Conversation
|
For the record, elastix/src/Core/ComponentBaseClasses/elxResamplerBase.hxx Lines 329 to 332 in 695f922 |
|
Just double-checked: a direction matrix for 3D images specified by elastix parameter |
| * \transformparameter Direction: The direction cosines matrix of the fixed image | ||
| * that was used during registration, and which is used for resampling the deformed moving image | ||
| * if the UseDirectionCosines parameter is set to "true".\n | ||
| * Please note: The matrix elements should be specified in column-major order!\n |
There was a problem hiding this comment.
I would describe more explicitly that this differs from ITK conventions:
| * Please note: The matrix elements should be specified in column-major order!\n | |
| * Please note: The matrix elements are specified in column-major order (Fortran-style). | |
| * This is not the same as ITK's convention of using row-major order (C-style) for flattening | |
| * direction matrix of an image into a list.\n |
There was a problem hiding this comment.
I think @lassoan's comments reduces the room for error even further!
There was a problem hiding this comment.
@lassoan Thank you for your suggestion 👍 I'm fine with your suggestion, at least if it's the only place where it should the documented. (If this specification should be documented on multiple places, I'd rather elaborate on the meaning of "column-major order" just once, rather than on each of those places. But this appears the one and only place where it needs to be documented, right?)
One nit-pick question, why do you suggest replacing
The matrix elements should be specified in column-major order!
with
The matrix elements are specified in column-major order!
?
I wanted to express that the user should specify the matrix elements in that order.
There was a problem hiding this comment.
I agree, this detailed note should only appear once in the Elastix documentation. So, if this needs to be mentioned elsewhere in the future than that place should refer to this note or this note could be moved out elsewhere.
To me, "should" has some degree of uncertainty. If "should" is needed to make it clear that it is up to the user has to specify the values then we could use "must" or "shall" to make it clear that column-major order is not just a recommendation but a requirement.
There was a problem hiding this comment.
OK, thanks, then shall we make it "must"?
There was a problem hiding this comment.
@lassoan @sorenchr2011 Stefan (@stefanklein) just pointer out to me that MetaIO also uses column-major order for its TransformMatrix representation. See https://discourse.itk.org/t/metaio-image-rotation-conventions/4525 and https://itk.org/Wiki/ITK/MetaIO/Documentation#Associated_transformations
So then, is ITK's convention really using row-major order? Shouldn't the comment say something like:
Please note: The matrix elements are specified in column-major order (Fortran-style, like MetaIO's matrix representation).
ITK often uses row-major order (C-style) for flattening direction matrix of an image into a list.
There was a problem hiding this comment.
@lassoan @sorenchr2011 Stefan (@stefanklein) just pointer out to me that MetaIO also uses column-major order for its
TransformMatrixrepresentation. See https://discourse.itk.org/t/metaio-image-rotation-conventions/4525 and https://itk.org/Wiki/ITK/MetaIO/Documentation#Associated_transformationsSo then, is ITK's convention really using row-major order?
ITK internally uses row-major order. Various file formats don't have to use the same conventions as ITK. ITK-based libraries (such as Elastix) don't have to use the same conventions as ITK either, but when a user gets a vector from an ITK method then he expects to be able to use it as input in another method in an ITK-based library without any conversion, so it is worth trying to harmonize conventions. Bringing in MetaIO into this discussion is unnecessary and may be distracting.
The matrix elements are specified in column-major order (Fortran-style, like MetaIO's matrix representation).
ITK often uses row-major order (C-style) for flattening direction matrix of an image into a list.
I don't think this is accurate, because ITK is consistent in how it stores matrices in memory and how they may be exposed as flat arrays via Python wrapping.
Anyway, any documentation would improve things and I don't really mind the details.
There was a problem hiding this comment.
Anyway, any documentation would improve things and I don't really mind the details.
@lassoan OK, thanks! We obviously agree that the documentation should specify the order of the matrix elements.
Stefan (@stefanklein) further clarified their choice for column-major order (back in 2008) by saying that as each column represents one vector of direction cosines, it may be more readable to have the direction cosines of a vector together, in the flattered representation.
448a43f to
16added
Compare
|
@lassoan I added you as co-author, please check! |
16added to
1771017
Compare
|
For the record, my little test to observe the ordering of matrix elements: static constexpr unsigned int ImageDimension = 3;
itk::Matrix<double, ImageDimension, ImageDimension> direction;
// Create a "flat" array of matrix elements, {1, 2, ..., N}
std::array<double, ImageDimension * ImageDimension> flatArray;
std::iota(flatArray.begin(), flatArray.end(), 1.0);
// Retrieve the elements from the flat array by a nested for-loop, just like elastix does at:
// https://github.com/SuperElastix/elastix/blob/995f471aeb17929f8c0557135997e78f54326390/Core/Kernel/elxElastixTemplate.hxx#L1081-L1090
for (unsigned int i = 0; i < ImageDimension; ++i)
{
for (unsigned int j = 0; j < ImageDimension; ++j)
{
direction(j, i) = flatArray[i * ImageDimension + j];
}
}
for (const double x : direction.GetVnlMatrix())
{
std::cout << " v" << x;
}
std::cout << std::endl;
Based on elastix/Core/Kernel/elxElastixTemplate.hxx Lines 1081 to 1090 in 995f471 Output:
Similar nested for loops are in |
|
I still wonder, shouldn't "Direction" be documented in "elxResamplerBase.h", if it's actually being retrieved by |
This comment was marked as resolved.
This comment was marked as resolved.
1771017 to
329558e
Compare
be28a43 to
6c58973
Compare
Maybe the best could be describe the convention in every method where the user can get or set a direction array, but keep it very short and simple, with a link to more details. Something like "direction: ..., matrix elements in column-major order (conversion may be needed, see details at URL)" . |
| * This is not the same as ITK's convention of using row-major order (C-style) for flattening | ||
| * direction matrix of an image into a list.\n | ||
| * example: <tt>(Direction -1.0 0.0 0.0 0.0 1.0 0.0 0.0 0.0 0.1)</tt>\n | ||
| * Default: identity matrix. Elements are sorted as follows: [ d11 d21 d31 d12 d22 d32 d13 d23 d33] (in 3D). |
There was a problem hiding this comment.
I see now, here it already says:
Elements are sorted as follows: [ d11 d21 d31 d12 d22 d32 d13 d23 d33] (in 3D)
6c58973 to
c2a6a96
Compare
Triggered by the discussion "Elastix idiosyncracy?", started by Soren Christensen, July 11, 2024, at https://discourse.itk.org/t/elastix-idiosyncracy/7049 Co-authored-by: Andras Lasso <lasso@queensu.ca>
Triggered by the discussion "Elastix idiosyncracy?", started by Soren Christensen, July 11, 2024, at https://discourse.itk.org/t/elastix-idiosyncracy/7049
c2a6a96 to
c304fb0
Compare
Direction parameterDirection and GridDirection parameters
|
Going to merge now, but maybe the documentation on Direction and GridDirection should be re-organized somehow, eventually. ( |
Triggered by the discussion "Elastix idiosyncracy?", started by Soren Christensen, July 11, 2024, at https://discourse.itk.org/t/elastix-idiosyncracy/7049