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.43k stars 668 forks source link

ENH: Fix dicom reading slices #4955

Closed hjmjohnson closed 1 week ago

hjmjohnson commented 2 weeks ago

Dicom files that do not have explicit "SliceThickness" listed can still be used. The actual "spacing between slices" is computed from the stack of slices when multiple dicoms are provided such that the initial setting of the "SliceThickness" placeholder is not used. (NOTE: SliceThickness is used when a single slice is read into a 3D image).

This fixes an issue when a stack of 124 dicom slices does not have "SliceThickness" fields, but does have "ImagePositionPatient" for each slice.

The previous implementation would throw an exception prior to getting to the code that explicitly computes the slice space from the "ImagePositionPatient" values.

The new code now allows ITK to make 3D nifti volumes that are compatible with those created by the dcm2niix package.

PR Checklist

hjmjohnson commented 2 weeks ago

Local mac testing:

100% tests passed, 0 tests failed out of 3011
hjmjohnson commented 1 week ago

@jhlegarreta I appreciate your comments, but I don't have the bandwidth to make this perfect. I'm going to add it as is.

seanm commented 1 week ago

Is this at all related to https://github.com/InsightSoftwareConsortium/ITK/issues/4794 ?

seanm commented 1 week ago

I just tracked down 916a40cb3cebf817f91b69ef80a428b346c46fef as the cause of one of our app's unit test failures.

With the new less-strict tolerance check, ITK_non_uniform_sampling_deviation is no longer added to the metadata dictionary, and our test was expecting it to be there.

We use the value of ITK_non_uniform_sampling_deviation to decide for ourselves if it is above/below an acceptable tolerance. But 916a40cb3cebf817f91b69ef80a428b346c46fef has now hardcoded a tolerance, making it impossible for us to choose ourselves.

It's not clear to me from the commit message what bug this commit was fixing... Yes, AlmostEquals() will return false for "slices that are only 1e-15 different for 1.5mm slices", but so what? Then ITK_non_uniform_sampling_deviation will contain a tiny number and callers can decide if they care. They can also just ignore ITK_non_uniform_sampling_deviation.

Where does this magic 1e-6 (DefaultImageCoordinateTolerance) number come from? What are its units? millimetres? metres? inches? How can we assume this number is appropriate for every kind of DICOM file under the sun?