ITK
ITK copied to clipboard
WIP: Fix MINC read/write to convert from/to RAS coordinate system
Convert the coordinate system from RAS to LPS during MINC loading, and convert back on writing.
Also adds a setting to keep old configuration, so that MINC-based ITK tools (EZMinc) can maintain their original configuration and return sensible coordinate values in MINC-space.
Paging @vfonov
Originally discussed at https://insightsoftwareconsortium.atlassian.net/browse/ITK-3503?attachmentSortBy=fileName
What about minc transform?
@vfonov I have the code for converting the MINC affine to from RAS to LPS, I was going to do a seperate PR. Would you rather I roll that into this?
@vfonov MINC vs ITK also defines its transforms in opposite directions, do you want that added to the reader, or leave that to the user?
I have added the conversion for Affine transforms.
I'm not quite sure where to add the switch to disable this conversion to support backwards compatibility.
Can anyone give some guidance here?
My transform work isn't right. It used to be combined with inverting the affines so that MINC and ITK transforms went in the same direction. I backed that part out and this needs some fixes as a result.
one of the main reason for me to have minc interface for ITK is to be able to run ANTs & Elastix. I think we should run extensive checks to make sure that this conversion does not screw up their results.
Totally agree. On the walk home I wrote up a bunch of tests in head to ensure this works properly. Will enumerate later.
Given two files, in both MINC and NIFTI format: GAD
- scan of me T1
- convert to MINC with dcm2mnc, minc-toolkit/1.9.16
- convert to NIFTI with dcm2niix/1.0.20171215
mni_icbm152_t1_tal_nlin_asym_09c
- original MINC version
- convert to NIFTI with mnc2nii
Tests to do:
- minc-to-minc affine registration with antsRegistration
- nifti-to-minc affine registration with antsRegistration
- minc-to-nifti affine registration with antsRegistration
- bestlinreg minc-to-minc
- save transforms as both ants and minc format
- apply all transforms using antsApplyTransforms
- apply minc-style transforms with mincresample
- ConvertImage to/from minc/nifti
Sounds like a plan
Additional tests:
- antsRegistration SyN to produce nonlinear
- save as minc/nifti
- confirm forward/reverse transformations also work as expected with antsApplyTransforms and mincresample
- nlfit to produce nonlinear
- confirm forward/reverse work with antsApplyTransforms
nlfit <-> antsApplyTransforms probably will not work.
All conversions and affine registrations now pass my listed tests.
The following still don't work
- nlfip -> antsApplyTransform
- mincresample applying the NLIN fields produced by ANTs
ITK and MINC have opposite transform directions, and nlpfit doesn't produce an inverse warp field and doesn't handle the invert flag of an XFM grid transform properly (yet...)
I think we have the issue that in grid-based transforms, the X and Y vector components point in opposite directions when produced in minc-land or ITK land.
Questions
- Should we flip all the voxel vectors?
- Can anyone suggest an efficient way to do this?
- @vfonov the TransformMINC doesn't have an option to not flip the transforms, do you need it?
PR https://github.com/InsightSoftwareConsortium/ITK/pull/194 addresses the issue with MINC NLIN transforms only being generated one-way and ITK not properly handling the invert flag on such transforms.
Still remaining is I need to figure out an efficient method to voxel-wise multiply the incoming grid file by (-1,-1,1) to flip the RAS vector components to LPS vectors.
Added patch now properly handles Grid transforms to/from MINC land.
My local tests of
- mincresample applying ITK generated transforms
- antsApplyTransforms applying grid transforms generated by nlpfit
Now work properly.
The question that remains is should ITK also invert all transforms it produces (and handle inversion on reading) so that the directionality of MINC vs ITK transforms is also transparently handled.
This patch also works properly on top of/with #194
Thanks @dzenanz I have removed the extraneous Fills.
@thewtex I think this change is breaking the current tests, is that correct? If so, any hints as to how to I would go about sorting that out?
@gdevenyi there are 51 minc tests failing on all platforms. Do the tests all pass locally?
@thewtex that's what I'm asking? how do I go about checking that locally :)
@gdevenyi ah, turn on BUILD_TESTING
in your ITK build's CMake configuration, re-build, then run ctest
from the build tree.
All the test are failing with errors like this, with can't read a file.
/scratch/ITK-build/bin/ITKIOMINCTestDriver "--compare" "/scratch/ITK-build/ExternalData/Modules/IO/MINC/test/Input/t1_z+_ushort_trans.mnc" "/scratch/ITK-build/Testing/Temporary/t1_z+_ushort_trans.mnc" "itkMINCImageIOTest4" "/scratch/ITK-build/ExternalData/Modules/IO/MINC/test/Input/t1_z+_ushort_trans.mnc" "/scratch/ITK-build/Testing/Temporary/t1_z+_ushort_trans.mnc" "427590.7957" "-8.195997123" "72.45943721" "-3.148635961"
Image:/scratch/ITK-build/ExternalData/Modules/IO/MINC/test/Input/t1_z+_ushort_trans.mnc sum=427590.7957 COM=[8.195997127, -72.45943721, -3.148635962]
Total mx mismatch:8.196
Image:/scratch/ITK-build/ExternalData/Modules/IO/MINC/test/Input/t1_z+_ushort_trans.mnc sum=427590.7957 COM=[8.195997127, -72.45943721, -3.148635962]
Total mx mismatch:8.196
/home/cic/devgab/projects/src/ITK.github/Modules/ThirdParty/MINC/src/libminc/libsrc2/volume.c:1400 (from MINC): Unable to open file '/scratch/ITK-build/Testing/Temporary/t1_z+_ushort_trans.mnc'
Exception detected while reading /scratch/ITK-build/ExternalData/Modules/IO/MINC/test/Input/t1_z+_ushort_trans.mnc : itk::ERROR: MINCImageIO(0x23e97a0): Could not open file "/scratch/ITK-build/Testing/Temporary/t1_z+_ushort_trans.mnc".<DartMeasurement name="BaselineImageName" type="text/string">INVALID_BASELINE_GIVEN</DartMeasurement>
It looks to me like the tests are getting setup slightly wrong maybe and the input files can't be found? Or am I misreading this?
The test runs, then it tries to compare a test output file, /scratch/ITK-build/Testing/Temporary/t1_z+_ushort_trans.mnc to a baseline file, but the test output file not exist. What is the test output file not being written?
Oops. I see it now. Its testing COM in RAS space, and that's wrong now :+1:
Patch incoming
All passing!
@vfonov the only remaining question is: do you need a flag to disable the correction of coordinate spaces for the MINCTransform code? I'm not sure how I would do that right now...
yes, i would say that it is critical, and it have to be very explicit to Enable new behaviour
All passing!
@vfonov the only remaining question is: do you need a flag to disable the correction of coordinate spaces for the MINCTransform code? I'm not sure how I would do that right now...
Also, i would like to completely disable the new behaviour where COM location is in different coordinates then what mincstats would report.
@vfonov the disable coordinate conversion is already added at https://github.com/InsightSoftwareConsortium/ITK/pull/147/files#diff-32784fd54fb5bc9cf57e46a4395d785bR94
I have to strongly disagree that these fixes should be opt-in behaviour. Right now the ITK MINC reader is wrong. The coordinates are incorrect according to the internal ITK standard when loading files, making it impossible to mix MINC with any other filetype. These changes make MINC transparently work with the other filetypes ITKIO supports.
To provide backwards compatibility I will figure out how to add a flag to TransformMINC to disable this as well.
If it was easy to disable, Why did you have to change the tests that were failing? I want to keep using old behavior by default.
@vfonov The tests were updated as the COM that were hard coded in those tests use the RAS coordinates. Now that the coordinates of the loaded MINC files are properly converted to ITK LPS space, the x and y values need to be -ve'd to have them correspond to the values printed by ITK's internal calculations.
@thewtex I need to add a feature/option to disable the RAS->LPS conversion for the TransformMINC work. Do you have a suggestion as to how I could add something that would be accessible? It looks like a common way to write out transforms is a TransformFileWriterTemplate which determines the underlying code based on the filename? If I add a option at the TransformMINC level can that propagate upwards so that its accessible?
@gdevenyi Yes, if an option is available on TransformMINC, that can be set, then passed to TransformFileWriterTemplate with SetTransformIO.