Skip to content

DOC: adjusted to fit more general case n != m#83

Merged
thewtex merged 2 commits intoInsightSoftwareConsortium:masterfrom
romangrothausmann:VectorGM-doc
Nov 12, 2018
Merged

DOC: adjusted to fit more general case n != m#83
thewtex merged 2 commits intoInsightSoftwareConsortium:masterfrom
romangrothausmann:VectorGM-doc

Conversation

@romangrothausmann
Copy link
Copy Markdown
Member

To my understanding eigenvalues and PCA are the special case for a m×n matrix with n = m, i.e. a square matrix. However, the first order derivative (Jacobian) can be a non-square matrix, i.e. m ≠ n, in case the image dimension does not equal the number of vector components (channels).
The PR is meant as a discussion initiation, as I am not an expert in this field and stumbled upon this while trying to understand the math behind this filter.

@romangrothausmann
Copy link
Copy Markdown
Member Author

Also, the output is a scalar image no matter how many channels the input has (which is the reason for the filter and the "magnitude" in its name), see e.g. the example WatershedSegmentation1.cxx

@romangrothausmann
Copy link
Copy Markdown
Member Author

How to see in the ci/circleci page ("Details" links) what caused the failure? I only see that ctest -V -S "${DASHBOARD_BRANCH_DIRECTORY}/circleci.cmake" is marked red but it is not clear to me from the logs what is the actual error/problem.

@kwrobot kwrobot added temp and removed temp labels Nov 6, 2018
@thewtex
Copy link
Copy Markdown
Member

thewtex commented Nov 9, 2018

@romangrothausmann thanks for contributing this fix!

We have now officially migrated to GitHub! :octocat:

Could you please rebase this patch with something like:

cd ITK
git checkout master
git pull origin master
git checkout VectorGM-doc
git rebase master
./Utilities/SetupForDevelopment.sh
# Follow prompt
git review-push --force

?

@thewtex thewtex requested a review from dzenanz November 9, 2018 23:51
Copy link
Copy Markdown
Member

@dzenanz dzenanz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution. As this is a purely documentation change, there should be no test CI failures. As Matt said, rebasing on current master and re-pushing will re-trigger all the checks, which should now all be OK.

adjusted documentation to fit the more general case for images of dimension N with M input channels, especially n != m.
Copy link
Copy Markdown
Member

@thewtex thewtex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the branch update @romangrothausmann , and the documentation correction 👍 Looks good.

The Windows failure is a false positive.

@thewtex thewtex merged commit 1293d50 into InsightSoftwareConsortium:master Nov 12, 2018
@romangrothausmann
Copy link
Copy Markdown
Member Author

Many thanks @thewtex and @dzenanz for taking a look and approving, and also for making contribution to ITK via GH possible. I find it much easier this way, especially the barrier to propose a contribute is much lower with GH PRs and the GH front-end allows many more ways to investigating the project and its code.

@dzenanz
Copy link
Copy Markdown
Member

dzenanz commented Nov 13, 2018

Ease of contribution was a major point of the move to GitHub!

@romangrothausmann romangrothausmann deleted the VectorGM-doc branch January 10, 2019 12:35
hjmjohnson added a commit to hjmjohnson/ITK that referenced this pull request Apr 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants