Repository navigation
Give every GPU transfer augmentation a configurable mix range - #137
Open
Hendrik-code wants to merge 2 commits into
Open
Hendrik-code wants to merge 2 commits into
Hendrik-code wants to merge 2 commits into
Conversation
Six of the seventeen transfer augmentations could already blend their output
back towards the input, under the name `mix_prob`, from three open-coded copies
of the same four lines -- and with the weight drawn from a hardcoded U(0, 1), so
the only expressible blends were "none" and "anywhere between the identity and
full strength". The other eleven had no blend at all.
`mix_alpha` is that weight made configurable: a scalar, or a [lo, hi] range
drawn from per sample. It is the weight on the *original*, so 0 is the
transform's output and 1 is the input untouched -- the direction the copies
used, which is what lets the default of (0, 1) reproduce their draw exactly.
`unit_tests/test_mix_prob.py` passes unchanged and is the check that it does.
It makes sense for all seventeen: every registered TA is intensity-only on a
fixed voxel grid, so none of them resamples, deforms, crops or touches the
segmentation, and a per-voxel convex combination is well defined throughout.
The geometric transforms are tagged GEO, not TA. `RandomGaussianBlurGPU` comes
along because it shares `_RandomConvBaseGPU` and already exposed `mix_prob`.
The blend sits before each transform's `retain_stats` restore, where the copies
already had it. That is what keeps the promise `retain_stats` makes: blending
after the restore would leave the output with the input's mean but a smaller
standard deviation, and `mix_prob` is above zero in 29 of the 33 shipped
configs, mostly on transforms that ask for `retain_stats` too.
One behaviour change, measured rather than assumed. The two per-sample copies
spent a draw per sample to decide something `mix_prob` had already settled:
if torch.rand(1).item() < self.mix_prob: # drawn even when mix_prob is 0.0
so `RandomInverseGPU` and `RandomHistogramEqualizationGPU` shifted the RNG
stream for every transform that ran after them, mixing on or off. Both helpers
return before drawing anything now, as the vectorised copy always did. 15 of the
28 runnable shipped configs therefore produce a different seeded sequence, same
distribution, and every transform in isolation is bit-identical except
`RandomInverseGPU` with `mix_prob > 0`, whose blend also had to move ahead of
its hoisted restore. What that buys is the property this feature needs: a
`mix_alpha` sweep can be compared against its own `mix_prob = 0` baseline at a
fixed seed, which it could not be before.
Also here, both found while wiring the above:
* `RandomSynthSegGPU.apply_transform` wrote `input[:, c] = ...` without cloning
first, mutating the caller's `batch["data"]`. Every sibling already clones;
it is uncovered because test_transforms_gpu.py does not list that module.
* `AnatomicalLabelOverlay.blend_strength` now shares the range validator, so a
value outside [0, 1] is rejected rather than documented.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two type errors CI caught that the local mypy (2.1.0 against the pinned 2.4.0) did not. `configure_mix` assigned `mix_prob` / `mix_alpha` from outside the class, so no type checker ever saw them declared and every read resolved through `nn.Module.__getattr__` -- typed `Tensor | Module`, neither of which is a number. That is the exact problem the `Mixing` Protocol was written to solve, and it did not, because structural matching reads the declared attributes. `_RegionSelecting` gets away with the same shape because its three attributes *are* assigned in the constructors. Declaring both on `ImageOnlyTransform`, at the values that mean "off", fixes every read site at once and costs nothing at runtime: only the transforms that take them as constructor arguments can be configured, since the signature is the config schema. `normalise_blend_range` also narrowed `bool` into its sequence branch, where it is not iterable. A bool was never a blend weight anyone meant -- and left to the numeric tower `True` would have quietly meant alpha 1.0, a transform configured to do nothing -- so it is now rejected by name. No behavioural change: every per-transform digest is unchanged, and the blend-before-retain_stats ordering check still holds bit-exactly. 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
Every GPU transfer augmentation (
AugType.TA, 17 of them) can now blend its output backtowards the image it was handed, at an alpha drawn from a range set in the config:
mix_probis the existing per-sample gate for whether to blend;mix_alphais new andsets how far. It takes a scalar or a
[lo, hi]range, mirroringAnatomicalLabelOverlay.blend_strength.alpha is the weight on the original:
0.0is the transform's output,1.0is theinput untouched. Counter-intuitive in a config, and kept that way deliberately — it is
the direction the existing code used, which is what lets the default range
(0.0, 1.0)reproduce the previous hardcoded
U(0, 1)draw exactly (draw * 1.0 + 0.0is exact inIEEE-754).
Why
Six of the seventeen already had half of this, from three open-coded copies of the same
four lines, with the weight hardcoded to
U(0, 1)— so the only expressible blends were"none" and "anywhere between the identity and full strength". The other eleven had no
blend at all. One shared implementation now lives in
smauglab/transforms/gpu/mixing.py.A configurable range also fixes a real hazard the hardcoded one had: inversion is
max - x, soalpha = 0.5onRandomInverseGPUismax / 2everywhere — a constantimage.
U(0, 1)samples straight through that dead zone;[0.7, 1.0]does not.Does it make sense for every TA?
Yes, for all seventeen. Every registered TA is intensity-only on a fixed voxel grid: none
resamples, deforms, flips or crops, none changes shape or channel count, none touches the
segmentation. The genuinely geometric transforms are tagged
GEO, notTA, so there isno augmentation in this group where a per-voxel convex combination would ghost or break
image/label alignment.
RandomGaussianBlurGPUcomes along because it shares_RandomConvBaseGPUand already exposedmix_prob.Three caveats are documented in the docstrings rather than designed around:
RandomSynthSegGPUis the only one with no stats restore after the blend. Itssynthesis is in
[0, 1]while a training patch is z-scored, so there the blend reallydoes mix two scales. Flagged in its Args, with a pointer to
RandomPaletteGPUorZscoreNormalizationGPU.RandomUnsharpMaskGPU/RandomBiasFieldGPU— the blend is close to a rescale ofunsharp_amount/coefficients. Offered anyway: interpolating toward the identity isa different axis from scaling the effect, and uniformity is what makes a sweep writable.
RandomPaletteGPU/PaletteSynthesisGPUblend in the[0, 1]space the synthesisis built in, before their closing foreground z-score, so both sides are in-units and the
* flat_m_allmasking still holds the background at exactly zero.Where the blend sits
Before each transform's
retain_statsrestore, where the existing copies already had it.That keeps the promise
retain_statsmakes — blending after the restore leaves theoutput with the input's mean but a smaller standard deviation (
s·sqrt((1+rho)/2), ~0.8sat rho = 0.3).
mix_probis above zero in 29 of the 33 shipped configs, mostly ontransforms that ask for
retain_statstoo, so this is not hypothetical. Confirmed byexecution: with
mix_alpha=0.75, retain_stats=Truethe output matchesrestore(blend(...))bit-exactly and differs fromblend(restore(...))by up to 1.36,and the output's mean/std match the input's to 9e-08 / 0.
One behaviour change, measured
The two per-sample copies spent a draw per sample to decide something
mix_probhadalready settled:
so
RandomInverseGPUandRandomHistogramEqualizationGPUshifted the RNG stream forevery transform that ran after them, whether or not mixing was on. Both helpers now
return before drawing anything, as the vectorised copy always did.
Measured, by hashing every shipped config's seeded output before and after:
distribution; a GPU augmentation run is not reproducible run-to-run under a fixed seed
anyway, and no published result is recomputed from these files.
RandomInverseGPUwithmix_prob > 0, whose blend also had to move ahead of its hoistedretain_stats.contrast.pyby reverting file-by-file, and attributed toRandomInverseGPU(6 draws → 4) with atorch.randcounter.What it buys is the property this feature needs: a
mix_alphasweep is comparableagainst its own
mix_prob = 0baseline at a fixed seed, which it was not before. Confighashes are computed over the document, not over defaults, so the paper hashes the README
pins (
c69b9872,580beb13) and the...-c-<hash>run folders are untouched.Also here
Both found while wiring the above:
RandomSynthSegGPU.apply_transformnever cloned its input, writinginput[:, c] = ...straight into the caller's
batch["data"]. Every sibling already clones; it slippedthrough because
test_transforms_gpu.pydoes not list that module. One-line fix.AnatomicalLabelOverlay.blend_strengthnow shares the range validator, so a valueoutside
[0, 1]is rejected rather than merely documented as being in it.Tests
unit_tests/test_mix_alpha.py, 19 cases. The ones that matter most iterate overregistry.entries(Backend.GPU, group=AugType.TA), so a TA added later cannot arrivewithout the knob, or with it wired into the constructor but never read:
0.0/(0.0, 1.0);mix_prob=1, mix_alpha=0.5differs frommix_prob=0— the guard against a dead knob;mix_alpha=1.0is the identity, for 14 of the 17 (allclose: aretain_statsrestoreof the input's own stats carries its own
1e-8). The two palette classes are skippedwith a reason — their unconditional z-score runs after the blend;
gives every sample the same alpha;
mix_prob=0is stream-neutral, and a degenerate range still spends its alpha draw, soan A/B over ranges stays comparable;
list, throughvalidate_sectionandbuild_gpu_pipeline.unit_tests/test_mix_prob.pypasses unchanged — that is the check thatmix_alpha'sdefault still reproduces the draw it replaced.
Full suite in
seg11: 11 failed, 485 passed, against a pre-change baseline of11 failed, 466 passed on the same interpreter — zero new failures. The 11 are
pre-existing: 9
test_trainerssubtests (nnunetv2 2.4.2 here against the>=2.5pin)and 2 from the stale
transform_params_gpu_random-normalization.json, which names fivetransforms that are not registered.
pre-commit run --all-filesclean.A second commit makes the two mix attributes visible to mypy.
configure_mixassignedthem from outside the class, so every read resolved through
nn.Module.__getattr__(
Tensor | Module) and theMixingProtocol could not match — the exact problem it waswritten to solve. They are declared on
ImageOnlyTransformnow, at the values that mean"off"; only the transforms that take them as constructor arguments are configurable,
since the signature is the config schema. It also rejects a
boolrange, which thenumeric tower would otherwise have read as alpha 1.0. No behavioural change — every
per-transform digest is unchanged. All 8 CI checks green.
🤖 Generated with Claude Code