Repository navigation
feat: augmentation-consistency Dice loss for domain generalization - #134
Open
Hendrik-code wants to merge 3 commits into
Open
Hendrik-code wants to merge 3 commits into
Hendrik-code wants to merge 3 commits into
Conversation
Heavy appearance augmentation never tells the network that the augmented and the original patch are the same anatomy and should get the same segmentation. This adds an optional second loss term that does: both go through the network and their predictions are compared with a soft Dice. The reference is the output of the geometric stage, not the dataloader's patch. The geometric transforms are the only ones that move voxels, so running them once and sharing the result is what makes the two images voxel-aligned and lets one segmentation be correct for both branches. `build_gpu_stages` does the split; `build_gpu_pipeline` is now a thin wrapper over it, so the unsplit path cannot diverge, and `_split_shared` refuses to hoist anything that is not already a prefix of the pipeline rather than silently reordering it. `RandomCropTransformGPU` moves from `AugType.GEO` to a new `AugType.OCC`. It occludes the field of view instead of moving voxels, and `PIPELINE_ORDER` puts it near the end for a reason: in front of the intensity stage, the flat slab it leaves behind gets texture painted into it, which is the failure its own `pad_value` docstring describes. `build.SEQUENTIAL_GROUPS` lists OCC beside GEO so nothing about the built pipelines changes -- `test_builder.TestShippedPipelinesAreUnchanged` hashes all 84 (config, mode) pairs to prove it -- while `build.SHARED_GROUPS` leaves it out of the shared stage. Configured by a new top-level `loss` section, off by default, validated like `pipeline` is. A null setting is rejected rather than read as "use the default": `loss_options` merges the block over `LOSS_DEFAULTS`, so `"weight": null` would reach the trainer as None and `"enabled": null` would quietly turn the term off while the recorded config looked like it asked for it. The term reuses `MemoryEfficientSoftDiceLoss`, not `SoftDiceLoss`: on nnunetv2 2.6 `get_tp_fp_fn_tn` builds `y_onehot` as a bool tensor and computes `~y_onehot`, which is a TypeError on the soft target this needs. Only the full-resolution head takes part, and the epoch summary prints the component on its own line because `train_loss` now carries both terms. Known interaction, warned about at startup and documented in the README: the crop is not applied to the reference, so in the batches where it fires the term asks the network to agree with the reference inside the occluded region, where the supervised target says background. Nothing masks it; `p: 0` avoids it. Verified: full suite in two environments with no new failures, and an end-to-end run of the real trainer on Dataset400_nose_nosemain confirming the total is the supervised loss plus weight times the term, and that `weight: 0` measures it while changing nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Both sides add to `nnUNetTrainerDAExt.py` and the trainer tests, and the four conflicts are additive: main's AMP gradient-scale floor and this branch's consistency term are independent guards. `train_step` now calls `_guard_grad_scale()` after `grad_scaler.step` and adds the consistency term after `self.loss`, and `on_train_epoch_end` reports all three summaries. The one real resolution is in the tests: main extended the inline trainer stub in `TestNonFiniteLossGuard` with the two scale-floor counters, while this branch had already lifted that stub into a module-level `bare_trainer` so three test classes could share it. The counters move into `bare_trainer`, which is what it is for. Main's narrower `scaler_trainer` stub is untouched -- it only drives `_guard_grad_scale`, which reads none of the consistency attributes. 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.
What does this change?
Adds an optional second loss term to
nnUNetTrainerDAExtGPU: the augmented patch and the unaugmented one both go through the network, and their predictions are compared with a soft Dice. Off unless a config asks for it.{ "loss": { "consistency": { "enabled": true, "weight": 0.5, "detach_reference": true } }, "GPU": { "...": {} } }The reference is taken after the geometric transforms and before the intensity ones. The geometric group is the only one that moves voxels, so running it once and sharing the output is what makes the two images voxel-aligned and lets a single segmentation be correct for both branches.
Mechanics:
build_gpu_stages(..., split_shared=True)returns(shared, augmented).build_gpu_pipelineis now a thin wrapper over it (shared + augmented), so the unsplit path cannot drift._split_sharedraises rather than hoisting anything that is not already a prefix of the pipeline.AugType.OCC, holdingRandomCropTransformGPU.build.SEQUENTIAL_GROUPSlists it besideGEOso no built pipeline changes;build.SHARED_GROUPSleaves it out of the shared stage.lossconfig section, validated likepipelinewith difflib suggestions. It is insideconfig_hash, so enabling the term gives a new...-c-<hash>run folder, and_record_configalready copies it into the run as provenance.MemoryEfficientSoftDiceLosswith the trainer's ownbatch_dice/smooth/ region settings, so the two Dice terms in the sum are computed identically.Why?
Nothing in the current objective expresses the invariance the augmentations are there to produce. This states it directly as a loss.
Why the crop is not
GEORandomCropTransformGPUoccludes the field of view rather than translating contents, andPIPELINE_ORDERputs it near the end for a reason: run in front of the intensity stage, the flat fill it leaves behind gets texture painted into it — the failure its ownpad_valuedocstring describes for PALETTE and the edge filters. A newAugTyperather than an exclusion list because the group is what_bucketsalready reads, and becauseGEOnow means exactly "moves voxels", which is the property the split depends on.Why not
SoftDiceLossOn nnunetv2 2.6,
get_tp_fp_fn_tnbuildsy_onehotas a bool tensor and computesfp = net_output * (~y_onehot). A soft target of matching shape takes they_onehot = gtbranch and~gtraisesTypeError: ~ (operator.invert) is only implemented on integer and Boolean-type tensors.MemoryEfficientSoftDiceLoss, from the same module and the one_build_lossitself uses, only ever doesy_onehot.sum(axes)andx * y_onehot, so it takes the soft target directly. (2.4.2 used1 - y_onehotand would have worked — the kind of upstream driftpyproject.tomlalready documents.)How was it tested?
pytestpasses locallypre-commit run --all-filespassesUnit tests — 50 new tests, 190 new subtests.
test_consistency_loss.py(new)verdict=consistencyNaN report; that chaining the two stages reproduces the single container exactly on the shipped config; the guardrailstest_builder.pytest_config_schema.pylosssection: defaults, copies, difflib suggestions, type and range checks, and that a JSONnullis rejected rather than read as "use the default"test_registry.py,test_cli.pysmauglab list --group occworksThe pipeline-structure hash was captured on
mainbefore any edit and is unchanged after, so nothing about the existing pipelines moved.Two environments, because this machine's default interpreter has nnunetv2 2.4.2:
seg11nnunet11The 9 are the documented
>=2.5signature mismatch; the 2 are abatchgeneratorsv2RemoveLabelTansform→RemoveLabelTransformrename. Both sets are identical before and after this change.mypy smauglab/clean (locally 2.1.0; the pin is 2.4.0, so CI is the real check).End to end, real trainer and dataloader,
Dataset400_nose_nosemain3d_fullres fold 0, one GPU:Total equals supervised plus
weighttimes the term on every step, andweight: 0measures and logs the term while leaving the loss bit-identical to the supervised value. A same-config control established that this setup is not reproducible run to run (the augmented batch differs by up to 1e1 between two identical runs), so an on-vs-off batch comparison would have said nothing; bit-identity of the split is pinned deterministically on CPU in the unit tests instead.Anything reviewers should look at closely?
p, and the README says to setp: 0. Masking the term to the kept box is possible — a ones-channel through the crop's mask path gives the exact indicator — but it was deliberately left out of this PR._build_lossweights, includingweights[-1] = 0and its DDP-dependent1e-6variant; giving the consistency term a second weighting scheme would duplicate a decision that belongs upstream..clone()of the reference. It is an alias of the geometric stage's output, justified bytest_no_inplace_mutation.pyplus a new test that runs the shipped configuration rather than default-constructed transforms.train_lossis no longer comparable across the switch, since it now carries both terms. Hence the separate epoch line.nnUNetLogger.logasserts the key already exists inmy_fantastic_logging, so a proper curve would need an upstream patch.registry.names(Backend.GPU, group=AugType.GEO),filter_by_group(..., AugType.GEO)andsmauglab list --group geono longer returnRandomCropTransformGPU; it is underocc. Inrandom_ordermode a crop-enabled config would also now follow the shuffled buckets instead of leading — no shipped config combines the two (the three*-List*.jsonuserandom_orderand have no crop; all four crop configs are sequential), so no published run is affected.Checklist
loss.consistencysmauglab matrix --write; one cell,crop | GEO→crop | OCC).all_augmentations.jsonis unchanged, asrender_templatedoes not read the groupCONTRIBUTING.mdtest table gained a row🤖 Generated with Claude Code