Repository navigation
BUG: Fix WarpImageFilter fast path and displacement exponential scale (#6575 batch 2) - #6669
Closed
hjmjohnson wants to merge 9 commits into
Closed
hjmjohnson wants to merge 9 commits into
hjmjohnson wants to merge 9 commits into
Conversation
The fast path iterated the displacement field over the output thread region, but the gate compared only origin, spacing and direction. A legitimately larger output over a smaller matching field died in the iterator containment check instead of taking the general clamped path. Issue InsightSoftwareConsortium#6575, item B34.
Measured on linear anisotropic fields (spacing ratios 4:1 and 100:1), per-axis steps converge no better than the annealed axis-0 step, so the behavior is documented rather than changed. Issue InsightSoftwareConsortium#6575, item Q25.
SetDerivativeWeights turns UseImageSpacing off, so the effective precedence is last-writer-wins; the class doc described only one ordering. Issue InsightSoftwareConsortium#6575, item Q26.
vnl_matrix_inverse::determinant_magnitude() is a product of singular values and cannot be negative. Also document that GetCovariance() returns the matrix passed in even when evaluation substitutes a diagonal inverse for a singular covariance. Issue InsightSoftwareConsortium#6575, item Q29.
The first-order scale 2^numiter was computed as 1 << numiter, which is undefined behavior for 31 or more iterations and reachable through SetMaximumNumberOfIterations with automatic iterations off. Use ldexp. Also select zero squaring iterations for a zero field: the previous sentinel, NumericTraits::min(), is a tiny positive value, so the keep-zero branch was unreachable, and integral iteration estimates overshot the documented ceil by one. Issue InsightSoftwareConsortium#6575, items B35 and Q31.
The 2D sibling and the strict-topology checks iterate the label image over its buffered region; the 3D check used the never-set requested region. Issue InsightSoftwareConsortium#6575, item Q38.
Notes for issue InsightSoftwareConsortium#6575 items Q30 (FastSymmetricForcesDemonsRegistration filter uses ESMDemonsRegistrationFunction, not its namesake function), Q32 (WarpVectorImageFilter EdgePaddingValue unused with extrapolating interpolators), Q33 (gradient options coincide at zero displacement), Q34/Q35/Q37 (level-set-motion step, gradient and smoothing caveats), and Q36 (zero-iteration metric/RMS mismatch).
github-actions
Bot
added
type:Bug
This was referenced Jul 21, 2026
Merged
Member
Author
|
Superseded — split into three focused PRs so the documentation/style dispositions land independently of each bug fix (only the relevant one need be reverted if an issue surfaces):
Each bug PR's new GTest was rebuilt and run locally on current |
hjmjohnson
added a commit
that referenced
this pull request
Jul 22, 2026
DOC: Displacement-field/warp/demons quirk dispositions Q25-Q38 (split of #6669)
hjmjohnson
added a commit
that referenced
this pull request
Jul 23, 2026
BUG: Include region in WarpImageFilter same-information fast path (B34, split of #6669)
hjmjohnson
added a commit
that referenced
this pull request
Jul 23, 2026
BUG: Compute displacement exponential scale without undefined shift (B35, split of #6669)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes ledger items B34 and B35 plus quirks Q25–Q38 from the displacement-field / warp / demons audit in #6575: the
WarpImageFiltersame-information fast path now includes the region in its equality test, andExponentialDisplacementFieldImageFiltercomputes its scale without an undefined1 << numitershift. Documentation-only commits record the remaining quirk dispositions.Bug fixes
WarpImageFilterfast path: the "same information" test compared origin/spacing/direction but not size, so a valid larger-output configuration took the fast path and died in an iterator containment check instead of the general resampling path. The region is now part of the test; new GTestWarpImageFilter.LargerOutputThanFieldDoesNotThrowfails before / passes after.ExponentialDisplacementFieldImageFilter: the field was divided by1 << numiter, undefined behavior fornumiter >= 31. Unreachable by default (automatic cap 20), but explicitSetMaximumNumberOfIterations(>= 31)made it live. The scale is now computed in floating point; new GTestsThirtyOneIterationsScaleCorrectlyandZeroFieldStaysZero.Quirk dispositions (Q25–Q38, DOC/STYLE commits)
Local testing
Rebased on upstream/main (37ecafc).
pre-commit run --all-filesclean. All 7 affected tests pass locally (2 new GTest suites + itkExponentialDisplacementFieldImageFilterTest, itkWarpImageFilterTest, itkWarpImageFilterTest2, itkWarpVectorImageFilterTest).