ENH: Add MinMaxCurvatureFlow alternate baselines for ITK 3D threshold fix - #2627
Merged
blowekamp merged 1 commit intoAug 28, 2026
Merged
Conversation
Member
Please create a PR for SimpleITK/SimpleITKExternalData with the data. I am unfortunately unable to locate developer documentation on uploading data. Hopefully making the PR to the data repo is straight forward enough. |
Member
|
@hjmjohnson Please see the newly added sitk-upload-binary-data skill to upload data. |
Contributor
Author
|
Related to : InsightSoftwareConsortium/ITK#6667 for minimizing inter-project dependancies on test cases. |
hjmjohnson
force-pushed
the
update-minmax-baselines-itk6619
branch
from
July 24, 2026 16:47
72a81b9 to
1cb3913
Compare
Contributor
Author
|
Data uploaded per the sitk-upload-binary-data skill: SimpleITK/SimpleITKExternalData#50 (draft). The previous content-link hashes were transcribed and did not match any real file; the branch is now rebased onto |
hjmjohnson
marked this pull request as ready for review
July 24, 2026 16:55
blowekamp
force-pushed
the
update-minmax-baselines-itk6619
branch
from
August 27, 2026 21:54
1cb3913 to
6654ed2
Compare
… fix
ITK commit 2a04261664 ("BUG: Fix polar angle in MinMaxCurvatureFlowFunction
threshold", merged as InsightSoftwareConsortium/ITK#6603,
InsightSoftwareConsortium/ITK#6575 item context in #6619) fixes
MinMaxCurvatureFlowFunction::ComputeThreshold normalizing the gradient
to the stencil radius (2) instead of unit length before acos(), which
had silently selected the wrong polar angle since 2002. This changes
MinMaxCurvatureFlowImageFilter output on 3D volumes.
Add the post-fix center slices as .1.nrrd alternate baselines so the
defaults and longer tests pass with both current and fixed ITK.
Verified locally: BasicFilters.MinMaxCurvatureFlowImageFilter and
Python.MinMaxCurvatureFlowImageFilter both pass against an ITK build
containing the fix, using the corrected .1.nrrd content hashes (see
below).
Note: BinaryMinMaxCurvatureFlowFunction never calls ComputeThreshold,
so it is unaffected by ITK#6603 itself. However, it inherits
MinMaxCurvatureFlowFunction's stencil radius (2) and is therefore also
perturbed by a separate, later ITK commit, 4f0b3147be ("BUG: Decouple
CurvatureFlowFunction derivative scale from radius",
InsightSoftwareConsortium/ITK#6575 item B8), which fixes
CurvatureFlowFunction::ComputeUpdate() understating the derivative by
1/radius^2 for any subclass that repurposes GetRadius() for a larger
neighborhood, as MinMaxCurvatureFlowFunction (and by inheritance
BinaryMinMaxCurvatureFlowFunction) does. BasicFilters.
BinaryMinMaxCurvatureFlowImageFilter's defaults/longer baselines will
need their own alternates for that fix; tracked as a follow-up, not
included here.
Fixup: the defaults.1.nrrd and longer.1.nrrd content-link hashes in an
earlier revision did not match the objects actually uploaded to
SimpleITK/SimpleITKExternalData#50 (stale/transcribed hashes that
resolved on no mirror), which aborted the whole superbuild at the
ExternalData fetch step and cascaded into unrelated Java/R test
failures. Corrected to the real uploaded object hashes, confirmed
against the S3 mirror and validated locally as above.
blowekamp
force-pushed
the
update-minmax-baselines-itk6619
branch
from
August 28, 2026 00:14
6654ed2 to
39d5053
Compare
blowekamp
self-requested a review
August 28, 2026 17:31
blowekamp
approved these changes
Aug 28, 2026
4 tasks
blowekamp
added a commit
that referenced
this pull request
Aug 29, 2026
ITK commit 4f0b3147be2 ("BUG: Decouple CurvatureFlowFunction
derivative scale from radius", ITK issue #6575 item B8) fixed
CurvatureFlowFunction::ComputeUpdate(): it always samples immediate
(1-pixel-stride) neighbors, but scaled derivatives by dividing by
GetRadius(). Any subclass repurposing the radius for something else --
such as MinMaxCurvatureFlowFunction's stencil radius -- understated
the update by 1/radius^2 (4x at the default stencil radius of 2).
BinaryMinMaxCurvatureFlowFunction::ComputeUpdate() inherits this code
path unmodified (calls straight through to
CurvatureFlowFunctionType::ComputeUpdate), and SimpleITK's
BinaryMinMaxCurvatureFlowImageFilter defaults StencilRadius to 2, so
it hits the same bug every step. This is a sibling bug to the one
already fixed in #2627 for the non-binary MinMaxCurvatureFlowImageFilter,
but via a different ITK commit -- #2627 correctly noted Binary was
unaffected by *its* fix (the ComputeThreshold polar-angle bug), since
this is a separate commit.
Adds .1.nrrd alternate baselines (ImageCompare picks best-RMS across
the name.N.nrrd series) so the defaults/longer tests pass against both
pre- and post-fix ITK, without touching the primary baselines. Data
staged in SimpleITK/SimpleITKExternalData#54.
Verified the harness output is:
- byte-identical (sha512) across two separate runs
- pixel-identical between gzip-compressed and uncompressed NRRD
encodings
- pixel-identical across C++, Python, and Java bindings
All BasicFilters/Python/Java tests pass locally against ITK main.
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
2 participants
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.
Adds
.1.nrrdalternate baselines for theMinMaxCurvatureFlowImageFilterdefaultsandlongertests, so they pass with both current ITK and an ITK containing InsightSoftwareConsortium/ITK#6603 (3D polar-angle threshold fix; full context in InsightSoftwareConsortium/ITK#6619).Data: the two objects are staged in SimpleITK/SimpleITKExternalData#50 — CI cannot fetch them until that PR merges.
Why the output changes, and by how much
ITK's 3D
MinMaxCurvatureFlowFunction::ComputeThresholdnormalized the gradient to lengthStencilRadius(2) instead of unit length beforeacos(), silently selecting the wrong polar angle since 2002. ITK#6603 restores the published algorithm (Sethian 1999). Measured onRA-Float.nrrdwith this repo's exact test parameters, the center-slice RMS versus the current baselines is 343.0 (longer) against a tolerance of 0.01. The 2D dispatch andBinaryMinMaxCurvatureFlowImageFilter(user-supplied threshold, never callsComputeThreshold) are unaffected — their baselines need no change.How the alternates were generated and validated
The baselines are the actual harness outputs:
BasicFilters.MinMaxCurvatureFlowImageFilterwas run via ctest against an ITK main build containing the threshold fix, and the failing-compare outputs written byImageCompare(center z-slice, 64×64×1 float NRRD) were hashed directly, byte-identical across two runs. With the alternates placed in the ExternalData store, the test passes against ITK main; the primary baselines are untouched, so current-ITK builds keep passing viaImageCompare's best-RMS selection over thename.N.nrrdseries (sitkImageCompare.cxx). The generated test commands reference baselines asDATA{...nrrd,:}, so the.1.nrrdseries members are fetched automatically — no CMake changes needed.An earlier revision of this PR carried transcribed hashes that did not correspond to any real file; the content links now hold the verified hashes of the uploaded objects.