Skip to content

BUG: Fix polar angle in MinMaxCurvatureFlowFunction threshold#6603

Merged
hjmjohnson merged 1 commit into
InsightSoftwareConsortium:mainfrom
physwkim:bug-minmax-curvature-flow-acos-6575
Jul 13, 2026
Merged

BUG: Fix polar angle in MinMaxCurvatureFlowFunction threshold#6603
hjmjohnson merged 1 commit into
InsightSoftwareConsortium:mainfrom
physwkim:bug-minmax-curvature-flow-acos-6575

Conversation

@physwkim

@physwkim physwkim commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

The 3-D min/max threshold rescaled the gradient to length StencilRadius and then passed a component of it to acos(), which needs a unit-length cosine. Addresses B7 of #6575.

Root cause

MinMaxCurvatureFlowFunction::ComputeThreshold(const Dispatch<3> &, ...) (itkMinMaxCurvatureFlowFunction.hxx:311):

gradMagnitude = std::sqrt(gradMagnitude) / static_cast<PixelType>(m_StencilRadius);
...
theta = std::acos(gradient[2]);   // :326

After that division each gradient[k] is r * cos(theta_k), not cos(theta_k). The clamp to [-1, 1] just below hides the domain violation, so no NaN appears — the function silently returns the wrong polar angle. With the default StencilRadius of 2, a true cos(theta) = 0.4 (theta = 66.4 deg) is evaluated as acos(0.8) = 36.9 deg, which puts the stencil's perpendicular sample points on the wrong ring and selects the wrong min/max curvature update.

Normalizing to unit length is the fix: the downstream position reconstruction already multiplies by m_StencilRadius explicitly. The 2-D dispatch (:246) shares the rescale but reaches atan2 on a ratio, where the length cancels — no defect there, left alone.

Local validation

New GoogleTest itkMinMaxCurvatureFlowFunctionGTest.cxx, registered in a new ITKCurvatureFlowGTests driver.

  • MinMaxCurvatureFlowFunction.ComputeUpdateUsesCorrectPolarAngleAtRadiusTwo — fail-before: result = 1 (the buggy angle selects the wrong branch). Pass-after: result = 0. Both values were derived from the test field's geometry before running.
  • ctest -R CurvatureFlow: itkMinMaxCurvatureFlowImageFilterTest, itkBinaryMinMaxCurvatureFlowImageFilterTest, itkCurvatureFlowTesti all pass before and after. They are 100-iteration convergence checks with tolerances, so they do not pin the per-step angle.
AI assistance

Historical context for the removed division and the measured downstream impact (SimpleITK baselines) are documented in #6619.

@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 13, 2026
@greptile-apps

greptile-apps Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes the 3D min/max curvature threshold angle calculation. The main changes are:

  • Unit-normalizes the 3D gradient before computing the polar angle.
  • Keeps the existing stencil-radius scaling in the 3D sample reconstruction.
  • Adds a focused GoogleTest regression for the radius-two update path.
  • Registers the new CurvatureFlow GoogleTest driver.

Confidence Score: 5/5

This looks safe to merge.

No blocking issues found in the changed code.

T-Rex T-Rex Logs

What T-Rex did

  • The build command completed successfully with exit code 0, and Ninja reported no work to do, indicating a clean reusable build tree.
  • The focused CTest run completed successfully with exit code 0 and reported 100% tests passed (3 tests in total).
  • Artifacts documenting the build and test run, including commands, directories, timestamps, and verbose outputs, were captured for review.

View all artifacts

T-Rex Ran code and verified through T-Rex

Important Files Changed

Filename Overview
Modules/Filtering/CurvatureFlow/include/itkMinMaxCurvatureFlowFunction.hxx Normalizes the 3D threshold gradient to unit length before spherical angle calculation.
Modules/Filtering/CurvatureFlow/test/CMakeLists.txt Adds the new CurvatureFlow GoogleTest source list and driver registration.
Modules/Filtering/CurvatureFlow/test/itkMinMaxCurvatureFlowFunctionGTest.cxx Adds a direct regression test for the corrected radius-two 3D update decision.

Reviews (1): Last reviewed commit: "BUG: Fix polar angle in MinMaxCurvatureF..." | Re-trigger Greptile

The 3-D threshold gradient was rescaled to length StencilRadius
before acos(), instead of to unit length. For radius >= 2 this
returns the wrong polar angle, misplacing the perpendicular sample
points used to pick the min/max curvature update.

Addresses item B7 of InsightSoftwareConsortium#6575.
@hjmjohnson hjmjohnson merged commit 297af6b into InsightSoftwareConsortium:main Jul 13, 2026
21 checks passed
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