fix: index the domain-transfer LUT by label value, not by position - #116
Open
Hendrik-code wants to merge 1 commit into
Open
Hendrik-code wants to merge 1 commit into
Hendrik-code wants to merge 1 commit into
Conversation
Per-class weights came from `seg_region_masks(seg, max_regions=NC)`, which for a
single-channel label map orders its channels by the values that happen to be
present. `_accumulate` then indexes the bank's class axis by that position, so
which anatomy a class's transfer curve landed on depended on what else was in
the patch: measured on a synthetic bank, label 3 reached LUT class 1 in a patch
holding {0, 3} and class 3 in a patch holding {0, 1, 3, 2}.
The bank's class axis is a fixed taxonomy, so this is not a relabelling but a
mismatch, and it changed from batch to batch. Expanding the label map per value
makes channel c mean label c everywhere. One-hot callers are untouched.
Not run against the real bank: it is hundreds of megabytes, built offline and
deliberately not shipped (SMAUGLAB_DOMAIN_BANK is unset here). The test writes a
synthetic .npz in the bank's format whose class curves are constants, so which
class a voxel received reads straight off the output. What that pins is
stability -- a label reaches the same class whatever else is present -- which
holds whatever the bank's taxonomy turns out to be.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This branch has not been deployed
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.
The defect
Per-class weights came from
which, for a single-channel label map, orders its channels by the values that happen to be present, ascending.
_accumulatethen indexes the bank's class axis by that position:The bank's class axis is a fixed anatomical taxonomy, so which anatomy a class's transfer curve landed on depended on what else was in the patch — and changed from batch to batch.
Measured
On a synthetic bank whose class
cmaps every intensity to the constantc / NC, so the class each voxel received reads straight off the output:Same label, same anatomy, different transfer curve, decided by its neighbours.
The fix
Expand a single-channel label map per value (
seg.round() == arange(NC)) so channelcmeans labelceverywhere. One-hot callers keepseg_region_masksand are untouched.Not run against the real bank — stated plainly
The LUT bank is hundreds of megabytes, built offline and deliberately not shipped;
SMAUGLAB_DOMAIN_BANKis unset in this environment, so I could not exercise this against real data. The test writes a synthetic.npzin the bank's own format instead.What the test pins is stability — a label reaches the same class whatever else is present — which holds whatever the bank's taxonomy turns out to be. If the bank's class
cis not labelc, the mapping still needs an explicit table from whoever owns the bank; this PR makes the indexing deterministic, which it was not.Tests
unit_tests/test_domain_transfer_classes.py: a label must reach the same class regardless of which other labels are present (fails on the old code with 1.0 vs 3.0); each label maps to its own class when all are present (a control that passes either way, since position equals value there); output stays finite and correctly shaped.Verified failing before the fix and passing after. Full suite unchanged against baseline;
ruff check .clean.🤖 Generated with Claude Code