Skip to content

COMP: Harden streaming-equivalence tests with upstream CastImageFilter#6195

Open
hjmjohnson wants to merge 1 commit intoInsightSoftwareConsortium:mainfrom
hjmjohnson:harden-streaming-equivalence-tests
Open

COMP: Harden streaming-equivalence tests with upstream CastImageFilter#6195
hjmjohnson wants to merge 1 commit intoInsightSoftwareConsortium:mainfrom
hjmjohnson:harden-streaming-equivalence-tests

Conversation

@hjmjohnson
Copy link
Copy Markdown
Member

Hardens three streaming-equivalence tests in Modules/Filtering/ImageGrid/test/ that were silently weak: their input was an in-memory Image whose BufferedRegion always equals LargestPossibleRegion, so the filter under test never saw a chunked input under streaming. Inserts a CastImageFilter between the image and the filter so streaming actually exercises chunked input.

All four affected tests continue to pass — this is pure test hardening, no library changes.

Why the tests were weak

Each test ran the same pipeline twice (e.g., 1 vs 8 stream divisions) and compared outputs pixel-by-pixel. The intent was to verify that the filter under test produces identical output regardless of streaming.

But because the input was a hand-allocated Image::New(), its BufferedRegion is fixed at allocation and never shrinks under streaming. The downstream StreamingImageFilter chunks the output, ITK's pipeline propagates RequestedRegion upstream, but a raw Image ignores RequestedRegion propagation — its buffered data is fixed. So the filter under test always saw the full input regardless of stream-division count, and any bug specific to chunked-input handling slipped through.

Inserting a CastImageFilter (a streaming-aware no-op) propagates the chunked RequestedRegion and produces a per-chunk BufferedRegion downstream, putting the filter under test in a real streaming pipeline.

The warper tests already had this pattern on the displacement-field input (vcaster); this PR adds the same pattern on the image input. itkResampleImageTest7 had no upstream cast at all and gets one added.

How this was discovered

While diagnosing PR #1012 (a 7-year-old draft regression test for issue #1011 / fix #1202, dealing with GaussianInterpolateImageFunction + streaming), the same in-memory-Image topology turned out to silently mask the bug. After inserting CastImageFilter upstream, locally reverting #1202's fix produced 28 672 / 32 768 NaN-or-divergent pixels — strong regression guard. Without the cast, 0 / 32 768 pixel diffs even with the bug reverted.

That triggered an audit of other streaming-equivalence tests in the codebase. The three tests in this PR are the ones that match the same blind spot.

Files changed
File Change
itkResampleImageTest7.cxx Add #include "itkCastImageFilter.h"; route image → resample via CastImageFilter<ImageType,ImageType>; update ITK_TEST_SET_GET_VALUE accordingly
itkWarpImageFilterTest.cxx Add a second CastImageFilter<ImageType,ImageType> named icaster next to the existing vcaster; wire icaster->GetOutput() into warper2->SetInput
itkWarpVectorImageFilterTest.cxx Same pattern as the warper test

@github-actions github-actions Bot added type:Compiler Compiler support or related warnings type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct area:Filtering Issues affecting the Filtering module labels May 2, 2026
@hjmjohnson hjmjohnson marked this pull request as ready for review May 2, 2026 16:30
@greptile-apps

This comment was marked as resolved.

@hjmjohnson hjmjohnson requested a review from blowekamp May 2, 2026 18:01
@blowekamp
Copy link
Copy Markdown
Member

I would add the PipelineMonitorImageFilter to genially check that the filter was streaming as expected.

Again we have longer commit message that the amount of code changed.

@hjmjohnson hjmjohnson added this to the ITK 6.0 Release Candidate 1 milestone May 5, 2026
@hjmjohnson hjmjohnson force-pushed the harden-streaming-equivalence-tests branch 2 times, most recently from 46e5d71 to 5534e19 Compare May 5, 2026 19:33
@hjmjohnson
Copy link
Copy Markdown
Member Author

hjmjohnson commented May 5, 2026

@blowekamp If you are satisfied with this version and approve, I'll monitor and merge as soon as the dashboards are green.

Good catch — your first suggestion (PipelineMonitorImageFilter) was the more important one, and I missed it on the first pass. Now addressed in 5534e19.

Each test now reads:

Image -> CastImageFilter -> PipelineMonitorImageFilter -> <FilterUnderTest> -> StreamingImageFilter

After the streamed Update with N divisions, the test asserts monitor->VerifyAllInputCanStream(N) and prints the monitor on failure. So the streaming behaviour is now actively verified — not just compared for value-equivalence:

  • itkResampleImageTest7numStreamDiv = 8 (with ClearPipelineSavedInformation() between the non-streamed and streamed runs so the verify only counts the streamed pass)
  • itkWarpImageFilterTestnumStreamDivisions = 3
  • itkWarpVectorImageFilterTestnumStreamDivisions = 3

Locally: 3/3 tests still pass. Title and commit message also tightened on the same push.

@hjmjohnson hjmjohnson force-pushed the harden-streaming-equivalence-tests branch 2 times, most recently from e1b8d52 to 9fe055d Compare May 5, 2026 20:11
Insert PipelineMonitorImageFilter on each test's streamable input path
(image for Resample, displacement field for Warp/WarpVector) and assert
VerifyInputFilterExecutedStreaming(N) after the streamed Update.
@hjmjohnson hjmjohnson force-pushed the harden-streaming-equivalence-tests branch from 9fe055d to b436bb0 Compare May 5, 2026 20:11
@hjmjohnson
Copy link
Copy Markdown
Member Author

/azp run ITK.macOS.Python

@hjmjohnson
Copy link
Copy Markdown
Member Author

@blowekamp I think everything asked for has been delivered in this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:Filtering Issues affecting the Filtering module type:Compiler Compiler support or related warnings type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants