Skip to content

Poi label names - #115

Closed
Hendrik-code wants to merge 759 commits into
mainfrom
poi-label-names
Closed

Hendrik-code wants to merge 759 commits into
mainfrom
poi-label-names

Conversation

@Hendrik-code

Copy link
Copy Markdown
Owner

Summary

Overhauls POI naming across POI and POI_Global per the requested spec. Three focused commits; full unit suite 406 passed / 4 skipped (395 baseline + 11 new POI tests). Coloring/serialization of .mrk.json is kept byte-identical.

What changed (mapped to the requirements)

  1. Names survive all POI ops — POI.copy, _set_inplace, POI_Global.to_other, calc_poi_average, and POI_Descriptor.copy now carry level_one_info/level_two_info/info/definition.
    Since reorient/rescale/extract/map_labels/to_global/average route through copy(), the names follow.
  2. Index by level names — poi[idx, "level2name"] and poi["level1name", "level2name"] work for get/set/in (via unpack_poi_id → level_*_info._get_id → descriptor definition), alongside ids and Enums.
  3. Enums everywhere + warning — Enum keys use .value; a warning fires when the matching level_*_info is still Any (unset).
  4. poi.level_one_name(region) — returns the group (level-one) name.
  5. poi.label_name(region, subregion) — returns the point name. set_label_name/set_level_one_name write them.
  6. New nested label_name format — {region: {subregion: name, "name": group}} instead of flat {"(1, 2)": "C2"}. normalize_label_name() migrates the old flat form (and JSON string keys) on load;
    save always writes the new form. .mrk.json export/import stays byte-identical, now fed from the nested dict. join_left/txt-import updated.
  7. Priority + warning — a custom label_name wins over the level_x_info enum name, with a warning when it diverges from a given enum name.

Notes for reviewers

  • Location.save_as_name() is False (subregions saved as ints), so its _get_name returns the id string — the accessor treats that as "no enum name" and falls back to the id/custom name. Vertebra_Instance uses real names.
    (Tell me if you'd like Location to also resolve to enum names.)
  • Name indexing is wired into __getitem__/__setitem__/__contains__; extract_region/extract_subregion/remove still take ids+Enums (not yet level names) — easy to extend.
  • Backwards compatible: old flat-format POI files load and migrate transparently; adds unit_tests/test_poi_label_names.py (11 tests) locking migration, accessors, round-trip, old-format load, name indexing, warnings, and metadata transfer.

Commits:

  • 18d9096 fix(poi): carry level_one_info/level_two_info/info through all POI ops
  • 38be00e feat(poi): nested label_name format + name accessors + migration
  • 3ab343d feat(poi): index POIs by level names + warn on enum without level_*_info

robert-graf and others added 30 commits July 15, 2025 10:48
robert-graf and others added 23 commits May 29, 2026 08:37
All sliding-window tiles share the same patch_size shape, so cuDNN can
autotune the fastest conv algorithms once and reuse them across tiles and
images. TF32 accelerates fp32 matmul/conv on Ampere+ GPUs with negligible
accuracy impact. Gated behind a new fast_perf flag (default True, CUDA only).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…erence

inference_mode disables autograd view/version tracking entirely, so it is
slightly faster and uses less memory than no_grad. All tensors here are pure
inference outputs (moved to CPU / converted to numpy downstream), so the
stricter inference-tensor semantics are safe.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
predict_sliding_window_return_logits runs once per fold; calling
torch.cuda.empty_cache() at its start and end returned allocator blocks to
the driver right before the next fold reallocated them (slow cudaMalloc +
sync). The pool is still cleared once per image after fold averaging, and all
OOM-recovery empty_cache() calls are untouched.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
initialize_from_trained_model_folder built loaded_networks by appending the
same self.network object once per fold, so every entry ended up holding the
LAST fold's weights. predict_logits_from_preprocessed_data then ran that one
weight set N times and averaged it, silently collapsing the N-fold ensemble to
a single fold while still paying Nx compute. Separately, cache_state_dicts=False
left loaded_networks as [] (not None), causing an IndexError in the predict
loop.

Now loaded_networks is None unless exactly one fold is present (preloaded for
zero per-prediction reloads). For >1 fold it stays None and the predict loop
swaps weights per fold via load_state_dict, restoring a correct ensemble using
a single network's worth of GPU memory. Device-availability warnings now fire
once regardless of fold count.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Every run_inference_on_file call previously reloaded the predictor from disk
and re-uploaded weights to the GPU. With cache_model=True the loaded predictor
is kept in a process-wide cache keyed by model path + folds + device/runtime
settings, so a loop over many files reuses the in-memory model. When caching,
the end-of-call del/empty_cache is skipped so the CUDA allocator pool stays
warm between images. Default is False to preserve current memory semantics; the
flag forwards through run_VibeSeg/run_vibeseg/run_nnunet via **kwargs.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
_run_sub processed one tile per forward pass, leaving the GPU underutilised for
small patches. Tiles all share patch_size, so they batch densely: with
tile_batch_size>1 they are stacked into a (B, C, *patch) tensor and run in one
pass, then scattered back per tile. Mirroring/gaussian/accumulation are
unchanged. tile_batch_size=1 (default) takes the original view-based path
verbatim, so behaviour and memory are unchanged unless opted in. Threaded
through nnUNetPredictor, load_inf_model and run_inference_on_file.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds benchmarks/ with a phase-resolved timing tool for run_inference_on_file
(load / preprocess / sliding-window predict / postprocess), using CUDA
synchronisation, warmup, repeats and peak-memory tracking. It instruments the
pipeline by monkeypatching (no library changes) and signature-filters its kwargs
so the same file runs against every commit in the optimisation range.

- benchmark_nnunet_inference.py: run/compare subcommands; synthetic or real input.
- bench_across_commits.sh: replays one config across baseline+each commit and
  prints per-commit deltas (isolates the always-on changes; the fold fix shows up
  as the fold_status column flipping from DUPLICATED to lazy-per-fold/distinct).
- bench_flag_sweep.sh: sweeps the opt-in flags at HEAD (cache_model needs >1 call,
  tile_batch_size, max_folds, step_size).
- README.md: usage, which tool measures which commit, and caveats.

pyproject: exempt benchmarks/ from docstring rules, mirroring the test dirs.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
POI.copy(), _set_inplace(), POI_Global.to_other(), and calc_poi_average() dropped
the naming metadata (level_one_info, level_two_info, and with it info['label_name']),
and POI_Descriptor.copy() dropped its name<->id definition. Now every copy/derive
path preserves them, so names survive reorient/rescale/extract/map_labels/to_global/
average (all of which route through copy()). Verified preserved across those ops;
76 existing POI tests still pass.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
label_name now uses the nested form {region:{subregion:name,'name':group}} instead
of flat {'(1, 2)':'C2'}. Old flat files (and JSON string keys) are migrated on load
(normalize_label_name); saving always writes the new format (mrk.json export stays
byte-identical, fed from the nested dict). New accessors POI.label_name(region,
subregion) and POI.level_one_name(region) resolve custom names with priority over
level_two_info/level_one_info enum names (warning when a custom name diverges from a
given enum name); set_label_name/set_level_one_name write them. mrk/txt import and
join_left updated to the nested form. 89 POI tests + 8 new tests pass.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
unpack_poi_id now resolves a key part via level_one_info/level_two_info enum names
(when set) before the descriptor's definition, so poi[idx, 'level2name'] and
poi['level1name', 'level2name'] work for get/set/contains alongside ids and Enums.
Using an Enum key while the matching level_*_info is still 'Any' (unset) now emits a
warning (the .value is still used). 100 POI tests pass.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@robert-graf

Copy link
Copy Markdown
Collaborator

Do not merge for the new release. Needs a lot of testing.

@Hendrik-code Hendrik-code linked an issue Jul 1, 2026 that may be closed by this pull request
@robert-graf robert-graf closed this Aug 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make POI names first level support

4 participants