search

InsightSoftwareConsortium / ITK

Insight Toolkit (ITK) -- Official Repository. ITK builds on a proven, spatially-oriented architecture for processing, segmentation, and registration of scientific images in two, three, or more dimensions.
https://itk.org
Apache License 2.0
1.42k stars 664 forks source link

Nifti files with non-orthonormal directional cosines should be read and the error changed to a warning. #3994

Open tomvars opened 1 year ago

tomvars commented 1 year ago

Hi all,

In this PR - https://github.com/InsightSoftwareConsortium/ITK/pull/1868/files ITK introduced an additional constraint on NiFTI files being read.

This has affected users across a variety of technologies which use ITK such as TorchIO, TotalSegmentator, Nvidia AAIA, HD-BET, nnDetection and even ITK-SNAP (with some files working on < v3.6 but not on v3.8 >)

Given so many users see this as unexpected behaviour could the developers investigate whether the constraint is still a reasonable one?

Note: In this issue this was discussed but the solution solved only one failure case related to small voxel sizes - https://github.com/InsightSoftwareConsortium/ITK/issues/2674 - this issue is still affecting users with larger voxel sizes across a variety of technologies

References: https://github.com/MIC-DKFZ/nnDetection/issues/24 https://github.com/SimpleITK/SimpleITK/issues/1433 https://github.com/fepegar/torchio/issues/669 https://github.com/wasserth/TotalSegmentator/issues/32

github-actions[bot] commented 1 year ago

Thank you for contributing an issue! πŸ™

Welcome to the ITK community! πŸ€—πŸ‘‹β˜€οΈ

We are glad you are here and appreciate your contribution. Please keep in mind our community participation guidelines. πŸ“œ Also, please check existing open issues and consider discussion on the ITK Discourse. πŸ“–

dzenanz commented 1 year ago

People who use NIFTI a lot should weigh in. Candidates: @hjmjohnson @cookpa @vfonov @seanm. Maybe make the error/warning CMake configurable, with the default being a warning?

hjmjohnson commented 1 year ago

ITK does not support non-orthogonal images, to read the images that are non-orthogonal invalidates all the requirements about processing and analyzing images.

vfonov commented 1 year ago

Maybe we should add a flag that forces Nifti reader to convert the non-confirming non-orthogonal matrix to the closest possible orthogonal?

hjmjohnson commented 1 year ago

@vfonov I think such a flag would have some utility, would not break backward compatibility, and would require the end-user to explicitly ask for the risky behavior.

I am supportive of adding such a flag if a pull request is made.

I have also thought that the non-orthogonal part could be returned as part of the meta-data dictionary as a transform so that it could be used by the developer for other purposes.

dzenanz commented 1 year ago

@tomvars please take a look at #4009.

cookpa commented 1 year ago

I looked through the linked issues in the first post, but I didn't see any links to example data that reproduce the problem. Even the headers by themselves would be useful, these can be extracted with

head -c 352 image.nii > header.nii

If the sform checks are too strict, it would be good to fix them. Otherwise everyone is just going to turn on this flag at compile time and the end users will likely not even know about it

vfonov commented 1 year ago

I think, it's possible that, other software is quietly fixing this issue by using nifti_make_orthog_mat44 , for example ITK NIFTI writer is doing it when saving NIFTI files: https://github.com/InsightSoftwareConsortium/ITK/blob/c44f9c1b39e627c5ef7fabec252232a92d570565/Modules/IO/NIFTI/src/itkNiftiImageIO.cxx#L2261

vfonov commented 1 year ago

I looked through the linked issues in the first post, but I didn't see any links to example data that reproduce the problem. Even the headers by themselves would be useful, these can be extracted with

I created a file from another one by using nifti_tool to make sform non-orthogonal, but it is probably not representative of the real world problem.

mike-barrow commented 9 months ago

I think there is some kind of bug with the implementation of this check anyway. I spent 4 days ripping my hair out when getting an error message indicating a non-orthonormal transform for nifti files generated using the dicom2nifti python package. I found that the translation vector elements in the matrix ([1,4],[2,4],[3,4]) were causing this issue purely by being in the order of 1e2. I do not understand this since those elements are orthogonal in the Cartesian co-ordinate system, are independent of the rotational sub-matrix, and it is unreasonable to force a normal length constraint on the column vector of the translation anyway.

elazarcoh commented 2 months ago

Following @mike-barrow analysis, I've uploaded couple of examples of the same (full-zeros) image which the only difference is in the translation vector. Note the rotation is orthogonal. The rotation matrix remained the same. Note that I'm observing this using the itk library directly, but only through the ITK-Snap app. When I'm trying to read the same image with the python package (itk.__version__ # 5.3.0) seems to be working fine:

import itk
io = itk.NiftiImageIO.New()
io.SetFileName("fail_to_open.nii.gz")
io.ReadImageInformation()

I'm not sure what the differences are, maybe different build flags?

Examples (both work and don't work)

❌ fail_to_open.nii.gz (original affine)

[[   0.33203125    0.            0.          -84.83398438]
 [   0.            0.33203125    0.         -198.83398438]
 [   0.            0.            0.69999999 -318.3999939 ]
 [   0.            0.            0.            1.        ]]

❌ fail_to_open_1.nii.gz

[[   0.33203125    0.            0.            0.        ]
 [   0.            0.33203125    0.         -198.83398438]
 [   0.            0.            0.69999999 -318.3999939 ]
 [   0.            0.            0.            1.        ]]

βœ… success_to_open_2.nii.gz

[[   0.33203125    0.            0.            0.        ]
 [   0.            0.33203125    0.            0.        ]
 [   0.            0.            0.69999999 -318.3999939 ]
 [   0.            0.            0.            1.        ]]

βœ… success_to_open_3.nii.gz

[[0.33203125 0.         0.         0.        ]
 [0.         0.33203125 0.         0.        ]
 [0.         0.         0.69999999 0.        ]
 [0.         0.         0.         1.        ]]

βœ… success_to_open_4.nii.gz

[[   0.33203125    0.            0.          -84.83398438]
 [   0.            0.33203125    0.            0.        ]
 [   0.            0.            0.69999999 -318.3999939 ]
 [   0.            0.            0.            1.        ]]

βœ… success_to_open_norm.nii.gz

[[ 0.33203125  0.          0.         -0.22043331]
 [ 0.          0.33203125  0.         -0.51665181]
 [ 0.          0.          0.69999999 -0.82733309]
 [ 0.          0.          0.          1.        ]]
cookpa commented 2 months ago

Thanks for these examples. I believe the translation issue was fixed by this commit as part of #4009.

cookpa commented 2 months ago

Looks like ITK-SNAP is using an older ITK

image
dzenanz commented 2 months ago

Yes, ITK-SNAP is somewhat behind on ITK version.

jilei-hao commented 2 months ago

The next major update of ITK-SNAP (v4.4.0) will be using ITK 5.4

On Tue, Aug 20, 2024 at 9:21 AM Dženan Zukić @.***> wrote:

Yes, ITK-SNAP is somewhat behind on ITK version.

β€” Reply to this email directly, view it on GitHub https://github.com/InsightSoftwareConsortium/ITK/issues/3994#issuecomment-2298848529, or unsubscribe https://github.com/notifications/unsubscribe-auth/ARDT6PVKDPDYYPFHY4HCA2TZSM7FTAVCNFSM6AAAAAAWSXMCJOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEOJYHA2DQNJSHE . You are receiving this because you are subscribed to this thread.Message ID: @.***>