Conversation
The raw transcripts carry a vendor (e.g. Xenium morphology) `cell_id` that segmentation methods condition on as a prior. The previous process_dataset stripped it alongside `nucleus_id`/`cell_type`, leaving methods with no prior signal. Stop stripping `cell_id` so it flows through to spatial_unlabelled, and declare it as an optional integer column on the transcripts in src/api/file_spatial_unlabelled.yaml. The held-out ground truth used for evaluation remains in spatial_solution. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Wraps segger (graph neural network transcript-to-cell segmentation) as
a Viash method that consumes the standard spatial_unlabelled SpatialData
zarr and produces a Labels2DModel prediction.
Initial boundary set is taken from the transcripts' `cell_id` prior
(the vendor morphology assignment that `process_dataset` now keeps in
spatial_unlabelled) when present and falls back to Cellpose on
`morphology_mip` otherwise. The initial polygons plus the original
transcripts are materialized as a Xenium-style directory
(transcripts.parquet + cell_boundaries.parquet +
nucleus_boundaries.parquet) so segger's stock `10x_xenium` preprocessor
drives the prediction graph — no SpatialData-loader branch of segger
required. `segger segment` is run from `dpeerlab/segger@main`. The
per-transcript segger_cell_id assignments are projected back onto the
initial mask by majority vote and rasterized as the final segmentation.
Docker engine pins the dependency stack that segger is known to work
with on the openproblems base image:
- PyG wheels matched to torch 2.5+cu121
- RAPIDS 24.10 (last release binary-compatible with the base image's
CUDA 12.1 runtime)
- Cellpose for the nucleus pre-segmentation
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Robrecht Cannoodt <rcannood@gmail.com>
* use pytorch 25.04 instead of 26.02 as a base image Signed-off-by: Robrecht Cannoodt <rcannood@gmail.com> * try with 25.06 Signed-off-by: Robrecht Cannoodt <rcannood@gmail.com> * update image location Signed-off-by: Robrecht Cannoodt <rcannood@gmail.com> * print format Signed-off-by: Robrecht Cannoodt <rcannood@gmail.com> --------- Signed-off-by: Robrecht Cannoodt <rcannood@gmail.com>
The new base image (nvcr.io/nvidia/pytorch:25.06-py3 from PR #33) ships NumPy 2.x, which no longer silently overflows -1 into uint32. Cellpose internally compares mask sizes (a uint32 array) against min_size, so passing -1 now raises `OverflowError: Python integer -1 out of bounds for uint32`. Modern cellpose treats min_size=0 as "keep all masks", matching the original intent. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Test data carries the vendor cell_id prior with -1 marking unassigned transcripts. The old filter (notna + non-empty-string) treated -1 as a real cell id, built a convex hull over every unassigned transcript, and then `labels[rr, cc] = int(-1)` blew up on uint32 under NumPy 2.x with `OverflowError: Python integer -1 out of bounds for uint32`. Add `_valid_cell_id_mask`, the pandas analog of segger's canonical `valid_cell_id_expr` (rejects null / -1 / "UNASSIGNED" / "NONE"), and apply the same predicate to segger's own output before majority-voting labels, since segger emits -1 for unassigned transcripts too. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The component installs segger from dpeerlab/segger's default branch (main), whose `segment` CLI no longer accepts --prediction-scale-factor, --min-similarity, or --fragment-mode (those live on v2-incremental). Rename our --prediction_scale_factor arg to --prediction_expansion_ratio to match upstream (`buffer_dist = sqrt(area/pi) * ratio`, so 2.2 is no longer a sensible default — drop it to 0.5). Drop --min_similarity and --fragment_mode since there is nothing to forward them to. Bump n_epochs default to 20. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
segger@main eagerly `import cudf` in `data/tiling.py`, and cudf's `validate_setup()` raises cudaErrorInsufficientDriver on CPU-only CI hosts before any of segger's documented CPU fallbacks get a chance to run. Forward RAPIDS_NO_INITIALIZE / CUDF_NO_INITIALIZE / RMM_NO_INITIALIZE to the `segger segment` subprocess so the import succeeds; real GPU operations still error if hit, but small test fixtures stay on CPU paths. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
segger needs a real CUDA GPU end-to-end (cudf, cuspatial, kernels). The viash test matrix lands on a CPU-only GitHub runner, so we can't make the algorithm actually run there. Two complementary changes: * `script.py`: after reading the input, short-circuit on `torch.cuda.is_available() == False` by writing a valid all-zeros SpatialData stub (segmentation labels matching the input image shape + AnnData table with dataset_id / method_id) and exiting 0. The test passes; the resulting prediction obviously scores poorly, which the log calls out loudly. * `config.vsh.yaml`: re-add the `cuspatial-cu12` install that PR #33 dropped. NGC's pytorch:25.06-py3 bundles cudf but not cuspatial, and segger eagerly imports it from `segger.geometry.conversion`. No version pin — pip resolves against the bundled cudf. GPU hosts now go through the real pipeline; CPU CI never reaches the import path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Promote the no-GPU heads-up to a `warnings.warn(UserWarning, ...)` emitted right after device detection — fires before the input is read so logs surface the situation as early as possible, on top of the existing in-flow print where the stub is written. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Reverts the CPU stub-and-exit path: segger now raises RuntimeError immediately after device detection if no CUDA GPU is visible. This makes CPU CI fail loudly (which is what we actually want for a GPU- only method) instead of silently emitting an all-zeros stub that could be mistaken for a real result. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Robrecht Cannoodt <rcannood@gmail.com>
Signed-off-by: Robrecht Cannoodt <rcannood@gmail.com>
Signed-off-by: Robrecht Cannoodt <rcannood@gmail.com>
Replace the older majority-vote-over-initial-nuclei label image with the team's canonical Delaunay alpha-shape per cell (vendored from dpeerlab/segger@v2-incremental, see boundary.py). The model itself stays on dpeerlab/segger@main — only the export side is uplifted to v2's boundary builder so the rest of the lab's tooling (segger-analysis fov_analysis, plot_vd_pixel_method_masks.py) renders the same masks. The prediction.zarr now contains, all in segger's native semantics: * labels.segmentation — uint label image rasterized from each cell's smooth Delaunay boundary, so every transcript segger assigned (including ones rescued outside the initial nucleus) falls inside its cell's mask. * shapes.cell_boundaries — a GeoDataFrame of the same per-cell polygons in global coordinates so downstream morphology / FOV analyses can use them directly without re-rasterizing. * tables.table — a proper cells x genes AnnData with per-cell counts derived from segger's transcript-level assignments (not from a post-hoc pixel lookup), uns.dataset_id, uns.method_id, and a counts layer. process_prediction will still rederive its own table from labels.segmentation downstream; this one is for direct inspection. `generate_boundaries` is wrapped in a 16 -> 8 -> 4 -> 2 worker fallback so saturated or low-core hosts don't make the export step fail. If the boundary builder still fails or returns nothing usable, the script degrades gracefully to the initial nucleus mask for the label image. rtree + tqdm added to the python pypi deps for the vendored boundary module. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
README spec for `file_prediction.zarr` is: labels: 'segmentation' tables: 'table' (uns.dataset_id, uns.method_id) process_prediction derives the cells x genes table by indexing `label_image[int(t.y), int(t.x)]` for every input transcript. The smallest valid label image is therefore one where each segger-assigned transcript's truncated pixel carries its `segger_cell_id`; unassigned transcripts fall on un-painted 0 and are filtered out by process_prediction's `cell_id != 0` rule. That's now `_rasterize_assigned_transcripts`: one affine, one int cast, one fancy-indexed numpy write. O(N) in #assigned transcripts, no KDTree, no per-cell loop, no Delaunay. Drops the vendored boundary.py and the rtree/tqdm deps along with the shapes export and the proper-anndata helper (process_prediction rebuilds the table from the label image anyway). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Both args only had `info.test_default` (consumed by viash test) and no
production `default`, so Nextflow runs of the workflow passed nothing
to the component and `par["init_segmentation"]` arrived as None, which
fell through to `raise ValueError("Unknown init_segmentation mode: None")`.
Set the production defaults to match what tests already used (auto and 30).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pixel-paint only lit each assigned transcript's own pixel, so the output looked like sparse dots even though `process_prediction`'s pixel-lookup still recovered correct cells x genes counts. Switch to a transcript-Voronoi raster: one cKDTree build over the assigned transcripts plus one batched `tree.query(pixels, workers=-1)` parallelizes the per-pixel nearest-neighbor lookup across all cores in C. A density-based distance cutoff (5 * median nearest-neighbor distance, floored at 5px) keeps cells from bleeding into empty space. Same accuracy as pixel-paint for transcript-lookup metrics (each transcript is closest to itself, so its own pixel lookup returns its own cell), but now produces filled cell footprints — usable for visualization and any future shape-based metric. Model invocation is unchanged: segger still installs from dpeerlab/segger@main with the same CLI flags. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
Describe your changes
Follow-up work on the segger method after PR #28 merged into main.
Production defaults: set real
default:values for--init_segmentation(auto) and--cellpose_diameter(30). Bothargs only had
info.test_default(consumed only byviash test),so Nextflow workflow runs were passing
Noneand trippingValueError: Unknown init_segmentation mode: None.Filled-cell raster via transcript-Voronoi (cKDTree). Replaces the
older majority-vote-over-initial-nuclei label image. One cKDTree
build over assigned transcripts plus one batched
tree.query(pixels, workers=-1)(parallel in C) gives every pixelthe cell of its nearest segger-assigned transcript. A density-based
cutoff (
5 * median nearest-neighbor distance, floored at 5 px)keeps cells from bleeding into empty regions. Output is correct for
process_prediction's pixel lookup AND looks like filled cells —not sparse dots — for visualization / future shape-based metrics.
Hard-fail on CPU. segger requires CUDA end-to-end (cudf,
cuspatial, GPU kernels). The script now raises
RuntimeErrorimmediately after
torch.device(...)if no GPU is visible insteadof silently producing a stub.
cuspatial-cu12install restored. PR Segger - use pytorch 25.04 instead of 26.02 as a base image #33 dropped it during thebase-image swap; NGC's
pytorch:25.06-py3bundles cudf but notcuspatial, which segger's
geometry.conversionimports eagerly.RAPIDS_NO_INITIALIZE/CUDF_NO_INITIALIZE/RMM_NO_INITIALIZEare forwarded to the
segger segmentsubprocess so cudf's import-time driver validation doesn't abort a run if the driver isn't yet
visible to the container.
Model invocation itself is unchanged: segger installs from
dpeerlab/segger@main, same CLI flags as before.Checklist before requesting a review
I have performed a self-review of my code
Check the correct box. Does this PR contain:
Proposed changes are described in the CHANGELOG.md ← needs entry
CI Tests succeed and look good! ← see caveat
Two things still to address before submit
Bug fixes
methods/segger: set productiondefault:for--init_segmentation(auto) and--cellpose_diameter(30);workflow previously passed
Noneand crashed.methods/segger: restorecuspatial-cu12install dropped by thebase-image swap.
methods/segger: forwardRAPIDS_NO_INITIALIZEto the seggersubprocess to bypass cudf's import-time driver check.
Minor changes
methods/segger: rebuildlabels.segmentationas atranscript-Voronoi raster (
scipy.spatial.cKDTree) so cells arefilled regions, not sparse transcript dots.
methods/segger: raiseRuntimeErroron CPU hosts — seggerrequires a CUDA GPU end-to-end.
test for segger), which omits run_and_check_output.py from its test_resources. So CI only runs the static check_config.py metadata
linter; the segger CLI is never executed in CI. The functional run that produced the Voronoi raster happened on a GPU host outside
CI. Tick the box knowing that.