Repository navigation
Fix FlexibleBatchSampler DDP wiring + epoch advance + dataset mixed-C guard - #408
Merged
Merged
Conversation
Covers the ``batch_group_by="marker"`` regime used by DynaCLR-2D-MIP-BagOfChannels. Three tests verify: - every batch contains exactly one marker (single-marker batches), - all markers surface at least once across one epoch, and - consecutive batches interleave markers rather than draining each marker group sequentially. The third check would catch a pathological sampler that yields all Phase3D batches first, then all TOMM20, etc. — previous tests only asserted per-batch homogeneity, which does not rule out sequential draining. Uses a new fixture ``multi_marker_anchors`` (1 experiment × 4 markers × 2 conditions × 40 rows). Full sampler suite still passes (37/37). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Four scripts under applications/dynaclr/scripts/profiling/ that measure DynaCLR data-pipeline throughput on VAST under varying ``TensorStoreConfig.recheck_cached_data`` settings: - ``benchmark_recheck_cached_data.py`` — raw ``ts.stack(...).read()`` loop against 2-experiment parquet. First showed +8% median / -24% p95 for ``"open"`` vs driver default on cold reads. - ``benchmark_dataloader_recheck.py`` — full ``MultiExperimentDataModule .train_dataloader()`` A/B at ``num_workers=4``. Exposed that warmup length matters: short warmup hid prefetch-buffer state. - ``benchmark_dataloader_workers_sweep.py`` — ``num_workers × recheck_cached_data`` grid (0, 1, 4 × None, "open", False) with generous warmup. Showed the three ``recheck`` legs are within ~5% of each other once steady-state is reached, and that ``num_workers=1`` actually beats ``num_workers=4`` on a small 30-FOV dataset. - ``benchmark_boc2d_real.py`` — combines a batch-composition sanity check (asserts ``batch_group_by="marker"`` yields single-marker, shuffled batches) with a production-config A/B on the real DynaCLR-2D-MIP-BagOfChannels-v2 parquet (3.3M rows, 11 markers, 13 stores). - ``README.md`` — explains what to run when. Scripts use the viscy monorepo's dev environment via ``uv run``. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PyTorch Lightning does not call set_epoch on custom batch_sampler instances — auto-wrap only targets DataLoader.sampler, not DataLoader.batch_sampler, and we explicitly set use_distributed_sampler: false. Without self-advancement the sampler's epoch counter stayed at 0 forever, so every training epoch replayed the same batch sequence (seed + 0), freezing the visible dataset at ~0.5% coverage for 81M-row OPS runs. Advance at the START of __iter__ so the fix is robust against early generator termination — Lightning cuts iteration via limit_train_batches, which would skip any end-of-iter bookkeeping. Verified with a real Lightning Trainer probe: sampler.epoch advances 0 → 1 → 2 across real epochs with limit_train_batches=3. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Lightning's auto-wrap hook (which would normally hand world_size/rank to the sampler) only targets DataLoader.sampler, not batch_sampler. With use_distributed_sampler: false and a batch_sampler, we must pass the DDP topology ourselves. Without this, every rank iterated the full batch sequence and yielded identical batches, turning 4-GPU DDP into 4x redundant compute on the same data — no effective batch size increase, and limit_train_batches=800 shrank to 800 (not 3200) unique batches per epoch. Reads trainer.world_size and trainer.global_rank at train_dataloader / val_dataloader call time via a small _ddp_topology() helper that defaults to (1, 0) when no trainer is attached (single-process unit tests). Verified end-to-end with a real Lightning Trainer on local GPU: trainer.world_size=1 global_rank=0 propagates to sampler.num_replicas/rank. Standalone 2-rank simulation shows rank 0 and rank 1 yield disjoint batch sets with 0 overlap. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drop redundant assertions that duplicated coverage from existing tests or leaked sampler-internal state into the behavioral check. Each fix now has a single focused test. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
When channels_per_sample=None and a batch mixes samples from experiments with different channel counts, torch.stack fails with a cryptic "tensors not equal size" deep in a dataloader thread. Raise a clear RuntimeError at the stack site instead, pointing at channels_per_sample=1 (bag-of-channels) or a fixed channel list as the fix. This path is not exercised by any production config (all set channels_per_sample: 1) — it shows up on debug probes and mixed-C research setups. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- sampler.py: drop # Why: block; docstring already covers the reasoning - dataset.py: drop mixed-C comment; RuntimeError message is self-documenting - datamodule.py: move DDP-wiring explanation into _ddp_topology docstring - test_datamodule.py: drop SimpleNamespace implementation comment Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4 of 5 tasks
alxndrkalinin
added a commit
that referenced
this pull request
Apr 27, 2026
* chore(deps): pin iohub to PR #408 (RFC-9 zipped OME-Zarr); bump Python to >=3.12 Pin iohub to czbiohub-sf/iohub PR #408 head SHA (53b10acb7a30a2c7e8dfd9b04258dea073e14088 — "feat: Added RFC-9 Zipped OME-Zarr"). The PR adds zipped OME-Zarr store support; this pin unblocks downstream consumers before the PR merges upstream. The PR's iohub head requires Python >=3.12, so bump every workspace pyproject.toml's `requires-python` from >=3.11 to >=3.12, drop the 3.11 classifier, and update ruff `target-version` to py312. Also folds in the previously-pending dynacell[wandb] optional-extra registration that the lockfile was already lagging on. SHA pin (not branch) survives force-pushes; bump deliberately when the PR moves. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(dynacell): bake A100 exclusion into hardware_4gpu launcher profile After repeat NCCL BROADCAST/ALLREDUCE coordination hangs at first-batch on this cluster's A100 partition (FCMAE jobs 31474030 + 31474038 timed out at 1h13m / 1h21m on watchdog; joint smoke 31480607 hung indefinitely at `Loading 'train_dataloader'` on gpu-a-3), every 4-GPU train leaf needed the same `--override launcher.sbatch.constraint='h100|h200|a40|a6000|l40s'` workaround. Job 31481032 with that override scheduled to gpu-h-3 (H100) and pushed past the same hang point, confirming the workaround. Bake the exclusion into the shared profile so it applies by default to all 11 4-GPU consumers (FCMAE pretrained × 4 + scratch × 4 + er/unext2 + joint celldiff train.yml + train_smoke_4gpu.yml). Leaves that genuinely need A100 must opt out via `--override launcher.sbatch.constraint=null`. Lock-in test: `test_4gpu_train_leaves_inherit_a100_exclude` walks every train leaf under benchmarks/virtual_staining/ and asserts the constraint on 4-GPU leaves; new leaves picking up the profile are covered automatically. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(viscy-data): port to iohub 0.3.2 ImageArray API (#407) The iohub PR #408 pin (Commit 1) ships a refactored ``ImageArray`` API that removed ``ImageArray.name`` and other legacy attributes used across viscy-data, viscy-utils, dynaclr, and qc. Without this port, the existing test suite fails immediately on dataset iteration (``AttributeError: 'ImageArray' object has no attribute 'name'`` at ``sliding_window.py``). Cherry-picks the migration shipped in PR #407 (squash 737cedf on ``modular-viscy-staging``), preserving local extensions: - ``sliding_window.py``: take ``f"/{img.path}"`` from #407 (replaces ``img.name``) but keep dynacell-models's ``to(torch.float32, copy=True)`` cast on the mmap_preload path (post-dates #407). - ``viscy-data/pyproject.toml``: keep ``iohub>=0.3.2`` lower bound from #407; the ``[tool.uv.sources]`` SHA pin in the workspace root pyproject.toml (Commit 1) is what actually controls resolution. - ``uv.lock``: regenerate. All 25 ``packages/viscy-data/tests/test_combined.py`` tests pass post- migration, including the previously-failing ``test_combined_datamodule_fit``. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(viscy-data): propagate trainer to children in ConcatDataModule.setup Under DDP, ``Trainer.fit`` calls ``prepare_data`` only on rank 0 of each node when ``prepare_data_per_node=True``, but ``setup`` runs on every rank. ``ConcatDataModule.prepare_data`` forwarded ``self.trainer`` to each child datamodule; ``setup`` did not. As a result, non-rank-0 children kept ``self.trainer = None``, and trainer-gated paths in the children's ``on_after_batch_transfer`` silently skipped — most visibly ``HCSDataModule``'s ``if self.trainer and self.trainer.training`` guard at ``hcs.py:628``, which gates ``gpu_augmentations``. Production failure: SLURM 31481032 (joint celldiff smoke on 4× H100, post-PR #413 + post-A100-exclude). Ranks 1, 2, 3 raised ``ValueError: x spatial size [13, 624, 624] does not match expected [8, 512, 512]`` at the first training step — rank 0 ran fine because its child had received the trainer via ``prepare_data``. Rank asymmetry was the smoking gun. Fix: propagate ``dm.trainer = self.trainer`` at the top of each loop iteration in ``setup``, mirroring ``CombinedDataModule.setup`` (which was already correct). Same fix applied to ``CachedConcatDataModule``, which has the identical latent bug. ``BatchedConcatDataModule`` inherits from ``ConcatDataModule`` so the fix flows through. Single-process regression test exercises the bug by skipping ``prepare_data`` entirely (mimicking the non-rank-0 lifecycle) and asserting that ``setup`` alone propagates the trainer and that ``gpu_augmentations`` actually run through ``on_after_batch_transfer``. The existing ``test_combined_ddp.py`` (gloo, ``pin_memory=False``, no ``gpu_augmentations``) cannot catch this — it doesn't exercise the gpu-aug branch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(dynacell): mirror a549-mantis manifest fixtures (3 plates) Add three local fixture manifests so VisCy's dataset_ref resolver tests can resolve a549-mantis datasets without requiring a ``dynacell-paper`` install. Mirrors the canonical manifests at ``dynacell-paper/_configs/datasets/a549-mantis/<date>/manifest.yaml`` landed via dynacell-paper PRs A1.1 (#14) and A2.1 (#15) plus the prior aeef64c registration. - ``a549-mantis-2024_10_29`` — TOMM20 plate (mito cross-eval). - ``a549-mantis-2024_11_07`` — SEC61B plate (er cross-eval; also feeds the joint celldiff train leaf). - ``a549-mantis-2026_03_26`` — h2b/caax mantis_v2 plate (nucleus + membrane cross-eval). Resolver discovery: ``DYNACELL_MANIFEST_ROOTS`` is set to ``tests/fixtures/manifests/`` by the autouse ``_dynacell_manifest_root_env`` fixture in ``tests/conftest.py``. Layout matches existing aics-hipsc fixture (``<root>/<dataset_name>/manifest.yaml`` per ``resolver.py:131-140``); ``splits/`` subdirs are not mirrored because the resolver doesn't read them. Closes Stage 5 of A549_EXPANSION_ROADMAP.md on the VisCy side and is the prerequisite for Stage 6 cross-eval leaves. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(dynacell): add a549_mantis predict_set fragments + Stage 6 cross-eval leaves Closes Stage 6 of A549_EXPANSION_ROADMAP.md: every existing iPSC-trained ``<organelle>/<model>/ipsc_confocal/`` cell now has a sibling ``predict__a549_mantis.yml`` + ``eval__a549_mantis.yaml`` so iPSC-trained models can be evaluated on the a549 test split. 8 cells × 2 leaves + 8 eval symlinks + 6 predict_set fragments = 30 file additions. Predict_set fragments are per-plate because the a549-mantis registry is split by acquisition date: - ``2024_11_07`` — SEC61B (er cross-eval) - ``2024_10_29`` — TOMM20 (mito cross-eval) - ``2026_03_26`` — H2B + CAAX (nucleus + membrane cross-eval) Each fragment exists on both the model side (``_internal/shared/model/predict_sets/``) and the Hydra side (``dynacell/evaluation/_configs/predict_set/``), matching the existing ipsc_confocal pattern. Stage 6 leaves keep the iPSC-trained checkpoint and inherit the cell's per-model predict configuration; only the predict_set base, output store, and ``benchmark.experiment_id`` differ. CellDiff outputs use ``_iterative`` suffix; UNetViT3D outputs do not. Nucleus + membrane leaves additionally set ``benchmark.dataset_ref.target: h2b`` (or ``caax``) at the leaf level — the iPSC manifest keys those targets by `nucleus`/`membrane` while a549 keys them by gene. Eval leaves also override ``target_name: h2b`` (or ``caax``) so the segmentation / cache layers (``mask_plate(target_name)`` → ``{target_name}.zarr``) remain consistent if ``compute_feature_metrics`` is enabled later. A549 manifests don't currently carry ``cell_segmentation`` or ``gt_cache_dir`` paths (no segmentation pipeline yet), so eval leaves set ``compute_feature_metrics: false``. Pixel metrics (PCC, SSIM, NRMSE, PSNR, FSC, spectral PCC) work without segmentation. Flip the flag in a follow-up once segmentation lands in dynacell-paper. Composition tests: - ``test_a549_predict_leaf_composes`` parametrizes over the 8-cell matrix and asserts ``data.init_args.{data_path, source_channel, target_channel}``, ``benchmark.dataset_ref.{dataset, target}``, ``experiment_id``, and ``launcher.sbatch.constraint == 'h200'`` (single-GPU predict topology). - ``test_a549_eval_leaf_composes_and_splices`` composes each Hydra eval leaf, calls ``apply_dataset_ref(cfg)``, and asserts ``io.{gt_path, gt_channel_name, pred_channel_name}``, ``cell_segmentation_path is None``, ``gt_cache_dir is None``, manifest spacing (mantis_v1 0.1494 µm vs mantis_v2 0.116 µm), and ``compute_feature_metrics is False``. 16 new tests, all green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor: /simplify cleanup of joint-training + a549 cross-eval branch Apply post-implementation simplification per /simplify review: - ``combined.py``: drop SLURM job ID from ``ConcatDataModule`` Notes block. Symptom (rank-asymmetric un-cropped batches) is non-obvious and stays; the specific failing job belongs in PR description, not a docstring that ages out. - ``test_combined.py``: switch ``SimpleNamespace`` fake trainer to ``MagicMock`` to match the existing pattern in ``test_hcs.py``. Move imports to module level (per CLAUDE.md "Import at the top of the file"). Drop redundant inline narration comments. - ``hardware_4gpu.yml``: trim 4 SLURM job IDs from the rationale block (task narration; belongs in PR/commit messages) and drop the rotting "11 4-GPU consumers (FCMAE × 8 + ...)" count — the data-driven ``test_4gpu_train_leaves_inherit_a100_exclude`` enforces the invariant without needing a maintenance-prone count in a YAML comment. No behavior change. All previously-green tests remain green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(dynacell): clarify _A549_EVAL_EXPECTATIONS shape comment The comment listed four fields, but the dict values are 3-tuples (the organelle is the dict key, not part of the value). Rewrite as ``{organelle: (target_group, gt_channel, gt_suffix)}`` so the shape is unambiguous when the matrix is extended. 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>
alxndrkalinin
added a commit
that referenced
this pull request
May 18, 2026
iohub PR #408 (RFC-9 zipped OME-Zarr, .ozx) merged into iohub main as afe11f6. Bumps pin from the pre-merge SHA on the PR branch to the merge commit on main and drops the "until merged" comment. Verified fork-safety hook for DataLoader(num_workers>0) on .ozx archives before merge. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
alxndrkalinin
added a commit
that referenced
this pull request
May 18, 2026
Brings in 4 commits from staging: - 651f969 Port virtual_staining examples into applications/cytoland (#409) - 0c5b834 bump iohub to 0.3.3 - 737cedf Port to iohub 0.3.2 ImageArray API (#407) - a10d4c4 Update packages/viscy-data/pyproject.toml Resolutions: - packages/viscy-data/pyproject.toml: kept staging's iohub>=0.3.3 floor; kept the lightning>=2.3 SECURITY comment from dynacell-models warning against 2.6.2/2.6.3 supply-chain compromise (matches viscy-utils). - packages/viscy-data/src/viscy_data/sliding_window.py: kept dynacell-models' .to(torch.float32, copy=True) on the preloaded read path so it produces the same dtype as the non-preloaded path's .astype(np.float32) below. .clone() would leave the preloaded tensor in its source dtype. - uv.lock: regenerated with `uv lock` from the merged pyproject set; iohub git pin (afe11f6, PR #408 merge commit on iohub main) survives the regen. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
edyoshikun
added a commit
that referenced
this pull request
May 22, 2026
* 2D MIP single-marker 192: 384->256->192 patch variant
Larger final crop (192 vs 160) for more subcellular detail at ~2x I/O cost.
Affine corner safety holds: 384 / sqrt(2) * 0.8 = 217 > 192 final crop.
Layered config (base + single-marker + single-marker-192):
- yx_patch_size: 256 -> 384
- final_yx_patch_size: 160 -> 192
- BatchedRandSpatialCropd roi_size: [10, 192, 192] -> [10, 256, 256]
- example_input_array_shape: [1,1,1,160,160] -> [1,1,1,192,192]
- All other augmentation knobs (scale_range, rotate, contrast, smooth,
noise, ChannelWiseZReduction) preserved verbatim from the 160 recipe.
Warm-start uses the 160 single-marker epoch-0 checkpoint (0rhpwh77).
ConvNeXt-Tiny stem is fully convolutional (1x4x4 kernel, stride 1x4x4)
so the state_dict loads cleanly at the new input size.
Run name: 2d-mip-ntxent-t0p2-lr2e5-bs256-384to192-zext16-single-marker-fix-shuffler
sbatch job-name: dynaclr_2d_sm192
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Lineage-aware PHATE subsampling in combined-dim-reduction
REDUCE_COMBINED on the infectomics-annotated matrix (350k cells x 768
features) was running PHATE on a 50k random subsample, fragmenting
tracks across the diffusion graph and producing slow + biologically
incoherent embeddings. The viscy_utils compute_phate already supports
lineage-aware subsampling but only when the caller passes lineage_ids;
nothing was constructing them.
This commit derives lineage_ids in reduce_combined.py from the
(fov_name, track_id) columns already present in obs (prefixed by store
index to keep namespaces disjoint across stores), then passes the
combined array to PHATE. compute_phate now picks N whole lineages and
fits on all their timepoints, then transforms the full 350k.
Recipe (recipes/reduce.yml) updates the `subsample` field with a
comment explaining the unit semantics flip when lineage cols are
present (subsample = N lineages, not N cells) and lowers the value to
1500 — at mean track length 17 across ~17k lineages, that yields
~25k fitting cells. Comparable wall time to the previous random 20k
but with coherent trajectories.
Falls back to random-cell subsampling when neither `lineage_id` nor
`fov_name + track_id` is in obs.
Validation log in evaluation_matrix.md §9 will be updated after the
resubmitted Wave-1 finishes.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(dynacell): add hardware_h200_single_smoke launcher profile
Adds a smoke-sized variant of hardware_h200_single that bounds
launcher.sbatch.time to 30 min. Identical otherwise (single H200,
ntasks_per_node=1, 256G mem). Re-points train_smoke.yml at the new
profile so a smoke job cannot sit on a multi-day allocation by
default. Pair with --override trainer.fast_dev_run=true (or
trainer.max_steps=N) so the run actually exits inside the wall.
test_joint_train_smoke_leaf_composes now asserts sbatch.time matches
the smoke value, locking the invariant into the leaf.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* fix(2D MIP 192): tighten random crop to 216 + drop warm-start
Job 31442612 hit a 30-min NCCL all-reduce timeout in optimizer.step.
Two suspected causes addressed:
- BatchedRandSpatialCropd roi_size 256 -> 216 to fit fully inside the
affine-safe inscribed region (384/sqrt(2)*0.8 = 217 px).
- Warm-start commented out to remove the 160-trained encoder loaded
into a 192-input model as a possible source of rank divergence.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(dynacell): /simplify smoke profile via wall-only overlay
Replaces the duplicated `hardware_h200_single_smoke.yml` (which copied
8 of 9 fields from `hardware_h200_single.yml`) with a 4-line
`wall_smoke.yml` that only overrides `launcher.sbatch.time`. The smoke
leaf now stacks `hardware_h200_single + wall_smoke + runtime_shared`,
matching the existing compose-by-overlay idiom. Future hardware tweaks
(cpus, mem, constraint) propagate to smoke automatically; future
hardware variants reuse `wall_smoke.yml` for free.
Also trims the smoke leaf header: drops the numbered "Differences from
sibling train.yml" enumeration (would rot the moment either leaf
changes; the README's "Joint smoke sibling" section already documents
the contract) and shortens the anchor-mechanism comment to a one-line
cross-ref into train.yml.
No behavior change — composed `launcher.sbatch` is byte-identical to
the previous form (verified by `test_joint_train_smoke_leaf_composes`,
which still asserts `time == "00:30:00"` and `constraint == "h200"`).
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* feat(dynacell): disable logger at the smoke leaf level
Consumers of train_smoke.yml shouldn't have to remember
`--override trainer.logger=false` at submit time, and smokes don't
need a logger by definition. Sets `trainer.logger: false` directly in
the leaf so future smokes for other models/organelles inherit the
same disable when the recipe defaults a WandbLogger.
Recipe `fit.yml` also defaults a `LearningRateMonitor` callback that
raises `MisconfigurationException` when `trainer.loggers` is empty
(`lr_monitor.py:121-122`), so the smoke leaf's `callbacks` list is
trimmed to just `ModelCheckpoint`. Lists replace wholesale under
deep_merge, so this cleanly drops LRMonitor without affecting prod.
`test_joint_train_smoke_leaf_composes` now asserts both invariants:
`trainer.logger is False` and LRMonitor is not in the callbacks list.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* fix(2D MIP 192 sbatch): mem-per-cpu 8G -> 14G to avoid host RAM OOM
* fix(viscy-data): drop non-tensor metadata in BatchedConcatDataModule combine
`BatchedConcatDataModule.on_after_batch_transfer` combined per-key
values across micro-batches by assuming each value was either a list
(extend) or a tensor (`torch.cat`). HCSDataModule emits `norm_meta`
(dict of per-channel normalization stats, when
`load_normalization_metadata=True`, the default) and `index` (tuple
of `(img_name, t_idx, z_idx)`) per sample. After collation those keys
hit the cat branch and `torch.cat([dict_a, dict_b])` raised
`TypeError: expected Tensor as element 0 in argument 0, but got dict`,
killing joint training during Lightning's `_run_sanity_check`.
Joint training across heterogeneous children has no well-defined
combined semantic for these keys — channels need not align between
zarrs and FOV identifiers are dataset-specific — and Lightning's
training/validation only reads source/target tensors. Skip the key
during combine instead.
Existing tests (`test_batched_concat_datamodule_with_hcs_children`
and the DDP variants) iterate `next(iter(loader))` to inspect the
micro-batch contract but never invoke `on_after_batch_transfer`, so
the bug never surfaced. New regression test calls
`on_after_batch_transfer` against the real HCS fixture (which does
write per-FOV normalization metadata) and asserts tensor keys
concatenate cleanly while dict-valued metadata is dropped.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* Move pseudotime package out of evaluation/ and add io.py
The pseudotime tree was duplicated under both
applications/dynaclr/src/dynaclr/evaluation/pseudotime/ and
.../pseudotime/. Make .../pseudotime/ the canonical location:
- Add io.py: template-zarr layout helpers + dataset-routing helpers
(save_template_zarr, load_template_flavor, find_embedding_zarr,
date_prefix_from_dataset_id, get_dynaclr_versions, ...).
- Populate __init__.py with the curated public API and re-export
io helpers.
- Rename POSITIVE_CLASSES -> DEFAULT_POSITIVE_CLASSES and
build_infection_template -> build_template for clarity.
- Migrate .values -> .to_numpy() in pandas usage (pyarrow string
arrays compatibility).
- Delete the legacy evaluation/pseudotime/ tree (no remaining
imports anywhere in the repo).
- Update test_pseudotime.py and scripts/pseudotime/3-organelle-remodeling/
to import from dynaclr.pseudotime, replacing the
sys.path-based shim into 1-build_template/evaluate_template.py.
- Refresh docs/DAGs/pseudotime.md with the new package layout and
the alignment-funnel drop_log.json sidecar note.
Verified: uv run pytest applications/dynaclr/tests/test_pseudotime.py
(29/29 pass), ruff check + format clean.
* fix(OnlineEvalCallback): all_gather features under DDP
Per-rank validation shards were being averaged via sync_dist=True,
but effective_rank, kNN-CV/holdout, and Spearman rho are non-linear
in the sample set — averaging shard scalars yields the wrong value.
The val DataLoader is sharded (datamodule passes num_replicas/rank
to FlexibleBatchSampler with use_distributed_sampler: false), so
this affected every multi-GPU run.
Fix: gather features + per-sample arrays (labels, track_ids,
timepoints) across ranks before computing metrics. All ranks see
the full set, produce identical scalars, and sync_dist=True
becomes a no-op average that still satisfies Lightning's epoch-end
all-reduce (avoiding the original rank-0-only deadlock).
- Add OnlineEvalCallback._gather_across_ranks: passthrough on
world_size=1; pl_module.all_gather + concat on world_size>1.
Equalize per-rank shard sizes by truncating to the minimum length
before gather to keep all_gather happy with fixed shapes.
- Add an availability all-reduce so optional metadata arrays
(track_ids, timepoints) return None consistently across ranks
when missing, instead of stalling on a no-op gather.
- Update class + method docstrings to reflect the new contract.
- Test the multi-rank gather path with a fake pl_module exposing
trainer.world_size and an all_gather that block-stacks. Single
consolidated test covers the world_size=2 concat plus the
missing-optional-array passthrough.
* refactor(cli_utils): hoist load_composed_config import to module top
Per CLAUDE.md "Import at the top of the file." Audited all 17 in-repo
callers of load_config across applications/dynaclr/, applications/
airtable/, applications/qc/, and packages/viscy-utils/: every existing
top-level base: key in their YAMLs is a recipe-style list of relative
paths (no data-shaped collisions), so load_composed_config is a strict
superset of yaml.safe_load on legacy configs. No opt-in flag needed.
- Move 'from viscy_utils.compose import load_composed_config' from
inside load_config() to the module-level imports.
- Drop the redundant existence check; load_composed_config already
raises FileNotFoundError when opening the file.
- Update the docstring to describe the base: composition behavior
rather than the prior raw-yaml.safe_load contract; remove the
yaml.YAMLError raises stanza (yaml is no longer imported here).
* fix(prepare): subclass Dumper instead of mutating yaml.Dumper
`save_yaml_no_aliases` was assigning a lambda to
`yaml.Dumper.ignore_aliases`, which mutates the class attribute
globally. Every other yaml.dump() in the same Python process inherited
the alias-disabled behavior — silent action-at-a-distance for any
caller that happens to import this module first.
Define a local _NoAliasDumper subclass and pass it as Dumper=...
instead. Behavior unchanged for this call site; the rest of the process
gets stock yaml.Dumper back.
* refactor: hoist inline imports to module top
Per CLAUDE.md "Import at the top of the file." Three trivial inline
imports of mandatory dependencies become module-level imports:
- preprocess_cell_index.py: viscy_data.cell_index.preprocess_cell_index
- prepare.py: iohub.open_ome_zarr (was inlined in two functions)
- dimensionality_reduction.py: numpy as np
Inline imports of optional/heavy dependencies (phate, sklearn in
dimensionality_reduction.py; copairs in embedding_map.py with explicit
ImportError handling) are intentional lazy-load patterns and left
alone.
* fix(mmd): cap pooled-kernel size and drop dead helper
mmd_permutation_test allocates an (N, N) float32 kernel where
N = len(X) + len(Y). Default callers pass max_cells=2000 per group
(N=4000 ≈ 64 MB), but a user passing max_cells=None or a too-large
value would silently OOM (50k → 10 GB; 100k → 40 GB).
- Raise ValueError up front when N exceeds 20000 (≈1.6 GB), with
a message that names the would-be allocation and tells the caller
to subsample. Threshold matches the practical statistical regime
for permutation MMD; anyone needing larger samples should switch
to a linear-time MMD estimator.
- Delete _mmd2_from_kernel (~28 lines) — it was never called from
any code path. The live permutation test uses an inner
_mmd2_from_labels closure instead.
23/23 test_mmd.py tests still pass.
* Move LC registry to per-task-domain sub-registries
Restructure the central LC registry from {model}/v{N}/ to
{model}/{task_domain}/v{N}/ so different annotation domains can each
have their own versioned bundle. Wave-2 leaves choose which
sub-registry to fetch based on what their data's markers need.
Layout change:
/linear_classifiers/{model}/v1/ -> /linear_classifiers/{model}/infectomics/v1/
/linear_classifiers/{model}/latest -> /linear_classifiers/{model}/infectomics/latest
Leaves updated:
- DynaCLR-2D-MIP-BagOfChannels/infectomics-annotated.yaml: publish_dir
now ends in /infectomics/ (Wave-1 trainer for infectomics task domain)
- DynaCLR-2D-MIP-BagOfChannels/{alfi,microglia}.yaml: pipelines_dir
ends in /infectomics/latest. alfi.yaml has a comment noting the
marker mismatch (DIC vs G3BP1/SEC61B/Phase3D/viral_sensor) means
predictions will be NaN until an alfi-annotated trainer publishes
to /alfi/. This is a smoke test for the reader path.
Matrix doc (evaluation_matrix.md) §1 + §2 reflect the sub-registry
design and the addition of alfi-annotated as a Wave-1 column.
Existing on-disk registry was relocated by hand:
cd /hpc/.../linear_classifiers/DynaCLR-2D-MIP-BagOfChannels/
mkdir infectomics
mv v1 infectomics/v1
rm latest
ln -s v1 infectomics/latest
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(slurm): make training/debug shell scripts location-portable
Several training launchers hardcoded my home checkout path
(/hpc/mydata/eduardo.hirata/repos/viscy/...) in their `source` line
or in `cd` calls, so they broke when run from a worktree or by
another user. Two scripts (DynaCLR-2D-BagOfChannels-v3.sh,
already-untracked OPS-373genes.sh) used `$(dirname "$0")/slurm/...`
which silently looked for a sibling 'slurm/' that doesn't exist —
those launchers are broken in HEAD today.
Switch every affected launcher to `$(dirname "$0")/../slurm/train.sh`
(one level up to the shared `training/slurm/`) and the three debug
scripts to `cd "$(dirname "$0")/../../../../.."` (5 levels up to the
repo root). Now any clone or worktree runs without editing.
Scripts already using ${WORKSPACE_DIR} (OPS-1000genes-{allmarkers,
lite,multimarker-BoC}.sh, Phase-contrastive-timeaware.sh) are left
alone — that pattern lets the user override the location via env
and works correctly from worktrees as long as WORKSPACE_DIR is set.
* fix(dynacell): cut smoke leaf batch_size to 1 to fit a single H200
train.yml's batch_size=4 across 4 GPUs OOM'd when run on a single H200
(per-step memory is batch_size * num_samples patches at [8, 512, 512]).
Drops the smoke's child batch_size to 1 so per-step VRAM stays inside
one GPU's budget while the patch shape stays identical to train.yml —
the validation remains apples-to-apples.
`RandWeightedCropd.num_samples` also drops from 2 to 1: HCSDataModule
requires `batch_size % num_samples == 0` and rejects the leaf at
schema-validation time otherwise. num_samples=1 is the largest value
that satisfies the constraint with batch_size=1.
`test_joint_train_smoke_leaf_composes` follows along (asserts
batch_size=1 instead of 4).
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* deps: cap anndata <0.12.9 in viscy-utils
anndata 0.12.9 hard-requires pandas<3, but the rest of the stack runs
on pandas 3 (we explicitly downcast ArrowStringArray columns in
embedding_writer.py to bridge the two). Locking anndata to 0.12.6 by
default works today, but a future `uv sync` could pick up 0.12.9+ and
silently break embedding writes.
Cap both the optional `[anndata]` extra and the test dep-group at
<0.12.9 so the resolver fails fast instead of installing a known-broken
combo. The TODO at embedding_writer.py:159 already points to anndata
0.13 as the lift-the-cap signal.
uv.lock unchanged — 0.12.6 already satisfies the new constraint, so a
lock refresh is cosmetic and will land separately if/when the lockfile
is otherwise touched.
* refactor(cell_index): parquet-only preprocess + logger over print
- preprocess_cell_index now returns None. The function's job is to
write the preprocessed parquet; callers that need the dataframe can
read_cell_index afterwards. Drops the dual return + write contract
flagged in review. Only caller is the click CLI, which ignored the
return.
- Convert all five print(...) calls in cell_index.py to
_logger.info(...). Logger was already wired at module top — these
calls were vestigial. Lets users silence the chatter via standard
logging config and routes through the same handler as the existing
_logger.warning / _logger.info elsewhere in the file.
* chore: rank-zero warn in effective_rank, pytest.raises in z_reduction test
- online_eval.effective_rank's NaN-row warning fires once per call,
but with the recent all_gather fix every rank computes on the same
full-set features matrix, so the warning would emit world_size times
with identical content. Switch to lightning_utilities' rank_zero_warn
so DDP runs see one log line, not N.
- test_z_reduction.test_invalid_strategy used try/assert False/except
ValueError. Replace with `with pytest.raises(ValueError)` (and add
the missing pytest import) — clearer intent and a better failure
mode if the error type ever drifts.
* feat(dynacell): add 4-GPU DDP smoke leaf for joint celldiff SEC61B
Sibling of train_smoke.yml that swaps hardware_h200_single -> hardware_4gpu
and adds trainer.strategy=ddp, devices=4, max_steps=5. Validates that
BatchedConcatDataModule + ShardedDistributedSampler integrate correctly
on the production DDP topology - single-GPU smoke already proved the
joint loader and training/val loops work; this isolates sharding.
batch_size=1 / num_samples=1 kept identical to train_smoke.yml so the
per-rank memory profile is the proven one - sharding is the only new
variable. max_steps=5 baked in (not override) after train_smoke.yml
TIMEOUT'd at 30 min from a missed --override at submit time.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* Untrack evaluation_matrix.md (local-only planning doc)
The model x dataset evaluation matrix doc is internal planning that
shouldn't ship upstream. It carries running notes (work in progress
status, validation logs, open questions) that aren't useful in the
public docs and would constantly need rebasing.
- git rm --cached the file (preserved on disk)
- .gitignore now excludes it
Prior history at:
ff04365a Move LC registry to per-task-domain sub-registries
f44b8ffb Lineage-aware PHATE subsampling in combined-dim-reduction
8a7245ef Add evaluation matrix DAG doc with central LC registry design
Those commits remain in history but the doc is no longer tracked.
The pipeline-level DAG (evaluation.md) stays tracked since it
documents the actual production pipeline.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* perf(viscy-utils): bf16-precision SSIM helper for Hopper FCMAE training (#412)
* feat(viscy-utils): add bf16-precision SSIM helper
Replaces monai.metrics.regression.compute_ssim_and_cs (which
unconditionally casts both inputs to fp32 internally) with a precision-
aware variant in viscy_utils.evaluation.metrics. The helper runs the 5
Gaussian-mean convolutions in bf16 and promotes only the variance
subtractions and C1/C2-guarded divisions to fp32. Squared products are
computed in fp32 first, then cast to bf16 for the conv input, which
preserves precision on the precision-sensitive squaring step.
Why: FCMAE VSCyto3D training on Hopper (H100/H200) runs at ~45 s/step
vs ~3-5 s/step on Ampere/Ada because monai's 25 fp32 conv ops per loss
invocation can't use Hopper's bf16 tensor cores. The bf16 helper
restores tensor-core path while keeping the precision-sensitive math in
fp32 to match monai's NaN-resistance properties.
Numerical contract validated by 7 tests in test_metrics.py:
- per-pixel random: rtol=5e-2, atol=1e-1 (>=2x margin over measured
0.0418 max abs drift)
- aggregate random: rtol=1e-2, atol=1e-2 (>=25% margin over measured
0.00776)
- aggregate correlated-pair (pred=target+0.05*randn): rtol=2e-3,
atol=5e-3
- gradient flow: cosine-similarity >= 0.99 between helper and monai-ref
flat gradients; sign-flip fraction < 1% on |grad_ref| > 1e-3 voxels
- dtype invariance: helper returns fp32 regardless of fp32/bf16/fp16
input dtype
The decorators on MixedLoss/SpotlightLoss are addressed in a follow-up
commit; this commit only changes the SSIM compute path.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(viscy-utils): drop redundant fp32 cast on MixedLoss/SpotlightLoss
Removes @torch.amp.custom_fwd(device_type="cuda",
cast_inputs=torch.float32) from MixedLoss.forward and SpotlightLoss.forward.
For MixedLoss: the decorator was redundant because monai's
compute_ssim_and_cs already cast both SSIM inputs to fp32 internally
(monai/metrics/regression.py:402-403), so the outer @custom_fwd only
affected F.l1_loss / F.mse_loss / outer F.avg_pool3d — none of which
are the slow path. Now that the SSIM compute path goes through
_compute_ssim_and_cs_bf16 (preceding commit) the fp32 island the
decorator created actively defeats the purpose. The L1 and MSE branches
remain numerically identical because PyTorch's autocast policy already
promotes them to fp32 (cf. test_mixed_loss_l1_only_matches_torch_l1).
For SpotlightLoss: the decorator's fp32 island was layered onto a
function with no conv-heavy ops — only squared error, masking, sums,
and divisions, all of which are in autocast's promote-to-fp32 list
already. Removal is a no-op for behaviour but unblocks autocast
plumbing for future precision experiments.
New tests:
- test_mixed_loss.py (4): forward outside autocast, forward under bf16
autocast (drift vs explicit-fp32 baseline within rtol=1e-2, atol=1e-2),
gradient flow under autocast, L1-only bit-exact F.l1_loss.
- test_spotlight_loss.py (+2): autocast bf16 forward+backward finite,
autocast result matches explicit-fp32 baseline within rtol=1e-2,
atol=1e-2.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs(dynacell): record Hopper FCMAE slowdown investigation + bf16 fix
Captures the full diagnostic chain from "Hopper trains 10-15x slower
than Ampere/Ada" to "fp32 MS-DSSIM is the dominant slow term" to the
landed bf16 SSIM helper, with measured numbers for every claim.
Notable artefacts the doc records (so future readers don't redo the
work):
- T2 probe (job 31452082) showed compute_ms collapses from 45.5 s/step
to ~0.9 s/step on identical N=4 H100 hardware when MS-DSSIM is
removed.
- monai's compute_ssim_and_cs unconditionally casts both inputs to fp32
at regression.py:402-403; the @torch.amp.custom_fwd decorator on
MixedLoss/SpotlightLoss was layered redundantly on top.
- T6 probe (job 31453564) with the bf16 helper measured 4151 ms/step
steady-state mean (6 consecutive STEP_TIMER lines) on identical
N=4 H100 — 10.97x speedup, fix landed in commits e42c49a and
3a7fa05.
Falsified hypotheses that were time-consuming to evaluate are also
recorded (cuDNN [240,960,1,1] stride mismatch, DDP bucket-view
mismatch, Lightning vs synthetic data, sync_batchnorm, MS-DSSIM-only
symmetric probe T4 cancelled due to 8h queue slip) so the next person
investigating Hopper performance regressions doesn't re-run them.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test(viscy-utils): tighten gradient sign-flip + spotlight autocast bounds
Two findings from /verify-plan:
1. test_ssim_helper_gradient_flow used |grad_ref| > 1e-3 to mask
non-tiny voxels for the sign-flip-fraction assertion, but the
measured |grad_ref| max is ~1.7e-6 — so the predicate selected zero
voxels and the assertion was vacuous. Switch to a relative threshold
(10% of the reference grad max) so the assertion is scale-invariant
and meaningful regardless of the loss scale.
2. test_spotlight_loss_autocast_matches_fp32_baseline used rtol=1e-2,
atol=1e-2 but measured drift is 0.0 — SpotlightLoss has no
conv-heavy ops and autocast policy promotes the precision-sensitive
pieces to fp32 anyway. Tighten to rtol=1e-3, atol=1e-3 to match the
plan's spec and catch any future drift.
Cosine-similarity check (>= 0.99 between flat helper-grad and flat
reference-grad) was already meaningful and unchanged.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(viscy-utils): specialize bf16 SSIM helper to 3D, drop redundant tensors
Two simplify passes consolidated:
1. Drop the ``spatial_dims`` parameter and the ``conv_fn = getattr(F,
f"conv{spatial_dims}d")`` indirection. The helper is already
specialized to "uniform kernel only" per the original plan; the only
caller (``ssim_25d``) only ever passes ``spatial_dims=3``. Hardcoding
``F.conv3d`` directly removes a parameter from the public surface and
removes a getattr lookup per conv call. Docstring updated to spell out
the 3D specialization.
2. Drop the explicit ``y_pred_bf`` and ``y_bf`` intermediate tensors. The
bf16 versions of the simple (non-squared) inputs are only consumed
once each (in ``mu_x`` / ``mu_y`` convs), so caching them as named
variables is pure memory duplication — peak working-set tensors drop
from 7 to 5. Inline the cast at the conv site so the temporary lives
only during the conv. Squared-product bf16 tensors stay named because
the squaring must happen in fp32 first to preserve precision and the
result is genuinely a different tensor.
Test wrapper ``_bf16(...)`` updated to drop the ``spatial_dims=3``
kwarg; ``_SPATIAL_DIMS`` module constant removed; ``_ref(...)`` keeps
``spatial_dims=3`` since monai's signature still requires it.
All 34 tests still pass; numerical contract unchanged.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(viscy-utils): apply PR review nits to bf16 SSIM helper
Three small improvements from PR #412 review:
1. metrics.py:178 — switch ``data_range: Union[float, torch.Tensor]``
to ``data_range: float | torch.Tensor`` (modern typing per global
CLAUDE.md). The pre-existing ``Union[...]`` at line 270 in
``ssim_25d`` is left alone per surgical-changes rule; the import
stays since it's still used.
2. metrics.py:233 — add a brief comment explaining why the kernel is
rebuilt per call (small tensor, negligible per-step cost relative
to the conv) and what the cache-key would be if profiling ever
shows it matters. Pre-empts the "should we cache this?" question
future readers will have.
3. test_metrics.py:108 — update the gradient-flow test docstring to
reference the relative threshold (10% of grad max) that the code
actually uses; it previously said ``|grad_ref| > 1e-3`` which was
the absolute threshold from before commit a498d59.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(viscy-utils): address Copilot review on PR #412
Four of Copilot's six findings applied; one rejected.
Applied:
- ``test_metrics.py``: monai is not a hard dep of viscy-utils (only
transitively pulled via viscy-models / optimizers.py). Use
``pytest.importorskip("monai.metrics.regression")`` so the suite
degrades gracefully if monai is ever decoupled.
- All three test files: skip on ``not torch.cuda.is_bf16_supported()``
in addition to ``not torch.cuda.is_available()``. bf16 conv on
pre-Ampere CUDA falls back to software emulation rather than failing,
but the equivalence-vs-monai-fp32 tolerances were measured on Hopper
— exclude older hardware where emulated bf16 could push drift past
the configured rtol/atol.
- ``metrics.py`` docstring: "five Gaussian-mean convolutions" →
"five uniform-window mean convolutions". Kernel is uniform
(ones / prod(kernel_size)), not Gaussian.
- ``metrics.py`` helper: keep named ``y_pred_bf`` / ``y_bf`` views of
the simple inputs going straight from the caller's dtype, instead of
the fp32 round-trip ``y_pred_fp32.to(bf16)``. Saves a cast per simple
input on the autocast path; under autograd the conv inputs are
retained for backward either way, so this doesn't change peak
memory.
Rejected:
- ``metrics.py`` capability gate / fp32 fallback. The plan deliberately
chose unconditional bf16 convs (explicit casts, not autocast policy
detection). bf16 conv works on every CUDA generation we ship to
(sm_80+); on older CUDA it would emulate, not hard-fail. Falling back
to fp32 for "any non-bf16 path" would reintroduce the Hopper
bottleneck this PR exists to fix.
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(viscy-data): drop use_thread_workers to fix DDP deadlock (#413)
* fix(viscy-data): drop use_thread_workers to fix DDP deadlock
BatchedConcatDataModule used ThreadDataLoader(use_thread_workers=True),
which substitutes a thread-shim for multiprocessing.Process inside
PyTorch's worker iterator and silently forces persistent_workers=False.
That combination × real init_process_group hangs Lightning's
barrier("train_dataloader()") on a 4-GPU H200 node (SLURM 31453225).
Single-GPU and CPU+gloo DDP both worked, so the deadlock is
GPU/CUDA-specific (pin-memory thread × thread-shim worker context ×
per-rank CUDA context).
Drop use_thread_workers=True from train_dataloader and val_dataloader,
move the lambda collate to a module-level _identity_collate (spawn-safe),
and document why workers are now real subprocesses. The new
test_combined_ddp.py uses mp.start_processes(start_method="fork") to
spawn 2 real DDP ranks under pytest (mp.spawn requires start_method
"spawn", which can't re-resolve pytest's --import-mode=importlib path)
and parameterizes (num_workers, mmap_preload) ∈ {(0,F), (2,F), (2,T)}
— the last cell is the regression guard. Uses a deadline-loop join
because ctx.join returns False as soon as any rank exits, so a single
ctx.join(timeout=...) call is unsafe.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(viscy-data): /simplify cleanup of DDP fix + regression test
Simplify pass on combined.py + test_combined_ddp.py:
- Drop the homegrown ``_identity_collate`` helper; reuse
``monai.data.utils.no_collation``, which is character-identical and
already available via the same ``monai.data`` namespace the file
imports ``ThreadDataLoader`` from.
- Trim the change-narration tail on the ``BatchedConcatDataModule``
docstring; the WHY belongs in the commit message, not the long-lived
class doc.
- In the new test: drop the SLURM job number reference (transient task
artifact), drop the hard-coded ``data_connector.py:93`` line number
(rots across Lightning versions), drop the contributor-private
``/home/...`` path from the deadline-loop comment, and delete two
WHAT-narration comments. Extract ``_kill_survivors`` so the
terminate→join→kill sequence isn't duplicated between the deadline
loop and the ``finally`` block. Replace the ``out_file.exists()``
pre-check with a direct ``read_text()`` (per repo style: prefer
raising errors over TOCTOU stat checks). Only ``mkdir`` the
``scratch_dir`` when ``mmap_preload=True`` actually needs it.
No behavior change. ``packages/viscy-data/tests/test_combined_ddp.py``
still passes all three parameter cells and ``test_combined.py`` is
unaffected.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test(viscy-data): skip DDP test on platforms without fork
Copilot review on PR #413 flagged that the new
``test_combined_ddp.py`` hard-codes ``start_method="fork"`` and would
fail at runtime on the ``windows-latest`` matrix in
``.github/workflows/test.yml`` (Windows has no fork). Add a
``pytest.skip`` when ``"fork"`` is absent from
``mp.get_all_start_methods()``. Linux (CI default) keeps running all
three parameter cells; Windows now skips cleanly.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs(viscy-data): scope test docstring honestly to CPU+gloo coverage
Code-review feedback flagged that the test claims to "guard the
regression that broke production" but the deadlock is GPU/CUDA-specific
(pin-memory thread × thread-shim worker context under NCCL) per the
fix commit's own diagnosis — gloo/CPU passes even with the bug present.
The test still has value (locks down collate, sampler-attachment, rank-0
prepare_data ordering, mmap_preload) but the docstring oversold its
coverage. Reword to state the scope honestly and note that catching
a direct revert of ``use_thread_workers=True`` needs a GPU runner.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test(viscy-data): guard against re-introducing use_thread_workers=True
CPU+gloo regression test in test_combined_ddp.py cannot reproduce the
GPU/NCCL-specific deadlock that PR #413 fixed (pin-memory thread ×
thread-shim worker context under CUDA), so a future revert of
``use_thread_workers=True`` would slip past CI.
This source-level check runs without DDP on every CI matrix cell.
It builds the joint loader with ``num_workers=2`` (the threshold MONAI
uses for the substitution) and asserts the resulting DataLoader's
``multiprocessing_context`` is not the
``monai.data.thread_buffer._ProcessThreadContext`` shim that
``use_thread_workers=True`` would install
(``monai/data/thread_buffer.py:189-191``).
Validated by temporarily re-adding ``use_thread_workers=True`` to
``BatchedConcatDataModule.train_dataloader`` and confirming the guard
fails on both zarr_v2 and zarr_v3 fixture parameterizations, then
removing the temp change before commit.
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>
* feat(dynacell): nucleus + membrane FCMAE_VSCyto3D scratch + pretrained leaves
Adds the four FCMAE_VSCyto3D leaves needed to extend the existing
ER (sec61b) + Mito (tomm20) benchmark matrix to all four iPSC
organelles (ER / Mito / Nucleus / Membrane), each with a paired
{scratch, pretrained-encoder} pair so we have a paper-adjacent
init-ablation baseline per organelle.
Layout matches the existing ER/Mito leaves under
``benchmarks/virtual_staining/{er,mito}/fcmae_vscyto3d_{pretrained,scratch}/
ipsc_confocal/train.yml`` — same shared overlays
(``train_sets/ipsc_confocal``, ``data_overlays/fcmae_vscyto3d_fit``,
``model_overlays/fcmae_vscyto3d_fit``, ``hardware_4gpu``) and the same
``benchmark.dataset_ref`` pattern. Pretrained leaves load encoder-only
weights from ``/hpc/projects/virtual_staining/models/mehta-lab/VSCyto3D/
fcmae.ckpt`` (the canonical 400 ep VSCyto3D ckpt); scratch leaves are
random-init.
Both nucleus and membrane targets resolve to the multi-marker
``cell.zarr`` (Nuclei / Membrane channels respectively) per the
aics-hipsc manifest at ``configs/datasets/aics-hipsc/manifest.yaml``,
so no new zarrs needed.
Resolved configs validated end-to-end via
``submit_benchmark_job.py --print-resolved-config``; all four compose
cleanly with the bf16-SSIM-helper-equipped ``viscy_utils`` at HEAD.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(dynacell): override Structure aug-keys for nucleus + membrane FCMAE leaves
Job 31474018 (FCMAE_VSCyto3D_Pretrained_Nucleus) crashed with
``KeyError: 'Structure'`` from ``RandWeightedCropd`` 47 minutes in.
Root cause: ``_internal/shared/model/data_overlays/fcmae_vscyto3d_fit.yml``
hardcodes ``keys: [Phase3D, Structure]`` and ``w_key: Structure`` — that
overlay was authored for ER (sec61b) and Mito (tomm20), which both
have ``target_channel == "Structure"``. Compose's list-replace semantics
mean the data overlay's augmentations list completely overrides the
target overlay's augmentations, so the per-organelle channel name from
``targets/{nucleus,membrane}.yml`` (Nuclei / Membrane) never reaches
``RandWeightedCropd``.
Fix at the leaf level: each of the 4 nucleus + membrane FCMAE leaves
now declares its own ``data.init_args.augmentations`` with the right
channel name. The leaf is composed last, so its augmentations list wins
over the data overlay. The augmentation policy (spatial_size [20, 600,
600], num_samples 4, RandWeightedCropd as the only CPU aug) is kept
identical to the FCMAE overlay so behaviour matches ER/Mito.
A more robust fix would be to make the FCMAE data overlay's CPU
augmentation block parameterized by ``target_channel`` (or to drop it
from the overlay entirely and rely on the per-target overlay), but
that's broader infra surgery — leaving for a separate refactor.
ER (sec61b) and Mito (tomm20) FCMAE leaves are unchanged because they
already work — their target_channel is "Structure".
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(dynacell): add eval script for FNet3D paper predictions on iPSC confocal
Evaluates SEC61B, membrane, TOMM20, and nucleus FNet3D predictions against
ground truth using pixel and DynaCLR feature metrics.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Disable PHATE; bump wrapper RAM; add microglia Wave-2 SLURM script
PHATE in reduce_combined silently deadlocks under scipy 1.17.1 +
sklearn 1.8.0 (graphtools -> sklearn PCA -> scipy.lu, ~0% CPU forever
even with BLAS thread caps). Set phate: null in the recipe; PCA-only
reductions are sufficient for Table 1 + matrix comparison plots.
Re-enable when scipy < 1.17, X_pca_combined is wired through PHATE,
or upstream fix lands.
Wave-1 wrapper bumped to 32G/4cpu (was 8G/2cpu). Per-experiment PLOT
runs locally on the wrapper node and 19 sequential plots on 350k cells
were OOM-killing the 8G allocation.
run_microglia.sh: new Wave-2 SLURM submission for the microglia leaf.
Same pattern as run_infectomics_annotated.sh; reads pipelines from
.../linear_classifiers/{model}/infectomics/latest.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(dynacell): unblock joint training + add A549 cross-eval (#415)
* chore(deps): pin iohub to PR #408 (RFC-9 zipped OME-Zarr); bump Python to >=3.12
Pin iohub to czbiohub-sf/iohub PR #408 head SHA
(53b10acb7a30a2c7e8dfd9b04258dea073e14088 — "feat: Added RFC-9 Zipped
OME-Zarr"). The PR adds zipped OME-Zarr store support; this pin
unblocks downstream consumers before the PR merges upstream.
The PR's iohub head requires Python >=3.12, so bump every workspace
pyproject.toml's `requires-python` from >=3.11 to >=3.12, drop the
3.11 classifier, and update ruff `target-version` to py312. Also
folds in the previously-pending dynacell[wandb] optional-extra
registration that the lockfile was already lagging on.
SHA pin (not branch) survives force-pushes; bump deliberately when
the PR moves.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(dynacell): bake A100 exclusion into hardware_4gpu launcher profile
After repeat NCCL BROADCAST/ALLREDUCE coordination hangs at first-batch
on this cluster's A100 partition (FCMAE jobs 31474030 + 31474038 timed
out at 1h13m / 1h21m on watchdog; joint smoke 31480607 hung
indefinitely at `Loading 'train_dataloader'` on gpu-a-3), every 4-GPU
train leaf needed the same `--override
launcher.sbatch.constraint='h100|h200|a40|a6000|l40s'` workaround.
Job 31481032 with that override scheduled to gpu-h-3 (H100) and pushed
past the same hang point, confirming the workaround.
Bake the exclusion into the shared profile so it applies by default to
all 11 4-GPU consumers (FCMAE pretrained × 4 + scratch × 4 + er/unext2
+ joint celldiff train.yml + train_smoke_4gpu.yml). Leaves that
genuinely need A100 must opt out via
`--override launcher.sbatch.constraint=null`.
Lock-in test: `test_4gpu_train_leaves_inherit_a100_exclude` walks
every train leaf under benchmarks/virtual_staining/ and asserts the
constraint on 4-GPU leaves; new leaves picking up the profile are
covered automatically.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(viscy-data): port to iohub 0.3.2 ImageArray API (#407)
The iohub PR #408 pin (Commit 1) ships a refactored ``ImageArray`` API
that removed ``ImageArray.name`` and other legacy attributes used
across viscy-data, viscy-utils, dynaclr, and qc. Without this port,
the existing test suite fails immediately on dataset iteration
(``AttributeError: 'ImageArray' object has no attribute 'name'`` at
``sliding_window.py``).
Cherry-picks the migration shipped in PR #407 (squash 737cedf on
``modular-viscy-staging``), preserving local extensions:
- ``sliding_window.py``: take ``f"/{img.path}"`` from #407 (replaces
``img.name``) but keep dynacell-models's ``to(torch.float32, copy=True)``
cast on the mmap_preload path (post-dates #407).
- ``viscy-data/pyproject.toml``: keep ``iohub>=0.3.2`` lower bound from
#407; the ``[tool.uv.sources]`` SHA pin in the workspace root
pyproject.toml (Commit 1) is what actually controls resolution.
- ``uv.lock``: regenerate.
All 25 ``packages/viscy-data/tests/test_combined.py`` tests pass post-
migration, including the previously-failing
``test_combined_datamodule_fit``.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(viscy-data): propagate trainer to children in ConcatDataModule.setup
Under DDP, ``Trainer.fit`` calls ``prepare_data`` only on rank 0 of each
node when ``prepare_data_per_node=True``, but ``setup`` runs on every
rank. ``ConcatDataModule.prepare_data`` forwarded ``self.trainer`` to
each child datamodule; ``setup`` did not. As a result, non-rank-0
children kept ``self.trainer = None``, and trainer-gated paths in the
children's ``on_after_batch_transfer`` silently skipped — most visibly
``HCSDataModule``'s
``if self.trainer and self.trainer.training`` guard at ``hcs.py:628``,
which gates ``gpu_augmentations``.
Production failure: SLURM 31481032 (joint celldiff smoke on 4× H100,
post-PR #413 + post-A100-exclude). Ranks 1, 2, 3 raised
``ValueError: x spatial size [13, 624, 624] does not match expected
[8, 512, 512]`` at the first training step — rank 0 ran fine because
its child had received the trainer via ``prepare_data``. Rank
asymmetry was the smoking gun.
Fix: propagate ``dm.trainer = self.trainer`` at the top of each loop
iteration in ``setup``, mirroring ``CombinedDataModule.setup`` (which
was already correct). Same fix applied to ``CachedConcatDataModule``,
which has the identical latent bug. ``BatchedConcatDataModule``
inherits from ``ConcatDataModule`` so the fix flows through.
Single-process regression test exercises the bug by skipping
``prepare_data`` entirely (mimicking the non-rank-0 lifecycle) and
asserting that ``setup`` alone propagates the trainer and that
``gpu_augmentations`` actually run through ``on_after_batch_transfer``.
The existing ``test_combined_ddp.py`` (gloo, ``pin_memory=False``, no
``gpu_augmentations``) cannot catch this — it doesn't exercise the
gpu-aug branch.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test(dynacell): mirror a549-mantis manifest fixtures (3 plates)
Add three local fixture manifests so VisCy's dataset_ref resolver
tests can resolve a549-mantis datasets without requiring a
``dynacell-paper`` install. Mirrors the canonical manifests at
``dynacell-paper/_configs/datasets/a549-mantis/<date>/manifest.yaml``
landed via dynacell-paper PRs A1.1 (#14) and A2.1 (#15) plus the
prior aeef64c registration.
- ``a549-mantis-2024_10_29`` — TOMM20 plate (mito cross-eval).
- ``a549-mantis-2024_11_07`` — SEC61B plate (er cross-eval; also
feeds the joint celldiff train leaf).
- ``a549-mantis-2026_03_26`` — h2b/caax mantis_v2 plate
(nucleus + membrane cross-eval).
Resolver discovery: ``DYNACELL_MANIFEST_ROOTS`` is set to
``tests/fixtures/manifests/`` by the autouse
``_dynacell_manifest_root_env`` fixture in ``tests/conftest.py``.
Layout matches existing aics-hipsc fixture
(``<root>/<dataset_name>/manifest.yaml`` per
``resolver.py:131-140``); ``splits/`` subdirs are not mirrored
because the resolver doesn't read them.
Closes Stage 5 of A549_EXPANSION_ROADMAP.md on the VisCy side and is
the prerequisite for Stage 6 cross-eval leaves.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(dynacell): add a549_mantis predict_set fragments + Stage 6 cross-eval leaves
Closes Stage 6 of A549_EXPANSION_ROADMAP.md: every existing iPSC-trained
``<organelle>/<model>/ipsc_confocal/`` cell now has a sibling
``predict__a549_mantis.yml`` + ``eval__a549_mantis.yaml`` so iPSC-trained
models can be evaluated on the a549 test split.
8 cells × 2 leaves + 8 eval symlinks + 6 predict_set fragments
= 30 file additions.
Predict_set fragments are per-plate because the a549-mantis registry is
split by acquisition date:
- ``2024_11_07`` — SEC61B (er cross-eval)
- ``2024_10_29`` — TOMM20 (mito cross-eval)
- ``2026_03_26`` — H2B + CAAX (nucleus + membrane cross-eval)
Each fragment exists on both the model side
(``_internal/shared/model/predict_sets/``) and the Hydra side
(``dynacell/evaluation/_configs/predict_set/``), matching the existing
ipsc_confocal pattern.
Stage 6 leaves keep the iPSC-trained checkpoint and inherit the cell's
per-model predict configuration; only the predict_set base, output
store, and ``benchmark.experiment_id`` differ. CellDiff outputs use
``_iterative`` suffix; UNetViT3D outputs do not.
Nucleus + membrane leaves additionally set
``benchmark.dataset_ref.target: h2b`` (or ``caax``) at the leaf level —
the iPSC manifest keys those targets by `nucleus`/`membrane` while a549
keys them by gene. Eval leaves also override
``target_name: h2b`` (or ``caax``) so the segmentation / cache layers
(``mask_plate(target_name)`` → ``{target_name}.zarr``) remain consistent
if ``compute_feature_metrics`` is enabled later.
A549 manifests don't currently carry ``cell_segmentation`` or
``gt_cache_dir`` paths (no segmentation pipeline yet), so eval leaves
set ``compute_feature_metrics: false``. Pixel metrics (PCC, SSIM, NRMSE,
PSNR, FSC, spectral PCC) work without segmentation. Flip the flag in a
follow-up once segmentation lands in dynacell-paper.
Composition tests:
- ``test_a549_predict_leaf_composes`` parametrizes over the 8-cell
matrix and asserts ``data.init_args.{data_path, source_channel,
target_channel}``, ``benchmark.dataset_ref.{dataset, target}``,
``experiment_id``, and ``launcher.sbatch.constraint == 'h200'``
(single-GPU predict topology).
- ``test_a549_eval_leaf_composes_and_splices`` composes each Hydra eval
leaf, calls ``apply_dataset_ref(cfg)``, and asserts ``io.{gt_path,
gt_channel_name, pred_channel_name}``, ``cell_segmentation_path is
None``, ``gt_cache_dir is None``, manifest spacing (mantis_v1 0.1494
µm vs mantis_v2 0.116 µm), and ``compute_feature_metrics is False``.
16 new tests, all green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor: /simplify cleanup of joint-training + a549 cross-eval branch
Apply post-implementation simplification per /simplify review:
- ``combined.py``: drop SLURM job ID from ``ConcatDataModule`` Notes block.
Symptom (rank-asymmetric un-cropped batches) is non-obvious and stays;
the specific failing job belongs in PR description, not a docstring
that ages out.
- ``test_combined.py``: switch ``SimpleNamespace`` fake trainer to
``MagicMock`` to match the existing pattern in ``test_hcs.py``. Move
imports to module level (per CLAUDE.md "Import at the top of the
file"). Drop redundant inline narration comments.
- ``hardware_4gpu.yml``: trim 4 SLURM job IDs from the rationale block
(task narration; belongs in PR/commit messages) and drop the rotting
"11 4-GPU consumers (FCMAE × 8 + ...)" count — the data-driven
``test_4gpu_train_leaves_inherit_a100_exclude`` enforces the invariant
without needing a maintenance-prone count in a YAML comment.
No behavior change. All previously-green tests remain green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs(dynacell): clarify _A549_EVAL_EXPECTATIONS shape comment
The comment listed four fields, but the dict values are 3-tuples (the
organelle is the dict key, not part of the value). Rewrite as
``{organelle: (target_group, gt_channel, gt_suffix)}`` so the shape is
unambiguous when the matrix is extended.
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>
* docs(dynacell): final findings + 8-job FCMAE benchmark, open items
Adds a "Final findings (2026-04-26) — bf16 fix shipped, 4-organelle
benchmark in flight" section that:
- States the solution in one paragraph (bf16 SSIM helper +
redundant-decorator removal, with reference to the merged PR #412
squash commit 48f4878).
- Records the live 4-organelle x {scratch, pretrained} = 8-job FCMAE
matrix with median + p10 s/step, sourced directly from wandb
loss/train_step step x timestamp deltas (dt < 60s filter excludes
ckpt/val/epoch boundaries — directly measured per the
no-walltime-estimates rule).
- Calls out three claims we can support today:
* pre-fix Hopper at 45-75 s/step -> post-fix at 4.77-5.76 s/step,
9-14x recovery
* Hopper now competitive with L40S but not visibly faster
* A40 (gpu-c-1) still wins at 2.40 s/step steady-state for
reasons not yet measured (node-local I/O, /dev/shm topology,
etc.); same shared-FS data path as Hopper, so the difference is
on the compute-node side.
- Lists what's still open: warmup-vs-steady-state on Hopper (current
numbers are gstep < 1.7k vs A40 baseline at gstep 35k), an unprofiled
A40-vs-Hopper compute-path gap, the 20h host-RAM leak (separate
thread), and the A100 NCCL BROADCAST hang (mitigation in place via
--constraint='h100|h200|a40|a6000|l40s' but root cause not
investigated).
Adds Recommendation #4 (the A100 exclude). Numbers explicitly noted as
"will be re-pulled once new Hopper jobs reach gstep >=10k" so the
follow-up is on the page, not a chat-only TODO.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs(dynacell): clean up resolved planning docs; refresh A549 roadmap
Remove three temporary planning docs whose work has shipped, and
refresh the overall A549 expansion roadmap to reflect current
progress, gaps, and next steps.
Removed:
- DATASET_REF_RESOLVER_SPEC.md — Stages 1-3 spec; all merged
(38d47b3, 4bb9f09, 11836c8, 326b2d0, 6273439, 8924ab2, f5a6e56,
a984384).
- fcmae_hopper_slowdown.md — bf16 SSIM kernel fix shipped via PR #412
(squash 48f4878); 4-organelle FCMAE benchmark complete.
- submit-reliability-plan.md — Gap 1 (--exclude SBATCH directive)
and Gap 3 (NCCL preflight smoke test) merged into
submit_benchmark_job.py + sbatch_template.sbatch +
tools/nccl_smoke_test.py; Gap 2 (auto-requeue) explicitly deferred
upstream of the doc.
A549_EXPANSION_ROADMAP.md refresh:
- Status snapshot table: Stages 1-3 done with commit refs; Stage 5
partial (canonical manifests in dynacell-paper @ aeef64c, VisCy
fixture mirror still missing); Stage 6 not started; Stage 7 (joint
training) in flight with first leaf shipped.
- Reconcile commit-message "Stage 7" with the roadmap by promoting
joint-training leaves to a first-class stage (was implicit under
the old Stage 6 description).
- Drop the now-stale DATASET_REF_RESOLVER_SPEC.md cross-reference.
- Trim ~234 → ~130 lines: collapse done stages, keep open work.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(dynacell): rename ER+Mito FCMAE pretrained outputs to _ws8500
The original ER and Mito FCMAE pretrained leaves predated the
warmup_steps=8500 model overlay (commit d296c7f, 2026-04-23) and
were submitted with the engine default warmup_steps=3, while their
scratch counterparts ran with warmup_steps=8500. To make the
pretrained vs scratch pairs directly comparable on training
protocol, rename outputs to a sibling _ws8500 directory so both
the abandoned ws=3 baselines and the new ws=8500 runs coexist on
disk. README.md files in the parent dirs document the dual-dir
layout.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* chore(dynacell): pin compute-job CWD to repo_root in sbatch template
Lets the dynacell launcher work from a git worktree: uv resolves
project context (pyproject + .venv) from CWD, so without an
explicit cd the compute-node srun would inherit whatever CWD sbatch
was invoked from and may pick up the wrong worktree's environment.
Pinning to @@repo_root (the worktree containing the launcher
script) makes the env deterministic regardless of submit-side CWD.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* docs(dynacell): refresh A549 roadmap with 2026-04-24 status
Stage 5 (a549 manifest): A549 zarr normalization-stats backfill
closed 2026-04-24 in dynacell-paper (f4120e0 + 17-zarr backfill);
no VisCy-side action remaining for that subtask.
Stage 7 (joint training leaves): mark "in flight - blocked". PR #413
(0b04b24) closed one DDP deadlock surface but the 4-GPU smoke
(234819a) still hangs at the same milestone; a second deadlock
surface remains. Joint leaf expansion is paused until that resolves.
Cross-reference the followup handoff and the _test48 debug-zarr
convention for short-wall smoke runs.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* fix(dynacell): repoint mito A549 cross-eval to 2024_11_21 plate
The 2024_10_29 plate the Stage 6 mito leaves were wired to is
train-only by design — its splits author 0 test FOVs. Repoint the
fixture mirror, predict_set fragments (model + Hydra side), the four
mito predict/eval leaves, and the two test matrices to
a549-mantis-2024_11_21, which has 11 authored test FOVs.
This unblocks all 8 Stage 6 cross-eval cells; ER, mito, nucleus, and
membrane × {celldiff, unetvit3d} now point at plates with real test
splits on disk.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(dynacell): add fnet3d_paper Stage 6 A549 predict leaves
Author 4 a549_mantis predict leaves for fnet3d_paper × {er, mito,
nucleus, membrane}, mirroring the iPSC fnet predict leaves. Plates
follow the same wiring used by celldiff/unetvit3d a549 leaves: ER/
sec61b → 2024_11_07, mito/tomm20 → 2024_11_21, nucleus and membrane
→ 2026_03_26 (with benchmark.dataset_ref.target overrides h2b and
caax for the gene-keyed a549 targets). Each leaf reuses the same
iPSC best-val checkpoint as predict__ipsc_confocal.yml — Stage 6 is
cross-eval, not retrain.
This rounds out the third and final model row for Stage 6's
"full-but-predictable-only" sub-scope (celldiff + unetvit3d already
landed in #415; fnet3d_paper completes the trio).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(dynacell): add fcmae_vscyto3d Stage 6 A549 predict scaffolding
Add a shared `fcmae_vscyto3d_predict.yml` overlay (mirrors the model
block from `fcmae_vscyto3d_fit.yml` so checkpoints load with the
matching architecture, plus predict-time hparams) and 8 a549_mantis
predict leaves: 4 organelles × {pretrained, scratch}. Plates follow
the same wiring as the celldiff/unetvit3d/fnet3d_paper a549 leaves
(ER/sec61b → 2024_11_07, mito/tomm20 → 2024_11_21, nucleus/h2b and
membrane/caax → 2026_03_26 with `dataset_ref.target` overrides).
`ckpt_path` is intentionally a `/TODO_FILL_BEFORE_SUBMIT/...` path
on every leaf — iPSC FCMAE training is still in flight (jobs
31475094-31523064). Submitting any of these as-is will fail loudly
with FileNotFoundError on torch.load. Each leaf header documents the
expected post-training path so the swap is mechanical.
Note: ER + Mito pretrained outputs use a `_ws8500` warmup-steps
suffix (per commit 9af8bdf); nucleus + membrane pretrained do not.
Suggested paths in the headers reflect that asymmetry.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(dynacell): add FNet3D ER joint single-GPU smoke leaf
Author the FNet3D counterpart to the existing celldiff
joint_ipsc_confocal_a549_mantis/train_smoke.yml. Pairs the iPSC
SEC61B_test12 zarr (12 FOVs, 2.4 GB) with the 4-FOV a549_mantis
SEC61B store so a single A40 / H200 can iterate the joint loader
end-to-end without the full ~250 GB iPSC SEC61B mmap_preload wait.
Verified end-to-end on a local A40: 10 fwd+bwd+optim steps in 1:19,
26 GB peak GPU mem, loss/train_step 0.680 → 1.187 epoch loss, three
checkpoints written. Joint sharding on FNet3D's 35.3 M-param model
with z=32 / yx=64 / bs=8 fp32 works at single-GPU.
Note: leaf bakes `num_workers: 0` + `pin_memory: false` because the
default `num_workers > 0` + `pin_memory: True` reliably hangs on
forking the dataloader workers from a CUDA-initialized parent on
this cluster's interactive nodes (same pattern as the 4-GPU joint
smoke deadlock). Header documents the workaround so consumers tune
up only when they need throughput.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* perf(cytoland): enable mmap+persistent+bf16 for A549 infected finetune
Three flips to maximize throughput on the 4xH200 prod run now that PR #411
lifted the mmap_preload + exclude_fov_names restriction:
- persistent_workers: true on the 3 sub-DMs avoids the worker re-spawn
cost that dominated dynacell's per-epoch overhead.
- mmap_preload: true caches each plate to local /tmp scratch (28 TB on
H200 nodes), eliminating per-batch NFS reads. D3 now mmaps cleanly
even with its 27 exclude_fov_names entries thanks to PR #411.
- precision: bf16-mixed dodges the Hopper fp16 cuDNN slowdown
documented in applications/dynacell/configs/examples/fcmae_hopper_slowdown.md.
D2 smoke explicitly overrides both DataModule flags to false so
single-GPU smokes on the full prod plate stay under their walltime;
the bf16 flip is kept on the smoke to validate the AMP path.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* chore(cytoland): drop h200 constraint on A549 infected sbatch
The 4xH200 slot was scheduling 5 days out. bf16-mixed (landed in
ad1df84) avoids the Hopper fp16 slowdown that was the original
reason to pin to H200, so any 4-GPU node (H100/A100/A6000) is fine
and starts much sooner.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* feat(dynacell): bundle manifest registry as first-class package data
Promote the test fixture manifest mirror to a real registry shipped with
the dynacell wheel. The resolver auto-discovers it via the
``dynacell.manifest_roots`` entry point declared in pyproject.toml, so a
fresh clone resolves ``benchmark.dataset_ref`` lookups out of the box —
no ``DYNACELL_MANIFEST_ROOTS`` env var needed for predict/eval/test runs.
Replaces the previous workaround where users had to either set the env
var manually (multiple sessions hit ``ManifestNotFoundError`` because of
this) or rely on the autouse pytest fixture that injected it for tests
only. The env var stays as an explicit override path for testing
discovery precedence.
Splits/ subdirs were missing in the old fixture mirror but present in
canonical; copy them alongside their manifests so consumers that
resolve ``target.splits`` (currently latent in VisCy, used by
dynacell-paper) work correctly.
Drift between this bundled copy and dynacell-paper canonical is guarded
by ``test_manifest_sync.py`` — parametrized over every shipped dataset,
parses both YAML files and asserts canonical is a subset of VisCy
(VisCy may carry additive fields like ``gt_cache_dir``). Skipped unless
``DYNACELL_PAPER_PATH`` is set, so CI behaves predictably while local
dev catches drift early.
This is the cheapest fix for the viscy <-> dynacell-paper logical
cycle: single PR, no new package. Heavier options (extracting a
``dynacell-manifests`` leaf package + ``DYNACELL_DATA_ROOT`` for path
portability) deferred per architecture discussion.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(cytoland): require >=80 GB VRAM for A549 infected sbatch
Adds --constraint='h200|h100_80|a100_80' so the 4xGPU prod run is
guaranteed at least 80 GB per device. The 3-DM CombinedDataModule in
MAX_SIZE_CYCLE produces 16x4x3 = 192 crops/step/GPU — measured
against a live dynacell FCMAE H200 reference (~45 GB at single-DM
128 crops/step), the projected per-GPU footprint is ~67 GB. That
fits comfortably on H200, ~84% of an 80 GB H100/A100, and would OOM
on a 40 GB A100 or 48 GB A40/A6000.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* docs(dynacell): note manifest registry drift policy in README
Direct follow-up to 2395e1d (bundled manifest registry). Explains the
two-source-of-truth split: VisCy ships the manifest content
(``src/dynacell/_manifests/``, auto-discovered via the
``dynacell.manifest_roots`` entry point), and ``dynacell-paper`` remains
the source of truth for manifest authoring. When a new plate lands in
``dynacell-paper``, mirror it back into VisCy; ``test_manifest_sync.py``
catches drift when ``DYNACELL_PAPER_PATH`` is set.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(dynacell): submit_benchmark_job.py supports optional --dependency, --parsable (#416)
* feat(dynacell): submit_benchmark_job.py supports optional --dependency, --parsable
Adds two opt-in flags to enable orchestration that needs to chain
sbatch jobs by ID:
- --dependency afterok:<job_id> appends --dependency=<value> to the
sbatch invocation. Default off.
- --parsable invokes sbatch with --parsable (which makes sbatch print
just the numeric job ID instead of "Submitted batch job <id>"),
captures that ID, and forwards it to stdout. Default off.
Both flags default off. Manual submit_benchmark_job.py <leaf>
invocations build the same ["sbatch", str(sbatch_path)] command as
before, with stdout untouched — every existing workflow (smoke jobs,
ad-hoc submissions, model-iteration scripts) sees no behavior change.
Why: dynacell-paper's upcoming benchmarks orchestrator (Phase 5F)
needs to capture train job IDs so it can submit predict with
--dependency=afterok:<train_id>, chaining the train → predict pair
via SLURM. Without these flags, the orchestrator would have to parse
"Submitted batch job <id>" prose from a non-captured stdout — brittle.
Tests: 5 new monkeypatched cases covering the default shape (backward
compat) plus the three new flag combinations. No live sbatch.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(dynacell): keep stderr attached + clarify --parsable wording
Two Copilot findings on PR #416:
1. ``--parsable`` previously used ``capture_output=True``, which
captures both stdout and stderr — sbatch warnings and diagnostics
were silently swallowed on success. Switch to
``stdout=subprocess.PIPE`` so stderr stays attached to the parent
and only the parsable line is captured.
2. ``--parsable`` help text claimed to "forward the parsed job ID"
but the implementation forwards sbatch's parsable stdout verbatim,
which can be ``job_id;cluster`` on multi-cluster setups. Update
wording to describe the forwarding accurately.
Test asserts the new kwarg shape (``stdout=PIPE``, no ``stderr``,
no ``capture_output``).
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>
* feat(dynacell): bundle 4 missing A549 manifests + predict_set fragments
Adds bundled manifests for a549-mantis plates 2024_10_31, 2024_11_05,
2025_07_24, 2025_08_26 — the per-plate predict_set fragments below need
their dataset_ref to resolve at compose time, so the manifests are
mirrored into dynacell._manifests parity with canonical dynacell-paper
configs (test_manifest_sync.py enforces).
Predict_set fragments author one-line dataset_ref redirects per plate so
existing per-plate predict leaves can switch plates without touching the
data block.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* feat(dynacell): per-plate A549 predict leaves for ER + MITO across 5 models
Today's predict__a549_mantis.yml only covered ONE plate (ER: 2024_11_07,
MITO: 2024_11_21). The A549 SEC61B test budget spans 4 plates and TOMM20
spans 4 plates, so a single-plate predict leaf cannot cover the full
test set. Plan-time gap: per-plate iteration was deferred under the
assumption a single demo plate was e…
alxndrkalinin
added a commit
that referenced
this pull request
Jun 2, 2026
…429) * fix(dynacell): drop NaN bars from cross-model metric figure In the cross-model bar chart, ``stats["mean"].reindex(plot_metrics)`` produces NaN entries for any metric a given model did not report. Passing those NaNs through ``ax.bar(..., yerr=stds.values, ...)`` yields version-dependent matplotlib behavior — recent versions raise or warn on NaN yerr, older ones quietly draw a zero-height error bar. Mask each model's missing metrics before plotting: skip the model entirely when no metrics are present, otherwise drop the NaN bars and ``fillna(0)`` on the surviving stds so yerr stays finite. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor(dynacell): use Field(default_factory=list) for Pydantic list defaults Replace bare ``list[T] = []`` defaults on Pydantic ``BaseModel`` fields with ``Field(default_factory=list)`` in ``BenchmarkSpec``, ``Provenance``, and ``CollectionExperiment``. Pydantic v2 deep-copies mutable defaults per-instance so the bare form is not actually a shared-state bug, but the factory form matches the convention already used in ``viscy_utils.evaluation.linear_classifier_config`` and is the explicit-intent style most readers expect for list defaults. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(viscy-utils): strip top-level _-prefixed keys from composed config YAML merge anchors must be defined at top level (the only scope ``yaml.safe_load`` resolves), but the defining key survives parse and gets passed through to consumers. LightningCLI / jsonargparse reject unknown top-level keys, so a leaf that uses ``_hcs_init_args: &hcs_init_args`` to factor shared init_args across joint-datamodule children fails at ``dynacell fit`` with:: Subcommand 'fit' does not accept option '_hcs_init_args.augmentations' This pattern slipped past the CPU-only smoke (which feeds the ``data:`` block straight into jsonargparse and never sees the top-level keys), so the only place it would have surfaced is the actual SLURM run. Establish the convention that top-level keys starting with ``_`` are private to the YAML composition layer and are stripped before the returned dict is handed to LightningCLI. The strip applies at every recursion level so anchor defs in ``base:`` fragments don't leak through ``deep_merge`` either. Resolvers still see the full dict — strip is the final step before return. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * docs(dynacell): document anchor convention; pin no-anchor-leak invariant The first joint train leaf relies on a top-level ``_hcs_init_args:`` YAML merge anchor to factor shared init_args across the two child HCSDataModule blocks. Now that ``load_composed_config`` strips underscore-prefixed top-level keys before returning, the leaf is LightningCLI-safe — but the convention only matters if leaf authors know about it. - Annotate the leaf's anchor block with a one-paragraph note pointing at the strip behavior and explaining why the prefix is required. - Pin the invariant in ``test_joint_train_leaf_composes``: assert no top-level key starting with ``_`` leaks through composition. Without this guard, a regression in ``load_composed_config`` would make the joint leaf fail at SLURM-run time, the same gap that allowed this bug to ship in the first place. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * chore(dynacell): declare wandb optional extra; document the runtime gap The default `trainer.logger` in `configs/recipes/trainer/fit.yml` is `lightning.pytorch.loggers.WandbLogger`, but `applications/dynacell` depends on `viscy-utils` (not `viscy-utils[eval]`) and `wandb` is in viscy-utils' eval extra — so a vanilla `pip install dynacell` ships without wandb. jsonargparse then rejects every fit/predict leaf at schema validation time with `Requirement 'wandb>=0.12.10' not met`, before `fast_dev_run` (or any other runtime mechanism) gets a chance to disable the logger. Add a `dynacell[wandb]` extra so consumers (dynacell-paper, end users) can pin the runtime expectation cleanly. Document the gap in the virtual_staining README's "Running" section: install via the extra, or override `trainer.logger.class_path` to a non-W&B logger like `CSVLogger`. No behavior change — the dep declaration just catches up with what the recipes already require at runtime. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat(dynacell): add canonical joint smoke leaf for celldiff/joint_ipsc_confocal_a549_mantis `submit_benchmark_job.py`'s dotlist override parser cannot index into list elements, so a smoke variant of a joint leaf cannot be made by overriding a single child's `data_path` at submit time. Adds a `train_smoke.yml` sibling next to the production `train.yml`: pre-swaps the iPSC child to `SEC61B_test48.zarr` (48 FOVs, smoke-staging budget), keeps the a549 child untouched (its train split is already 4 FOVs), and swaps `hardware_4gpu` for `hardware_h200_single` so a single-GPU smoke node can run it without DDP. The leaf comment header documents the diff against `train.yml` and points consumers at `--override trainer.fast_dev_run=true` (or `trainer.max_steps=N`) for bounding wall time. README "Joint smoke sibling" section explains why this is a sibling leaf rather than a submit-time override. New `test_joint_train_smoke_leaf_composes` brackets single-GPU topology, both children's zarr paths, and the no-anchor-leak invariant. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat(dynacell): add hardware_h200_single_smoke launcher profile Adds a smoke-sized variant of hardware_h200_single that bounds launcher.sbatch.time to 30 min. Identical otherwise (single H200, ntasks_per_node=1, 256G mem). Re-points train_smoke.yml at the new profile so a smoke job cannot sit on a multi-day allocation by default. Pair with --override trainer.fast_dev_run=true (or trainer.max_steps=N) so the run actually exits inside the wall. test_joint_train_smoke_leaf_composes now asserts sbatch.time matches the smoke value, locking the invariant into the leaf. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor(dynacell): /simplify smoke profile via wall-only overlay Replaces the duplicated `hardware_h200_single_smoke.yml` (which copied 8 of 9 fields from `hardware_h200_single.yml`) with a 4-line `wall_smoke.yml` that only overrides `launcher.sbatch.time`. The smoke leaf now stacks `hardware_h200_single + wall_smoke + runtime_shared`, matching the existing compose-by-overlay idiom. Future hardware tweaks (cpus, mem, constraint) propagate to smoke automatically; future hardware variants reuse `wall_smoke.yml` for free. Also trims the smoke leaf header: drops the numbered "Differences from sibling train.yml" enumeration (would rot the moment either leaf changes; the README's "Joint smoke sibling" section already documents the contract) and shortens the anchor-mechanism comment to a one-line cross-ref into train.yml. No behavior change — composed `launcher.sbatch` is byte-identical to the previous form (verified by `test_joint_train_smoke_leaf_composes`, which still asserts `time == "00:30:00"` and `constraint == "h200"`). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat(dynacell): disable logger at the smoke leaf level Consumers of train_smoke.yml shouldn't have to remember `--override trainer.logger=false` at submit time, and smokes don't need a logger by definition. Sets `trainer.logger: false` directly in the leaf so future smokes for other models/organelles inherit the same disable when the recipe defaults a WandbLogger. Recipe `fit.yml` also defaults a `LearningRateMonitor` callback that raises `MisconfigurationException` when `trainer.loggers` is empty (`lr_monitor.py:121-122`), so the smoke leaf's `callbacks` list is trimmed to just `ModelCheckpoint`. Lists replace wholesale under deep_merge, so this cleanly drops LRMonitor without affecting prod. `test_joint_train_smoke_leaf_composes` now asserts both invariants: `trainer.logger is False` and LRMonitor is not in the callbacks list. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(viscy-data): drop non-tensor metadata in BatchedConcatDataModule combine `BatchedConcatDataModule.on_after_batch_transfer` combined per-key values across micro-batches by assuming each value was either a list (extend) or a tensor (`torch.cat`). HCSDataModule emits `norm_meta` (dict of per-channel normalization stats, when `load_normalization_metadata=True`, the default) and `index` (tuple of `(img_name, t_idx, z_idx)`) per sample. After collation those keys hit the cat branch and `torch.cat([dict_a, dict_b])` raised `TypeError: expected Tensor as element 0 in argument 0, but got dict`, killing joint training during Lightning's `_run_sanity_check`. Joint training across heterogeneous children has no well-defined combined semantic for these keys — channels need not align between zarrs and FOV identifiers are dataset-specific — and Lightning's training/validation only reads source/target tensors. Skip the key during combine instead. Existing tests (`test_batched_concat_datamodule_with_hcs_children` and the DDP variants) iterate `next(iter(loader))` to inspect the micro-batch contract but never invoke `on_after_batch_transfer`, so the bug never surfaced. New regression test calls `on_after_batch_transfer` against the real HCS fixture (which does write per-FOV normalization metadata) and asserts tensor keys concatenate cleanly while dict-valued metadata is dropped. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(dynacell): cut smoke leaf batch_size to 1 to fit a single H200 train.yml's batch_size=4 across 4 GPUs OOM'd when run on a single H200 (per-step memory is batch_size * num_samples patches at [8, 512, 512]). Drops the smoke's child batch_size to 1 so per-step VRAM stays inside one GPU's budget while the patch shape stays identical to train.yml — the validation remains apples-to-apples. `RandWeightedCropd.num_samples` also drops from 2 to 1: HCSDataModule requires `batch_size % num_samples == 0` and rejects the leaf at schema-validation time otherwise. num_samples=1 is the largest value that satisfies the constraint with batch_size=1. `test_joint_train_smoke_leaf_composes` follows along (asserts batch_size=1 instead of 4). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat(dynacell): add 4-GPU DDP smoke leaf for joint celldiff SEC61B Sibling of train_smoke.yml that swaps hardware_h200_single -> hardware_4gpu and adds trainer.strategy=ddp, devices=4, max_steps=5. Validates that BatchedConcatDataModule + ShardedDistributedSampler integrate correctly on the production DDP topology - single-GPU smoke already proved the joint loader and training/val loops work; this isolates sharding. batch_size=1 / num_samples=1 kept identical to train_smoke.yml so the per-rank memory profile is the proven one - sharding is the only new variable. max_steps=5 baked in (not override) after train_smoke.yml TIMEOUT'd at 30 min from a missed --override at submit time. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * perf(viscy-utils): bf16-precision SSIM helper for Hopper FCMAE training (#412) * feat(viscy-utils): add bf16-precision SSIM helper Replaces monai.metrics.regression.compute_ssim_and_cs (which unconditionally casts both inputs to fp32 internally) with a precision- aware variant in viscy_utils.evaluation.metrics. The helper runs the 5 Gaussian-mean convolutions in bf16 and promotes only the variance subtractions and C1/C2-guarded divisions to fp32. Squared products are computed in fp32 first, then cast to bf16 for the conv input, which preserves precision on the precision-sensitive squaring step. Why: FCMAE VSCyto3D training on Hopper (H100/H200) runs at ~45 s/step vs ~3-5 s/step on Ampere/Ada because monai's 25 fp32 conv ops per loss invocation can't use Hopper's bf16 tensor cores. The bf16 helper restores tensor-core path while keeping the precision-sensitive math in fp32 to match monai's NaN-resistance properties. Numerical contract validated by 7 tests in test_metrics.py: - per-pixel random: rtol=5e-2, atol=1e-1 (>=2x margin over measured 0.0418 max abs drift) - aggregate random: rtol=1e-2, atol=1e-2 (>=25% margin over measured 0.00776) - aggregate correlated-pair (pred=target+0.05*randn): rtol=2e-3, atol=5e-3 - gradient flow: cosine-similarity >= 0.99 between helper and monai-ref flat gradients; sign-flip fraction < 1% on |grad_ref| > 1e-3 voxels - dtype invariance: helper returns fp32 regardless of fp32/bf16/fp16 input dtype The decorators on MixedLoss/SpotlightLoss are addressed in a follow-up commit; this commit only changes the SSIM compute path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(viscy-utils): drop redundant fp32 cast on MixedLoss/SpotlightLoss Removes @torch.amp.custom_fwd(device_type="cuda", cast_inputs=torch.float32) from MixedLoss.forward and SpotlightLoss.forward. For MixedLoss: the decorator was redundant because monai's compute_ssim_and_cs already cast both SSIM inputs to fp32 internally (monai/metrics/regression.py:402-403), so the outer @custom_fwd only affected F.l1_loss / F.mse_loss / outer F.avg_pool3d — none of which are the slow path. Now that the SSIM compute path goes through _compute_ssim_and_cs_bf16 (preceding commit) the fp32 island the decorator created actively defeats the purpose. The L1 and MSE branches remain numerically identical because PyTorch's autocast policy already promotes them to fp32 (cf. test_mixed_loss_l1_only_matches_torch_l1). For SpotlightLoss: the decorator's fp32 island was layered onto a function with no conv-heavy ops — only squared error, masking, sums, and divisions, all of which are in autocast's promote-to-fp32 list already. Removal is a no-op for behaviour but unblocks autocast plumbing for future precision experiments. New tests: - test_mixed_loss.py (4): forward outside autocast, forward under bf16 autocast (drift vs explicit-fp32 baseline within rtol=1e-2, atol=1e-2), gradient flow under autocast, L1-only bit-exact F.l1_loss. - test_spotlight_loss.py (+2): autocast bf16 forward+backward finite, autocast result matches explicit-fp32 baseline within rtol=1e-2, atol=1e-2. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(dynacell): record Hopper FCMAE slowdown investigation + bf16 fix Captures the full diagnostic chain from "Hopper trains 10-15x slower than Ampere/Ada" to "fp32 MS-DSSIM is the dominant slow term" to the landed bf16 SSIM helper, with measured numbers for every claim. Notable artefacts the doc records (so future readers don't redo the work): - T2 probe (job 31452082) showed compute_ms collapses from 45.5 s/step to ~0.9 s/step on identical N=4 H100 hardware when MS-DSSIM is removed. - monai's compute_ssim_and_cs unconditionally casts both inputs to fp32 at regression.py:402-403; the @torch.amp.custom_fwd decorator on MixedLoss/SpotlightLoss was layered redundantly on top. - T6 probe (job 31453564) with the bf16 helper measured 4151 ms/step steady-state mean (6 consecutive STEP_TIMER lines) on identical N=4 H100 — 10.97x speedup, fix landed in commits e42c49a and 3a7fa05. Falsified hypotheses that were time-consuming to evaluate are also recorded (cuDNN [240,960,1,1] stride mismatch, DDP bucket-view mismatch, Lightning vs synthetic data, sync_batchnorm, MS-DSSIM-only symmetric probe T4 cancelled due to 8h queue slip) so the next person investigating Hopper performance regressions doesn't re-run them. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(viscy-utils): tighten gradient sign-flip + spotlight autocast bounds Two findings from /verify-plan: 1. test_ssim_helper_gradient_flow used |grad_ref| > 1e-3 to mask non-tiny voxels for the sign-flip-fraction assertion, but the measured |grad_ref| max is ~1.7e-6 — so the predicate selected zero voxels and the assertion was vacuous. Switch to a relative threshold (10% of the reference grad max) so the assertion is scale-invariant and meaningful regardless of the loss scale. 2. test_spotlight_loss_autocast_matches_fp32_baseline used rtol=1e-2, atol=1e-2 but measured drift is 0.0 — SpotlightLoss has no conv-heavy ops and autocast policy promotes the precision-sensitive pieces to fp32 anyway. Tighten to rtol=1e-3, atol=1e-3 to match the plan's spec and catch any future drift. Cosine-similarity check (>= 0.99 between flat helper-grad and flat reference-grad) was already meaningful and unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(viscy-utils): specialize bf16 SSIM helper to 3D, drop redundant tensors Two simplify passes consolidated: 1. Drop the ``spatial_dims`` parameter and the ``conv_fn = getattr(F, f"conv{spatial_dims}d")`` indirection. The helper is already specialized to "uniform kernel only" per the original plan; the only caller (``ssim_25d``) only ever passes ``spatial_dims=3``. Hardcoding ``F.conv3d`` directly removes a parameter from the public surface and removes a getattr lookup per conv call. Docstring updated to spell out the 3D specialization. 2. Drop the explicit ``y_pred_bf`` and ``y_bf`` intermediate tensors. The bf16 versions of the simple (non-squared) inputs are only consumed once each (in ``mu_x`` / ``mu_y`` convs), so caching them as named variables is pure memory duplication — peak working-set tensors drop from 7 to 5. Inline the cast at the conv site so the temporary lives only during the conv. Squared-product bf16 tensors stay named because the squaring must happen in fp32 first to preserve precision and the result is genuinely a different tensor. Test wrapper ``_bf16(...)`` updated to drop the ``spatial_dims=3`` kwarg; ``_SPATIAL_DIMS`` module constant removed; ``_ref(...)`` keeps ``spatial_dims=3`` since monai's signature still requires it. All 34 tests still pass; numerical contract unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(viscy-utils): apply PR review nits to bf16 SSIM helper Three small improvements from PR #412 review: 1. metrics.py:178 — switch ``data_range: Union[float, torch.Tensor]`` to ``data_range: float | torch.Tensor`` (modern typing per global CLAUDE.md). The pre-existing ``Union[...]`` at line 270 in ``ssim_25d`` is left alone per surgical-changes rule; the import stays since it's still used. 2. metrics.py:233 — add a brief comment explaining why the kernel is rebuilt per call (small tensor, negligible per-step cost relative to the conv) and what the cache-key would be if profiling ever shows it matters. Pre-empts the "should we cache this?" question future readers will have. 3. test_metrics.py:108 — update the gradient-flow test docstring to reference the relative threshold (10% of grad max) that the code actually uses; it previously said ``|grad_ref| > 1e-3`` which was the absolute threshold from before commit a498d59. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(viscy-utils): address Copilot review on PR #412 Four of Copilot's six findings applied; one rejected. Applied: - ``test_metrics.py``: monai is not a hard dep of viscy-utils (only transitively pulled via viscy-models / optimizers.py). Use ``pytest.importorskip("monai.metrics.regression")`` so the suite degrades gracefully if monai is ever decoupled. - All three test files: skip on ``not torch.cuda.is_bf16_supported()`` in addition to ``not torch.cuda.is_available()``. bf16 conv on pre-Ampere CUDA falls back to software emulation rather than failing, but the equivalence-vs-monai-fp32 tolerances were measured on Hopper — exclude older hardware where emulated bf16 could push drift past the configured rtol/atol. - ``metrics.py`` docstring: "five Gaussian-mean convolutions" → "five uniform-window mean convolutions". Kernel is uniform (ones / prod(kernel_size)), not Gaussian. - ``metrics.py`` helper: keep named ``y_pred_bf`` / ``y_bf`` views of the simple inputs going straight from the caller's dtype, instead of the fp32 round-trip ``y_pred_fp32.to(bf16)``. Saves a cast per simple input on the autocast path; under autograd the conv inputs are retained for backward either way, so this doesn't change peak memory. Rejected: - ``metrics.py`` capability gate / fp32 fallback. The plan deliberately chose unconditional bf16 convs (explicit casts, not autocast policy detection). bf16 conv works on every CUDA generation we ship to (sm_80+); on older CUDA it would emulate, not hard-fail. Falling back to fp32 for "any non-bf16 path" would reintroduce the Hopper bottleneck this PR exists to fix. 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(viscy-data): drop use_thread_workers to fix DDP deadlock (#413) * fix(viscy-data): drop use_thread_workers to fix DDP deadlock BatchedConcatDataModule used ThreadDataLoader(use_thread_workers=True), which substitutes a thread-shim for multiprocessing.Process inside PyTorch's worker iterator and silently forces persistent_workers=False. That combination × real init_process_group hangs Lightning's barrier("train_dataloader()") on a 4-GPU H200 node (SLURM 31453225). Single-GPU and CPU+gloo DDP both worked, so the deadlock is GPU/CUDA-specific (pin-memory thread × thread-shim worker context × per-rank CUDA context). Drop use_thread_workers=True from train_dataloader and val_dataloader, move the lambda collate to a module-level _identity_collate (spawn-safe), and document why workers are now real subprocesses. The new test_combined_ddp.py uses mp.start_processes(start_method="fork") to spawn 2 real DDP ranks under pytest (mp.spawn requires start_method "spawn", which can't re-resolve pytest's --import-mode=importlib path) and parameterizes (num_workers, mmap_preload) ∈ {(0,F), (2,F), (2,T)} — the last cell is the regression guard. Uses a deadline-loop join because ctx.join returns False as soon as any rank exits, so a single ctx.join(timeout=...) call is unsafe. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(viscy-data): /simplify cleanup of DDP fix + regression test Simplify pass on combined.py + test_combined_ddp.py: - Drop the homegrown ``_identity_collate`` helper; reuse ``monai.data.utils.no_collation``, which is character-identical and already available via the same ``monai.data`` namespace the file imports ``ThreadDataLoader`` from. - Trim the change-narration tail on the ``BatchedConcatDataModule`` docstring; the WHY belongs in the commit message, not the long-lived class doc. - In the new test: drop the SLURM job number reference (transient task artifact), drop the hard-coded ``data_connector.py:93`` line number (rots across Lightning versions), drop the contributor-private ``/home/...`` path from the deadline-loop comment, and delete two WHAT-narration comments. Extract ``_kill_survivors`` so the terminate→join→kill sequence isn't duplicated between the deadline loop and the ``finally`` block. Replace the ``out_file.exists()`` pre-check with a direct ``read_text()`` (per repo style: prefer raising errors over TOCTOU stat checks). Only ``mkdir`` the ``scratch_dir`` when ``mmap_preload=True`` actually needs it. No behavior change. ``packages/viscy-data/tests/test_combined_ddp.py`` still passes all three parameter cells and ``test_combined.py`` is unaffected. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(viscy-data): skip DDP test on platforms without fork Copilot review on PR #413 flagged that the new ``test_combined_ddp.py`` hard-codes ``start_method="fork"`` and would fail at runtime on the ``windows-latest`` matrix in ``.github/workflows/test.yml`` (Windows has no fork). Add a ``pytest.skip`` when ``"fork"`` is absent from ``mp.get_all_start_methods()``. Linux (CI default) keeps running all three parameter cells; Windows now skips cleanly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(viscy-data): scope test docstring honestly to CPU+gloo coverage Code-review feedback flagged that the test claims to "guard the regression that broke production" but the deadlock is GPU/CUDA-specific (pin-memory thread × thread-shim worker context under NCCL) per the fix commit's own diagnosis — gloo/CPU passes even with the bug present. The test still has value (locks down collate, sampler-attachment, rank-0 prepare_data ordering, mmap_preload) but the docstring oversold its coverage. Reword to state the scope honestly and note that catching a direct revert of ``use_thread_workers=True`` needs a GPU runner. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(viscy-data): guard against re-introducing use_thread_workers=True CPU+gloo regression test in test_combined_ddp.py cannot reproduce the GPU/NCCL-specific deadlock that PR #413 fixed (pin-memory thread × thread-shim worker context under CUDA), so a future revert of ``use_thread_workers=True`` would slip past CI. This source-level check runs without DDP on every CI matrix cell. It builds the joint loader with ``num_workers=2`` (the threshold MONAI uses for the substitution) and asserts the resulting DataLoader's ``multiprocessing_context`` is not the ``monai.data.thread_buffer._ProcessThreadContext`` shim that ``use_thread_workers=True`` would install (``monai/data/thread_buffer.py:189-191``). Validated by temporarily re-adding ``use_thread_workers=True`` to ``BatchedConcatDataModule.train_dataloader`` and confirming the guard fails on both zarr_v2 and zarr_v3 fixture parameterizations, then removing the temp change before commit. 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> * feat(dynacell): nucleus + membrane FCMAE_VSCyto3D scratch + pretrained leaves Adds the four FCMAE_VSCyto3D leaves needed to extend the existing ER (sec61b) + Mito (tomm20) benchmark matrix to all four iPSC organelles (ER / Mito / Nucleus / Membrane), each with a paired {scratch, pretrained-encoder} pair so we have a paper-adjacent init-ablation baseline per organelle. Layout matches the existing ER/Mito leaves under ``benchmarks/virtual_staining/{er,mito}/fcmae_vscyto3d_{pretrained,scratch}/ ipsc_confocal/train.yml`` — same shared overlays (``train_sets/ipsc_confocal``, ``data_overlays/fcmae_vscyto3d_fit``, ``model_overlays/fcmae_vscyto3d_fit``, ``hardware_4gpu``) and the same ``benchmark.dataset_ref`` pattern. Pretrained leaves load encoder-only weights from ``/hpc/projects/virtual_staining/models/mehta-lab/VSCyto3D/ fcmae.ckpt`` (the canonical 400 ep VSCyto3D ckpt); scratch leaves are random-init. Both nucleus and membrane targets resolve to the multi-marker ``cell.zarr`` (Nuclei / Membrane channels respectively) per the aics-hipsc manifest at ``configs/datasets/aics-hipsc/manifest.yaml``, so no new zarrs needed. Resolved configs validated end-to-end via ``submit_benchmark_job.py --print-resolved-config``; all four compose cleanly with the bf16-SSIM-helper-equipped ``viscy_utils`` at HEAD. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(dynacell): override Structure aug-keys for nucleus + membrane FCMAE leaves Job 31474018 (FCMAE_VSCyto3D_Pretrained_Nucleus) crashed with ``KeyError: 'Structure'`` from ``RandWeightedCropd`` 47 minutes in. Root cause: ``_internal/shared/model/data_overlays/fcmae_vscyto3d_fit.yml`` hardcodes ``keys: [Phase3D, Structure]`` and ``w_key: Structure`` — that overlay was authored for ER (sec61b) and Mito (tomm20), which both have ``target_channel == "Structure"``. Compose's list-replace semantics mean the data overlay's augmentations list completely overrides the target overlay's augmentations, so the per-organelle channel name from ``targets/{nucleus,membrane}.yml`` (Nuclei / Membrane) never reaches ``RandWeightedCropd``. Fix at the leaf level: each of the 4 nucleus + membrane FCMAE leaves now declares its own ``data.init_args.augmentations`` with the right channel name. The leaf is composed last, so its augmentations list wins over the data overlay. The augmentation policy (spatial_size [20, 600, 600], num_samples 4, RandWeightedCropd as the only CPU aug) is kept identical to the FCMAE overlay so behaviour matches ER/Mito. A more robust fix would be to make the FCMAE data overlay's CPU augmentation block parameterized by ``target_channel`` (or to drop it from the overlay entirely and rely on the per-target overlay), but that's broader infra surgery — leaving for a separate refactor. ER (sec61b) and Mito (tomm20) FCMAE leaves are unchanged because they already work — their target_channel is "Structure". Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(dynacell): add eval script for FNet3D paper predictions on iPSC confocal Evaluates SEC61B, membrane, TOMM20, and nucleus FNet3D predictions against ground truth using pixel and DynaCLR feature metrics. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(dynacell): unblock joint training + add A549 cross-eval (#415) * chore(deps): pin iohub to PR #408 (RFC-9 zipped OME-Zarr); bump Python to >=3.12 Pin iohub to czbiohub-sf/iohub PR #408 head SHA (53b10acb7a30a2c7e8dfd9b04258dea073e14088 — "feat: Added RFC-9 Zipped OME-Zarr"). The PR adds zipped OME-Zarr store support; this pin unblocks downstream consumers before the PR merges upstream. The PR's iohub head requires Python >=3.12, so bump every workspace pyproject.toml's `requires-python` from >=3.11 to >=3.12, drop the 3.11 classifier, and update ruff `target-version` to py312. Also folds in the previously-pending dynacell[wandb] optional-extra registration that the lockfile was already lagging on. SHA pin (not branch) survives force-pushes; bump deliberately when the PR moves. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(dynacell): bake A100 exclusion into hardware_4gpu launcher profile After repeat NCCL BROADCAST/ALLREDUCE coordination hangs at first-batch on this cluster's A100 partition (FCMAE jobs 31474030 + 31474038 timed out at 1h13m / 1h21m on watchdog; joint smoke 31480607 hung indefinitely at `Loading 'train_dataloader'` on gpu-a-3), every 4-GPU train leaf needed the same `--override launcher.sbatch.constraint='h100|h200|a40|a6000|l40s'` workaround. Job 31481032 with that override scheduled to gpu-h-3 (H100) and pushed past the same hang point, confirming the workaround. Bake the exclusion into the shared profile so it applies by default to all 11 4-GPU consumers (FCMAE pretrained × 4 + scratch × 4 + er/unext2 + joint celldiff train.yml + train_smoke_4gpu.yml). Leaves that genuinely need A100 must opt out via `--override launcher.sbatch.constraint=null`. Lock-in test: `test_4gpu_train_leaves_inherit_a100_exclude` walks every train leaf under benchmarks/virtual_staining/ and asserts the constraint on 4-GPU leaves; new leaves picking up the profile are covered automatically. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(viscy-data): port to iohub 0.3.2 ImageArray API (#407) The iohub PR #408 pin (Commit 1) ships a refactored ``ImageArray`` API that removed ``ImageArray.name`` and other legacy attributes used across viscy-data, viscy-utils, dynaclr, and qc. Without this port, the existing test suite fails immediately on dataset iteration (``AttributeError: 'ImageArray' object has no attribute 'name'`` at ``sliding_window.py``). Cherry-picks the migration shipped in PR #407 (squash 737cedf on ``modular-viscy-staging``), preserving local extensions: - ``sliding_window.py``: take ``f"/{img.path}"`` from #407 (replaces ``img.name``) but keep dynacell-models's ``to(torch.float32, copy=True)`` cast on the mmap_preload path (post-dates #407). - ``viscy-data/pyproject.toml``: keep ``iohub>=0.3.2`` lower bound from #407; the ``[tool.uv.sources]`` SHA pin in the workspace root pyproject.toml (Commit 1) is what actually controls resolution. - ``uv.lock``: regenerate. All 25 ``packages/viscy-data/tests/test_combined.py`` tests pass post- migration, including the previously-failing ``test_combined_datamodule_fit``. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(viscy-data): propagate trainer to children in ConcatDataModule.setup Under DDP, ``Trainer.fit`` calls ``prepare_data`` only on rank 0 of each node when ``prepare_data_per_node=True``, but ``setup`` runs on every rank. ``ConcatDataModule.prepare_data`` forwarded ``self.trainer`` to each child datamodule; ``setup`` did not. As a result, non-rank-0 children kept ``self.trainer = None``, and trainer-gated paths in the children's ``on_after_batch_transfer`` silently skipped — most visibly ``HCSDataModule``'s ``if self.trainer and self.trainer.training`` guard at ``hcs.py:628``, which gates ``gpu_augmentations``. Production failure: SLURM 31481032 (joint celldiff smoke on 4× H100, post-PR #413 + post-A100-exclude). Ranks 1, 2, 3 raised ``ValueError: x spatial size [13, 624, 624] does not match expected [8, 512, 512]`` at the first training step — rank 0 ran fine because its child had received the trainer via ``prepare_data``. Rank asymmetry was the smoking gun. Fix: propagate ``dm.trainer = self.trainer`` at the top of each loop iteration in ``setup``, mirroring ``CombinedDataModule.setup`` (which was already correct). Same fix applied to ``CachedConcatDataModule``, which has the identical latent bug. ``BatchedConcatDataModule`` inherits from ``ConcatDataModule`` so the fix flows through. Single-process regression test exercises the bug by skipping ``prepare_data`` entirely (mimicking the non-rank-0 lifecycle) and asserting that ``setup`` alone propagates the trainer and that ``gpu_augmentations`` actually run through ``on_after_batch_transfer``. The existing ``test_combined_ddp.py`` (gloo, ``pin_memory=False``, no ``gpu_augmentations``) cannot catch this — it doesn't exercise the gpu-aug branch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(dynacell): mirror a549-mantis manifest fixtures (3 plates) Add three local fixture manifests so VisCy's dataset_ref resolver tests can resolve a549-mantis datasets without requiring a ``dynacell-paper`` install. Mirrors the canonical manifests at ``dynacell-paper/_configs/datasets/a549-mantis/<date>/manifest.yaml`` landed via dynacell-paper PRs A1.1 (#14) and A2.1 (#15) plus the prior aeef64c registration. - ``a549-mantis-2024_10_29`` — TOMM20 plate (mito cross-eval). - ``a549-mantis-2024_11_07`` — SEC61B plate (er cross-eval; also feeds the joint celldiff train leaf). - ``a549-mantis-2026_03_26`` — h2b/caax mantis_v2 plate (nucleus + membrane cross-eval). Resolver discovery: ``DYNACELL_MANIFEST_ROOTS`` is set to ``tests/fixtures/manifests/`` by the autouse ``_dynacell_manifest_root_env`` fixture in ``tests/conftest.py``. Layout matches existing aics-hipsc fixture (``<root>/<dataset_name>/manifest.yaml`` per ``resolver.py:131-140``); ``splits/`` subdirs are not mirrored because the resolver doesn't read them. Closes Stage 5 of A549_EXPANSION_ROADMAP.md on the VisCy side and is the prerequisite for Stage 6 cross-eval leaves. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(dynacell): add a549_mantis predict_set fragments + Stage 6 cross-eval leaves Closes Stage 6 of A549_EXPANSION_ROADMAP.md: every existing iPSC-trained ``<organelle>/<model>/ipsc_confocal/`` cell now has a sibling ``predict__a549_mantis.yml`` + ``eval__a549_mantis.yaml`` so iPSC-trained models can be evaluated on the a549 test split. 8 cells × 2 leaves + 8 eval symlinks + 6 predict_set fragments = 30 file additions. Predict_set fragments are per-plate because the a549-mantis registry is split by acquisition date: - ``2024_11_07`` — SEC61B (er cross-eval) - ``2024_10_29`` — TOMM20 (mito cross-eval) - ``2026_03_26`` — H2B + CAAX (nucleus + membrane cross-eval) Each fragment exists on both the model side (``_internal/shared/model/predict_sets/``) and the Hydra side (``dynacell/evaluation/_configs/predict_set/``), matching the existing ipsc_confocal pattern. Stage 6 leaves keep the iPSC-trained checkpoint and inherit the cell's per-model predict configuration; only the predict_set base, output store, and ``benchmark.experiment_id`` differ. CellDiff outputs use ``_iterative`` suffix; UNetViT3D outputs do not. Nucleus + membrane leaves additionally set ``benchmark.dataset_ref.target: h2b`` (or ``caax``) at the leaf level — the iPSC manifest keys those targets by `nucleus`/`membrane` while a549 keys them by gene. Eval leaves also override ``target_name: h2b`` (or ``caax``) so the segmentation / cache layers (``mask_plate(target_name)`` → ``{target_name}.zarr``) remain consistent if ``compute_feature_metrics`` is enabled later. A549 manifests don't currently carry ``cell_segmentation`` or ``gt_cache_dir`` paths (no segmentation pipeline yet), so eval leaves set ``compute_feature_metrics: false``. Pixel metrics (PCC, SSIM, NRMSE, PSNR, FSC, spectral PCC) work without segmentation. Flip the flag in a follow-up once segmentation lands in dynacell-paper. Composition tests: - ``test_a549_predict_leaf_composes`` parametrizes over the 8-cell matrix and asserts ``data.init_args.{data_path, source_channel, target_channel}``, ``benchmark.dataset_ref.{dataset, target}``, ``experiment_id``, and ``launcher.sbatch.constraint == 'h200'`` (single-GPU predict topology). - ``test_a549_eval_leaf_composes_and_splices`` composes each Hydra eval leaf, calls ``apply_dataset_ref(cfg)``, and asserts ``io.{gt_path, gt_channel_name, pred_channel_name}``, ``cell_segmentation_path is None``, ``gt_cache_dir is None``, manifest spacing (mantis_v1 0.1494 µm vs mantis_v2 0.116 µm), and ``compute_feature_metrics is False``. 16 new tests, all green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor: /simplify cleanup of joint-training + a549 cross-eval branch Apply post-implementation simplification per /simplify review: - ``combined.py``: drop SLURM job ID from ``ConcatDataModule`` Notes block. Symptom (rank-asymmetric un-cropped batches) is non-obvious and stays; the specific failing job belongs in PR description, not a docstring that ages out. - ``test_combined.py``: switch ``SimpleNamespace`` fake trainer to ``MagicMock`` to match the existing pattern in ``test_hcs.py``. Move imports to module level (per CLAUDE.md "Import at the top of the file"). Drop redundant inline narration comments. - ``hardware_4gpu.yml``: trim 4 SLURM job IDs from the rationale block (task narration; belongs in PR/commit messages) and drop the rotting "11 4-GPU consumers (FCMAE × 8 + ...)" count — the data-driven ``test_4gpu_train_leaves_inherit_a100_exclude`` enforces the invariant without needing a maintenance-prone count in a YAML comment. No behavior change. All previously-green tests remain green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(dynacell): clarify _A549_EVAL_EXPECTATIONS shape comment The comment listed four fields, but the dict values are 3-tuples (the organelle is the dict key, not part of the value). Rewrite as ``{organelle: (target_group, gt_channel, gt_suffix)}`` so the shape is unambiguous when the matrix is extended. 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> * docs(dynacell): final findings + 8-job FCMAE benchmark, open items Adds a "Final findings (2026-04-26) — bf16 fix shipped, 4-organelle benchmark in flight" section that: - States the solution in one paragraph (bf16 SSIM helper + redundant-decorator removal, with reference to the merged PR #412 squash commit 48f4878). - Records the live 4-organelle x {scratch, pretrained} = 8-job FCMAE matrix with median + p10 s/step, sourced directly from wandb loss/train_step step x timestamp deltas (dt < 60s filter excludes ckpt/val/epoch boundaries — directly measured per the no-walltime-estimates rule). - Calls out three claims we can support today: * pre-fix Hopper at 45-75 s/step -> post-fix at 4.77-5.76 s/step, 9-14x recovery * Hopper now competitive with L40S but not visibly faster * A40 (gpu-c-1) still wins at 2.40 s/step steady-state for reasons not yet measured (node-local I/O, /dev/shm topology, etc.); same shared-FS data path as Hopper, so the difference is on the compute-node side. - Lists what's still open: warmup-vs-steady-state on Hopper (current numbers are gstep < 1.7k vs A40 baseline at gstep 35k), an unprofiled A40-vs-Hopper compute-path gap, the 20h host-RAM leak (separate thread), and the A100 NCCL BROADCAST hang (mitigation in place via --constraint='h100|h200|a40|a6000|l40s' but root cause not investigated). Adds Recommendation #4 (the A100 exclude). Numbers explicitly noted as "will be re-pulled once new Hopper jobs reach gstep >=10k" so the follow-up is on the page, not a chat-only TODO. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(dynacell): clean up resolved planning docs; refresh A549 roadmap Remove three temporary planning docs whose work has shipped, and refresh the overall A549 expansion roadmap to reflect current progress, gaps, and next steps. Removed: - DATASET_REF_RESOLVER_SPEC.md — Stages 1-3 spec; all merged (38d47b3, 4bb9f09, 11836c8, 326b2d0, 6273439, 8924ab2, f5a6e56, a984384). - fcmae_hopper_slowdown.md — bf16 SSIM kernel fix shipped via PR #412 (squash 48f4878); 4-organelle FCMAE benchmark complete. - submit-reliability-plan.md — Gap 1 (--exclude SBATCH directive) and Gap 3 (NCCL preflight smoke test) merged into submit_benchmark_job.py + sbatch_template.sbatch + tools/nccl_smoke_test.py; Gap 2 (auto-requeue) explicitly deferred upstream of the doc. A549_EXPANSION_ROADMAP.md refresh: - Status snapshot table: Stages 1-3 done with commit refs; Stage 5 partial (canonical manifests in dynacell-paper @ aeef64c, VisCy fixture mirror still missing); Stage 6 not started; Stage 7 (joint training) in flight with first leaf shipped. - Reconcile commit-message "Stage 7" with the roadmap by promoting joint-training leaves to a first-class stage (was implicit under the old Stage 6 description). - Drop the now-stale DATASET_REF_RESOLVER_SPEC.md cross-reference. - Trim ~234 → ~130 lines: collapse done stages, keep open work. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(dynacell): rename ER+Mito FCMAE pretrained outputs to _ws8500 The original ER and Mito FCMAE pretrained leaves predated the warmup_steps=8500 model overlay (commit d296c7f, 2026-04-23) and were submitted with the engine default warmup_steps=3, while their scratch counterparts ran with warmup_steps=8500. To make the pretrained vs scratch pairs directly comparable on training protocol, rename outputs to a sibling _ws8500 directory so both the abandoned ws=3 baselines and the new ws=8500 runs coexist on disk. README.md files in the parent dirs document the dual-dir layout. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * chore(dynacell): pin compute-job CWD to repo_root in sbatch template Lets the dynacell launcher work from a git worktree: uv resolves project context (pyproject + .venv) from CWD, so without an explicit cd the compute-node srun would inherit whatever CWD sbatch was invoked from and may pick up the wrong worktree's environment. Pinning to @@repo_root (the worktree containing the launcher script) makes the env deterministic regardless of submit-side CWD. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * docs(dynacell): refresh A549 roadmap with 2026-04-24 status Stage 5 (a549 manifest): A549 zarr normalization-stats backfill closed 2026-04-24 in dynacell-paper (f4120e0 + 17-zarr backfill); no VisCy-side action remaining for that subtask. Stage 7 (joint training leaves): mark "in flight - blocked". PR #413 (0b04b24) closed one DDP deadlock surface but the 4-GPU smoke (234819a) still hangs at the same milestone; a second deadlock surface remains. Joint leaf expansion is paused until that resolves. Cross-reference the followup handoff and the _test48 debug-zarr convention for short-wall smoke runs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(dynacell): repoint mito A549 cross-eval to 2024_11_21 plate The 2024_10_29 plate the Stage 6 mito leaves were wired to is train-only by design — its splits author 0 test FOVs. Repoint the fixture mirror, predict_set fragments (model + Hydra side), the four mito predict/eval leaves, and the two test matrices to a549-mantis-2024_11_21, which has 11 authored test FOVs. This unblocks all 8 Stage 6 cross-eval cells; ER, mito, nucleus, and membrane × {celldiff, unetvit3d} now point at plates with real test splits on disk. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(dynacell): add fnet3d_paper Stage 6 A549 predict leaves Author 4 a549_mantis predict leaves for fnet3d_paper × {er, mito, nucleus, membrane}, mirroring the iPSC fnet predict leaves. Plates follow the same wiring used by celldiff/unetvit3d a549 leaves: ER/ sec61b → 2024_11_07, mito/tomm20 → 2024_11_21, nucleus and membrane → 2026_03_26 (with benchmark.dataset_ref.target overrides h2b and caax for the gene-keyed a549 targets). Each leaf reuses the same iPSC best-val checkpoint as predict__ipsc_confocal.yml — Stage 6 is cross-eval, not retrain. This rounds out the third and final model row for Stage 6's "full-but-predictable-only" sub-scope (celldiff + unetvit3d already landed in #415; fnet3d_paper completes the trio). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(dynacell): add fcmae_vscyto3d Stage 6 A549 predict scaffolding Add a shared `fcmae_vscyto3d_predict.yml` overlay (mirrors the model block from `fcmae_vscyto3d_fit.yml` so checkpoints load with the matching architecture, plus predict-time hparams) and 8 a549_mantis predict leaves: 4 organelles × {pretrained, scratch}. Plates follow the same wiring as the celldiff/unetvit3d/fnet3d_paper a549 leaves (ER/sec61b → 2024_11_07, mito/tomm20 → 2024_11_21, nucleus/h2b and membrane/caax → 2026_03_26 with `dataset_ref.target` overrides). `ckpt_path` is intentionally a `/TODO_FILL_BEFORE_SUBMIT/...` path on every leaf — iPSC FCMAE training is still in flight (jobs 31475094-31523064). Submitting any of these as-is will fail loudly with FileNotFoundError on torch.load. Each leaf header documents the expected post-training path so the swap is mechanical. Note: ER + Mito pretrained outputs use a `_ws8500` warmup-steps suffix (per commit 9af8bdf); nucleus + membrane pretrained do not. Suggested paths in the headers reflect that asymmetry. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(dynacell): add FNet3D ER joint single-GPU smoke leaf Author the FNet3D counterpart to the existing celldiff joint_ipsc_confocal_a549_mantis/train_smoke.yml. Pairs the iPSC SEC61B_test12 zarr (12 FOVs, 2.4 GB) with the 4-FOV a549_mantis SEC61B store so a single A40 / H200 can iterate the joint loader end-to-end without the full ~250 GB iPSC SEC61B mmap_preload wait. Verified end-to-end on a local A40: 10 fwd+bwd+optim steps in 1:19, 26 GB peak GPU mem, loss/train_step 0.680 → 1.187 epoch loss, three checkpoints written. Joint sharding on FNet3D's 35.3 M-param model with z=32 / yx=64 / bs=8 fp32 works at single-GPU. Note: leaf bakes `num_workers: 0` + `pin_memory: false` because the default `num_workers > 0` + `pin_memory: True` reliably hangs on forking the dataloader workers from a CUDA-initialized parent on this cluster's interactive nodes (same pattern as the 4-GPU joint smoke deadlock). Header documents the workaround so consumers tune up only when they need throughput. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * perf(cytoland): enable mmap+persistent+bf16 for A549 infected finetune Three flips to maximize throughput on the 4xH200 prod run now that PR #411 lifted the mmap_preload + exclude_fov_names restriction: - persistent_workers: true on the 3 sub-DMs avoids the worker re-spawn cost that dominated dynacell's per-epoch overhead. - mmap_preload: true caches each plate to local /tmp scratch (28 TB on H200 nodes), eliminating per-batch NFS reads. D3 now mmaps cleanly even with its 27 exclude_fov_names entries thanks to PR #411. - precision: bf16-mixed dodges the Hopper fp16 cuDNN slowdown documented in applications/dynacell/configs/examples/fcmae_hopper_slowdown.md. D2 smoke explicitly overrides both DataModule flags to false so single-GPU smokes on the full prod plate stay under their walltime; the bf16 flip is kept on the smoke to validate the AMP path. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * chore(cytoland): drop h200 constraint on A549 infected sbatch The 4xH200 slot was scheduling 5 days out. bf16-mixed (landed in ad1df84) avoids the Hopper fp16 slowdown that was the original reason to pin to H200, so any 4-GPU node (H100/A100/A6000) is fine and starts much sooner. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat(dynacell): bundle manifest registry as first-class package data Promote the test fixture manifest mirror to a real registry shipped with the dynacell wheel. The resolver auto-discovers it via the ``dynacell.manifest_roots`` entry point declared in pyproject.toml, so a fresh clone resolves ``benchmark.dataset_ref`` lookups out of the box — no ``DYNACELL_MANIFEST_ROOTS`` env var needed for predict/eval/test runs. Replaces the previous workaround where users had to either set the env var manually (multiple sessions hit ``ManifestNotFoundError`` because of this) or rely on the autouse pytest fixture that injected it for tests only. The env var stays as an explicit override path for testing discovery precedence. Splits/ subdirs were missing in the old fixture mirror but present in canonical; copy them alongside their manifests so consumers that resolve ``target.splits`` (currently latent in VisCy, used by dynacell-paper) work correctly. Drift between this bundled copy and dynacell-paper canonical is guarded by ``test_manifest_sync.py`` — parametrized over every shipped dataset, parses both YAML files and asserts canonical is a subset of VisCy (VisCy may carry additive fields like ``gt_cache_dir``). Skipped unless ``DYNACELL_PAPER_PATH`` is set, so CI behaves predictably while local dev catches drift early. This is the cheapest fix for the viscy <-> dynacell-paper logical cycle: single PR, no new package. Heavier options (extracting a ``dynacell-manifests`` leaf package + ``DYNACELL_DATA_ROOT`` for path portability) deferred per architecture discussion. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(cytoland): require >=80 GB VRAM for A549 infected sbatch Adds --constraint='h200|h100_80|a100_80' so the 4xGPU prod run is guaranteed at least 80 GB per device. The 3-DM CombinedDataModule in MAX_SIZE_CYCLE produces 16x4x3 = 192 crops/step/GPU — measured against a live dynacell FCMAE H200 reference (~45 GB at single-DM 128 crops/step), the projected per-GPU footprint is ~67 GB. That fits comfortably on H200, ~84% of an 80 GB H100/A100, and would OOM on a 40 GB A100 or 48 GB A40/A6000. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * docs(dynacell): note manifest registry drift policy in README Direct follow-up to 2395e1d (bundled manifest registry). Explains the two-source-of-truth split: VisCy ships the manifest content (``src/dynacell/_manifests/``, auto-discovered via the ``dynacell.manifest_roots`` entry point), and ``dynacell-paper`` remains the source of truth for manifest authoring. When a new plate lands in ``dynacell-paper``, mirror it back into VisCy; ``test_manifest_sync.py`` catches drift when ``DYNACELL_PAPER_PATH`` is set. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(dynacell): submit_benchmark_job.py supports optional --dependency, --parsable (#416) * feat(dynacell): submit_benchmark_job.py supports optional --dependency, --parsable Adds two opt-in flags to enable orchestration that needs to chain sbatch jobs by ID: - --dependency afterok:<job_id> appends --dependency=<value> to the sbatch invocation. Default off. - --parsable invokes sbatch with --parsable (which makes sbatch print just the numeric job ID instead of "Submitted batch job <id>"), captures that ID, and forwards it to stdout. Default off. Both flags default off. Manual submit_benchmark_job.py <leaf> invocations build the same ["sbatch", str(sbatch_path)] command as before, with stdout untouched — every existing workflow (smoke jobs, ad-hoc submissions, model-iteration scripts) sees no behavior change. Why: dynacell-paper's upcoming benchmarks orchestrator (Phase 5F) needs to capture train job IDs so it can submit predict with --dependency=afterok:<train_id>, chaining the train → predict pair via SLURM. Without these flags, the orchestrator would have to parse "Submitted batch job <id>" prose from a non-captured stdout — brittle. Tests: 5 new monkeypatched cases covering the default shape (backward compat) plus the three new flag combinations. No live sbatch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(dynacell): keep stderr attached + clarify --parsable wording Two Copilot findings on PR #416: 1. ``--parsable`` previously used ``capture_output=True``, which captures both stdout and stderr — sbatch warnings and diagnostics were silently swallowed on success. Switch to ``stdout=subprocess.PIPE`` so stderr stays attached to the parent and only the parsable line is captured. 2. ``--parsable`` help text claimed to "forward the parsed job ID" but the implementation forwards sbatch's parsable stdout verbatim, which can be ``job_id;cluster`` on multi-cluster setups. Update wording to describe the forwarding accurately. Test asserts the new kwarg shape (``stdout=PIPE``, no ``stderr``, no ``capture_output``). 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> * feat(dynacell): bundle 4 missing A549 manifests + predict_set fragments Adds bundled manifests for a549-mantis plates 2024_10_31, 2024_11_05, 2025_07_24, 2025_08_26 — the per-plate predict_set fragments below need their dataset_ref to resolve at compose time, so the manifests are mirrored into dynacell._manifests parity with canonical dynacell-paper configs (test_manifest_sync.py enforces). Predict_set fragments author one-line dataset_ref redirects per plate so existing per-plate predict leaves can switch plates without touching the data block. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat(dynacell): per-plate A549 predict leaves for ER + MITO across 5 models Today's predict__a549_mantis.yml only covered ONE plate (ER: 2024_11_07, MITO: 2024_11_21). The A549 SEC61B test budget spans 4 plates and TOMM20 spans 4 plates, so a single-plate predict leaf cannot cover the full test set. Plan-time gap: per-plate iteration was deferred under the assumption a single demo plate was enough, which conflicts with the data-design test-set spec. For each (organelle, model) in {er, mito} x {celldiff, fcmae_vscyto3d_*, fnet3d_paper, unetvit3d}, replace predict__a549_mantis.yml with one plate-scoped leaf per applicable test plate (ER: 2024_10_31, 2024_11_07, 2025_07_24, 2025_08_26; MITO: 2024_11_05, 2024_11_21, 2025_07_24, 2025_08_26). Each leaf composes its plate's predict_set and writes to a plate-suffixed output zarr so concurrent runs never clobber. 40 leaves total (4 models x 4 plates x 2 organelles + 5th model on each side: FNet ER already had 4 from the SEC61B-only refactor, MITO adds 4). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat(dynacell): submit_benchmark_batch.py — chain N predict leaves into one sbatch Path-1 batching for predict-only workflows: composes each leaf via load_composed_config, writes one resolved config per leaf with stable plate-scoped names under {run_root}/resolved/, and renders ONE sbatch script that loops `python -m dynacell predict --config <resolved>` over all of them. Amortizes queue submission and GPU allocation; total compute is unchanged from N per-leaf jobs. Validates that all leaves agree on launcher.mode (predict only), launcher.run_root, and the sbatch hardware profile (nodes/gpus/mem/etc). trainer.devices must equal sbatch.ntasks_per_node — same Lightning SLURMEnvironment invariant as submit_benchmark_job.py. predict_all_a549.sh wraps the new tool: globs every predict__a549_mantis_*.yml under <organelle>/<model>/ipsc_confocal/ and hands them to submit_benchmark_batch.py with a composite job_name. Used to re-run FNet ER + MITO predictions across all 4 plates (jobs 31558746 + 31558818). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat(dynacell): compute FID/KID at dataset level across all cells Pool all cell features from every FOV and timepoint before computing FID and KID, replacing the previous per-timepoint image-to-image comparison. Cosine similarity remains per-timepoint. The single dataset-level FID/KID value is broadcast to every row in the output CSV so the column structure is preserved. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(dynacell): add per-model eval scripts in model subfolders One script per CELL-Diff variant (denoise, sliding_window, iterative) in celldiff/, one for UNetViT3D in unetvit3d/, and one for FNet3D in fnet3d/. Each script covers all four organelles (ER, membrane, mitochondria, nucleus) following the run_eval_fnet.sh format. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(dynacell): use uv run consistently and fix pred path in eval scripts Add missing force_recompute.all=true and uv run prefix to run_eval_fnet.sh. Fix nucleus UNetViT3D pred path (nucleus_ -> nucl_) in run_eval_nucl.sh. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(dynacell): consolidate eval scripts into model subfolders Remove top-level per-organelle and per-model scripts; canonical scripts now live in celldiff/, unetvit3d/, and fnet3d/. Fix source path to use relative .envrc in all subfolder scripts. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore(manifests): point a549-mantis stores at .ozx Flip stores.{train,test}: paths from .zarr to .ozx across all 7 bundled a549-mantis plate manifests. Mirrors the same change in dynacell-paper (commit 6514090); test_manifest_sync.py asserts semantic equality with canonical. OZX is the new test corpus container — RFC-9 packed, 512x512 center-crop. 9 test stores exist on disk (all hosting plates). The 2026_03_26 train side has no OZX yet; both CAAX/H2B train: paths are placeholders that will land when train assembly runs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat(dynacell): add --override + --overwrite to submit_benchmark_batch.py The batch submitter previously had no way to inject per-leaf flags, so re-running a chained predict over plates whose output stores already contained the prediction channel required either rm-ing the stores or hand-editing every leaf. Add two CLI flags, both off by default: - --override KEY.PATH=VALUE (repeatable): same dotlist semantics as submit_benchmark_job.py, applied to each composed leaf via viscy_utils.compose.deep_merge. - --overwrite: dedicated alias that walks trainer.callbacks per leaf and sets init_args.overwrite=True on every entry whose class_path ends with HCSPredictionWriter. Sidesteps deep_merge's dict-key-only limitation (callbacks[0]-style overrides silently no-op). predict_all_a549.sh forwards extra args via "$@" already; doc comment updated to mention --overwrite as the canonical way to re-run a plate with existing predictions. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(dynacell): add --overwrite alias to submit_benchmark_job.py for parity Mirror the alias just added to submit_benchmark_batch.py: a dedicated --overwrite flag that walks trainer.callbacks and sets init_args.overwrite=True on every HCSPredictionWriter entry, off by default. The existing --override "trainer.callbacks[0].init_args.overwrite=true" recommendation was a latent no-op — viscy_utils.compose.deep_merge is dict-key-only and the [0] segment becomes a literal key, leaving the real callback untouched. The alias matches by class_path suffix and works against the actual list, so single-leaf submitters now have the same path as the batch submitter. Also clarifies the --override help text on the same caveat so callers don't reach for the broken syntax. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(dynacell): predict_local_a549.sh — local-GPU per-plate predict batcher Mirror of predict_all_a549.sh but for local execution rather than sbatch submission. Stages resolved YAMLs through submit_benchmark_job.py --dry-run (so the resolved/ tree gets the same provenanced timestamped filenames a sbatch run would), then invokes `dynacell predict` directly on each, optionally N at a time on the same GPU. Args: - <organelle> <model> — discover predict__a549_mantis_*.yml leaves under configs/.../<organelle>/<model>/ipsc_confocal/ - --overwrite — forwarded to submit_benchmark_job.py; required to re-run a leaf whose output store already contains the prediction channel - --parallel N (default 1) — run N predicts concurrently on one GPU, waiting between batches. Tune to fit VRAM (~2 fnet predicts fit on an A40) Logs land at $run_root/slurm/local_<TS>_<organelle>_<model>_<plate>.log. Uses `unbuffer` when available so TQDM progress bar streams to the log; falls back to plain `uv run` with a warning otherwise. SIGINT trap kills backgrounded predicts on early exit. Fail-fast: a failing plate aborts. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(dynacell): predict_local_a549.sh dying after first plate Under set -e, ((i++)) returns exit status 1 when i is 0 (the post- increment value 0 is treated as a failure by bash's arithmetic-eval exit-status rule). On the first loop iteration i goes 0 → 1, the ((i++)) expression evaluates to 0 → script exits → the EXIT trap fires kill 0 → both the script and the already-launched predict child are SIGTERM'd. Symptom: stage phase completes, run phase prints the first [start] line, then job control reports "terminated" with no logs and no zarr output. Replace with a pure-assignment increment (i=$((i + 1))) which has no exit-status side effect. Loop now actually iterates. Reproducer: bash -c 'set -e; i=0; ((i++)); echo survived' || echo killed # → killed Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(dynacell): predict_local_a549.sh - probe unbuffer + catch child failures Two issues exposed by the mito fnet3d run that completed in 30s with no output: 1. unbuffer detection was PATH-only. On nodes that ship the wrapper without the Tcl Expect package, unbuffer crashes immediately ("can't find package Expect"), all backgrounded predicts die in <1s. Add a runtime probe (`unbuffer true`) so a broken install falls through to the plain-uv runner. 2. set -e does NOT propagate background-job failures through `wait` (bash treats async failures as already-handled), so a batch where every child died still printed "[done] all leaves complete". Track each backgrounded predict's PID + plate + log path; after each batch, wait per-PID and record non-zero exits in FAILED_PLATES; on script end, print a [fail] summary and exit 1 if any plate failed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(dynacell): tomm20 fcmae scratch predict leaves pinned to best ckpt Add the 4 mito/fcmae_vscyto3d_scratch/ipsc_confocal predict leaves (3 a549_mantis + 1 ipsc_confocal) pinned to epoch=69-step=21840.ckpt (val 0.5527) from training run J31475715. Run was can…
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
Fixes three correctness bugs in the DynaCLR training data pipeline, discovered while debugging an OPS training run that plateaued with healthy loss but frozen dataset coverage.
Bug 1: Epoch counter never advanced
FlexibleBatchSampler.set_epoch()existed but Lightning never called it — PL's auto-wrap hook only targetssampler, notbatch_sampler. Every training epoch reseeded RNG withseed + 0, replaying the same ~0.5% of the dataset forever. Augmentation diversity masked it, but the model saw the same cells every epoch.Fix: auto-advance
self.epochat the start of__iter__, robust againstlimit_train_batchesearly termination.Bug 2: DDP rank/replicas never wired
Datamodule constructed
FlexibleBatchSamplerwithoutnum_replicas/rank, defaulting to(1, 0). On 4-GPU DDP, every rank iterated the full sequence and yielded identical batches — 4× redundant compute on the same data, no effective batch-size increase.Fix: add
_ddp_topology()helper that readstrainer.world_size/trainer.global_rankattrain_dataloader()call time and passes them to the sampler.Bug 3: Mixed channel-count batches crash with cryptic error
When
channels_per_sample=Noneand a batch mixes experiments with different channel counts,torch.stackin_slice_patchesfails with "tensors not equal size" deep in a dataloader thread. Not exercised by any production config (all usechannels_per_sample: 1), but traps debug probes.Fix: raise a clear
RuntimeErrorat the stack site pointing at the config fix.Verification
trainer.world_size=2→sampler.num_replicas=2, correct per-ranksampler.ranksampler.epochadvances 0→1 across real Lightning epochs withlimit_train_batches=3Test plan
packages/viscy-data/tests/test_sampler.py,applications/dynaclr/tests/test_datamodule.py::TestTrainDataloaderWiresDDPTopology,applications/dynaclr/tests/test_dataset.py::TestMixedChannelCountErrors)world_size/rankpropagation and epoch advance🤖 Generated with Claude Code