feat: one trainer, a CLI, and a coverage matrix that cannot go stale - #71
Open
Hendrik-code wants to merge 1 commit into
Open
feat: one trainer, a CLI, and a coverage matrix that cannot go stale#71Hendrik-code wants to merge 1 commit into
Hendrik-code wants to merge 1 commit into
Conversation
The three nnU-Net trainers collapse into one. Which sections a config populates
decides whether augmentation runs on the dataloader, on the batch, or both, and
pipeline.mode decides how the GPU pipeline is arranged, so the CPU/GPU/Hybrid
split no longer needs a class each.
The survivor keeps the name nnUNetTrainerDAExtGPU. nnU-Net writes the trainer
class name into every checkpoint and resolves the class from it at inference, so
renaming it would strand every model already trained.
smauglab/cli.py answers, from the registry, the questions that previously needed
reading four `if` ladders side by side:
smauglab list --backend gpu what a GPU config can name, in pipeline order
smauglab matrix which backends implement each augmentation
smauglab show RandomScharrGPU one augmentation's parameters and defaults
smauglab validate cfg.json every problem in a config, at once
smauglab template a config naming every augmentation
smauglab hash cfg.json the content-addressed config identity
`hash` is the only one that does not need the registry, so it is the only one
that does not pay for importing torch.
The README coverage matrix and smauglab/configs/all_augmentations.json are
generated by `--write` and checked by `--check`, and a test runs `--check`. They
are derived from the registry, so they cannot describe augmentations that do not
exist or miss ones that do.
Two migration rules were missing and are added, with the configs regenerated:
* A config carrying a RandomChooseXTransforms block was only ever used with the
list trainer, and that choice was the trainer *class*. It becomes
pipeline.mode=random_order, or those configs silently switch to the sequential
pipeline.
* nnU-Net's own SpatialTransform used to be appended by the trainer with its
settings hardcoded, so no config named it. The trainer builds its CPU pipeline
from the config now, so it has to be in the file or it silently stops running.
The block is verbatim from get_training_transforms; patch_size and rotation
stay out because nnU-Net supplies them at runtime.
Both were caught by the trainer tests rather than by inspection, which is the
argument for having them.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Stacked on #70. Last PR in the chain.
Three trainers become one
Which sections a config populates decides whether augmentation runs on the dataloader, on the batch, or both;
pipeline.modedecides how the GPU pipeline is arranged. So the CPU/GPU/Hybrid split no longer needs a class each.nnUNetTrainerDAExtGPU. nnU-Net writes the trainer class name into every checkpoint and resolves the class from it at inference — renaming it would strand every model already trained.A CLI that reads the registry
These answer questions that previously needed reading four
ifladders side by side.hashis the only one that doesn't need the registry, so it's the only one that doesn't pay for importing torch.The README coverage matrix and
smauglab/configs/all_augmentations.jsonare generated by--writeand verified by--check, and a test runs--check. Being derived from the registry, they cannot describe augmentations that don't exist or miss ones that do.Two migration rules were missing
Both were caught by the new trainer tests rather than by inspection — which is the argument for having them. The configs are regenerated with the fixed migrator:
pipeline.modewas never emitted. A config carrying aRandomChooseXTransformsblock was only ever used with the list trainer, and that choice was the trainer class. Without translating it topipeline.mode=random_order, those configs silently switch to the sequential pipeline.CPU.SpatialTransformwas never added. nnU-Net's ownSpatialTransformused to be appended by the trainer with its settings hardcoded, so no config named it. The trainer builds its CPU pipeline from the config now — so it has to be in the file or it silently stops running. The block is verbatim fromget_training_transforms;patch_sizeandrotationstay out because nnU-Net supplies them at runtime (they'recontext_paramson the registry entry).Testing
New
test_trainers.py(the two rules above are its findings),test_cli.py, and the generated-artefact tests deferred from #68.Full suite: 208 passed, 5 skipped, 569 subtests. The builder equivalence test from #70 still passes against the regenerated configs.
ruffandmypyclean.