ITKUltrasound icon indicating copy to clipboard operation
ITKUltrasound copied to clipboard

BUG: `itk::SliceSeriesSpecialCoordinatesImage` warning and unexpected behavior

Open tbirdso opened this issue 1 year ago • 0 comments

Overview

Building itk::SliceSeriesSpecialCoordinatesImage produces a compiler warning. Examination of the surrounding code has raised additional questions regarding intended functionality.

Compiler Warning

include/itkSliceSeriesSpecialCoordinatesImage.h:477:42: warning: 'this' pointer is null [-Wnonnull]

Full results: https://open.cdash.org/viewBuildError.php?type=1&buildid=8372431

Questions on functionality

Copied from #214

After taking a closer look I am concerned about the procedure for attempting to extrapolate outside of the image extent represented with slice transforms.

itk::SliceSeriesSpecialCoordinatesImage::TransformIndexToPhysicalPoint appears to attempt to handle indices that lie outside the image extent along the N-1th pixel dimension by setting that element in point to the pixel distance between the requested index and the known image extent, then applying the slice transform. However, point is intended to already represent a physical point in (N-1)D space, so this is mixing indices with physical measurements.

https://github.com/KitwareMedical/ITKUltrasound/blob/master/include/itkSliceSeriesSpecialCoordinatesImage.h#L569

I believe the behavior in TransformIndexToPhysicalPoint was intended to be applied to TransformContinuousIndexToPhysicalPoint where the null-transform warning occurs. However, it seems like there is more fundamental discussion required to verify and/or update itk::SliceSeriesSpecialCoordinatesImage.

Proposed changes

In order of preference:

  1. Disallow extrapolation outside of the image extent. Each image slice is treated as having some independent spacing from each other, so extrapolation is not particularly meaningful and could be misleading. Discrete or continuous indices that lie outside of the image pixel extent should not be mapped to a physical point but instead treated as erroneous input.
  2. Make no changes to TransformIndexToPhysicalPoint and apply the same extrapolation behavior in which the closest slice transform is used to resolve null pointer warnings in TransformContinuousIndexToPhysicalPoint.

Additional Notes

The issue was originally discussed in #214 as a side effect of updating CI workflows. To avoid blocking more pressing changes an exception was added to ignore the warning in CI results and discussion was moved into this issue.

itk::SliceSeriesSpecialCoordinatesImage does not appear to be heavily used and is not wrapped for Python. Thus this issue is likely low priority.

cc @dzenanz @thewtex

tbirdso avatar Jan 02 '23 20:01 tbirdso