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

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

Open
tomvars opened this issue Apr 4, 2023 · 15 comments
Labels
type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances

Comments

@tomvars
Copy link

tomvars commented Apr 4, 2023

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 - #2674 - this issue is still affecting users with larger voxel sizes across a variety of technologies

References:
MIC-DKFZ/nnDetection#24
SimpleITK/SimpleITK#1433
fepegar/torchio#669
wasserth/TotalSegmentator#32

@tomvars tomvars added the type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances label Apr 4, 2023
@github-actions
Copy link

github-actions bot commented Apr 4, 2023

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
Copy link
Member

dzenanz commented Apr 4, 2023

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
Copy link
Member

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
Copy link
Contributor

vfonov commented Apr 5, 2023

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

@hjmjohnson
Copy link
Member

@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
Copy link
Member

dzenanz commented Apr 12, 2023

@tomvars please take a look at #4009.

@cookpa
Copy link
Contributor

cookpa commented Apr 12, 2023

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
Copy link
Contributor

vfonov commented Apr 12, 2023

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:

@vfonov
Copy link
Contributor

vfonov commented Apr 12, 2023

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
Copy link

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.

Palollo added a commit to chaimeleon-eu/workstation-images that referenced this issue Feb 22, 2024
Required to open some nifti files
generated with old versions of tools like simpleitk 2.2.0.
The new version shows the error
"ITK only supports orthonormal direction cosines. No orthonormal definition found!".
More details here:
InsightSoftwareConsortium/ITK#3994
@elazarcoh
Copy link

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
Copy link
Contributor

cookpa commented Aug 20, 2024

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

@cookpa
Copy link
Contributor

cookpa commented Aug 20, 2024

Looks like ITK-SNAP is using an older ITK

image

@dzenanz
Copy link
Member

dzenanz commented Aug 20, 2024

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

@jilei-hao
Copy link

jilei-hao commented Aug 20, 2024 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances
Projects
None yet
Development

No branches or pull requests

8 participants