Skip to content

ENH: Streaming-equivalence GTest for GaussianInterpolateImageFunction#1012

Merged
hjmjohnson merged 1 commit intoInsightSoftwareConsortium:mainfrom
romangrothausmann:TestResampleSDI4GaussInterpolator
May 6, 2026
Merged

ENH: Streaming-equivalence GTest for GaussianInterpolateImageFunction#1012
hjmjohnson merged 1 commit intoInsightSoftwareConsortium:mainfrom
romangrothausmann:TestResampleSDI4GaussInterpolator

Conversation

@romangrothausmann
Copy link
Copy Markdown
Member

@romangrothausmann romangrothausmann commented Jun 12, 2019

Adds a GTest regression guard for issue #1011, which was fixed by #1202: ResampleImageFilter + GaussianInterpolateImageFunction must produce identical output whether or not its downstream is streamed.

Force-pushed to revive this dormant draft from 2019. Original three commits superseded by a single modern GTest TEST block; authorship preserved via Co-Authored-By: trailer.

Why the original draft no longer reproduced

The pipeline in the original draft fed the resampler an in-memory Image object directly. An in-memory image's BufferedRegion always equals its LargestPossibleRegion regardless of downstream RequestedRegion propagation, so the bug surface in GaussianInterpolateImageFunction::ComputeBoundingBox (which read from BufferedRegion pre-#1202) never triggered.

The new TEST inserts a CastImageFilter upstream of the resampler — a streaming-aware no-op that produces a chunked BufferedRegion per stream division. With the fix in place: 0 / 32 768 pixel diffs. With #1202 reverted locally: 28 672 / 32 768 pixels diverge, output contains NaN. Strong regression guard.

Test summary
Form New TEST block appended to Modules/Filtering/ImageGrid/test/itkResampleImageFilterGTest.cxx (no new files)
Geometry 32³ float ramp, AffineTransform Scale(0.9), GaussianInterp σ=1, α=1
Pipeline Image → CastImageFilter → ResampleImageFilter → StreamingImageFilter
Comparison streamed (8 divisions) vs non-streamed (1 division), pixel-by-pixel EXPECT_FLOAT_EQ
Local timing 12 ms

@romangrothausmann
Copy link
Copy Markdown
Member Author

Since all tests succeed, it seem that just using GaussianInterpolateImageFunction with a test based on itkResampleImageTest7 does not seem to trigger the problem of #1011. I also tried using 3D data instead of 2D, but then the test fails already without using GaussianInterpolateImageFunction probably due to the fixed tests concerning the chunk sizes. @thewtex How did you come up with these values:

ITK_TEST_SET_GET_VALUE( 48, finalRequestedRegion.GetIndex(1) );
ITK_TEST_SET_GET_VALUE( 60, finalRequestedRegion.GetSize(0) );
ITK_TEST_SET_GET_VALUE( 12, finalRequestedRegion.GetSize(1) );

@dzenanz
Copy link
Copy Markdown
Member

dzenanz commented Jun 12, 2019

@romangrothausmann examining what the actual values are? The GetRadius patch has increased the region by 1 pixel on all sides.

@kwrobot-v1
Copy link
Copy Markdown

kwrobot-v1 Bot commented Jun 14, 2019

Errors:

  • Failed to run the checks: service error.

@romangrothausmann
Copy link
Copy Markdown
Member Author

examining what the actual values are?

@dzenanz What do you mean by that?
The increase in the reagion is clear but how to get those values for a 3D case as e.g. in: 3f5fd25
And why does the test succeed even for GaussianInterpolateImageFunction (where the radius should be larger)?

@dzenanz
Copy link
Copy Markdown
Member

dzenanz commented Jun 14, 2019

examining what the actual values are?

Run the program under debugger and examine the values.

GaussianInterpolateImageFunction's radius depends on the sigma parameter. I guess the default sigma results in radius of 1, which is the same as linear interpolation.

@dzenanz
Copy link
Copy Markdown
Member

dzenanz commented Aug 29, 2019

This PR should be revived after #1191 and #1213 are merged.

@dzenanz
Copy link
Copy Markdown
Member

dzenanz commented Oct 22, 2019

@romangrothausmann can you rebase this PR on current master?

@romangrothausmann
Copy link
Copy Markdown
Member Author

That should be possible, I'll give it a try.

@romangrothausmann romangrothausmann force-pushed the TestResampleSDI4GaussInterpolator branch from 3f5fd25 to 26c91e7 Compare October 22, 2019 15:34
@romangrothausmann
Copy link
Copy Markdown
Member Author

3D version without GaussianInterpolateImageFunction (26c91e7) now works for me locally. Let's see if it still does with GaussianInterpolateImageFunction.

@romangrothausmann
Copy link
Copy Markdown
Member Author

Oddly, on my local PC ctest -R ResampleImageTest8 passes even though I would expect that it fails with only the changes of 452bba1 because I would expect that the 3rd dimension would demand changes to

// Verify that we only requested a smaller chunk when streaming
const ImageRegionType finalRequestedRegion( image->GetRequestedRegion() );
ITK_TEST_SET_GET_VALUE( 0, finalRequestedRegion.GetIndex(0) );
ITK_TEST_SET_GET_VALUE( 48, finalRequestedRegion.GetIndex(1) );
ITK_TEST_SET_GET_VALUE( 60, finalRequestedRegion.GetSize(0) );
ITK_TEST_SET_GET_VALUE( 12, finalRequestedRegion.GetSize(1) );

@dzenanz How to "Run the program under debugger" with ctest?

@dzenanz
Copy link
Copy Markdown
Member

dzenanz commented Oct 24, 2019

ctest -VV will print the exact command line for each test. That way, you can run that command line under debugger (e.g. copy command-line parameters into Visual Studio).

@stale
Copy link
Copy Markdown

stale Bot commented Feb 21, 2020

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

@stale stale Bot added the status:Use_Milestone_Backlog Use "Backlog" milestone instead of label for issues without a fixed deadline label Feb 21, 2020
@hjmjohnson hjmjohnson marked this pull request as draft February 15, 2026 21:16
@hjmjohnson hjmjohnson force-pushed the TestResampleSDI4GaussInterpolator branch from 26c91e7 to 68d3c56 Compare May 2, 2026 15:12
@github-actions github-actions Bot added 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 changed the title WIP: Test resampling with Gauss interpolator when employing streaming ENH: Streaming-equivalence GTest for GaussianInterpolateImageFunction May 2, 2026
@hjmjohnson
Copy link
Copy Markdown
Member

Hi @romangrothausmann — I hope you don't mind, but I took the liberty of force-pushing a single commit to this branch to revive your dormant draft as a regression test for #1011.

The original three commits have been replaced (your authorship is preserved via Co-Authored-By: in the new commit's message). The new test is a TEST block appended to itkResampleImageFilterGTest.cxx, and inserts a CastImageFilter upstream of the resampler so the bug actually surfaces under streaming — with that change, locally reverting #1202's fix produces 28 672 / 32 768 NaN-or-divergent pixels, which gives us the strong regression guard you set out to add seven years ago.

cc @dzenanz — keeping this PR draft pending CI; happy to flip to ready-for-review once CI is green if you'd like.

If you'd prefer a different attribution arrangement or a separate PR from my fork, just say the word and I'll redo it.

@greptile-apps

This comment was marked as resolved.

Comment thread Modules/Filtering/ImageGrid/test/itkResampleImageFilterGTest.cxx
Comment thread Modules/Filtering/ImageGrid/test/itkResampleImageFilterGTest.cxx
@dzenanz
Copy link
Copy Markdown
Member

dzenanz commented May 4, 2026

Let's give @romangrothausmann a few days to review this.

Regression guard for issue InsightSoftwareConsortium#1011 (fixed by InsightSoftwareConsortium#1202): a Gaussian-interpolated
ResampleImageFilter must produce the same output whether or not its
downstream is streamed.

Reverts of the bounding-box fix (BufferedRegion vs LargestPossibleRegion)
reproduce a 28672 / 32768-pixel divergence with NaN output values. A
CastImageFilter is inserted upstream of the resampler so the bug surfaces;
an in-memory image's BufferedRegion never shrinks under streaming, which
is why the abandoned WIP draft InsightSoftwareConsortium#1012 no longer reproduced.

Supersedes InsightSoftwareConsortium#1012 by @romangrothausmann; uses the modern GTest harness
and fits as a TEST block in itkResampleImageFilterGTest.

Co-Authored-By: Roman Grothausmann <roman.grothausmann@mh-hannover.de>
@hjmjohnson hjmjohnson force-pushed the TestResampleSDI4GaussInterpolator branch from 68d3c56 to 4002ed5 Compare May 4, 2026 17:15
@hjmjohnson
Copy link
Copy Markdown
Member

Folded the two P2 style nits from greptile's review into the existing commit (autosquash, force-push to refresh CI). Both are pure stylistic improvements — no logic change, test still passes locally.

Greptile P2 Fix
Brace-aggregate { { 32, 32, 32 } } for SizeType Replaced with ImageType::SizeType::Filled(32) (matches ITK convention for uniform sizes)
Magic literal 8 for stream divisions Extracted as constexpr unsigned int kNumberOfStreamDivisions{ 8 }

@romangrothausmann — the patchset is still a single commit so the diff you'll see vs. upstream/main reflects the latest polish. No need for you to address greptile's findings yourself.

cc @dzenanz — taking the courtesy window into account; this just preempts the polish so Roman gets the final form on first read.

@github-actions github-actions Bot added the type:Enhancement Improvement of existing methods or implementation label May 4, 2026
@hjmjohnson hjmjohnson self-requested a review May 6, 2026 11:10
@hjmjohnson hjmjohnson merged commit 8736796 into InsightSoftwareConsortium:main May 6, 2026
15 checks passed
hjmjohnson added a commit to blowekamp/ITK that referenced this pull request May 6, 2026
Regression guard for issue InsightSoftwareConsortium#1011 (fixed by InsightSoftwareConsortium#1202): a Gaussian-interpolated
ResampleImageFilter must produce the same output whether or not its
downstream is streamed.

Reverts of the bounding-box fix (BufferedRegion vs LargestPossibleRegion)
reproduce a 28672 / 32768-pixel divergence with NaN output values. A
CastImageFilter is inserted upstream of the resampler so the bug surfaces;
an in-memory image's BufferedRegion never shrinks under streaming, which
is why the abandoned WIP draft InsightSoftwareConsortium#1012 no longer reproduced.

Supersedes InsightSoftwareConsortium#1012 by @romangrothausmann; uses the modern GTest harness
and fits as a TEST block in itkResampleImageFilterGTest.

Co-Authored-By: Roman Grothausmann <roman.grothausmann@mh-hannover.de>
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 status:Use_Milestone_Backlog Use "Backlog" milestone instead of label for issues without a fixed deadline type:Enhancement Improvement of existing methods or implementation 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.

3 participants