Skip to content

Conversation

@maxpietsch
Copy link
Member

As reported on the forum, transformcalc header fails for large rotations in mrview's transformation tool due to the axis interpretation.

The proposed solution is to set Header::do_realign_transform = false;. I think this is sensible given the purpose of the tool but I am not sure if this brings any negative side effects for other use cases. Any opinions?

@jdtournier
Copy link
Member

Yes, I can see this is going to be an issue. I'm not sure what the best way to fix this is going to - I imagine setting Header::do_realign_transform = false is likely to cause trouble in other use cases - for example with NIfTI images that often have very large rotations in their headers. It's likely to alter behaviour that would have previously been considered 'normal'. We'll have to think about this one...

@Lestropie
Copy link
Member

  • Transform(orig_header).voxel2scanner.inverse() seems an unusual call given the presence of Transform::scanner2voxel.

  • The fact that the variable named "forward_transform" has to be inverted when writing to file comes off as suspicious. I'm not accustomed to dealing with transforms myself - I tend to just tinker until I get the right answer and then stop - but this nevertheless sets off alarms in my head.

  • The transformations that were performed during header load are now stored here. You could theoretically use this info to construct the effective corresponding affine transform matrix if necessary. Don't have a sense of whether or not this actually solves anything that RealignTransform false doesn't; just making sure you're aware.

@jdtournier jdtournier added this to the 3.1.0 updates milestone Nov 30, 2022
@Lestropie
Copy link
Member

Suggested alteration: In #2526 I propose that the Header class should have a method that reconstructs, from the transformation and the set of axis flips & permutations that were applied at image load, what the image transformation for the image header as stored on the filesystem was. I think this is what is wanted here, but by doing it this way it would not be necessary to manipulate that static class member, which would otherwise be assumed to be fixed for the duration of command execution.

@Lestropie
Copy link
Member

Pinging @ppruc.

This needs to be revisited following #3011. What I expect is necessary is that any calculations that involve header transforms of input images need to be made with respect to the transform of the image as it is stored on disk, not as it is interpreted by MRtrix, which has possibly experienced a realignment through shuffling of transform axes & strides. With the changes in #3011 there is explicit tracking of the transform of the image on disk, as well as what such shuffling was applied at the point of image load; the former should I think be what any header-transform-dependent calculations are based on. It's possible that the scope of erroneous transform calculations extends beyond just transformcalc header. Having some sample tests with which the prblem can be demonstrated & fix verified would be useful.

@Lestropie
Copy link
Member

  • Ensure that current code using Transform(orig_header).voxel2scanner.inverse() is replaced with Transform(orig_header).scanner2voxel if the relevant code remains
    (as the code currently in this PR may need to be replaced entirely to fully resolve the issue)

@Lestropie Lestropie force-pushed the transformcalc_fix branch 2 times, most recently from f80219e to 91ac6ac Compare August 29, 2025 07:10
@Lestropie Lestropie marked this pull request as draft September 24, 2025 08:21
Resolves changes to transformcalc header originally proposed in #2102 against explicit tracking of header transform realignment as implemented in #3011.
Comment on lines -208 to -209
transform_type forward_transform = Transform(modified_header).voxel2scanner * Transform(orig_header).voxel2scanner.inverse();
save_transform (forward_transform.inverse(), output_path);
Copy link
Member

Choose a reason for hiding this comment

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

Not sure why this composed the transform from modified header to original header and then inverted it, as opposed to just computing the transform from original heder to modified header...

@Lestropie Lestropie marked this pull request as ready for review September 28, 2025 12:30
@Lestropie
Copy link
Member

I initially commenced re-writing this operation to always make use of the transform as stored on disk. However I think there is scope for this to also be erroneous: the transform being sought could conceivably be the difference between two voxel grids after MRtrix3's closest-to-RAS re-interpretation. So what I have opted for instead is to produce a warning in any circumstance where the value of RealignTransform is consequential. The availability of Header::realignment() from #3011 means that it is not necessary to load the same images twice while changing Header::do_realign_transform to achieve this.

@Lestropie Lestropie marked this pull request as draft September 28, 2025 13:09
@Lestropie
Copy link
Member

Lestropie commented Sep 28, 2025

Back to draft: requires more comprehensive tests.
I expected that for the first test:

echo $'0 1 0 0\n1 0 0 0\n0 0 1 0\n0 0 0 1' > tmp1.txt && \
mrtransform dwi_mean.mif -linear tmp1.txt tmp.mif -force && \
transformcalc dwi_mean.mif tmp.mif header tmp2.txt -force -config RealignTransform true

, the output transform should be the identity matrix, but that's not what comes out...

Edit: Presumably this is because I'm applying a transform relative to scannerspace axes. So what I need is to define the appropriate scannerspace transform such that, following the MRtrix3 internal transform realignment, there is no difference in the transforms of the two interpreted headers.

@Lestropie
Copy link
Member

Still having trouble devising one of the tests that I actually want to apply to ensure correct operation (as opposed to adding a test that checks for regression against the wrong answer).

I want to apply a linear transform to an image whose header transform is not the identity matrix (eg. dwi_mean.mif), such that when I run transformcalc comparing the original and modified images, specifically with -config RealignTransform true, it reports the identity matrix, as the difference between those two images on the filesystem disappears as a consequence of the internal realignment process. But it's not clicking in my mind how to derive that transform.

- Test issuing of warnings and evaluation of appleid transform for pure transverse input image.
Attempted tests involving non-pure-transverse image intentionally commented out; derivation of the appropriate transform that results in equivalence of two images following internal image realignment is a topic of future work.
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