BUG: Fix MultiLabelSTAPLE label-loop wrap and undecided-label propagation

[physwkim]

MultiLabelSTAPLEImageFilter hangs forever on a saturated uint8 label space, and its voting-seeded confusion matrices count ties as label-0 consensus. Addresses B84 and B85 of #6575. Follows #6579 and #6597 in the same filter.

B84 — infinite loop (reproduced, not source analysis)

18 loop counters in itkMultiLabelSTAPLEImageFilter.hxx are bounded by m_TotalLabelCount (a size_t) but typed by the pixel type:

for (InputPixelType l = 0; l < this->m_TotalLabelCount; ++l)

With InputPixelType == unsigned char and all 256 label values in use, m_TotalLabelCount == 256, the counter wraps 255 -> 0, and the loop never terminates. The first one reached is in InitializeConfusionMatrixArrayFromVoting(), before InitializePriorProbabilities() or the E-M loop. This was hit as a live hang (process pinned at 100% CPU) while testing #6597 — it is a path #6597's new exception does not cover, because an explicit SetLabelForUndecidedPixels() is legal there.

All 18 counters are retyped to size_t, matching what m_TotalLabelCount actually is. itkLabelVotingImageFilter.hxx:125 already does this, which is why the sibling filter never had the bug. Anchor rg 'for \((InputPixelType|OutputPixelType|PixelType)\s+\w+\s*=' over Modules/Segmentation/LabelVoting/include/: 18 hits, all in this one file, all fixed; no sibling file has any.

B85 — voting-seed sentinel collides with a real label

InitializeConfusionMatrixArrayFromVoting() seeds the confusion matrices from an internal LabelVotingImageFilter, whose automatic undecided sentinel is m_TotalLabelCount cast to its output pixel type. Instantiated on TOutputImage, a saturated label space wraps that cast onto real label 0. Tie pixels then seed label-0 consensus, and the skip test

if (static_cast<size_t>(out.Get()) < this->m_TotalLabelCount)

never excludes them, because it only means "is this the undecided sentinel" while the sentinel happens to equal the auto-computed default.

The seed pass now votes into NumericTraits<InputPixelType>::AccumulateType — one step wider than the input — so the sentinel is always representable and distinct from every real label, and the skip test compares against the sentinel the voting filter actually used. LabelVotingImageFilter constrains its input to an unsigned integer (InputUnsignedIntCheck), so AccumulateType is always a wider unsigned integer here.

Note that the caller's SetLabelForUndecidedPixels() is deliberately not forwarded to the internal voting filter. The seed pass needs a sentinel disjoint from every real label, not the caller's output label; forwarding it reintroduces the collision whenever the caller picks a label that is also in use (e.g. SetLabelForUndecidedPixels(0)).

The intermediate voting image is AccumulateType rather than a fixed-width SizeValueType, keeping it at 2 bytes/voxel for the common uint8 case instead of 8.

Local validation

Four tests added to the existing itkMultiLabelSTAPLEImageFilterGTest.cxx:

• SaturatedUnsignedCharLabelCountDoesNotHang — runs the filter on a worker thread behind a watchdog, aborting rather than leaking a spinning thread into the rest of the driver. Fail-before: watchdog expires, still spinning.
• SaturatedLabelsWithoutExplicitUndecidedLabelThrows — a saturated space with no representable undecided label must refuse rather than wrap the default onto real label 0.
• SaturatedConsensusOnLabelZeroSeedsMatrixWhenUndecidedLabelIsZero — label-0 consensus must still seed the matrices when the caller maps undecided pixels to 0.
• ConsensusOnLabelZeroSeedsMatrixWhenUndecidedLabelIsZero — same invariant unsaturated; only genuine ties are excluded.

ctest -R "LabelVoting|MultiLabelSTAPLE": 14/14 pass, including #6579's VotingUndecidedPixelsDoNotCorruptSeededConfusionMatrix and the pre-existing itkMultiLabelSTAPLEImageFilterTest, unchanged. Both test drivers build without warnings.

Negative control, to confirm the new tests have teeth rather than merely passing: forcing VotingLabelType = InputPixelType makes SaturatedConsensusOnLabelZeroSeedsMatrixWhenUndecidedLabelIsZero fail and SaturatedUnsignedCharLabelCountDoesNotHang abort on its watchdog.

AI assistance

• Tool: Claude Code (claude-opus-4-8)
• Role: implementation and regression-test authoring from the B84/B85 analysis in BUG: Findings collected while porting ~60 filters — reported together in one issue #6575; selection of the seed-sentinel type
• Contribution: identified that forwarding the caller's undecided label to the seed pass reintroduces the label-0 collision, and that the sentinel must instead be carried in a type one step wider than the input
• All code was reviewed, built, and tested locally before committing

[greptile-apps]

Greptile Summary

This PR fixes two MultiLabelSTAPLEImageFilter label-handling paths.

• Retypes label-count loops from pixel types to size_t.
• Forwards the configured undecided label into the internal voting seed filter.
• Adds tests for saturated uint8 labels and custom undecided-label propagation.

Confidence Score: 4/5

The changed undecided-label seeding path can produce incorrect confusion matrices.

A caller-selected undecided value can collide with a valid label, causing the seed pass to drop real voting outputs for that label. The loop-counter changes otherwise match the intended saturated-label fix.

Modules/Segmentation/LabelVoting/include/itkMultiLabelSTAPLEImageFilter.hxx

T-Rex Logs

What T-Rex did

• The team attempted to validate the undecided label masking real votes by building a narrow C++ harness that uses ITK MultiLabelSTAPLEImageFilter with two raters agreeing on label 0 and zero EM iterations.
• They checked the available build tools and found cmake, ctest, and ninja unavailable, while make and g++ are present.
• They attempted to compile the harness directly against ITK source headers with g++, but the build failed because the generated ITK configuration header itkConfigure.h is missing from the source tree.
• They also tried preprocessing with the same ITK include paths, and it failed for the same missing itkConfigure.h before the ITK template code could run.
• Artifacts show tool availability and test-registration evidence, and a narrowed CTest invocation failed with ctest not found.

_{Ran code and verified through T-Rex}

Important Files Changed

Filename	Overview
Modules/Segmentation/LabelVoting/include/itkMultiLabelSTAPLEImageFilter.hxx	Updates the voting seed setup and label-count loops, but the new undecided-label comparison can skip valid votes when the undecided value is also a real label.
Modules/Segmentation/LabelVoting/test/itkMultiLabelSTAPLEImageFilterGTest.cxx	Adds tests for the saturated-label hang and custom undecided-label propagation.

_{Reviews (1): Last reviewed commit: "BUG: Propagate undecided label to MultiL..." | Re-trigger Greptile}

[hjmjohnson]

First force-push = plain rebase on main; second = review fix only (sentinel undecided label for the voting seed). Local ctest -R 'MultiLabelSTAPLE|LabelVoting': 7/7 pass.

[hjmjohnson]

Follow-up force-push = high-effort review fixes for the saturated-label path and the new tests.

What changed

• The voting seed now relies on LabelVotingImageFilter's own sentinel default (one past the last label) instead of duplicating it; the caller's undecided label is propagated only in the saturated case where no sentinel is representable.
• MultiLabelSTAPLEImageFilter::GenerateData() now throws (matching BUG: Undecided-vote label silently wraps to a real label #6597's design for LabelVotingImageFilter) when the label space saturates the pixel type and no explicit undecided label was set — previously the default silently wrapped onto real label 0, merging undecided output pixels into the background.
• Hang test: worker thread is joined on the passing path (no teardown race) and the deadline is 30 s to avoid flakes on instrumented/loaded CI executors.
• New SaturatedLabelsWithoutExplicitUndecidedLabelThrows regression test; comments trimmed of prior-behavior narration and internal finding codes.

Known limitation (documented, not fixable from the voting output alone): in the saturated case the seeding pass cannot distinguish a genuine consensus on the caller's undecided label from a tie, so those samples are excluded from the seed. The non-saturated path — every realistic case — now seeds exactly.

Local ctest -R 'MultiLabelSTAPLE|LabelVoting': 8/8 pass.

[hjmjohnson]

Force-push = plain rebase onto current main, folding the new GTest source into the GTests list that #6589 introduced for this module (two creategoogletestdriver calls in the CI merge build caused/would cause a duplicate-target configure error). Verified locally: configure clean, module tests pass.

[hjmjohnson]

Force-push = rebase onto current upstream/main, resolving two real merge conflicts. ctest -R 'MultiLabelSTAPLE|LabelVoting': 13/13 pass.

Why there were conflicts

This branch predates 23a4da9cd4e (BUG: Undecided-vote label silently wraps to a real label, #6575 B15), which has since merged to main via a different route. Both that commit and this PR's own BUG: Type MultiLabelSTAPLE label loops by count, not pixel (B84) independently added new TEST() cases immediately after the same pre-existing test — a parallel-insertion conflict, not semantic duplication: the two sets of tests cover different bugs (B15 representability vs. B84 loop-wrap). Resolution: kept both test sets. A second, trivial conflict was two competing one-line doc-comments above the same test; kept the one citing the specific issue/item number (B84).

One substantive-but-small conflict: this PR's own second commit had already tightened this->m_TotalLabelCount > itk::NumericTraits<OutputPixelType>::max() to this->m_TotalLabelCount > static_cast<size_t>(NumericTraits<OutputPixelType>::max()), since m_TotalLabelCount is size_t after the B84 fix. Kept this PR's more explicit cast over upstream's version of the same guard.

[hjmjohnson]

Content-only force-push (a8e390a → 9eff1e7; plain rebase kept separate — range-diff shows commit 1 identical, only commit 2 changed): fixes a confusion-matrix seeding bug in the saturated label space, hardens the hang-guard test, and trims a comment.

Undecided-sentinel collision (dominant fix). In the saturated path (m_TotalLabelCount > max(OutputPixelType), reachable with a single label-255 pixel + SetLabelForUndecidedPixels(0)) the voting-undecided label was set to a real caller label, so out.Get() != votingUndecidedLabel silently excluded genuine label-0 consensus pixels from confusion-matrix seeding — biasing the STAPLE estimate for that label. Root-caused to the sentinel being forced into OutputPixelType. Fixed by voting into a wide SizeValueType image so the sentinel (maxLabel+1) is always representable and never collides; the saturated-case caller-label workaround (and its 3-line comment) is no longer needed. Non-saturated behavior is bit-identical. New regression test SaturatedConsensusOnLabelZeroSeedsMatrixWhenUndecidedLabelIsZero fails without the fix.

Hang-guard test. On timeout the test detached the worker, leaking a CPU-spinning thread that could corrupt later tests in the same binary. A stuck Update() can't be killed portably, so it now aborts loudly instead.

Note on the pre-existing ConsensusOnLabelZero test

The existing ConsensusOnLabelZeroSeedsMatrixWhenUndecidedLabelIsZero uses labels {0,1} → m_TotalLabelCount = 2, which is not saturated, so it never took the collision branch and gave false confidence. The new test uses a label-255 pixel to actually saturate unsigned char.

Local validation

New test fails on the prior code (cm(0,0) = 0.0), passes after the fix (1.0). MultiLabelSTAPLEImageFilter GTest 7/7; ctest -L ITKLabelVoting 9/9, zero regressions. pre-commit run --all-files exit 0.

[hjmjohnson]

Force-push 9eff1e7 → 1ed9e6f = plain rebase onto latest upstream/main (36 commits), no content change — git range-diff shows both commits identical (=). Now 0 behind.

[hjmjohnson]

Use "VotingLabelType" instead of the size_t.

[hjmjohnson]

Left as size_t here, deliberately. size_t is m_TotalLabelCount's declared type, which is the point of commit 1 (BUG: Type MultiLabelSTAPLE label loops by count, not pixel): the counter should follow the bound, not the pixel. VotingLabelType is pixel-derived, so a counter typed on it has the same shape as B84 — it is safe in practice (unsigned short reaches 257 for uint8 input, unsigned int reaches 65537 for uint16) and only wraps for uint32 input, which needs a confusion matrix of ~10^20 bytes and is unreachable. But it reads as a counterexample to the commit sitting directly beneath it.

VotingLabelType's job is narrower: carry the voting sentinel in a type where it cannot collide with a real label. Scoping it to that keeps the two concerns separate.

Happy to switch if you would rather have the consistency within the function — say the word and it is a one-line change.