Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

-export_grad_fsl fix for highly non-RAS acquisitions #2912

Closed
wants to merge 1 commit into from

Conversation

Lestropie
Copy link
Member

Here we go again...

So I was left scratching my head at this one for quite some time. I am trying to get to #2477 among other things, but was seemingly having issues at the bvec write stage, which meant I couldn't yet trust anything I was observing at the bvec read stage.

  1. Eigen function .rotation() was being used, which is not the same as .linear(). For a DICOM DWI series acquired on a Siemens magnet with a sagittal plane, these two functions yield very different results. I'm sure we've had at least one issue in the past with conflating these two, though haven't had success in finding anything on GitHub (@MRtrix3/mrtrix3-devs anyone with better recollection?).

  2. Code was doing a post-multiply with the image2scanner transform in order to do a scanner2image transformation., rather than constructing the scanner2image transform and using that.

  3. Code was doing a scanner2image transformation based on the transformation for the internal in-memory representation of the header, and then performing subsequent permutations & flips based on the secondary output of the File::NIfTI::adjust_transform() function and the internal strides, knowing that at NIfTI export time there would be similar modifications applied. However the File::NIfTI::adjust_transform() function returns what will become the header transformation of the output image. So it seems to me that if one wants to perform an image2scanner transformation of gradient directions that will correspond to the output image, and one has what will be the transformation of the output image, might as well do it in one step?

I'm reasonably confident given the test data & tool I have:

  • Direct verification of bvec contents based on non-realigned header transformation and known gradient table
  • Manual checking of dwi2tensor outputs:
    DICOM mrconverted to NIfTI / bvec / bval, both with & without internal header transform realignment

In the absence of this change, the specific combination of sagittally-acquired DICOM DWI data, running mrconvert to NIfTI / bvec / bval with internal header transform realignment disabled, produces an incorrect result.

@jdtournier I'll fill in more details in a separate email chain; content to leave as a draft until I can explain the broader project there.

@Lestropie Lestropie added the bug label May 21, 2024
@Lestropie Lestropie requested a review from jdtournier May 21, 2024 13:33
@Lestropie Lestropie self-assigned this May 21, 2024
@Lestropie
Copy link
Member Author

Realised that the testing I'm doing currently has commented out the evaluation of manually requested output strides. Will leave as draft until my tests have been expanded in scope.

@Lestropie
Copy link
Member Author

Superseded by #3011.

@Lestropie Lestropie closed this Sep 20, 2024
@Lestropie Lestropie deleted the bvec_save_fix branch September 20, 2024 23:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

1 participant