fix(labeling): only rotate cutouts for models trained on rotated ones - #68
Open
Hendrik-code wants to merge 1 commit into
Open
fix(labeling): only rotate cutouts for models trained on rotated ones#68Hendrik-code wants to merge 1 commit into
Hendrik-code wants to merge 1 commit into
Conversation
`run_all_seg_instances` has rotated every vertebra patch to the spine axis since 9bb4585 (2025-06-11). The released t2w labeling checkpoint was trained on `sagittal_v3corpus_npz`, i.e. axis-aligned cutouts -- only the `v4corpus`/ROT variants saw rotated ones. Since that commit the shipped model has therefore been fed patches of a kind it never saw in training. Measured on NAKO blocks 106/107 (n=1000, same weights, same ground truth): rotation on (current) : 96.00 perfect / 78.05 TEA / 82.07 LEA (40 errors) rotation off (this) : 98.20 perfect / 86.59 TEA / 94.48 LEA (18 errors) The rotation-off numbers are the published VERIDAH ones. `patch_rotation` is now derived from the checkpoint's own `ds_name`, so weights and preprocessing travel together and the `v4corpus` models keep rotating. Default stays True, so a model folder without a recognisable `ds_name` behaves as before. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Owner
Author
|
Verification run finished. The released That matches the published VERIDAH numbers exactly, against 96.00 / 78.05 / 82.07 (40 errors) on the current Cross-checked against the earlier reference run: decoded label sequences agree on all 1974 scans in the excel, with a ~5e-6 median softmax difference (GPU floating-point noise). 🤖 Generated with Claude Code |
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 bug
Since
9bb4585("update labeling to VERIDAH methodology", 2025-06-11),VertLabelingClassifier.run_all_seg_instancesrotates every vertebra cutout to the local spine axis before classification: it takes the angle between the neighbouring vertebra centroids, pads the patch by 64 voxels, rotates, and crops back.The released t2w labeling checkpoint was trained on
sagittal_v3corpus_npz— axis-aligned cutouts. Only the separatev4corpus/_ROTmodels were trained on rotated ones. So since that commit the shipped model has been fed patches of a kind it never saw during training, and every user has silently been getting degraded labeling.Nothing binds the checkpoint's weights to the preprocessing the inference code applies, so there was no way for this to fail loudly.
Evidence
Bisected on NAKO blocks 106/107 (n = 1000 subjects), holding weights, segmentations and ground truth fixed — the only difference is the rotation:
main)TEA / LEA = thoracic / lumbar enumeration anomaly rate, i.e. how often transitional vertebrae (T11/T13, L4/L6) are enumerated correctly — the metrics the VERIDAH paper is about.
The rotation-off column reproduces the published VERIDAH numbers exactly. Per-vertebra VERT argmax agreement between the two paths is only 97.11 %, with a median max-softmax difference of 0.21, so this is not a rounding-level effect.
Ruled out along the way:
1c80c35("fixed veridah not resampling and cropping") is a bit-identical no-op on this cohort — NAKO T2w is already at the model's target resolution 0.8571/0.8571/3.3, so the rescale does nothing and the outer crop does not change the per-instance cutouts (softmax difference 0.00e+00).The fix
Derive the setting from the checkpoint instead of applying it unconditionally:
plus an axis-aligned branch in
run_all_seg_instancesthat passesangle=Nonefor every instance.patch_rotationis initialised toTruein__init__, so a model folder whoseds_nameis missing or unrecognised behaves exactly as it does today.v4corpus/ ROT models keep rotating — they were trained that way.run_all_arraysnever rotated.patch_rotation=... (ds_name=...)) so the choice is visible in any inference log.Tests
Test_Labeling_Patch_Rotation_Gateinunit_tests/test_bugfixes.py:load()derivesFalseforsagittal_v3corpus_npzandTrueforsagittal_v4corpus_npz;ds_namekeeps the current rotating behaviour;run_all_seg_instancesrequests every patch withangle=None, and with it on it still computes non-zero angles.Full suite: 221 passed.
Test_Merged_Vertebra_Background::test_top_two_instances_are_mergedfails, but it fails identically on the base branch without this commit — pre-existing and unrelated.End-to-end on this branch: the released
t2w_labelingmodel folder loaded through the ordinaryVertLabelingClassifier(...).load()entry point — no pinned-checkpoint helper, no legacy path — reportspatch_rotation=False (ds_name=sagittal_v3corpus_npz)at load and scoreswhich is the published VERIDAH result to the decimal. Decoded label sequences are identical to the earlier reference run on all 1974 scans in the excel; the residual softmax difference between the two runs is ~5e-6 median, i.e. GPU floating-point noise.
Base branch
Branched off
bugfinder, which is already contained in #67, so this PR currently shows that commit too; it will narrow to a single commit once #67 merges.The dependency is real rather than incidental:
2ab7335is what re-reads the patch arrays in(I, P, L)before the crop-back. Without it the axis-aligned branch would crop the wrong axes for any input not already in that orientation.Follow-up worth considering
Deriving
patch_rotationfrom a dataset-name substring is a working fix but still an implicit convention — one more model naming scheme and it breaks silently again. The durable version is to serialize the preprocessing contract (rotation, resolution, orientation, crop) intoinference_config.jsonand have the loader assert the checkpoint against it, so weights and preprocessing cannot drift apart in the first place.🤖 Generated with Claude Code