Repository navigation
Conversation
…f by default (#33) * Expose resumable polygon extraction in CELLMEASUREMENT * Default neighbors to 0 * Split resource profiles into public and WEHI heavy-compute sets --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: mcmero <7799828+mcmero@users.noreply.github.com>
* cellmeasurement fix: channel detection for OPAL OME-TIFFs --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: mcmero <7799828+mcmero@users.noreply.github.com>
KRONOSEMBEDDINGS was called three times, once inside each of mesmer_segment, sopa_segment and cellsam_segment, with the _wbacksub variants forwarding its emits. That placement makes any cohort-wide channel operator unsound: a collect() inside mesmer_segment only ever observes the Mesmer samples, so a statistic computed across the cohort would silently differ depending on which segmenter produced a sample. Each segmentation subworkflow now emits kronos_input ([meta, tiff, whole_cell_mask]) and the top-level workflow mixes all six and invokes KRONOSEMBEDDINGS once. KRONOS is segmenter-agnostic, so this also removes the same ~30-line block duplicated three times. Mixing across subworkflows exposed a latent pairing hazard: the module takes the image/mask and the GeoJSON as two separate channels, which Nextflow consumes positionally. Both inputs are now derived from a single channel joined on meta, so a sample's mask can no longer be paired with another sample's annotations. Pipeline outputs are unchanged. SEGMENTATIONREPORT now receives the CELLMEASUREMENT GeoJSON rather than the KRONOS-merged one, which does not affect the report: its template selects measurement columns by anchored regex (^(Cell|Nucleus): (Area|Length|...) and : (Mean|Median|Min|Max| Std.Dev)$), and kronos_emb_* matches neither, with no broad column sweep anywhere in the template. Subworkflow emit names changed (kronos_embeddings/kronos_marker_report -> kronos_input), so nf-test snapshots need regenerating. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
All 7 tests across the 6 changed subworkflows were executed and PASSED; every snapshot here comes from a successful run. Two independent changes appear in this diff: 1. Caused by this PR: kronos_embeddings/kronos_marker_report are replaced by kronos_input, which carries [meta, tiff, whole_cell_mask] rather than the previously-empty channels (KRONOS is disabled by default, so the old emits always snapshotted as []). 2. NOT caused by this PR: the test.geojson.gz and versions.yml md5s change. This is pre-existing drift, reproduced on unmodified dev (ef47666) in the same environment, which fails against its own committed snapshot with the identical new md5 (f3d8c2ee -> cf3e776b). The cause is that the cellpose model weights are not pinned: the container is pinned to sopa:2.1.11-cellpose, but the 1.2GB cpsam checkpoint is downloaded from cellpose.org at runtime, so segmentation output drifts as that file changes upstream. Note cellsam_segment.snap retains an obsolete entry ("cellsam_segment - with background subtraction") from a test that no longer exists; it predates this branch and is left untouched, though it is now the last reference to the removed emit names. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Swaps the KRONOS1 extractor for KRONOS2 (768-d, marker-aware) and fixes
three normalisation defects carried by the v1 script. Outputs from the two
versions are NOT comparable: both the model and the patch content change.
Behaviour changes
-----------------
* 384-d -> 768-d embeddings.
* Cell isolation: pixels outside the target cell's polygon are zeroed, so an
embedding describes that cell rather than the ~18 um neighbourhood a 64 px
box spans at 0.28 um/px. Verified empirically -- isolated vs raw-box
embeddings differ (L2 mean 9.4, cosine mean 0.973), so this is material.
* The intensity divisor is derived from the image dtype (uint8=255,
uint16=65535, float=400) rather than hardcoded to 65535, matching KRONOS2's
own sp_image._scaling_factor. A float32 backsub output was previously
divided by 65535 instead of 400.
* The samplesheet's nuclear_channel is passed as preferred_dapi. This is
KRONOS2's ONLY marker-alias mechanism, so DRAQ5/Hoechst slides previously
received default normalisation stats.
* kronos_batch_size defaults to 16 (was 32): KRONOS2 is fp32 and cuBLAS picks
batch-dependent kernels, so 16 reproduces the published values.
* The per-cell CSV is gone. It held the same vectors already merged into the
GeoJSON, which is now the single embedding artefact.
Marker names are now our responsibility
---------------------------------------
KRONOS2 matches names exactly on a separator-insensitive key with NO alias or
fuzzy step (its marker_utils docstring calls this out as deliberate, to avoid
the "CD24 -> cd4" class of mis-normalisation). An unmatched marker is not
dropped -- it is silently normalised with default stats. Against the real
288-marker vocabulary, 45 of a 49-marker CODEX panel matched; the 4 that did
not ("Collagen 4", "Cytokeritin", "DAPI-01", "VISA") are all naming variants
of markers that ARE in the vocabulary.
So kronos_marker_mapping is retained rather than removed, unmatched markers
fail the run by default, and the error points at mapping first, since mapping
onto a vocabulary name preserves the pretrained stats and text embedding
whereas a novel-marker registration would not.
Structure
---------
bin/kronos2_common.py holds everything that is not the model -- channel names,
marker resolution, cell geometry, lazy patch reads, GeoJSON output -- so a
future encoder differs only in preprocess/forward instead of becoming a second
thousand-line script. Two efficiency changes come with it: centroids are
computed analytically from the polygons (v1 rasterised, and wrote to disk, a
full-slide uint32 mask just to locate cells), and pixels are read through a
lazy zarr view so memory scales with the batch rather than the slide.
Also fixed
----------
* The SOPA_PATCHIFYIMAGE stub wrote an empty patch-count file that the caller
parses with .toInteger(), so every stub run of the Cellpose path died with
`For input string: ""`.
* KRONOSEMBEDDINGS passed '--nv' unconditionally, which is invalid under
Docker; the replacement selects by container engine as CELLSAMSEGMENT does.
Validation
----------
Run against the real gated weights on a GPU: 119 cells -> (119, 768), no
NaN/inf, no all-zero rows, no near-identical pairs, and the 210 existing
cellmeasurement measurements preserved.
The container reference is a PLACEHOLDER: the cu124 torch/xformers stack has
no suitable public image and needs a Seqera Wave build, as KRONOS1's did.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Committed as 100644 while every other script in bin/ is 100755. Nextflow puts bin/ on PATH and invokes the script directly, so the task failed with exit 126 (found, not executable) rather than anything diagnosable from its output. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The kronos_input emit added in 79c9b5c re-exports the input image. On the backsub paths that image is produced by the backsub container, which stamps a fresh UUID into the OME-XML on every run: UUID="urn:uuid:f1aad240-8a5a-11f1-b846-7ee323878ed5" Creator="backsub v0.4.1" Its md5 therefore changes every execution, so the three *_wbacksub snapshots committed in 4a761f1 can never pass a second time. They appeared to pass because they were generated and compared within a single --update-snapshot run, which always agrees with itself; the defect only surfaces on an independent verification run. kronos_input is plumbing for the top-level KRONOS2 call and carries no value the other emits do not already cover, so it is excluded from the snapshot assertions rather than the emit being reshaped. Each exclusion carries a comment explaining why, so it does not read as a muted failure. Snapshots still need regenerating against these assertions, and then verifying with a second pass that does NOT use --update-snapshot. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The plan records the design decisions and the evidence behind them, read from the model source rather than assumed -- notably that KRONOS2 matches marker names exactly with no alias or fuzzy step, which is why kronos_marker_mapping is retained rather than removed. The examples are referenced from docs/usage.md and are validated against the real 288-marker vocabulary: every mapping target is in vocabulary, and the additional_markers.csv rows use legal compartment/family categories and name markers that genuinely are absent. .gitignore blocks the KRONOS2 marker vocabulary and anything derived from it. The model is CC-BY-NC-ND-4.0, so the table is generated locally from the user's own model directory instead of being redistributed from this public repo -- which also removes any question of the committed copy drifting from the model in use. The handoff is a working document; delete it before merging. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Container
---------
Built from the module's environment.yml with the Wave CLI and frozen to the
community registry, so the digest is reproducible:
java -jar wave.jar --conda-file modules/local/kronos2embeddings/environment.yml \
--platform linux/amd64 --freeze --await 30m
Wave's anonymous tier publishes to community.wave.seqera.io -- the same
registry the KRONOS1 image used -- so no Seqera account was needed. Verified to
carry torch 2.6.0+cu124 (not a CPU fallback), a working CUDA runtime, and to
reproduce a standalone run's embeddings to cosine 1.000000.
The reference must stay fully qualified: docker.registry='quay.io' in
nextflow.config silently rewrites anything that is not, producing a confusing
401 from quay.
rasterio -> Pillow
------------------
The first Wave build (14ecc418) imported fine standalone but failed under
Nextflow with:
ImportError: libtiff.so.6: undefined symbol: jpeg12_write_raw_data
a conda libtiff/libjpeg mismatch reached through rasterio. Rather than pin
around it, rasterio and its GDAL stack are dropped: they were pulled in to fill
one small polygon per patch, which Pillow's C polygon filler does natively.
This is the same choice CORAL makes, for the same reason. Image size drops from
10.1GB to 9.78GB.
The two rasterisers do not agree exactly -- boundary-pixel conventions differ,
and at 64px cell patches the boundary is a meaningful fraction of area. The
effect was measured rather than assumed:
* Pillow vs rasterio embeddings: cosine 0.99974 (min 0.99133)
* for scale, cell isolation vs raw box: cosine 0.97267
* rasterio's own all_touched=True vs False: IoU 0.858, i.e. the two
rasterisers agree MORE closely with each other than rasterio does with
itself across its two modes
So the shift is a rasterisation-convention detail, roughly 15x smaller than the
isolation decision, not a change of the same class.
Docs
----
Records a trap that costs real time: staging the weights under /opt breaks the
container. Nextflow bind-mounts an input's parent to make it visible, so a host
/opt shadows the container's /opt/conda and Python disappears -- reported as
"/usr/bin/env: 'python3': No such file or directory" from an image that plainly
has python3.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Fixes the snapshot fix in 5040fe3, which was incomplete. workflow.out carries every channel TWICE -- once positionally ("0", "1", ...) and once by name -- so filtering on `entry.key != 'kronos_input'` removed the named copy while the positional duplicate kept the unreproducible value. The assertions now keep only the named emits, which drops the positional duplicates entirely (they were pure redundancy) along with kronos_input. Verified properly this time: each snapshot was regenerated and then checked by a SECOND pass WITHOUT --update-snapshot. That distinction is the whole point -- --update-snapshot compares against what it just wrote, so it always agrees with itself, which is how the original defect survived. mesmer_segment PASS mesmer_segment_wbacksub PASS sopa_segment PASS sopa_segment_wbacksub PASS cellsam_segment_wbacksub PASS cellsam_segment 1 of 2 tests PASS (see below) Known-flaky, NOT caused by this PR ---------------------------------- `cellsam_segment - tiff` still fails an independent verify. The only differing values are test_nuclear.tiff and the GeoJSON derived from it; the whole-cell mask is byte-identical. CellSAM's nuclear segmentation is therefore non-deterministic between runs. Nothing in this PR touches segmentation. Note these snapshots were generated with a GPU, while CI has none, so CellSAM runs on CPU there and may well be deterministic. `BACKGROUNDSUBTRACT`'s own snapshot has the same class of problem and predates this branch entirely: it stores the md5 of the backsub OME-TIFF, which carries a fresh UUID per run (Creator="backsub"), so it can never pass twice. Untouched here; worth its own issue. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both linting failures on this PR were self-inflicted. prettier: the docs and JSON added here were written directly rather than through the formatter -- nextflow_schema.json in particular went through a json.dump(indent=4), which does not match prettier's style. Reformatted with prettier 3.2.5, the version pinned in .pre-commit-config.yaml. rocrate_readme_sync: this PR rewrote the README's KRONOS section, so the descriptions embedded in ro-crate-metadata.json no longer matched. Regenerated with `nf-core pipelines lint --fix rocrate_readme_sync` rather than hand-edited, using nf-core 3.3.2 (the version pinned in .nf-core.yml, which is what CI installs) and without --release, since CI only passes that when the base branch is master. ro-crate-metadata.json sits under both tools' jurisdiction and is not in .prettierignore, so it was checked for a formatting fight: prettier alters one line of what nf-core writes, and nf-core still reports 315 tests passed afterwards. They converge, so no ignore rule is needed. Verified locally: `nf-core pipelines lint` -> 315 passed, 1 module warning (smoothmasks meta.yml, pre-existing and unrelated). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
It was built and verified during the same session; leaving it in "blocked on you" would send the next reader chasing work that is already done. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
refactor: invoke KRONOS once at top level instead of per segmenter Swap kronos1 with kronos2 and validate there was improvement over existing implementation. Built new wave container for kronos2
Cellpose does not error on a --pretrained-model path that is missing: it falls back to its built-in weights. A typo therefore produced a complete, plausible, silently wrong run rather than a failure. Check cellpose_pretrained_model and cellpose_models_dir in PIPELINE_INITIALISATION, before any compute is scheduled. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every patch task downloaded the ~1.2 GB cpsam checkpoint from cellpose.org itself -- about 18 identical requests for a single sample. That produced HTTP 429 rate limiting in CI and a truncated download locally (PytorchStreamReader failed reading zip archive), and left no guarantee that the patches of one run were segmented with the same weights. A new CELLPOSEMODEL process fetches them once and emits a directory that SOPA_SEGMENTATIONCELLPOSE takes as a staged input, so -resume also reuses them across runs. It is invoked at the top level rather than in the Cellpose subworkflows for the same reason KRONOS was hoisted: SOPA_SEGMENT is instantiated twice (with and without background subtraction) and instantiates SOPA_SEGMENT_COMPARTMENT twice again (nuclear and whole-cell), so a call sited any lower would fetch up to four times. Its trigger is the Cellpose samplesheet reduced to a constant, so Mesmer- and CellSAM-only runs pay nothing and the resume key does not depend on sample ordering. The consumer links the staged contents into a task-local directory rather than pointing CELLPOSE_LOCAL_MODELS_PATH at the shared stage directly: a download -- of a missing or newly requested model -- would otherwise be a concurrent write from every patch task into one directory. Adds --cellpose_models_dir for sites with a pre-staged copy, which skips the download entirely and is what you want where compute nodes have no outbound network. This pins where the weights come from, not which weights: cellpose.org still serves whatever is current. Only --cellpose_models_dir makes them reproducible over time. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The upstream backsub container stamps each output OME-TIFF with a fresh UUID (Creator="backsub"), so its md5 differs on every run and the backgroundsubtract snapshot could never pass twice. The container is not ours to change, so drop the file from the snapshot and assert it is produced by name instead -- the same treatment kronos_input already gets. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
SOPA_CONVERT gained a second input, val(compartment), in b015b6f, but the setup block of sopa_patchifyimage - zarr was never updated. The test has been failing since with: Process `SOPA_CONVERT` declares 2 input channels but 1 were specified Nothing caught it because the snapshot suite has never run in CI. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…OLVED The snapshot suite has never run in CI, so it had rotted. This records what was repaired and, for what was not, exactly what is known and what is not. Explicitly unresolved: 1. Whether CellSAM is deterministic on CPU. The first attempt at this was invalid and is recorded as such: conf/base.config sets accelerator = 1 for withLabel:process_gpu, which Nextflow's docker executor expands into `--gpus all`, so `-profile test,docker` runs on the GPU even though no `--gpus` appears in any config. Those runs showed cellsam_segment - tiff byte-identical across two passes, but on GPU, which says nothing about a GPU-less CI runner. The CPU re-run was stopped before completing rather than left running overnight. 2. Why segmentationreport's HTML md5 drifts. A Quarto render timestamp is the obvious candidate but is unverified, so it is written down as a hypothesis rather than a cause. 3. Why the module-level cellsamsegment mask md5s drift. Deliberately not regenerated: doing so before (1) is settled risks committing a GPU-produced mask that CI could never reproduce. Also records that CellSAM fetches its 1.70 GB weights per task from DeepCell (41m38s measured; one task hit the 1h limit at 78%). That is long-standing behaviour rather than a regression, but it has almost no headroom and is a poor fit for CI. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Eight tests across five files, none of which had passed in this environment. All were pre-existing: unmodified dev fails them identically. Two distinct causes. versions.yml md5s (sopa_convert x2, sopa_resolvecellpose, and sopa_segmentationcellpose) were recorded in an environment that no longer matches. The committed value had flip-flopped across commits -- e1cb9bf9 -> 136a272d -> 451efc07 -> 136a272d -- and 451efc07 is the md5 of a versions.yml where the sopa version rendered empty, so at least one past regeneration ran somewhere `sopa --version` produced nothing. They are stable within an environment (three consecutive runs agree), so this is staleness, not flakiness. The sopa_segment_compartment snapshots recorded one patch. b015b6f created them under the 3000 px default; 18c4b81 then set patch_width_pixel = 250 in the test profile, which yields nine. patches_file_image moved from the md5 of "1" to the md5 of "9", and the parquets, shapes and masks all moved with it. sopa_patchifyimage - zarr - stub was likewise stale since PR #35 fixed its empty patch-count file. Also drops two obsolete entries from sopa_segmentationcellpose, left behind by tests that no longer exist. Every snapshot here was regenerated in one run and then reproduced by a separate run WITHOUT --update-snapshot. That second pass is the only thing that proves reproducibility -- --update-snapshot compares against what it just wrote, so it always agrees with itself, which is how the backsub-UUID defect survived the first time. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The existing CI runs only `nextflow run`, which proves the pipeline completes but not that it reproduces its outputs -- so the snapshot suite has never executed automatically, which is exactly why it had rotted. This adds the workflow, but wired to workflow_dispatch ONLY. Three tests are still known-red for reasons nobody has diagnosed (segmentationreport, and the two module-level cellsamsegment tests -- see docs/nf-test-status.md), and gating merges on those would make the check noise rather than signal. The header records what to add back to enable it. It caches the cellpose weights keyed on the sopa container tag and feeds them in via --cellpose_models_dir, so a run does not re-fetch 1.2 GB. That is plumbed through a CELLPOSE_MODELS_DIR env hook in tests/nextflow.config, which is useful on its own: export it locally and the sopa tests stop downloading the weights per run. Note the workflow does not yet resolve the GPU question. process_gpu tasks request an accelerator via conf/base.config, which Nextflow's docker executor turns into `--gpus all`, and ubuntu-latest has no GPU. That needs establishing before any test touching CELLSAMSEGMENT or MESMERSEGMENT runs in CI. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
MESMERSEGMENT was named as requesting a GPU. It does not -- it is process_multi. The only two processes carrying process_gpu are CELLSAMSEGMENT and KRONOS2EMBEDDINGS. Also reframes the point. `accelerator = 1` becoming `--gpus all` is intended behaviour, not a defect: CellSAM and KRONOS2 are meant to run on the GPU and do. It is relevant to CI only because ubuntu-latest has no GPU, and the current CI never reaches those processes anyway -- conf/test.config points at cellpose_samplesheet.csv. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The status doc still described them as pending phase-2 verification. That passed and they were committed in 41ad892. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
nf-core lint failed on it with two actions_nf_test errors: the workflow was wired to workflow_dispatch only, and that check requires the standard push / pull_request / release triggers. Adding it was a mistake for a more basic reason. .nf-core.yml already lists .github/workflows/nf-test.yml under lint.files_exist -- the project had already declared that file intentionally absent. There is no way to add it in a deliberately-inert state and keep lint green, and with three tests still known-red, wiring it live would gate merges on undiagnosed failures. The boilerplate is regenerable. What is not obvious is the weights-cache step, so that is preserved verbatim in docs/nf-test-status.md, along with why the workflow is absent. tests/nextflow.config keeps its CELLPOSE_MODELS_DIR hook, which is useful on its own and trips no lint rule. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Stage the Cellpose weights once per run, fail fast on bad model paths, and repair the nf-test suite
Moves to a Wave-built container carrying sopa 2.2.6 and cellpose 4.2.1.1,
which is the first release with cpsam_v2, cpdino and cpdino-vitb. The
previously pinned sopa:2.1.11-cellpose shipped cellpose 4.0.8, whose only
built-in model was cpsam, so there was nothing to select.
community.wave.seqera.io/library/python_pip_sopacellpose_cellpose:06f04d6833aa089b
sopa 2.2.6 | cellpose 4.2.1.1 | torch 2.13.0 (cuda 13.0) | zarr 3.2.1
MODEL_NAMES: ['cpsam_v2', 'cpdino', 'cpdino-vitb', 'cpsam']
Verified in the image: CUDA is available and the four sopa CLI commands the
pipeline calls all still exist. The `sopa segmentation cellpose` option
surface is byte-identical between 2.1.11 and 2.2.6, so no call sites changed.
cellpose_pretrained_model becomes the model selector and defaults to
cpsam_v2. It takes a built-in name or a path to a custom model; sopa forwards
the value straight to CellposeModel(pretrained_model=...), which accepts
either. The schema lists all four names.
cellpose_model_type is retired and now errors instead of being ignored.
Cellpose >=4.0.1 discards --model-type, and sopa only reads it when
pretrained_model is unset -- which never happens on v4 because sopa defaults
it to 'cpsam'. The old 'cyto3' default has therefore been producing cpsam
results on every run. Failing loudly beats repeating that.
Two follow-ons from the previous PR that would otherwise have broken:
- The startup path check would have rejected a model *name*. It now
validates as a path only when the value is not a known built-in.
- CELLPOSEMODEL fetched cellpose's default rather than the model in use.
Under 4.2 those differ (cpsam_v2 vs sopa's cpsam), so it would have staged
a checkpoint segmentation never asks for and silently left every patch task
downloading its own. It now takes the model name.
Also enables the GPU: sopa warns that cellpose >=4 "can be slow without a
GPU", and the pipeline never passed --gpu, so segmentation has been running
on CPU throughout. SOPA_SEGMENTATIONCELLPOSE moves to process_gpu.
Snapshots are NOT regenerated here -- results are expected to move, and that
needs measuring rather than blessing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
cpdino and cpdino-vitb are in cellpose 4.2.1.1's MODEL_NAMES, so they passed
validation -- and then failed deep in CELLPOSEMODEL:
File "cellpose/vit.py", line 171, in __init__
vit_model = dinov3_vitl16 if self.model_name == 'vitl' else dinov3_vitb16
NameError: name 'dinov3_vitl16' is not defined
They need Meta's `dinov3` package, which cellpose imports in a bare
try/except and only warns about:
try:
from dinov3.hub.backbones import dinov3_vitl16, dinov3_vitb16
except:
vit_logger.warning("Could not import CPDINO, run `pip install
git+https://github.com/facebookresearch/dinov3` ...")
So the failure arrives at model construction, after the 1.13 GB checkpoint
has already been fetched. That is the same silently-deferred failure mode this
pipeline has been removing elsewhere, so reject it in
PIPELINE_INITIALISATION with a message naming the missing package.
dinov3 is not on PyPI, installs from git, and carries a bespoke Meta licence
with gated access to the backbone weights, so adding it to the container is a
decision rather than a detail. The guard is a single list -- delete it if the
container ever carries dinov3.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Rebuilds the Wave container with Meta's dinov3, which cellpose needs for cpdino and cpdino-vitb, and removes the startup guard added in 64960f2. community.wave.seqera.io/library/python_pip_sopacellpose_cellpose:2bb51160896b005b sopa 2.2.6 | cellpose 4.2.1.1 | torch 2.13.0 (cuda 13.0) | dinov3 Verified in the image: dinov3.hub.backbones imports, CPDINO constructs (backbone dino_vitl, 303.4M params) and evaluates, and the four sopa CLI commands still work. Only dinov3's architecture code is used. cellpose builds the backbone with `pretrained=False` and then loads its own checkpoint; Meta's gated `dinov3_*_pretrain_lvd1689m` weights are only touched by CPDINO.load_pretrained, which inference never calls. So no Meta access request is needed. dinov3 is pinned to commit 6876159a. It has no releases and no tags, so tracking `main` would have reintroduced exactly the unpinned-dependency problem this pipeline has been removing. It is also not on PyPI and the image has no git, so it installs from the commit tarball rather than git+https. CITATIONS.md records the licence obligation: the DINOv3 License requires that a copy of the Agreement accompany any redistribution, which now includes this container, and that publications acknowledge the materials. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Documents the model table with the measured differences rather than just naming the options, so the choice can be made on evidence: holding the model at cpsam across the old and new stacks gives IoU 0.9987, cpsam_v2 gives 0.9552, and cpdino gives 0.9251 while finding 123 cells instead of 119. The first of those is the load-bearing number -- it says the version, sopa and device changes are effectively inert, so any movement is attributable to the model rather than the upgrade. Also records that cellpose_model_type is gone and why, the GPU requirement, and that populating a shared cache now needs the model named explicitly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…and gaussian Cellpose normalises by rescaling to the 1st and 99th percentiles and does nothing else -- sharpen_radius and smooth_radius both default to 0, and it contains no CLAHE code anywhere. Cellpose-SAM was trained on exactly that normalisation, and its augmentations covered global brightness/contrast jitter plus blur and noise, but no spatially varying local remapping of the kind CLAHE applies. sopa layers two stages on top, applied per patch before cellpose sees anything: gaussian_filter(sigma=1) and equalize_adapthist(clip_limit=0.2). Both are now off. Exposes cellpose_clip_limit (0), cellpose_gaussian_sigma (0), cellpose_clahe_kernel_size (null) and cellpose_tile_norm_blocksize (0, cellpose's own default) in a new "Cellpose preprocessing options" schema group. Restore the old behaviour with --cellpose_clip_limit 0.2 --cellpose_gaussian_sigma 1. Both stages were computed from the patch, and CLAHE's kernel is patch.shape // 8, so the contrast-enhancement scale tracked patch_width_pixel: at COMET's 0.28 um/px, 52 um at 1500 px, 105 um at 3000 and 210 um at 6000 for identical tissue. Patch overlap does not help -- it resolves cells straddling a boundary, not cells normalised differently. Measured on a real COMET image by segmenting the same region as part of a 1500 px vs a 3000 px patch and matching cells by IoU, the fraction of cell boundaries surviving a patch-size change (IoU >= 0.90): cpsam_v2 (default) 38.6% -> 81.9% cpsam 43.2% -> 83.9% cpdino 36.8% -> 86.6% Mean IoU 0.86 -> 0.93. Three architectures, same direction, similar magnitude. This changes results and they should not be pooled with earlier runs. Median cell area falls 23-43% for every model, and counts move +8-35% (cpsam_v2), -3-10% (cpsam), +28-54% (cpdino) -- CLAHE was suppressing detections in the newer models and inflating them in cpsam. Per-cell intensities are computed over these masks. cellpose_diameter needs no adjustment: Cellpose-SAM trained across 0.25x-4x scale, so the smaller cells sit well inside its range. Verified end to end on the test profile (30 tasks, nine-patch path), with the flags confirmed reaching model.eval in the pinned sopa 2.2.6. Run on a COMET brain-panel ROI with all three models and inspected in QuPath -- sensible and closely comparable (1742 / 1751 / 1648 cells), cellSAM drawing larger boundaries around membrane signal. Config only; no sopa or cellpose code changes. The single invocation site is unaliased, so withName: 'SOPA_SEGMENTATIONCELLPOSE' covers both compartments. These numbers measure reproducibility, not accuracy -- no annotations were scored, and the normalisation measurements are DAPI-only, so the whole-cell compartment is unmeasured. cellSAM also CLAHEs each block and performs well, but it was trained with CLAHE applied and pins its kernel at a fixed 128 px; cellpose's weights were not. Applying it to a model that never saw it is the problem, not CLAHE itself. Follow-up: nf-test snapshots for the cellpose path need regenerating. CI does not run nf-test, so this is repo hygiene rather than a merge gate. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Match Cellpose's preprocessing contract: disable sopa's CLAHE and gaussian
A CI run failed with
Failed to pull singularity image
status : 143
hint : Try and increase singularity.pullTimeout (current is "20m")
on community.wave.seqera.io/library/tifffile_pandas_typer -- a 230 MB image
that sat in "Copying blob" for 33 minutes before Nextflow SIGTERMed it. The
3.5 GB Cellpose image pulled concurrently and finished in 4.5. Nothing about
the failing PR is involved; it is a registry stall, and CI re-downloads ~6 GB
on every run, so it gets many chances to hit one.
Cache the converted SIFs in the workspace between runs, keyed on the module
files where the container URIs are declared. A container change invalidates
the key; restore-keys means an unrelated module edit still restores the
previous images and only the changed one is pulled. GitHub evicts past 10 GB
per repo, so a miss is today's behaviour rather than a failure. Restored after
the disk cleanup step, which the several GB of SIFs need.
Also raise pullTimeout to 1h on both the singularity and apptainer profiles.
Nextflow's 20m default is sized for the small images most nf-core pipelines
use; ours are 3.5 GB and 5.8 GB compressed with SIF conversion on top, so a
pull making steady progress can legitimately exceed it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ci: stop re-pulling every container image on every run
Every one of the four tuning parameters was described by restating its own name -- "Flow threshold for cellpose", "Cell probability threshold for cellpose" -- which tells nobody which way to turn them. Replace them with the direction of effect and when to reach for each, in both the schema (what Seqera renders) and the usage docs. Checked against the pinned stack rather than written from memory: - cellpose_diameter is only a resize factor. CellposeModel.eval computes `rescale = 30. / diameter`, so the default of 30 resamples nothing and does not assert your cells are 30 px. Cellpose-SAM trained on 7.5-120 px cells, so it is mostly a speed knob, and it filters nothing. - cellpose_min_area is sopa's, not Cellpose's: "Minimum area (in pixels^2) for a cell to be considered as valid", applied per patch before RESOLVECELLPOSE merges them. Units are px^2, so it does not track pixel_size_microns -- worth saying, since 200 px^2 is 15.7 um^2 only at COMET's 0.28 um/px. - flow_threshold rejects on shape: higher keeps more masks, and 0 disables the check outright (`if flow_threshold > 0` guards it). - cellprob_threshold moves the boundaries, not just the count, so it shifts cell areas and every intensity measured over them. Lower finds more and larger cells. Sources: cellpose 4.2 models.py/dynamics.py and the Cellpose-SAM settings docs; sopa 2.2.6's own CLI help, read out of the pinned wheel. No CHANGELOG entry: nothing here changes behaviour, and the file is already a conflict surface for the two open branches. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
cellsam_min_area defaulted to 0 -- no size floor at all -- while cellpose_min_area defaulted to 200, so the same slide was filtered differently depending only on which segmenter produced it. Default CellSAM to 200 as well. This changes CellSAM results: objects under 200 px^2 (15.7 um^2 at COMET's 0.28 um/px, about a 4.5 um disc) are now dropped as debris. --cellsam_min_area 0 restores the old behaviour. Cellpose output is untouched. The two were already compatible enough for one number to mean one thing. Checked in both implementations rather than assumed: - units: both pixels^2. sopa/segmentation/stainings.py:44 says so for the Cellpose path, and bin/cellsam_segment.py counts label pixels. - comparison: both keep area >= threshold. sopa does `cells[cells.area >= self.min_area]`; cellsam removes `counts < min_area`. - measure: not identical. cellsam counts pixels; sopa measures the polygon from `find_contours(mask, 0.5)` after `buffer(-r).buffer(2r).buffer(-r)` smoothing. Replicating that on round and ragged blobs puts the polygon at 0.93-1.00x the pixel count (median 0.99), so 200 here cuts at an equivalent ~202 px on the Cellpose side. Close enough to share a default, and now documented so nobody assumes they are exactly interchangeable. - timing: cellsam filters once after the WSI tiles are merged, sopa per patch during segmentation. Safe because patch_overlap_pixel means a cell truncated at one patch edge appears whole in its neighbour. mesmer_min_nuclei_area is deliberately not touched: it filters nuclei rather than whole cells, and is applied inside the Mesmer container. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Reviewer caught it: I described `cellpose_diameter` as "not a claim about how
big your cells are" and "mostly a speed knob". Both are wrong, and the second
follows from the first.
It is exactly a claim about how big your cells are. Cellpose's own docstring
is "diameters are used to rescale the image to 30 pix cell diameter": the 30
in `rescale = 30. / diameter` is the diameter the model targets, the image is
resized by that factor before the network sees it, and the masks are resized
back after. Declaring 60 presents a 60 px cell to the network at 30 px. The
default of 30 asserting "already at that scale" is why it resamples nothing --
a consequence of the assertion, not evidence the number is inert.
Speed is a side effect of the downsampling, and Cellpose does document it as a
use ("if you input a diameter of 90, then the image will be downsampled by a
factor of 3, which will increase run speed"), but leading with it buried the
actual mechanism. What is true and worth keeping is that Cellpose-SAM's 7.5-120
px tolerance makes the parameter optional for typical data, unlike earlier
Cellpose versions.
Two other corrections from the same docs page while here:
- cellprob_threshold is not a 0-1 probability. The values are inputs to a
sigmoid centred at zero, so they run from about -6 to +6 and 0 is the
midpoint. I had left the range out earlier because I could not verify it;
the settings page states it, so it is now documented -- it is the difference
between knowing -2 is a reasonable value and guessing.
- flow_threshold now says why the check exists: nothing stops the network
predicting flows matching no real shape, so Cellpose recomputes each mask's
implied flow gradients and takes the MSE against the predicted flows.
Source: https://cellpose.readthedocs.io/en/latest/settings.html, and
cellpose 4.2 models.py.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…-area Document the Cellpose tuning parameters, and give CellSAM the same minimum cell area
The trailing clause -- "the default of 30 says they already do" -- restated something the reader is already looking at. In the Seqera form the value 30 is displayed directly above the description, so the sentence spent its last third narrating the field above it. Wording from the reviewer. The fuller explanation of what the default implies stays in help_text, which is where someone who wants it will look. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A run on Milton died after an hour with
Failed to pull singularity image
FATAL: ... write /home/.../.apptainer/cache/oci-tmp/tmp_2881498929:
disk quota exceeded
An Apptainer pull writes two separate things. Nextflow places the finished
.img; Apptainer itself unpacks the OCI layers it converts from into
$APPTAINER_CACHEDIR, defaulting to ~/.apptainer/cache. The second is the
larger, and a home quota does not hold tens of GB of it.
Add `container_cache_dir`, which sets apptainer.cacheDir and
singularity.cacheDir. That is the half Nextflow controls. The layer cache is
read from the environment of the process that launched Nextflow -- image
pulls run in Nextflow's own JVM, not in a task -- so nothing in
nextflow.config can redirect it. PIPELINE_INITIALISATION therefore checks it:
setting container_cache_dir while the layer cache is still under $HOME is an
error naming the exact exports to run, and using Apptainer with the layer
cache under $HOME warns. Both fire before any download starts, instead of an
hour into one.
Also bind deepcell_cache_dir into the CELLSAMSEGMENT and MESMERSEGMENT
containers. It never was, and autoMounts covers only $HOME, /tmp and the work
directory -- so pointing it at scratch redirected $HOME inside the container
to a path that was not there. The 1.7 GB CellSAM download went to the
container's ephemeral layer and every task repeated it, which is the cost the
parameter exists to avoid, and MESMERSEGMENT's model-download flock file was
container-local and serialised nothing.
Verified against the pipeline: container_cache_dir without APPTAINER_CACHEDIR
errors at startup with the right suggested path; with both on scratch the run
proceeds silently; unset warns and proceeds. The binds were confirmed present
in the rendered container command.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
nf-core lint runs `nextflow config -flat`, which uses the strict config parser,
and it rejected the top-level conditional:
Error nextflow.config:220:1: If statements cannot be mixed with config
Same class of failure as 85cf35b. Use the elvis operator instead, so the value
is an expression rather than a statement.
That changes what lands in the config when the parameter is unset: `cacheDir`
is now present and null rather than absent. That matters, because CI's image
cache from #39 comes from NXF_SINGULARITY_CACHEDIR and a null that shadowed it
would break the cache silently -- nothing would fail, images would just be
re-pulled every run.
It does not shadow it. Verified with a stub `singularity` on PATH, reading back
the directory Nextflow reports in "Pulling Singularity image ... [cache X]":
param unset, NXF_SINGULARITY_CACHEDIR set -> env var wins
param set, env var also set -> the parameter wins
neither set -> $workDir/singularity, as before
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Keep container images and model caches off the home directory
Adds a way to withhold channels that are not markers -- autofluorescence, blanks, empty cycles -- from KRONOS2, so they contribute nothing to the embedding. Until now the only escape for an out-of-vocabulary channel was --allow-novel-defaults, which still *feeds* it to the model normalised with default statistics: the opposite of what is wanted for a channel that carries no biology. This is scoped to the embedding step and deletes nothing. A withheld channel is simply never read, so it stays in the image, remains available to segmentation and every other process, and keeps whatever intensity measurements CELLMEASUREMENT wrote for it in the GeoJSON. Contrast remove_markers, which does strip channels out of the background-subtracted image. The log line, the marker report and the docs all say "not shown to KRONOS2" rather than "removed", so the scope is not mistakable. Names are comma separated and case sensitive, matched against the image's own channel names before any kronos_marker_mapping rename. A name matching nothing fails the run and lists the channels that are present; the nuclear channel cannot be withheld because it is the model's preferred_dapi. While wiring this up, channel indices became load-bearing and exposed a silent correctness bug on OME-TIFFs that give each channel its own 2D series. ChannelReader sized itself from series[0] alone, so an 8-series image read as 1 channel while the patch buffer was still sized from the 8-name marker list -- and numpy broadcast that one channel across all eight. The run completed, reported 8 channels, and produced 768-d vectors computed from eight copies of DAPI. Verified on the COMET _fixed images. The layout is now detected once and shared by the name list and the pixel reader, each channel is read from its own series, read_window refuses to return fewer channels than were asked of it, and the run fails outright if the name count and the image's channel count disagree. Channel names are also built positionally, so an OME Channel carrying neither Name nor ID no longer shifts every later marker onto the wrong channel. The marker report gains the embedded/not-shown split and names withheld channels in place, and is now written before the novel-marker gate can exit -- a failing run is when it is most useful. Adds tests/python/test_kronos2_common.py, the first module under the tests/python scaffold, plus a pytest job in the linting workflow so it runs, and a stub nf-test for the module, which had none. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
feat: add kronos_exclude_markers, and fix split-series channel reads
parquet_to_tiff.py hardcoded dtype=np.uint16 when rasterising segmentation geometries. Label IDs are stored as pixel values, so that caps a mask at 65535 distinct cells -- and rasterio.features.rasterize does not raise on overflow, it silently saturates at the dtype maximum and drops every geometry past it. The failure is invisible on the crops used in testing and only surfaces on whole-slide images. On a 370 mm^2 section (~370k cells at 0.4964 um/px) it truncated to exactly 65535 labels, with the discarded cells collapsing onto low label IDs. That leaves single labels whose parts are scattered across the whole slide, whose bounding boxes span the image, and whose geometries are correspondingly expensive downstream. Select the narrowest dtype that fits the geometry count instead, and raise rather than truncate if even uint32 is too narrow. Masks under 65535 cells are unchanged, so existing outputs and snapshots stay byte-identical. Diagnostics go to stderr because stdout carries the TIFF. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
test_parquet_to_tiff.py exercises label dtype selection, which lives in bin/parquet_to_tiff.py. That module imports rasterio, shapely, pyarrow and typer at import time, so the tests cannot reach the function without them and collection failed with ModuleNotFoundError. Skipping the module on missing imports would have made CI green while leaving the truncation bug unguarded, which is the opposite of the point. These are plain wheels with no GPU or gated weights, so the job stays within its stated dependency-light intent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Widening the label dtype doubles these masks: on a 36583x40986 slide the PARQUETTOTIFF output goes from 3.0 GB to 6.0 GB, and SMOOTHMASKS then reads that and writes another copy. All of it lands on shared scratch, where a recent run spent six hours on a single 2.87 GB mask write. Label masks are long runs of a repeated ID, so deflate more than cancels the widening -- a dense whole-slide mask goes from 2.87 GB to 31 MB, and the compressed write is faster than the uncompressed one even on local disk. Verified end to end through the stdout redirect the module uses, since TIFF writing needs a seekable stream: 70,000 cells round-trip as uint32 with every label intact, 12x smaller on disk. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Replace the uint16/uint32 selection (select_label_dtype + LABEL_DTYPES) with a single fixed uint32 dtype. uint32 indexes 4.29e9 cells, orders of magnitude past any real slide, so there is no case where narrowing to uint16 is worth the extra code -- and it removes the sorted-tuple assumption raised in review. A fail-fast guard still rejects the implausible >uint32 case rather than truncating silently. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The smoothed mask is the viewer-facing output and some viewers choke on compressed-strip TIFFs, so drop compression here entirely and write a plain TIFF. No option is exposed -- this output is meant to be opened, not squeezed onto scratch. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The CellSAM segmenter wrote its label mask uncompressed, unlike the Cellpose path (parquet_to_tiff). Both are intermediates that get smoothed downstream, so compress this one the same way -- zlib level 1, ~90x on label masks. Only the final smoothed mask is left uncompressed for viewers. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
0.2.2 carries WEHI-SODA-Hub/cellmeasurement-py#7, the STRtree broad phase for the GeoJSON export overlap pass — the companion to the mask changes here. It also carries #6, which drops the geometric descriptors from the erosion and expansion bins. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
fix: label masks silently truncate past 65,535 cells
Document the label-mask dtype overflow fix, resource-profile/neighbors changes and resumable polygon extraction in CHANGELOG.md, which were missing from Unreleased, and bump the manifest version. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
The bash [[ == ]] comparison hardcoded lowercase wehi-soda-hub/sp_segment while the real repo is WEHI-SODA-Hub/sp_segment. That mismatch failed the check for every legitimate dev->main PR, even though the surrounding if: guard (which uses GitHub Actions' case-insensitive ==) let the step run. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Collaborator
|
I have used both the cellsam and cellpose workflows on real data and are working correctly. Trying to decide if its worth adding H&E and IHC functionality for Lachie in this release. I don't think we will worry for now, we can experiment with it in the future. |
Contributor
Author
Sounds good. I think we don't worry for now. I'm also still working on the AVITI stuff, which will be (hopefully) next release. |
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
2 participants
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.
Bump for 0.5.0 release.
Branch protection CI fails due to bug (this can be ignored).
PR checklist
nf-core pipelines lint).nextflow run . -profile test,docker --outdir <OUTDIR>).nextflow run . -profile debug,test,docker --outdir <OUTDIR>).docs/usage.mdis updated.docs/output.mdis updated.CHANGELOG.mdis updated.README.mdis updated (including new tool citations and authors/contributors).