Skip to content

BUG: Guard empty re-estimation pass in ConfidenceConnected filters#6578

Merged
hjmjohnson merged 1 commit into
InsightSoftwareConsortium:mainfrom
physwkim:bug-confidence-connected-empty-reestimation
Jul 10, 2026
Merged

BUG: Guard empty re-estimation pass in ConfidenceConnected filters#6578
hjmjohnson merged 1 commit into
InsightSoftwareConsortium:mainfrom
physwkim:bug-confidence-connected-empty-reestimation

Conversation

@physwkim

Copy link
Copy Markdown
Contributor

Addresses item B33 of #6575.

VectorConfidenceConnectedImageFilter re-estimates the mean and covariance from the previous pass's segmentation each iteration, but the Mahalanobis threshold is fixed before the loop (the seed-inclusion boost runs pre-loop only). Because the statistics move while the threshold does not, a pass can re-estimate statistics that exclude even the seeds; that pass's flood segments nothing, and the next pass counts num == 0 samples and divides by it. The resulting NaN mean makes every Mahalanobis squared distance NaN, which MahalanobisDistanceThresholdImageFunction clamps to a distance of 0.0 (NaN fails its > 0 test), and 0.0 <= threshold holds for any positive threshold — the next flood includes every pixel in the image.

The scalar sibling ConfidenceConnectedImageFilter has the same unguarded division in its re-estimation loop (both filters do guard the equivalent empty case in their initial statistics). Its failure mode is self-limiting — NaN bounds make every comparison false and the output goes silently empty — but the division is equally meaningless.

The fix adds the missing guard to both filters: when a re-estimation pass finds zero samples, stop iterating instead of dividing by zero. The segmentation is left exactly as the last completed flood produced it — the converged answer under the fixed threshold — and the previously estimated statistics remain finite.

The new regression test engineers the empty pass deterministically (single outlier seed against a uniform background; hand-traced pass by pass in the test's header comment): the pre-loop flood covers the whole image, the first re-estimation moves the mean far enough that the seed fails the fixed threshold (empty flood), and the next pass hits num == 0. Unfixed, the final output floods all 256 pixels; fixed, the filter stops at the converged empty segmentation with NaN-free statistics.

Verified locally (Linux, GCC 13, Release): the new test fails on unmodified main (Segmented pixels: 256 of 256) and passes with the fix; all 12 ITKRegionGrowing and 24 ITKDisplacementField 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 B33 of BUG: Findings collected while porting ~60 filters — reported together in one issue #6575; hand-traced the engineered empty-pass scenario before implementation
  • Testing: built locally and verified the regression test fails before / passes after the fix; full ITKRegionGrowing + ITKDisplacementField 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:Segmentation Issues affecting the Segmentation module labels Jul 10, 2026
@greptile-apps

This comment was marked as resolved.

@hjmjohnson hjmjohnson marked this pull request as draft July 10, 2026 11:49
The ConfidenceConnected and VectorConfidenceConnected filters fix the
threshold before the iteration loop (to the seed-inclusion-boosted
multiplier) but re-estimate the mean/covariance from the previous pass's
segmentation every iteration. Because the statistics move while the
threshold does not, a re-estimated pass can produce statistics that
exclude even the seed pixel; that pass's flood then segments nothing,
and the following pass counts zero samples. The accumulation loops then
divided the summed mean/covariance by that zero count with no guard.

In the vector filter this makes the mean NaN, which makes every
Mahalanobis squared distance NaN;
MahalanobisDistanceThresholdImageFunction treats a non-positive (and NaN
is not greater than zero) squared distance as distance 0.0, which passes
"<= threshold" for every pixel, so the next flood includes the ENTIRE
image reachable from the seeds. In the scalar filter the same missing
guard makes the mean/variance NaN, the threshold bounds NaN, and every
comparison false, silently emptying the output; the division is
meaningless either way.

Guard the re-estimation loop in both filters: when the previous pass
segmented no pixels there is nothing to re-estimate from, so break out
of the iteration loop. This leaves the segmentation exactly as the last
completed flood produced it (empty), which is the converged answer under
the fixed threshold.

Add a deterministic regression test
(itkVectorConfidenceConnectedImageFilterEmptyReestimationTest) that
engineers a single-outlier vector image where pass 0's re-estimated mean
moves off the seed, pass 0's re-flood segments nothing, and the next
pass would divide by zero. It asserts the output is not the whole image
(exactly the empty converged segmentation) and that the re-estimated
mean/covariance contain no NaN. The scalar guard prevents the same
division-by-zero; its self-limiting empty-output failure mode does not
warrant a second engineered scenario.

Addresses item B33 of InsightSoftwareConsortium#6575
@hjmjohnson hjmjohnson force-pushed the bug-confidence-connected-empty-reestimation branch from d9fc995 to 58a0789 Compare July 10, 2026 12:06
@hjmjohnson hjmjohnson marked this pull request as ready for review July 10, 2026 13:13
@hjmjohnson hjmjohnson merged commit 92a3d53 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
…pass

The re-estimation loop divides mean/covariance by num with no num == 0
guard (unlike the initial statistics' seed_cnt == 0 guard). When a pass's
mask is empty -- reachable because the multiplier is fixed pre-loop while
mean/covariance drift each round -- num == 0 makes the statistics NaN,
which clamps every Mahalanobis distance to 0 and floods the whole image.
Stop the loop at num == 0, keeping the previous pass's statistics and
mask (upstream fix PR InsightSoftwareConsortium/ITK#6578). The scalar
confidence_connected's num == 0 is already unreachable. Filed as #6575
item B33.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:Segmentation Issues affecting the Segmentation 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