Repository navigation
feat(dynacell): grouped multi-condition eval driver - #427
Merged
Merged
Conversation
Add ``dynacell evaluate-grouped``: run the same (model, organelle) across multiple I/O variants (the canonical case is the three A549 treatment plates mock/denv/zikv) in one Python process, paying the segmenter + DINOv3 + DynaCLR + CELL-DINO cold-start once instead of N times. On serial-mode interactive jobs and on per-condition SLURM jobs this is 30-90s of weight load + checkpoint warmup amortized across all conditions. Under runtime.executor=process, each condition still spawns its own pool whose workers re-load model copies, so only the parent-side load is amortized; the driver prints a note when it detects this. Mechanism: - ``dynacell.evaluation.model_loader.load_eval_models`` extracts the segmenter + extractor instantiation that previously lived inline in ``evaluate_predictions`` and ``_worker_setup``. The bundle carries identity tags (dinov3_model_name, dynaclr_ckpt source paths, encoder cfg, celldino weights) so ``init_cache_context`` callers get identical cache-identity values across conditions. - ``evaluate_predictions(config, *, models=None)`` accepts a pre-loaded bundle and skips its inline load when supplied. ``models=None`` preserves the historical single-condition behavior; existing callers and tests are unchanged. - ``evaluate_predictions_grouped(config)`` reads a top-level ``conditions:`` list, deep-merges each overlay into a copy of the base, and runs them sequentially with the shared models. Per-condition ``_final_metrics_cache_valid`` short-circuits are honored independently. - ``_check_grouped_field_invariants`` rejects overlays that touch ``target_name``, ``feature_extractor.*``, ``compute_feature_metrics``, or ``use_gpu`` -- those determine which models get loaded and must be identical across conditions. ``io.*``, ``save.*``, ``runtime.*``, ``limit_positions``, ``force_recompute.*`` are free to vary. - Hydra entry point ``evaluate_model_grouped`` with new ``_configs/eval_grouped.yaml`` (inherits ``eval.yaml`` via defaults). - New CLI subcommand registered in ``__main__._HYDRA_COMMANDS``. Tests: ``tests/test_evaluation_grouped.py`` builds a 2-condition cache-only fixture (separate pred plates + cache dirs) and asserts byte-equal parity against sequential per-condition runs, plus rejection cases for empty ``conditions`` and forbidden model-loading-field overrides. Cache-only path means no models actually load -- the test validates the loop structure, not the speedup. Reuses the iohub fixture builders from ``test_evaluation_pipeline_parallel_cpu.py``. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…context unification Followup polish on d69d970 (feat: grouped multi-condition eval driver). Addresses /simplify findings + the two related cleanups the user asked to fold in: Pipeline driver (pipeline.py): - Lazy model load — defer ``load_eval_models`` until the first condition that misses ``_final_metrics_cache_valid``. Restarts where every condition is cache-hit pay no 30–90 s cold start. - Invariant baseline is now the conditions-stripped input config, not the conditions[0]-merged config. A model-loading override smuggled into condition 0 is rejected symmetrically with conditions 1..N (previously it was silently re-baselined). New test guards the regression: ``test_grouped_rejects_condition0_model_field_override``. - ``_merge_condition`` drops the manual ``to_container`` round-trip deep-copy; ``OmegaConf.merge`` already returns a non-aliased config. - ``_check_grouped_field_invariants`` switched to ``OmegaConf.is_config`` in place of the ``type(OmegaConf.create([]))`` idiom; baseline snapshot is computed once and reused per iteration. - Extracted ``_load_cached_final_metrics`` so the three-NPY reload block is shared between ``evaluate_model`` and the grouped driver. - Removed redundant first-iteration merge + ``apply_dataset_ref`` for condition 0; the loop body now does both for every condition, including idx=0. Model loader (model_loader.py) and precompute-gt: - Added ``LoadFlags`` so ``load_eval_models`` can be called from both ``evaluate_predictions`` (all extractors gated by ``compute_feature_metrics``) and ``precompute_gt_artifacts`` (per-extractor ``build.masks`` / ``build.dinov3`` / ``build.dynaclr`` / ``build.celldino`` flags). ``LoadFlags.for_evaluate`` recovers the prior default. The 30-line inline load block in ``precompute_cli.py`` collapses to a single ``load_eval_models(config, flags=LoadFlags(...))`` call. Stricter celldino weights gate is preserved as an explicit pre-load raise. - Added ``init_cache_contexts(config, models)`` and ``init_gt_cache_context(config, models)`` so the four identical ``init_cache_context(..., side=..., **identity_tags)`` call sites in ``evaluate_predictions``, ``_worker_setup``, and ``precompute_gt_artifacts`` consolidate behind the EvalModels bundle. Test fixtures (tests/_eval_fixtures.py): - Promoted the cache-only fixture builders (``build_eval_config``, ``build_fixture``, ``make_hcs_plate``, ``make_mask_cache``, ``live_pipeline_module``, ``read_position_arrays``, ``N_POSITIONS``/``T``/``D``/``H``/``W``) out of ``test_evaluation_pipeline_parallel_cpu.py`` into a private ``_eval_fixtures.py`` module (underscore-prefixed so pytest skips collection). Both ``test_evaluation_pipeline_parallel_cpu.py`` and ``test_evaluation_grouped.py`` now import normally — no ``sys.path.insert`` (project's "no ``sys.path`` edits" rule). CI: - ``.github/workflows/test.yml`` adds ``test_evaluation_grouped.py`` to the ``test-dynacell-configs`` matrix so the grouped driver gets exercised on every push. All 104 tests in the affected suites pass. Pre-existing 48-test failure baseline (test_benchmark_config_composition + one test_evaluation_metrics case) is unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Split the existing ``[grouped] note:`` print into two tiers so users get proportional signal for the configuration they actually picked: - ``executor=process`` + ``require_complete_cache=true`` (cache-only, n_conditions > 1) keeps the mild informational note — workers re-init per condition but skip ``prepare_segmentation_model`` (returns None) and don't instantiate the deep extractors, so the only overhead is N pool spawns. - ``executor=process`` + ``require_complete_cache=false`` (cache-miss, n_conditions > 1) gets a loud multi-line WARNING. This is the actual worst case: each condition's worker pool independently loads SuperModel + DINOv3 + DynaCLR + CELL-DINO, so total wasted cold-start scales as ~30-90 s × N_workers × N_conditions. The warning explicitly points to ``executor=serial`` as the fix, since the parent's pre-loaded ``EvalModels`` is shared across conditions in serial mode but lost across pool spawns in process mode. Two new tests in ``test_evaluation_grouped.py``: - ``test_grouped_warns_loudly_on_process_plus_cache_miss`` confirms the WARNING text fires (executor=process + require_complete_cache=false + 2 conditions) with all the expected keywords. - ``test_grouped_mild_note_on_process_plus_cache_only`` confirms the cache-only fixture (require_complete_cache=true) gets the mild note and does NOT trip the loud WARNING. CLAUDE.md "Grouped multi-condition eval" section updated with the tiered behavior so users searching for the WARNING text find context. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds an opt-in “grouped multi-condition” evaluation path to Dynacell so multiple condition-specific I/O variants can be evaluated in a single Python process, amortizing model/extractor cold-start across conditions while keeping existing dynacell evaluate behavior unchanged.
Changes:
- Introduces
dynacell evaluate-grouped+ Hydraeval_groupedconfig, withevaluate_predictions_grouped()driving per-condition overlays and (optionally) shared preloaded models. - Extracts unified model-loading and cache-context initialization into
evaluation/model_loader.py, reused by both evaluation andprecompute-gt. - Adds a shared cache-only eval fixture module and new grouped-driver tests; updates CI to run the new suite.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| applications/dynacell/src/dynacell/evaluation/pipeline.py | Adds models= kwarg to evaluate_predictions, implements evaluate_predictions_grouped, and factors cached-metrics reload helper. |
| applications/dynacell/src/dynacell/evaluation/model_loader.py | New centralized loader for seg/extractor models + helpers for cache context initialization. |
| applications/dynacell/src/dynacell/evaluation/precompute_cli.py | Switches GT precompute to the unified model loader + GT cache-context helper. |
| applications/dynacell/src/dynacell/evaluation/_configs/eval_grouped.yaml | New Hydra config template describing the conditions: overlay list for grouped eval. |
| applications/dynacell/src/dynacell/main.py | Registers the evaluate-grouped CLI subcommand. |
| applications/dynacell/tests/_eval_fixtures.py | New shared tiny HCS + cache-only fixture builder utilities for integration tests. |
| applications/dynacell/tests/test_evaluation_grouped.py | New grouped-vs-sequential parity + invariant/warning behavior tests. |
| applications/dynacell/tests/test_evaluation_pipeline_parallel_cpu.py | Refactors to reuse _eval_fixtures helpers instead of inline definitions. |
| applications/dynacell/CLAUDE.md | Documents grouped eval usage, constraints, and process-mode caveats. |
| .github/workflows/test.yml | Extends the dynacell configs job to run the new grouped evaluation tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…riant Three fixes triggered by Copilot's review on PR #427 plus an independent struct-mode bug that would have crashed every real Hydra invocation of ``dynacell evaluate-grouped``: 1. ``_merge_condition`` was over-simplified during the /simplify pass (commit c271d3f). ``OmegaConf.merge`` propagates struct mode from its first argument, and Hydra composition always produces struct-mode configs. Result: merging an overlay carrying a ``name`` label (which isn't in the schema) raises ``ConfigKeyError``, and ``del merged["conditions"]`` raises ``ConfigTypeError`` even when the merge succeeds. Restore the ``to_container`` round-trip on the base — it escapes struct mode while still producing an independent config. Same fix applied to the ``models_base`` snapshot a few lines above. Existing tests didn't catch this because they construct configs via ``OmegaConf.create({...})`` directly (non-struct); real users going through ``@hydra.main`` would have hit the crash immediately. 2. Add ``io.require_complete_cache`` to ``_MODEL_LOADING_FIELDS`` so per-condition overlays cannot override it. The flag flips ``prepare_segmentation_model`` between "load SuperModel" and "return None" — letting a condition flip it would mean the shared model bundle is wrong for that condition (None when cache-miss expects a real segmenter, or loaded but never used). Treat as a grouped invariant. (Copilot thread on pipeline.py:1227.) 3. Add ``applications/dynacell/tests/__init__.py``. The new test files use ``from ._eval_fixtures import ...`` relative imports, which work under the project's ``--import-mode=importlib`` setting but are fragile under other pytest import modes. Adding ``__init__.py`` makes ``tests/`` a proper package and the relative imports work regardless of which import mode pytest is invoked with. (Copilot threads on test_evaluation_grouped.py:32 and test_evaluation_pipeline_parallel_cpu.py:42.) Two new regression tests in ``tests/test_evaluation_grouped.py``: - ``test_grouped_rejects_require_complete_cache_override`` covers the new invariant. - ``test_grouped_works_on_struct_mode_config`` wraps the parity fixture in ``OmegaConf.set_struct(grouped_cfg, True)`` to simulate a Hydra-composed config; would have failed before the ``_merge_condition`` fix. All 108 tests in affected suites pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…view Two 75-confidence findings from the parallel review on PR #427 that scored below the 80-threshold for the inline review comment but are worth fixing now while context is fresh: 1. "Reality check for process mode" subsection referenced ``pipeline.py:171`` as the parent seg-model load site. After the eval-parallelism refactor that line is a dataclass field (``pred_ts: list[np.ndarray] = ...``); the actual load now lives in ``load_eval_models(config)`` called from the top of ``evaluate_predictions``. Switch the reference from a brittle line number to a function-name pointer. 2. "Grouped multi-condition eval" subsection said the per-condition cache-load short-circuit fires "with ``force_recompute.final_metrics =false``", omitting the second gate: ``_final_metrics_cache_valid`` also returns False (forcing recompute) when ``force_recompute.all =true``, regardless of the ``final_metrics`` flag. Make both gates explicit so users who set ``force_recompute.all=true`` for unrelated reasons aren't surprised by unexpectedly recomputed metrics. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
alxndrkalinin
added a commit
that referenced
this pull request
May 20, 2026
…im bloat The eval README had grown stale relative to the runtime/grouped/cache work landed on this branch and PR #427: no mention of `io.require_complete_cache`'s cache-only fast path, no `runtime.*` block, no `dynacell evaluate-grouped`, no CELL-DINO. This rewrite folds those in and restructures: FOV parallelism + grouped eval move out of "Artifact caches" (where they were mis-classified) into a new top-level "Scaling" section; a new "Quick start" sits above Configuration; HF-cache operational details go to a new "HPC notes" section at the end. Bloat trim along the way: components table compressed (related modules combined, trivial files pushed to a note), HF-cache + external-users sections shortened, the 11-row force_recompute table collapsed to a YAML block plus 5 grouped descriptions, redundant Hydra group-discovery paragraphs removed. No flags, footguns, or CLI options dropped — every config knob and env var from the old version is still documented. 466 → 265 lines. Public-repo hygiene: replaced the hard-coded `/hpc/projects/comp.micro/...` default for the team-shared HF cache with a description that points at the `_DEFAULT_SHARED_HF_CACHE` constant in `dynacell/__main__.py` and the `DYNACELL_SHARED_HF_CACHE` env-var override. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
alxndrkalinin
added a commit
that referenced
this pull request
May 21, 2026
* feat(dynacell): add eval runtime module + thread budgeting (C1)
Adds dynacell.evaluation.runtime with three layered thread-cap entry points:
early_apply_env_caps() sets BLAS/OMP env vars before C-extension load (called
from __main__.main_cli); apply_thread_budget(threads) is the in-process safety
net (env + torch.set_num_threads + threadpool_limits); resolve_runtime(config)
materializes the new runtime config block.
The runtime block (eval.yaml) is opt-in for parallelism (default executor=serial,
fov_workers=1) but unconditionally tightens BLAS/OMP threads to cpu_count by
default. On SLURM nodes where --cpus-per-task < host CPU count, this corrects
previously-loose BLAS threading. Set DYNACELL_THREADS_PER_WORKER=N to override
at the env layer for cases where in-process caps come too late.
precompute_gt_artifacts validates that runtime.executor='serial' and
fov_workers=1 — FOV-level parallelism is restricted to evaluate_predictions
in this commit; precompute parallelism is out of scope (DeepFeatureBatcher
accumulates state across FOVs).
resolve_runtime supports two-phase resolution for evaluate_predictions:
top-of-function for parent BLAS cap, then post-position-list with
freeze_threads_per_worker= to keep parent and worker BLAS budgets consistent.
Worker primitives (_process_one_fov, ProcessPoolExecutor) land in C3+C4.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(dynacell): per-T memory hygiene + region timing instrumentation (C2)
Adds always-on region timers (RegionTimer context manager) that record
wall time of FOV-level and per-T regions to <save_dir>/eval_timing.csv.
Region taxonomy matches actual execution placement at pipeline.py:341-505:
FOV-level (t=None): mask_gt, mask_pred, cp_gt, deep_gt_dinov3,
deep_gt_dynaclr, deep_gt_celldino, features_pred_per_t, microssim, seg_write
Per-T: pixel_metrics, mask_metrics, feature_pairwise
Timing overhead is ~0.1 ms per region per timepoint; ~120 ms total on a
9-h eval (~4e-6 of wall time) — no gating knob.
Per-T memory hygiene knobs (runtime.cuda_empty_cache_every_n_timepoints,
runtime.gc_collect_every_n_fovs) ship at 0 (no behavior change) in this
commit. The plan calls for measuring and flipping defaults in C2 — that
measurement requires a real eval run on the test48 zarr that exceeds this
session's scope. Defaults stay at 0; observed late-T degradation in
prior A549 ER evals (18 -> 61 s/T over 10 timepoints) is the documented
motivation but not the measured fix in this commit.
DYNACELL_FORCE_PER_T_HYGIENE=1 env var flips both knobs on at runtime
regardless of YAML defaults — operator escape hatch for post-ship
mitigation without a code change.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(dynacell): extract per-FOV body into _process_one_fov + FovResult (C3)
Pure refactor: extract the per-FOV body of evaluate_predictions into a
module-level function _process_one_fov(...) -> FovResult and an
_aggregate_fov_result(...) parent-side aggregator. Serial-path behavior
is identical to the prior code (verified: 384/48 pass/fail parity with
HEAD~2; the 48 failures are all pre-existing).
FovResult carries everything one FOV contributes to the run as a
picklable dataclass:
- per_t_pixel_rows (with microssim already merged in per pipeline.py:471
semantics — no separate microssim_rows field),
- per_t_mask_rows, per_t_feature_rows,
- seg_array of shape (T, 2, D, H, W) bool (channel 0 = pred, channel 1 = GT),
- 4 _BackboneLists for cp/dinov3/dynaclr/celldino — each holds the per-T
pred_feats/gt_feats/pred_fovs/gt_fovs/pred_ts/gt_ts arrays. Workers
preserve the existing `if size > 0: append` guard so empty per-T slots
never enter the lists; aggregator extends parent lists unconditionally.
- timings slice from the per-process region_timer collector.
_aggregate_fov_result writes the seg array to the HCS plate (still in the
parent — concurrent create_position from workers would race plate
metadata), extends per-T row lists, and extends the 24 dataset-level
accumulators.
This refactor positions C4 to add ProcessPoolExecutor + worker primitives
without further changes to the FOV body. flush_manifest stays in the
parent's loop for now; under process mode (C4) each worker will flush
its own ctx at FOV end.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(dynacell): wire ProcessPoolExecutor + GPU serialization lock (C4)
Adds worker primitives in runtime.py (_worker_initializer,
gpu_serialization_lock, make_fov_executor) and the worker entry point
_worker_run_fov in pipeline.py. evaluate_predictions now branches on
runtime.executor: serial (default) runs _process_one_fov inline
identical to today; process spawns a ProcessPoolExecutor over FOVs.
Worker design:
- Spawn context avoids inheriting parent CUDA. _worker_initializer
sets BLAS/OMP env caps BEFORE apply_thread_budget lazy-imports torch.
- _worker_setup lazy-loads seg_model + extractors under
gpu_serialization_lock(): N workers don't init CUDA + load weights
concurrently. Plate handles cached per-worker.
- GPU touch sites inside _process_one_fov are wrapped in
gpu_serialization_lock(): mask_gt/pred, cp_gt, deep_gt_{dinov3,dynaclr,
celldino}, features_pred_per_t, pixel_metrics (per-T, GPU-bound
end-to-end including cubic spectral), and on-demand cellpose fallback
inside mask_metrics. In serial mode the lock has no path set and yields
immediately — bit-for-bit-identical behavior.
- Lock file at /tmp/dynacell_gpu_<SLURM_JOB_ID-or-PID>.lock (node-local,
not NFS). One lock per SLURM job, shared across workers.
Phase-2 runtime resolution: after pred_positions is built,
resolve_runtime is re-called with n_positions= to clamp "auto"
fov_workers to min(provisional, n_positions) and freeze
threads_per_worker at the Phase-1 value so worker initializers match
the parent's already-applied BLAS cap.
Order: results collected via as_completed (arrival order) but aggregated
in original pred_positions order so per-FOV CSV + embedding NPZ row
order is deterministic regardless of executor.
Workers flush their own ctx manifest at FOV end (global manifest lock at
_pos_write_lock(ctx, "manifest", "global") fcntl serializes flushes
across workers); interrupted process-mode runs preserve cache progress
the same way serial mode did.
Tests: full dynacell suite 384/48 pass/fail (identical to baseline; all
48 failures pre-existing). No new spawn-mode test in this commit —
covered by C5 CPU integration test.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test(dynacell): runtime + parallel tests, CI wiring, SLURM env caps, docs (C5)
Adds test coverage for the C1-C4 runtime module and FOV-parallelism wiring:
test_runtime.py (23 tests): apply_thread_budget no-crash semantics under
Hydra's main; resolve_runtime auto/freeze/validate paths; region timing
collector; DYNACELL_FORCE_PER_T_HYGIENE env-var hook; spawn-context
ProcessPoolExecutor smoke via stdlib os.getpid.
test_evaluation_pipeline_parallel.py (3 tests): FovResult pickle round-trip
with realistic per-backbone shapes (the path that ProcessPoolExecutor takes
for Future.result transport); _aggregate_fov_result extends all 24
dataset-level accumulator lists correctly. Re-imports the live pipeline
module inside test bodies because test_lazy_init.py pops dynacell.* modules
from sys.modules, which would invalidate pickle resolution of a
module-level imported FovResult.
CI wiring:
- Root pyproject.toml registers the "slow" marker and appends -m "not slow"
to existing addopts.
- test-dynacell-configs runs test_runtime.py + test_evaluation_pipeline_parallel.py
alongside existing benchmark-schema/submit-tool tests. No new heavy deps
needed (these tests don't import iohub, cubic, transformers).
SLURM env caps:
- run_eval_jointtrained_nucleus.slurm and
run_eval_unext2_jointtrained_membrane.slurm export
DYNACELL_THREADS_PER_WORKER=\$SLURM_CPUS_PER_TASK before invoking eval.
Docs:
- applications/dynacell/CLAUDE.md gains "Eval runtime / parallelism"
section documenting the config block, env-var hooks, timing
instrumentation, and process-mode caveats.
Full dynacell test suite: 410 passed / 48 failed / 65 skipped — the 48
failures are the same set that fails on dynacell-models@HEAD (verified
parity across all 5 commits in this branch).
End-to-end serial-vs-process parity test on a real iohub+cache fixture is
a follow-up (see .claude/plans/eval-parallelism.md §C5).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(dynacell): drop parent seg_model under executor=process to avoid duplicate residency
Under executor=process, workers lazy-load their own seg_model copies (under
the GPU lock) — but the parent had been holding a duplicate copy GPU-resident
since the unconditional prepare_segmentation_model(config) call at the top of
evaluate_predictions. On an 80 GB A100 with fov_workers=4, this meant
(N+1) × model_weights resident instead of N.
After Phase-2 runtime resolution (where the final executor is known), drop
the parent's copy when executor=process. The pre-warm side effect
(segmenter_model_zoo's validate_model populating the checkpoint dir to
avoid N workers racing on a cold quilt download) is already done by then.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(dynacell): resolve config interpolations in-place before worker submit
Belt-and-suspenders safety net suggested in PR #426 review: call
OmegaConf.resolve(config) in-place after Phase-1 runtime resolution and
before any worker pickle.dumps the DictConfig across the spawn boundary.
OmegaConf supports lazy interpolation resolution natively, so workers
*would* resolve ${...} refs on first attribute access — but doing it once
in the parent eliminates a tiny per-worker hot-path cost and guards against
the (unlikely) case where an interpolation depends on parent-only state.
??? MISSING fields are untouched: verified OmegaConf 2.3 `_resolve` walks
only nodes where `_is_interpolation()` is true, so bare ??? values pass
through. Confirmed: 63 tests pass across pipeline / compose / parallel /
runtime suites.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(dynacell): silence inner per-T tqdm in spawn workers
Adds a runtime.is_worker() helper that returns True iff
_worker_initializer has set _GPU_LOCK_PATH. Wire it into
_process_one_fov's inner-T tqdm via disable=is_worker(), so under
fov_workers>1 the N concurrent per-T bars don't interleave on the
parent's stderr. Outer per-FOV tqdm (in the parent) stays visible
because the parent's _GPU_LOCK_PATH is never set.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(dynacell): don't double-count timing rows under executor=serial
Bug: _process_one_fov writes region timings to the global _TIMINGS
collector via region_timer, and returns the slice in FovResult.timings.
_aggregate_fov_result then calls extend_timings(result.timings)
unconditionally. In serial mode this re-appends each row to the SAME
parent collector — every timed region appears twice in eval_timing.csv.
In process mode the second append is correct (workers have separate
per-process _TIMINGS, parent's is empty until aggregated).
Fix: add extend_worker_timings kwarg to _aggregate_fov_result. Pass
True in process mode, False in serial. Verified via 63-test suite green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(dynacell): context-manage worker plate handles per FOV
_worker_setup no longer caches pred/gt/seg plate handles in _WORKER_STATE.
Instead, _worker_run_fov opens them via `with open_ome_zarr(...) as ...`
scoped to one FOV — iohub file descriptors close between FOVs and the
worker has nothing to clean up on ProcessPoolExecutor shutdown.
Models + cache contexts stay cached in _WORKER_STATE (their load cost
~5-10 s per worker is too high to repeat per FOV; ProcessPoolExecutor
process exit reclaims them).
Position lookup by name uses a linear scan of plate.positions() (iohub
has no name->position index). Cheap for the ≤ tens of positions per
typical eval — sub-ms per scan.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(dynacell): skip prepare_segmentation_model under require_complete_cache
When io.require_complete_cache=true, the caller asserts that all needed
masks live in the cache — so loading SuperModel (which hits a quilt
download for nucleus/membrane on a cold node) is wasted work. Return
None unconditionally in that mode. If a cache miss happens despite the
assertion, the cache layer raises StaleCacheError before any segment()
call, so the None seg_model never gets dereferenced.
Enables the cache-only/no-model path that C5's integration test design
relies on (run evaluate_predictions end-to-end against prebuilt mask
caches without instantiating cellpose/SuperModel).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test(dynacell): add CPU integration test for serial-vs-process parity
Drives evaluate_predictions end-to-end on a tiny iohub fixture (3 positions
× T=2 × D=4 × H=W=32) with prebuilt mask caches. Runs the pipeline twice —
once with runtime.executor=serial, once with runtime.executor=process
runtime.fov_workers=2 — and asserts:
1. pixel_metrics + mask_metrics rows are byte-equal after sort by FOV.
2. segmentation_results.zarr per-position content is byte-equal across
both channels.
Cache-only design (matches plan §C5): target_name=er +
require_complete_cache=true + compute_feature_metrics=false +
compute_microssim=false + pixel_metrics.fsc=null +
pixel_metrics.spectral_pcc=null. The full pipeline runs without
instantiating cellpose, SuperModel, DinoV3, DynaCLR, CellDino, or cubic.
What this validates: spawn boot, DictConfig pickling, FovResult.timings +
arrays round-trip through pickle, _aggregate_fov_result extends parent
accumulators correctly, parent-side segmentation_results.zarr writes
don't race, sort-by-pos_name aggregation, and the new
extend_worker_timings semantics doesn't double-count timing rows.
_live_pipeline_module() clears dynacell.evaluation.* from sys.modules
before importing — necessary because test_evaluation_pipeline.py stubs
metrics/utils via monkeypatch.setitem(sys.modules, ...), and without the
clear my pipeline's compute_pixel_metrics resolves to the empty-returning
stub.
CI: runs ~16 s on the existing test-dynacell-configs job.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* ci(dynacell): include CPU integration test in dynacell-configs CI job
Adds tests/test_evaluation_pipeline_parallel_cpu.py to the dynacell
pytest invocation. The test runs end-to-end evaluate_predictions in
both serial and spawn-process modes against a tiny iohub fixture +
prebuilt mask cache — no eval extras (cellpose, transformers, cubic)
needed because target_name=er + io.require_complete_cache=true
short-circuit segmenter and feature-extractor loads.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(dynacell): harden runtime resolution + gate GPU lock on use_gpu
Four Copilot-review findings on the runtime layer:
1. apply_thread_budget threadpool_limits re-arm: the old handle becomes
unreachable and is GC'd. The previous comment said "stays alive"
which was misleading. Document the actual behavior: we WANT the new
limit active, dropping the old handle is intentional. Also remove
the dead if-branch since both arms did the same assignment.
2. resolve_runtime accepts literal fov_workers=0 / threads_per_worker=0
which produces divide-by-zero in the "auto" math or nonsensical
thread counts. Add explicit >= 1 validation with a clear error.
3. _cpu_count() prefers os.sched_getaffinity(0) over os.cpu_count() so
SLURM cgroup limits (cpuset etc.) are honored — otherwise auto-
resolved fov_workers/threads_per_worker can oversize past the
cgroup-visible CPU budget. Falls back to os.cpu_count() on non-Linux.
4. gpu_serialization_lock now takes a gate: bool = True kwarg. Callers
in pipeline.py pass gate=use_gpu so under config.use_gpu=false the
lock is a no-op even in process mode. Previously the lock fired
unconditionally, defeating cross-worker parallelism for CPU-only
eval runs. The lock still gates on _GPU_LOCK_PATH for the
parent/serial no-op (unchanged).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* perf(dynacell): stream aggregation under executor=process to bound peak memory
Previously buffered every FovResult in results_by_pos until ALL N
worker futures completed, then aggregated in pos_names_in_order. Peak
parent memory: N × seg_array (~N × 440 MB for T=10 D=21 H=W=1024 evals)
all live simultaneously.
Switch to streaming aggregation: drain the buffer opportunistically in
pos_names_in_order as futures complete. Only out-of-order FovResults
stay buffered; once the next-expected position lands we aggregate
(plate write + accumulator extend) and drop the ref. Peak buffer is
bounded by the number of out-of-order FOVs in flight, not by N.
Deterministic per-FOV CSV / embedding NPZ row order is preserved
because aggregation still happens in pos_names_in_order — early
completions just wait their turn in the buffer.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(dynacell): grouped multi-condition eval driver (#427)
* feat(dynacell): grouped multi-condition eval driver
Add ``dynacell evaluate-grouped``: run the same (model, organelle) across
multiple I/O variants (the canonical case is the three A549 treatment
plates mock/denv/zikv) in one Python process, paying the segmenter +
DINOv3 + DynaCLR + CELL-DINO cold-start once instead of N times. On
serial-mode interactive jobs and on per-condition SLURM jobs this is
30-90s of weight load + checkpoint warmup amortized across all
conditions. Under runtime.executor=process, each condition still spawns
its own pool whose workers re-load model copies, so only the parent-side
load is amortized; the driver prints a note when it detects this.
Mechanism:
- ``dynacell.evaluation.model_loader.load_eval_models`` extracts the
segmenter + extractor instantiation that previously lived inline in
``evaluate_predictions`` and ``_worker_setup``. The bundle carries
identity tags (dinov3_model_name, dynaclr_ckpt source paths, encoder
cfg, celldino weights) so ``init_cache_context`` callers get identical
cache-identity values across conditions.
- ``evaluate_predictions(config, *, models=None)`` accepts a pre-loaded
bundle and skips its inline load when supplied. ``models=None``
preserves the historical single-condition behavior; existing callers
and tests are unchanged.
- ``evaluate_predictions_grouped(config)`` reads a top-level
``conditions:`` list, deep-merges each overlay into a copy of the
base, and runs them sequentially with the shared models. Per-condition
``_final_metrics_cache_valid`` short-circuits are honored independently.
- ``_check_grouped_field_invariants`` rejects overlays that touch
``target_name``, ``feature_extractor.*``, ``compute_feature_metrics``,
or ``use_gpu`` -- those determine which models get loaded and must be
identical across conditions. ``io.*``, ``save.*``, ``runtime.*``,
``limit_positions``, ``force_recompute.*`` are free to vary.
- Hydra entry point ``evaluate_model_grouped`` with new
``_configs/eval_grouped.yaml`` (inherits ``eval.yaml`` via defaults).
- New CLI subcommand registered in ``__main__._HYDRA_COMMANDS``.
Tests: ``tests/test_evaluation_grouped.py`` builds a 2-condition
cache-only fixture (separate pred plates + cache dirs) and asserts
byte-equal parity against sequential per-condition runs, plus rejection
cases for empty ``conditions`` and forbidden model-loading-field
overrides. Cache-only path means no models actually load -- the test
validates the loop structure, not the speedup. Reuses the iohub fixture
builders from ``test_evaluation_pipeline_parallel_cpu.py``.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(dynacell): simplify grouped eval driver + fold load + cache-context unification
Followup polish on d69d970 (feat: grouped multi-condition eval driver).
Addresses /simplify findings + the two related cleanups the user asked to
fold in:
Pipeline driver (pipeline.py):
- Lazy model load — defer ``load_eval_models`` until the first condition
that misses ``_final_metrics_cache_valid``. Restarts where every
condition is cache-hit pay no 30–90 s cold start.
- Invariant baseline is now the conditions-stripped input config, not
the conditions[0]-merged config. A model-loading override smuggled
into condition 0 is rejected symmetrically with conditions 1..N
(previously it was silently re-baselined). New test guards the
regression: ``test_grouped_rejects_condition0_model_field_override``.
- ``_merge_condition`` drops the manual ``to_container`` round-trip
deep-copy; ``OmegaConf.merge`` already returns a non-aliased config.
- ``_check_grouped_field_invariants`` switched to ``OmegaConf.is_config``
in place of the ``type(OmegaConf.create([]))`` idiom; baseline
snapshot is computed once and reused per iteration.
- Extracted ``_load_cached_final_metrics`` so the three-NPY reload block
is shared between ``evaluate_model`` and the grouped driver.
- Removed redundant first-iteration merge + ``apply_dataset_ref`` for
condition 0; the loop body now does both for every condition,
including idx=0.
Model loader (model_loader.py) and precompute-gt:
- Added ``LoadFlags`` so ``load_eval_models`` can be called from both
``evaluate_predictions`` (all extractors gated by
``compute_feature_metrics``) and ``precompute_gt_artifacts``
(per-extractor ``build.masks`` / ``build.dinov3`` / ``build.dynaclr``
/ ``build.celldino`` flags). ``LoadFlags.for_evaluate`` recovers the
prior default. The 30-line inline load block in ``precompute_cli.py``
collapses to a single ``load_eval_models(config, flags=LoadFlags(...))``
call. Stricter celldino weights gate is preserved as an explicit
pre-load raise.
- Added ``init_cache_contexts(config, models)`` and
``init_gt_cache_context(config, models)`` so the four identical
``init_cache_context(..., side=..., **identity_tags)`` call sites in
``evaluate_predictions``, ``_worker_setup``, and
``precompute_gt_artifacts`` consolidate behind the EvalModels bundle.
Test fixtures (tests/_eval_fixtures.py):
- Promoted the cache-only fixture builders (``build_eval_config``,
``build_fixture``, ``make_hcs_plate``, ``make_mask_cache``,
``live_pipeline_module``, ``read_position_arrays``,
``N_POSITIONS``/``T``/``D``/``H``/``W``) out of
``test_evaluation_pipeline_parallel_cpu.py`` into a private
``_eval_fixtures.py`` module (underscore-prefixed so pytest skips
collection). Both ``test_evaluation_pipeline_parallel_cpu.py`` and
``test_evaluation_grouped.py`` now import normally — no
``sys.path.insert`` (project's "no ``sys.path`` edits" rule).
CI:
- ``.github/workflows/test.yml`` adds ``test_evaluation_grouped.py`` to
the ``test-dynacell-configs`` matrix so the grouped driver gets
exercised on every push.
All 104 tests in the affected suites pass. Pre-existing 48-test failure
baseline (test_benchmark_config_composition + one test_evaluation_metrics
case) is unchanged.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(dynacell): tier grouped-eval process-mode message by cache state
Split the existing ``[grouped] note:`` print into two tiers so users get
proportional signal for the configuration they actually picked:
- ``executor=process`` + ``require_complete_cache=true`` (cache-only,
n_conditions > 1) keeps the mild informational note — workers re-init
per condition but skip ``prepare_segmentation_model`` (returns None)
and don't instantiate the deep extractors, so the only overhead is
N pool spawns.
- ``executor=process`` + ``require_complete_cache=false`` (cache-miss,
n_conditions > 1) gets a loud multi-line WARNING. This is the actual
worst case: each condition's worker pool independently loads
SuperModel + DINOv3 + DynaCLR + CELL-DINO, so total wasted cold-start
scales as ~30-90 s × N_workers × N_conditions. The warning explicitly
points to ``executor=serial`` as the fix, since the parent's
pre-loaded ``EvalModels`` is shared across conditions in serial mode
but lost across pool spawns in process mode.
Two new tests in ``test_evaluation_grouped.py``:
- ``test_grouped_warns_loudly_on_process_plus_cache_miss`` confirms the
WARNING text fires (executor=process + require_complete_cache=false +
2 conditions) with all the expected keywords.
- ``test_grouped_mild_note_on_process_plus_cache_only`` confirms the
cache-only fixture (require_complete_cache=true) gets the mild note
and does NOT trip the loud WARNING.
CLAUDE.md "Grouped multi-condition eval" section updated with the
tiered behavior so users searching for the WARNING text find context.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(dynacell): grouped eval struct-mode + require_complete_cache invariant
Three fixes triggered by Copilot's review on PR #427 plus an independent
struct-mode bug that would have crashed every real Hydra invocation of
``dynacell evaluate-grouped``:
1. ``_merge_condition`` was over-simplified during the /simplify pass
(commit c271d3f). ``OmegaConf.merge`` propagates struct mode from
its first argument, and Hydra composition always produces struct-mode
configs. Result: merging an overlay carrying a ``name`` label (which
isn't in the schema) raises ``ConfigKeyError``, and ``del
merged["conditions"]`` raises ``ConfigTypeError`` even when the merge
succeeds. Restore the ``to_container`` round-trip on the base — it
escapes struct mode while still producing an independent config. Same
fix applied to the ``models_base`` snapshot a few lines above.
Existing tests didn't catch this because they construct configs via
``OmegaConf.create({...})`` directly (non-struct); real users going
through ``@hydra.main`` would have hit the crash immediately.
2. Add ``io.require_complete_cache`` to ``_MODEL_LOADING_FIELDS`` so
per-condition overlays cannot override it. The flag flips
``prepare_segmentation_model`` between "load SuperModel" and "return
None" — letting a condition flip it would mean the shared model
bundle is wrong for that condition (None when cache-miss expects a
real segmenter, or loaded but never used). Treat as a grouped
invariant. (Copilot thread on pipeline.py:1227.)
3. Add ``applications/dynacell/tests/__init__.py``. The new test files
use ``from ._eval_fixtures import ...`` relative imports, which work
under the project's ``--import-mode=importlib`` setting but are
fragile under other pytest import modes. Adding ``__init__.py``
makes ``tests/`` a proper package and the relative imports work
regardless of which import mode pytest is invoked with. (Copilot
threads on test_evaluation_grouped.py:32 and
test_evaluation_pipeline_parallel_cpu.py:42.)
Two new regression tests in ``tests/test_evaluation_grouped.py``:
- ``test_grouped_rejects_require_complete_cache_override`` covers the
new invariant.
- ``test_grouped_works_on_struct_mode_config`` wraps the parity fixture
in ``OmegaConf.set_struct(grouped_cfg, True)`` to simulate a
Hydra-composed config; would have failed before the
``_merge_condition`` fix.
All 108 tests in affected suites pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs(dynacell): fix two CLAUDE.md doc nits flagged in PR #427 self-review
Two 75-confidence findings from the parallel review on PR #427 that
scored below the 80-threshold for the inline review comment but are
worth fixing now while context is fresh:
1. "Reality check for process mode" subsection referenced
``pipeline.py:171`` as the parent seg-model load site. After the
eval-parallelism refactor that line is a dataclass field
(``pred_ts: list[np.ndarray] = ...``); the actual load now lives in
``load_eval_models(config)`` called from the top of
``evaluate_predictions``. Switch the reference from a brittle line
number to a function-name pointer.
2. "Grouped multi-condition eval" subsection said the per-condition
cache-load short-circuit fires "with ``force_recompute.final_metrics
=false``", omitting the second gate: ``_final_metrics_cache_valid``
also returns False (forcing recompute) when ``force_recompute.all
=true``, regardless of the ``final_metrics`` flag. Make both gates
explicit so users who set ``force_recompute.all=true`` for unrelated
reasons aren't surprised by unexpectedly recomputed metrics.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Alexandr Kalinin <alxndrkalinin@users.noreply.github.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(dynacell): keep SuperModel under require_complete_cache when pred cache off
`prepare_segmentation_model` was returning None unconditionally
whenever `io.require_complete_cache=true`, on the assumption that all
masks would be served from cache. That assumption breaks for
nucleus/membrane targets when `io.pred_cache_dir=None`: the per-T loop
in `_process_one_fov` falls back to `segment(predict[t],
seg_model=seg_model)`, which raises `ValueError` because `seg_model` is
None.
Narrow the early return to cases where no path can reach `segment()`
without a cached mask — organelle targets (workflow-based, no
`seg_model` needed) or nucleus/membrane runs that also have
`io.pred_cache_dir` configured. Reported by Copilot on PR #426.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* docs(dynacell): align gpu_serialization_lock docstring with implementation
The docstring claimed "no CUDA available in the worker" was one of the
no-op conditions, but the implementation only checks `gate` and
`_GPU_LOCK_PATH`. On a CPU-only node with `use_gpu=true` the lock would
still serialize across workers. Rather than add a runtime CUDA probe
(callers already pass `gate=use_gpu` to opt out for CPU-only regions),
correct the docstring to describe what the code actually does.
Reported by Copilot on PR #426.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* docs(dynacell): rewrite eval README — runtime/grouped/cache-only + trim bloat
The eval README had grown stale relative to the runtime/grouped/cache
work landed on this branch and PR #427: no mention of
`io.require_complete_cache`'s cache-only fast path, no `runtime.*`
block, no `dynacell evaluate-grouped`, no CELL-DINO.
This rewrite folds those in and restructures: FOV parallelism + grouped
eval move out of "Artifact caches" (where they were mis-classified)
into a new top-level "Scaling" section; a new "Quick start" sits above
Configuration; HF-cache operational details go to a new "HPC notes"
section at the end.
Bloat trim along the way: components table compressed (related modules
combined, trivial files pushed to a note), HF-cache + external-users
sections shortened, the 11-row force_recompute table collapsed to a
YAML block plus 5 grouped descriptions, redundant Hydra
group-discovery paragraphs removed. No flags, footguns, or CLI
options dropped — every config knob and env var from the old version
is still documented. 466 → 265 lines.
Public-repo hygiene: replaced the hard-coded
`/hpc/projects/comp.micro/...` default for the team-shared HF cache
with a description that points at the `_DEFAULT_SHARED_HF_CACHE`
constant in `dynacell/__main__.py` and the `DYNACELL_SHARED_HF_CACHE`
env-var override.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* fix(dynacell): address PR #426 code-review findings
Five fixes from a code-review pass on the controlled FOV-parallel eval
work:
- Restore the seg_model invariant for grouped multi-condition runs.
After 5829175, ``prepare_segmentation_model``'s load decision
depends on ``io.pred_cache_dir`` in addition to
``io.require_complete_cache``, but ``_MODEL_LOADING_FIELDS`` only
caught the latter. A per-condition overlay setting
``io.pred_cache_dir=None`` against a base that has it set (for
nucleus/membrane + ``require_complete_cache=true``) would crash
at runtime: the shared ``seg_model`` is None but the per-T loop
falls back to ``segment(predict[t], seg_model=None)``. New
``_seg_model_required()`` helper mirrors the load decision and
``_check_grouped_field_invariants`` rejects the dangerous direction
only, leaving canonical per-condition pred caches free to differ.
- Fix the process-mode progress bar stalling. ``pbar.update(1)`` only
fired inside the in-order drain loop, so if FOV 0 was slow and later
FOVs completed first, the bar stayed at 0% until FOV 0 drained.
Update on each worker completion instead.
- Correct the misleading ``[grouped] note:`` for ``executor=process`` +
``require_complete_cache=true``. The message claimed "no models
actually load" but deep extractors do load whenever
``compute_feature_metrics=true``, and the segmenter only short-
circuits in a subset of cases. Rewrite to match reality.
- Move ``runtime.*`` imports (12 symbols) from four inline
per-function blocks to one top-of-file import. No spawn-ordering or
circular-import constraint blocks them; the inline pattern just
violated the project's "no inline imports without strong reason"
rule and duplicated a ``flush_manifest`` import that already lives
at module top.
New regression test ``test_grouped_rejects_pred_cache_dir_flip_for_nucleus``
guards the dangerous direction; an existing test's assertion updates to
the corrected note wording.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Alexandr Kalinin <alxndrkalinin@users.noreply.github.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
edyoshikun
added a commit
that referenced
this pull request
May 22, 2026
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Eduardo Hirata-Miyasaki <edhiratam@gmail.com> Co-authored-by: dihan.zheng <dihan.zheng@ucsf.edu> fix(viscy-data): drop use_thread_workers to fix DDP deadlock (#413) fix(viscy-data): port to iohub 0.3.2 ImageArray API (#407) fix shipped via PR #412 fixes triggered by Copilot's review on PR #427 plus an independent fix two CLAUDE.md doc nits flagged in PR #427 self-review fix(dynacell): address PR #426 code-review findings fix(dynacell): harden DynacellGAN against silent failures (PR #428 review)
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.
Summary
Adds a new
dynacell evaluate-groupedsubcommand that runs the same(model, organelle)across multiple I/O variants — typically the three A549 treatment plates (mock/denv/zikv) — in one Python process, amortizing the segmenter + DINOv3 + DynaCLR + CELL-DINO cold-start (~30–90 s) across all conditions instead of paying it N times.The driver is opt-in. Existing per-condition runs via
dynacell evaluateare unchanged.Stacked on #426
This branch is based on
eval-parallelism(PR #426). The diff here is only this feature's 3 commits; #426 should land first. The base PR'sEvalModelsextraction was the prerequisite — that's whyc271d3f0consolidates the dualinit_cache_contextcall sites and the inline-load duplication inprecompute_cli.pybehind the same bundle.Commits
d69d9709evaluate_predictions(..., models=)kwarg +evaluate_predictions_grouped+ Hydra entry)c271d3f0LoadFlagsconsolidatesprecompute_cliload;init_cache_contextshelper)e84ed6a1require_complete_cache=true; loud WARNING for the cache-miss × N-conditions × N-workers worst case)Performance characteristics
The amortization win depends on the executor mode:
runtime.executor=serial(recommended for grouped): the parent'sEvalModelsbundle is passed toevaluate_predictions(..., models=...)for every condition. Models load once total. Full amortization.runtime.executor=process: only the parent-side pre-warm is amortized; each condition spawns a freshProcessPoolExecutorwhose workers re-load their own model copies. The driver detects this and tiers its output:require_complete_cache=true+n_conditions > 1→ mild informational note (workers don't actually load any models on the cache-only path, so re-init is cheap)require_complete_cache=false+n_conditions > 1→ loud multi-line WARNING with explicit cost estimate and pointer toexecutor=serialConstraints on per-condition overlays
Per-condition entries may freely override
io.*,save.*,runtime.*,limit_positions,force_recompute.*, and carry anamelabel. They MUST NOT changetarget_name,feature_extractor.*,compute_feature_metrics, oruse_gpu— those gate which models get loaded. The invariant check is symmetric across all conditions (including condition 0), so a model-loading override smuggled into the first condition is rejected, not silently re-baselined.Example config
Run:
Test plan
tests/test_evaluation_grouped.py(NEW, 6 tests):test_grouped_matches_sequential_per_condition— byte-equal parity vs sequential per-condition runs (pixel CSV, mask CSV, seg plate)test_grouped_rejects_model_loading_field_overrides— condition N>0 sneaking intarget_nameraisestest_grouped_rejects_condition0_model_field_override— condition 0 sneaking intarget_nameraises (regression guard for symmetric baseline)test_grouped_rejects_empty_conditions— missing/empty conditions list raisestest_grouped_warns_loudly_on_process_plus_cache_miss— WARNING fires on the bad-config combotest_grouped_mild_note_on_process_plus_cache_only— only mild note on cache-only pathtests/test_evaluation_precompute_cli.py— covers the unifiedLoadFlags/load_eval_modelspath now used by precompute-gttest_benchmark_config_composition, onetest_evaluation_metricscase) unchanged.github/workflows/test.ymlextended to runtest_evaluation_grouped.pyin thetest-dynacell-configsjobFiles touched
applications/dynacell/src/dynacell/evaluation/model_loader.pyapplications/dynacell/src/dynacell/evaluation/_configs/eval_grouped.yamlapplications/dynacell/tests/_eval_fixtures.py(extracted fromtest_evaluation_pipeline_parallel_cpu.pyso the new test imports normally instead ofsys.path.insert)applications/dynacell/tests/test_evaluation_grouped.pyapplications/dynacell/src/dynacell/evaluation/pipeline.py(kwarg + grouped driver + cache-load helper extraction)applications/dynacell/src/dynacell/evaluation/precompute_cli.py(uses unifiedload_eval_models+init_gt_cache_context)applications/dynacell/src/dynacell/__main__.py(registersevaluate-grouped)applications/dynacell/CLAUDE.md(new "Grouped multi-condition eval" subsection)applications/dynacell/tests/test_evaluation_pipeline_parallel_cpu.py(imports from_eval_fixtures).github/workflows/test.yml(adds grouped test to CI matrix)🤖 Generated with Claude Code