Skip to content

refactor: virtual-stain --init/--copy with Nextflow subworkflow - #259

Closed
aofei-liu wants to merge 5 commits into
refactor-reconstructfrom
refactor-virtual-stain
Closed

refactor: virtual-stain --init/--copy with Nextflow subworkflow#259
aofei-liu wants to merge 5 commits into
refactor-reconstructfrom
refactor-virtual-stain

Conversation

@aofei-liu

@aofei-liu aofei-liu commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add --init and --copy modes to biahub virtual-stain CLI, absorbing init-virtual-stain and copy-virtual-stain from nf.py
  • --init creates the output plate with <channel>_prediction channels from predict config, copies per-position OME metadata, emits RESOURCES: line
  • --copy copies viscy prediction data from a temp per-position zarr into the output plate, then removes the temp zarr
  • Predict config uses the shared -c option (replaces old --predict-config-filepath)
  • -i is now optional (not needed for --copy mode)
  • New nextflow/modules/virtual_stain.nf subworkflow: init → preprocess (viscy) → fan-out (predict + copy) × N positions
  • VisCy preprocess/predict remain as external commands called directly from Nextflow processes — only init and copy are biahub CLI commands

Chained PR — base is refactor-reconstruct (#258).

Test plan

  • Tier 1: 2/2 pytest tests pass (test_virtual_stain_cli_init_only, test_virtual_stain_cli_copy)
  • Tier 2: --init against real data — channels/shape/scale match reference exactly, emitted RESOURCES:10 120
  • No regressions: all 11 existing CLI tests (deskew, flat-field, reconstruct) still pass
  • Tier 3: Nextflow subworkflow end-to-end on GPU nodes — list_positions (1/1) → init (1/1) → preprocess (1/1) → run_virtual_stain (4/4) completed in 20m 56s, zero failures

Tier 3 output verification

  • 4 positions, channels=['nuclei_prediction', 'membrane_prediction'], shape=(5, 2, 86, 1600, 1370)
  • Scale matches reference: [1.0, 1.0, 0.17, 0.1133, 0.1133]
  • Pearson r vs reference ≈ 0.80/0.71 — expected divergence because the reference 3-virtual-stain was produced from a reconstruct output with a duplicate-channel bug (see below)

Data fixes (outside this PR)

  • Reference 2-reconstruct: had duplicate identical channels (Phase3D_recon_recon, Phase3D_recon). Fixed to single Phase3D channel. Old store backed up.
  • predict.yml: ckpt_path pointed to a stale path; fixed to include the 4gpu_bf16_bs16_to_ep7/ subdirectory.

🤖 Generated with Claude Code

aofei-liu and others added 5 commits June 8, 2026 13:53
…workflow

Add --init and --copy modes to the unified `biahub virtual-stain` CLI:

- --init: creates output plate with prediction channels from config,
  copies per-position OME metadata, emits RESOURCES: line
- --copy: copies viscy prediction from temp per-position zarr into
  the output plate, then cleans up the temp zarr

The predict config (-c) is now shared across all modes (replaces the
old --predict-config-filepath). The -i option is optional (not needed
for --copy mode).

Nextflow module (virtual_stain.nf) wires the subworkflow:
  init → preprocess (viscy) → fan-out (predict + copy) × N positions

Viscy preprocess/predict remain as external commands called directly
from Nextflow processes — only init and copy are biahub CLI commands.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…irtual_stain

Derive position_keys from input_position_dirpaths and read shape/scale
from the first position directly, matching the pattern used by deskew,
flat-field, reconstruct, and track.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
virtual_stain_wf now receives input_zarr, output_zarr, config as
explicit val parameters. Temp zarr and log paths derived from
output_zarr's parent directory. viscy_cmd() stays as-is.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@ieivanov

Copy link
Copy Markdown
Collaborator

Superseded by #271, which recreates this integration against the current in-process cytoland virtual-stain CLI (post-#267). This PR's viscy predict + --copy temp-zarr flow no longer matches the CLI. Closing in favor of #271.

@ieivanov ieivanov closed this Jun 16, 2026
ieivanov added a commit that referenced this pull request Jun 23, 2026
* feat(nextflow): integrate virtual-stain into mantis-v2 pipeline

Recreate the Nextflow virtual-stain integration against the current
in-process cytoland CLI (post-#267), replacing the #259 approach which
was built on the old `viscy predict` + `--copy` temp-zarr flow.

- New `nextflow/modules/virtual_stain.nf`: init → preprocess → fan-out
  per-position GPU prediction. Per-position work is a single
  `biahub virtual-stain --cluster debug` call (no temp zarr, no --copy).
- `viscy preprocess` runs over the whole input plate to compute the
  normalization statistics the model reads via read_norm_meta.
- Both `biahub virtual-stain` and `viscy` run under biahub's `stain`
  extra (cytoland → viscy-utils provides the `viscy` console script).
- Add a `gpu` process label (queue=gpu) to nextflow.config.
- Wire virtual_stain_wf after reconstruct in mantis-v2.nf.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix biahub virtual-stain mem request

* refactor: unify per-position resource requests across step CLIs

Every step CLI (deskew, flat-field, apply-inv-tf, virtual-stain) now emits
its per-position resource request through a single shared helper,
biahub.cli.utils.echo_resources, as a JSON payload:

    RESOURCES:{"cpus": 16, "mem_gb": 64, "time_min": 480}

The same (cpus, total mem_gb, time_min) values feed both the CLI's own
slurm_* submission args and, via parse_resources in nextflow, the Nextflow
per-position task directives — so the SLURM fan-out and the Nextflow fan-out
can no longer request different resources.

- Add echo_resources() and parse the JSON in common.nf::parse_resources,
  which now returns time_min alongside cpus/mem_gb.
- Emit memory as a TOTAL (mem_gb) and request it via slurm_mem rather than
  slurm_mem_per_cpu, fixing virtual-stain's per-cpu over-request.
- Wire each CLI's slurm_time from time_min; drop the hardcoded `time`
  literals in the run_* processes for `meta.time_min * task.attempt`.
- virtual-stain computes time_min before the init_only return so --init
  emits it.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* payload bugfix

* debug viscy predict

* refactor: rename time_min to time_minutes for clarity

Avoids confusion with "minimum time"; renames the resource payload key
and Nextflow meta field consistently across both languages.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* update iohub dep

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants