Repository navigation
Finish in_seg / out_seg: where an augmentation applies, at what granularity, and how sharply it stops - #138
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
Nothing calls it yet. The parameters it serves -- `in_seg`, `out_seg` and `mix_in_out` -- expressed about a third of what they looked like they expressed, and this is the one implementation the whole group will share, in the shape `gpu/mixing.py` established for `mix_prob` / `mix_alpha`. What it adds over the three helpers in gpu/contrast.py it will replace: * `in_seg` and `out_seg` both firing now means a *mixture* of foreground and background rather than falling through to "the whole patch", so `in_seg=1, out_seg=1` stops being indistinguishable from `in_seg=0, out_seg=0`. * `seg_unit` / `seg_unit_prob` replace `mix_in_out`, which keyed its keep/drop draw on the mask's *channel* axis. On the single-channel label map nnU-Net's trainer passes that is one draw per sample, deleting the whole mask half the time -- so the thirteen shipped configs setting it have been running a coin flip rather than a label mix. The replacement switches whole label values or whole connected components. * `bg_split` / `bg_em_clusters` let the background be split into intensity-coherent subregions by EM (SynthSeg sec. 5.4) so `out_seg` can apply to part of it instead of all or none. `em_subdivide_labels` does the work; this wraps it. * `seg_feather` turns the composite weight from a binary step into a Gaussian ramp. The old composite put a one-voxel discontinuity exactly on the label boundary -- an edge perfectly correlated with the segmentation target. `connected_components_3d` is the only new algorithm: label-equivalence propagation with a pointer jump, 6-connected to match `scipy.ndimage.label`'s default. Verified against scipy on random blobs (exact partition agreement over six trials), on corner-touching voxels (two components, which is what pins 6- over 26-connectivity), and on multi-label volumes where the component count must equal the sum of per-label scipy counts. Label boundaries are respected in a single pass: the propagation is gated on same-label neighbours, so it does not need a pass per label value. int64 throughout, because float32 is exact only to 2^24 and a 256^3 patch has exactly that many voxels. Three properties the code it replaces broke, each now tested: * a knob that is off spends no random numbers -- `_choose_region_mode` drew its two uniforms whenever a mask was attached, even at zero; * the number of uniforms a draw spends is a function of the configuration and never of the data, so a four-label patch and a five-label one leave the stream in the same place; * the EM draw is forked, so a cache hit and a cache miss leave the global stream in the same place. The decomposition is cached as an attribute on the mask tensor rather than in a dict keyed on it: `Tensor.__hash__` is identity-based but `__eq__` is elementwise, so a hash-bucket collision makes `dict` evaluate `bool(a == b)` and raise. Measured: a WeakKeyDictionary with five hundred same-shaped tensor keys raises reliably. `normalise_blend_range` gains a `bounds=` argument so `seg_feather`, a sigma in voxels with no upper limit, can share it with the three blend weights that need the unit interval. The default keeps the `[0, 1]` wording its existing callers' tests assert on. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…and boundary
Wires `gpu/regions.py` into all twenty-eight transforms that read the
segmentation, replacing the three helpers in gpu/contrast.py.
The surface, on every transform that had `in_seg` / `out_seg` and on the four
TAs that had neither:
in_seg / out_seg where: foreground labels, background, both
(a mixture), or the whole patch
seg_unit what a unit is: "whole", "label", "component"
seg_unit_prob the chance each unit takes part
bg_split "em" splits the background into intensity
bg_em_clusters clusters so part of it can be used
seg_feather Gaussian sigma for the boundary, 0 = a hard step
`mix_in_out` is gone. It keyed its keep/drop draw on the mask's *channel*
axis, which for nnU-Net's single-channel label map is one draw per sample
that deletes the whole mask half the time -- so `in_seg` applied the
transform nowhere and `out_seg` applied it everywhere. All thirty-two
shipped configs that set it are migrated to `seg_unit="label"`, which is
what it was trying to be; this is a behaviour fix, not a rename, and the
keep draw also moves from `randint(0, 2, ...)` to `rand(...) < prob`.
`RandomRedistributeSegGPU.in_seg` becomes `confine_prob`, on both backends.
It never meant what `in_seg` means elsewhere -- it confines each region's
Gaussian to that region's own voxels -- and twenty-eight shipped configs set
it. So this is the one transform that does not accept `in_seg` / `out_seg`:
had it accepted them, every stale config would have kept validating while
silently changing meaning, and no diagnostic was possible, because difflib
scores `in_seg` against `out_seg` at 0.615 and would answer "did you mean
out_seg?" before reaching the hint. Unaccepted, the hint fires. What the
class does take is the rest of the surface, which its per-region machinery
consumes directly: `seg_unit` chooses what a region is, `bg_split` gives it
more than one region of air, `seg_unit_prob` which regions contribute.
Its defaults are `seg_unit="label"`, `seg_unit_prob=1.0` -- the region set
and participation it has always had -- and the keep draw spends nothing at
1.0, so the default is bit-compatible. Verified: the unit decomposition is
tensor-identical to the `seg_region_masks` set it replaces, including when a
patch has no background at all.
The four TAs that held the segmentation but could not be confined to it now
can be. Each composites after its own stats restore, which is where
`_select_and_check` already sat in the other twenty-four:
* RandomSynthSegGPU, RandomPaletteGPU, PaletteSynthesisGPU with
`normalize=True`, reviving a parameter `_apply_region_mode` had carried
with no caller. Their output is a [0, 1] synthesis and the patch it lands
on is z-scored, so a raw composite would leave an intensity step on the
mask boundary -- a feature perfectly correlated with the target.
* RandomDomainTransferGPU after the z-restore rather than after its
LUT-space blend, which is the point at which both sides share a scale.
RandomPaletteGPU and PaletteSynthesisGPU also gain the non-finite guard the
other transforms have had; they used to write the synthesis unconditionally.
Two deliberate stream changes, both once and both documented in
gpu/regions.py: the mode draw is per sample rather than batch-wide (it is
the last decision in the package that was not), and it spends nothing when
both probabilities are zero. `RandomInverseGPU`'s per-sample loop existed
only to get a per-sample mode and is gone with it; its non-finite guard is
now per channel, which only matters for an input that was already broken,
since the transform is `max - x`.
Also removed: `_apply_region_mode`'s dead `normalize` branch is now live,
its dead `mix_in_out` branch is gone, `_choose_region_mode`'s unused
`seg_mask` parameter is gone, out-of-range probabilities are rejected rather
than silently clamped, and `ZscoreNormalizationGPU` no longer has to assign
`mix_in_out` by hand outside its signature.
Suite is at the documented 11-failure baseline in seg11 (9 from nnunetv2
2.4.2, 2 from the stale random-normalization config); `template --check` and
`matrix --check` both pass. Every migrated config gets a new content hash,
so experiment directory names change -- the old files are in git history.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`test_connected_components.py` checks the one new algorithm against `scipy.ndimage.label` rather than against itself: partition equality on random blobby volumes, on a serpentine ribbon whose geodesic length is far beyond its Euclidean extent (which is what the pointer jump is for), and on a multi-label volume where the count must equal the sum of scipy's per-label runs -- i.e. that one propagation pass respects label boundaries. Two tests pin the connectivity: corner-touching voxels are two components and face-touching ones are one, so a future switch to the cheaper single `max_pool3d` (a 3x3x3 neighbourhood, and therefore 26-connected) fails here rather than silently merging structures. `test_region_selection.py` is registry-driven over every GPU transform that accepts the surface, so one added later cannot arrive without it or with it wired into the constructor and never read. The "is it read" case runs several seeds, because a draw that happens to keep every label legitimately makes `seg_unit="label"` identical to `"whole"`, and skips a transform that leaves the test input alone -- `RandomClampGPU` does, which is a statement about the input rather than about the knob. Three of its cases are about the RNG stream rather than about pixels, and each is a property the replaced code broke: * region selection off, and `seg_unit_prob=1`, spend no random numbers, so a sweep is comparable with its own baseline at a fixed seed; * the draw width is a function of the configuration and never of the data -- a two-label patch and a five-label one leave the stream in the same place, which a per-unit draw would not; * `bg_split="em"` leaves the global stream exactly where `"none"` does, so whether the decomposition was cached cannot change the augmentation. Plus: the cache is shared across a pass and rebuilt per spec and per mask object; the whole-unit invariant (a label or component is applied or it is not, never partially); the feathered boundary is a monotone ramp bounded in [0, 1] whose `in` and `out` halves still sum to the full transform; an empty mask makes `in` a no-op and `out` everywhere, which is what the binary version did; and no shipped config still names `mix_in_out` or `RandomRedistributeSegGPU.in_seg`. `helpers.multi_label_seg` is new because neither existing fixture can express the granularities: `tiny_seg` is one binary cube, so it has one label value and one connected component and all three `seg_unit` settings agree on it. The new one has three values with one of them in two separated blocks, which is the only shape that tells "component" from "label" apart. Suite in seg11: 556 passed, 2701 subtests, and the same documented 11-failure baseline as before this branch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
6 tasks done
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 this is
in_segandout_seghave been in the GPU augmentations for a long time, and between them they expressed about a third of what they looked like they expressed. This finishes them, and adds the two things they implied but never had: sub-label granularity, and a background that is not all-or-nothing.Builds on
hm/ta-mix-alpha(merge that first), and follows that PR's structure deliberately: one shared module with the rationale in its docstring, aconfigure_*call per constructor, and class-level "off" defaults so mypy can see the attributes.What was broken
in_segandout_segcould not express a mixture. Two independent uniforms, and both fired fell through to the same"all"as neither fired — soin_seg=1, out_seg=1was indistinguishable fromin_seg=0, out_seg=0.mix_in_outnever worked on the layout training uses. It keyed its keep/drop draw onseg_mask.shape[:2], the channel axis. For nnU-Net's single-channel integer label map that is(B, 1)— one draw per sample that deletes the whole mask half the time. So"in"applied the transform nowhere and"out"applied it everywhere. It only ever meant "per label" on a one-hot mask. Every config that setmix_in_out: truehas been running a coin flip, not a label mix.out_segtook all of it or none.m * transformed + (1 - m) * origwith binarymleft a one-voxel discontinuity sitting exactly on the label boundary — an edge perfectly correlated with the segmentation target.RandomSynthSegGPU,RandomPaletteGPU,PaletteSynthesisGPU,RandomDomainTransferGPU) — precisely the set that already holds the segmentation._choose_region_modesilently clamped, soin_seg: 5.0andin_seg: truewere accepted quietly. And it drew its two uniforms whenever a mask was attached, including at zero, so an unselected run and a selected one at the same seed were never comparable — the same defect fix: pin the region-moment matmul to fp32, which autocast overflows #136 removed formix_prob.The surface
Everything resolves to one integer unit id map and a single gather off it. Nothing per-unit is materialised — a noisy mask under
seg_unit="component"can hold hundreds of components, and[2, 300, 128, 128, 128]of bool is 1.2 GB.bg_split="em"calls the existingem_subdivide_labels(SynthSeg §5.4) withn_foreground_clusters=1; the EM is not reimplemented.connected_components_3d, the one new algorithmLabel-equivalence propagation with a pointer jump, pure torch, on device, int64 throughout —
float32is exact only to 2²⁴ and a 256³ patch has exactly that many voxels. 6-connected, matchingscipy.ndimage.label's default; the obvious singlemax_pool3d(kernel=3)is a 3³ neighbourhood and would merge corner-touching structures.Components respect label boundaries in one propagation pass: the step is gated on same-label neighbours, so it does not need a pass per label value (a spine patch has five to twenty-five).
Checked against scipy rather than against itself — partition equality, not just counts:
Measured at the real patch size
A40, 128³, batch 2, 6 labels. Full output in the verification section below.
connected_components_3dbackground_em_units(EM, 3–10 clusters)That last line is why the decomposition is cached rather than recomputed per transform: without it the feature adds over a second to every training step. The feather blur is the one part that cannot be cached (it depends on the per-call draws) and at ~1 ms it does not need to be.
All 24
(mode, seg_unit, bg_split)combinations were checked at 128³ to change exactly a union of whole units, per sample — not merely "something changed".outon the whole background is 93.5% of voxels; withbg_split="em"it becomes 55.8%, andboth, component, em41.6%, which is the feature doing what it is for.RandomRedistributeSegGPUis the one exceptionIts
in_segnever meant whatin_segmeans elsewhere — it confines each region's Gaussian to that region's own voxels — and 28 config files set it. It becomesconfine_prob, on both backends, and the class deliberately does not acceptin_seg/out_seg.That is not fastidiousness. Had it accepted them, every stale config would have kept validating while silently changing meaning, and no diagnostic was possible: difflib scores
in_segagainstout_segat 0.615, aboveunknown_parameter_message's 0.6 cutoff, so it would answer "did you mean out_seg?" and never reach the hint. Unaccepted, the hint fires:What the class does take is the rest of the surface, which its per-region machinery consumes directly. Its defaults (
seg_unit="label",seg_unit_prob=1.0) are the region set and participation it has always had, and the keep draw spends nothing at 1.0 — verified tensor-identical to theseg_region_masksset it replaces, including on a patch with no background at all.Three stream properties, each one the old code broke
Tested, not asserted in prose:
in_seg = out_seg = 0, andseg_unit_prob = 1, leave the stream exactly where they found it. Without this no sweep is comparable with its own baseline at a fixed seed.B × 100 001uniforms on a cache miss and none on a hit, so without the fork the augmentation would depend on cache state: on which transform asked first, on which rowspselected, on whether a geometric transform replaced the mask.Deliberate behaviour changes
mix_in_outis gone, migrated toseg_unit="label". This is a fix, not a rename (see above), and the draw also moves fromrandint(0, 2, ...)torand(...) < prob.transform_params_gpu_inoutseg.jsonandtransform_params_hybrid{,_TAGE}.json, all at 0.5/0.5 — whose distribution goes from 25% in / 25% out / 50% all to 25 / 25 / 25% mixture / 25% all. Left as they are: the new reading is what they should have meant.in_seg=0.5read as "half the batches" whilepandmix_probboth meant "half the samples".RandomInverseGPU's per-sample loop existed only to get a per-sample mode and is gone with it; its non-finite guard is now per channel, which matters only for an input that was already broken, since the transform ismax - x.RandomPaletteGPUandPaletteSynthesisGPUgain the non-finite guard the other transforms have had; they used to write the synthesis unconditionally.Config hashes change
config_hashfeeds the experiment directory name. Seven tracked configs move, and both benchmark arms named in CLAUDE.md are among them:all_augmentations.json(generated)f47a0f39e3b5ca6atransform_params.json088b9bd58c8768b8transform_params_gpu.json49138fe4c80bd868transform_params_hybrid.jsonc95704cdc482b7d3transform_params_hybrid_TAGE.jsonc5fb06c0726e91fdtransform_params_one-sequence-to-segment-them-all.jsonc69b98721690ad25transform_params_paper-synthseg.json580beb135b0c6e1bc69b9872is theOurs (fp32)/Ours (post)arms and580beb13isSynthSeg (post). Existing run directories keep their names; a re-run from these files lands in new ones. The old files are in git history.A further 28 config files in
smauglab/configs/andsmauglab/configs_paul/are gitignored local sweep configs (the blanket*.jsonrule). They are migrated on disk so local runs keep working, but they are not in this diff — worth knowing if you have your own.The four synthesising TAs
Each composites after its own stats restore, which is where the region selection already sat in the other twenty-four:
RandomSynthSegGPU,RandomPaletteGPU,PaletteSynthesisGPUwithnormalize=True— reviving a parameter_apply_region_modehad carried with no caller. Their output is a[0, 1]synthesis and the patch it lands on is z-scored, so a raw composite would leave an intensity step on the mask boundary that has nothing to do with the augmentation.RandomDomainTransferGPUafter the z-restore rather than after its LUT-space blend, which is the point at which both sides share a scale.RandomSynthSegGPUtakes the parameters explicitly:_filter_generator_kwargsdrops anything outside_GENERATOR_KEYS, so they would have vanished silently. Its docstring says thatbg_em_clustersand the generator's ownem_background_clusters_rangeare different features one word apart.Implementation notes worth a reviewer's eye
The cache is a tensor attribute, not a dict keyed on tensors.
Tensor.__hash__is identity-based butTensor.__eq__is elementwise, so a hash-bucket collision between two masks makesdictevaluatebool(a == b)and raise "Boolean value of Tensor with more than one value is ambiguous". Measured: aWeakKeyDictionarywith five hundred same-shaped tensor keys raises reliably. An attribute has no buckets, dies with the tensor, and invalidates for free —MaskSequentialOpsCustom.transformreturns the same object for an image-only module and a new one for a geometric one, so identity changes exactly when the mask changes.Keyed on the mask as the transform was handed it, which is what keeps the image and the units row-aligned: kornia slices the image to the rows that drew below
pand_seg_for_applied_rowsslices the mask to match, so a decomposition built from the full batch would pair B-row units with a B'-row image — andbg_split="em"reads that image. Atp = 1the slice is the full mask object and every module of the pass hits one entry. This also makes theRandomChooseXTransformsGPUbare-tensor path work with no change to it: verified 1 decomposition for two children atsame_on_batch=True, and 2 for two samples × two children otherwise, rather than 4.em_subdivide_labelsis guarded against a patch with no background, where it falls back to the largest-area label and would otherwise split a foreground structure into three to ten pieces and call them background.gpu/regions.pyimportsem_subdivide_labelsfunction-locally.synthseg/__init__.pyimportssynthseg/transforms.py, which importsgpu/base.py, which imports this module for its defaults. A module-scope import closes that loop and breaks everyimport smauglab.transforms.gpu.base;test_imports.pyis the canary.The pinned-image caveat, stated rather than hidden:
bg_split="em"is the only decomposition that reads the image, and every transform's first statement isinput = input.clone(), so keying the cache on the image would mean a zero hit rate. The clusters are fitted once per pass, from the image as it stood when the first region-selecting transform asked. That is also the paper's reading — EM is a preprocessing step on the real paired image — and it means two region-selected transforms in one pass cut the background along the same lines instead of each along its own.normalise_blend_rangegainsbounds=soseg_feather, a sigma in voxels with no upper limit, shares it with the three blend weights that need the unit interval. The default keeps the[0, 1]wording its existing callers' tests assert on, and a test checks that widening one did not widenmix_alpha.Verification
Beyond the suite, on an A40 at 128³ batch 2:
RandomDomainTransferGPUcould not be exercised locally: it needs theSMAUGLAB_DOMAIN_BANKartefact, which is not set here, so its tests skip as they do upstream. Its region selection is the simplest of the four (nonormalize, both sides already on one scale) but it is the one path in this PR that has not run.🤖 Generated with Claude Code