Skip to content

BUG: Reset smallestError per pixel in IterativeInverseDisplacementField#6576

Merged
hjmjohnson merged 1 commit into
InsightSoftwareConsortium:mainfrom
physwkim:bug-iterative-inverse-stale-smallest-error
Jul 10, 2026
Merged

BUG: Reset smallestError per pixel in IterativeInverseDisplacementField#6576
hjmjohnson merged 1 commit into
InsightSoftwareConsortium:mainfrom
physwkim:bug-iterative-inverse-stale-smallest-error

Conversation

@physwkim

Copy link
Copy Markdown
Contributor

Addresses item B31 of #6575.

IterativeInverseDisplacementFieldImageFilter declares the coordinate-descent residual bar smallestError outside the per-pixel loop and reassigns it only when a pixel's initial mapped point lies inside the input buffer. A pixel whose initial mapped point falls outside the buffer therefore inherits the previous pixel's residual as the value its probes must beat: probes that land back inside the buffer are accepted or rejected based on how well the previous pixel converged, so the inverse field's values at such pixels depend on raster order.

The fix moves the declaration inside the per-pixel loop, initialized to NumericTraits<double>::max(): the bar a probe must beat is the error of this pixel's current guess when that guess is evaluable (the existing inside-buffer branch still sets it), and "unknown — accept any evaluable probe" otherwise. Behavior for pixels whose initial guess is evaluable is unchanged.

The new regression test builds a 24×24 field where the pixel before the affected one in raster order converges to a zero residual (poisoning the stale bar) and the affected pixel's first guess maps just outside the interpolation buffer, with an in-buffer probe available one step away. Unfixed, the probe loses to the stale zero bar and the pixel never moves off its zero-iteration first guess; fixed, the descent accepts the probe and refines the estimate.

Verified locally (Linux, GCC 13, Release): the new test fails on unmodified main (moved by 0) and passes with the fix; all 24 ITKDisplacementField and 12 ITKRegionGrowing tests pass with the fix applied.

PR Checklist

  • No API changes were made (or the changes have been approved)
  • No major design changes were made (or the changes have been approved)
  • Added test (or behavior not changed)
  • Updated API documentation (or API not changed)
  • Added license to new files (if any)
  • Added Python wrapping to new files (if any) as described in ITK Software Guide Section 9.5
  • Added ITK examples for all new major features (if any)
AI assistance
  • Tool: Claude Code (claude-fable-5, with claude-opus-4-8 subagents)
  • Role: implemented the fix and regression test from the analysis previously filed as item B31 of BUG: Findings collected while porting ~60 filters — reported together in one issue #6575; hand-traced the failing scenario before implementation
  • Testing: built locally and verified the regression test fails before / passes after the fix; full ITKDisplacementField + ITKRegionGrowing test suites pass with the fix
  • Note: clang-format 19.1.7 was not available locally; formatting was hand-matched to ITK style and may need the CI formatter's touch-up

@github-actions github-actions Bot added type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots 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 Jul 10, 2026
@greptile-apps

greptile-apps Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes per-pixel residual handling in IterativeInverseDisplacementFieldImageFilter. The main changes are:

  • Resets smallestError inside the per-pixel loop.
  • Uses NumericTraits<double>::max() when the initial mapped point is outside the input buffer.
  • Adds a GoogleTest case for the outside-buffer first-guess refinement path.
  • Registers the new displacement-field GoogleTest driver in CMake.

Confidence Score: 5/5

Safe to merge with minimal risk.

The code change is narrow and keeps the existing inside-buffer residual path unchanged. The new tests cover the fixed outside-buffer behavior.

No files require special attention.

T-Rex T-Rex Logs

What T-Rex did

  • T-Rex ran the requested verification and completed the general contract validation, but local artifact references were not uploaded.

T-Rex Ran code and verified through T-Rex

Important Files Changed

Filename Overview
Modules/Filtering/DisplacementField/include/itkIterativeInverseDisplacementFieldImageFilter.hxx Moves smallestError into the per-pixel refinement loop with a max initial value so outside-buffer initial guesses do not reuse the previous pixel's residual.
Modules/Filtering/DisplacementField/test/CMakeLists.txt Adds a GoogleTest driver for the new displacement-field regression test source.
Modules/Filtering/DisplacementField/test/itkIterativeInverseDisplacementFieldImageFilterGTest.cxx Adds tests for outside-buffer first guesses and a zero-field sanity check.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant PixelLoop as Per-pixel loop
participant Interp as Input field interpolator
participant Descent as Coordinate descent
participant Output as Output pixel

PixelLoop->>PixelLoop: "Initialize smallestError = max()"
PixelLoop->>Interp: Check initial mappedPoint
alt Initial guess inside buffer
    Interp-->>PixelLoop: forwardVector
    PixelLoop->>PixelLoop: Set smallestError to current residual
else Initial guess outside buffer
    PixelLoop->>PixelLoop: Keep max() so first evaluable probe can win
end
PixelLoop->>Descent: Probe +/- step per dimension
Descent->>Interp: Evaluate in-buffer probes
Interp-->>Descent: probe residuals
Descent->>PixelLoop: "Accept probe when residual < smallestError"
PixelLoop->>Output: Store refined inverse displacement
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant PixelLoop as Per-pixel loop
participant Interp as Input field interpolator
participant Descent as Coordinate descent
participant Output as Output pixel

PixelLoop->>PixelLoop: "Initialize smallestError = max()"
PixelLoop->>Interp: Check initial mappedPoint
alt Initial guess inside buffer
    Interp-->>PixelLoop: forwardVector
    PixelLoop->>PixelLoop: Set smallestError to current residual
else Initial guess outside buffer
    PixelLoop->>PixelLoop: Keep max() so first evaluable probe can win
end
PixelLoop->>Descent: Probe +/- step per dimension
Descent->>Interp: Evaluate in-buffer probes
Interp-->>Descent: probe residuals
Descent->>PixelLoop: "Accept probe when residual < smallestError"
PixelLoop->>Output: Store refined inverse displacement
Loading

Reviews (2): Last reviewed commit: "BUG: Reset smallestError per pixel in It..." | Re-trigger Greptile

@hjmjohnson
hjmjohnson marked this pull request as draft July 10, 2026 11:49
IterativeInverseDisplacementFieldImageFilter refines each pixel of the
inverse field by coordinate descent, keeping the best probe found so far
in "smallestError". That variable was declared once outside the per-pixel
loop and only (re)assigned inside the branch that evaluates the error of a
pixel's initial guess, which runs only when the pixel's initial mapped
point lies inside the input buffer.

For a pixel whose initial mapped point falls outside the buffer, that
branch is skipped, so smallestError retained the residual left over from
the previous pixel. The coordinate-descent probes are individually
buffer-guarded, so a probe that lands back inside the buffer was compared
against that stale bar and accepted or rejected based on how well an
unrelated earlier pixel converged. The inverse field's values at such
pixels therefore depended on raster order.

Move the declaration inside the per-pixel loop and initialize it to
NumericTraits<double>::max(). When the initial guess is not evaluable the
error is unknown, so the bar is the worst possible value and the first
evaluable probe is accepted rather than measured against a stale error.
When the initial guess is evaluable, the existing inside-buffer branch
still resets the bar to that guess's error, so behavior there is
unchanged. The early-stop test against StopValue is likewise unaffected
(default StopValue is 0.0). NumericTraits is already reachable through the
itkMath.h include.

Add a deterministic regression test that builds a field whose late-raster
pixel maps just outside the buffer while a probe steps back inside, and a
preceding pixel that converges to a zero residual. With the defect the
refined output equals the zero-iteration first guess; with the fix it
moves. The test asserts the two differ and that a well-behaved pixel stays
finite and near zero.

Addresses item B31 of InsightSoftwareConsortium#6575
@hjmjohnson
hjmjohnson marked this pull request as ready for review July 10, 2026 13:30
@hjmjohnson
hjmjohnson merged commit a3f5b92 into InsightSoftwareConsortium:main Jul 10, 2026
22 checks passed
physwkim added a commit to physwkim/sitk-rs that referenced this pull request Jul 12, 2026
… pixel

double smallestError = 0 is declared outside the per-pixel loop and only
reassigned inside the IsInsideBuffer branch, so a pixel whose initial
mapped point falls outside the buffer inherits the previous pixel's
residual as the bar its probes must beat -- the output depends on raster
order. Reset it to f64::MAX at the top of every pixel (upstream fix PR
InsightSoftwareConsortium/ITK#6576), so an outside-start pixel searches
from a neutral bar. Filed as #6575 comment item B31.
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:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots 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