Skip to content

Commit 7a8c84d

Browse files
lguerardclaude
andcommitted
perf(merge): ⚡ segment into the label group and relabel it in place
Takes the pipeline from three full passes over the label volume to two. The segment jobs write their tile-local ids straight into image.zarr/labels/<name>/0, and the merge rewrites that array where it stands instead of filling a scratch stage store and copying it across. Relabelling in place is safe because the boundary scan completes before any chunk is rewritten, so nothing still needs the original ids, and each worker owns a disjoint, chunk-aligned region. zarr_native_merge detects that its output is its input and skips creating the array -- creating it would delete the very data it is about to read. It only applies when the tile already fits the label chunk cap, because in place the chunking cannot be changed and level 0 has to stay pageable for a viewer. An oversized tile keeps the scratch store, and prepare records which route it took in tiles.json so segment and merge agree. The label group is not registered in labels/.zattrs until the merge finishes, so a run that dies midway leaves an unregistered group that NGFF readers do not list, and a rerun recreates it. That is the trade: the stage store used to be a clean redo point. Verified against the two-store path on a cross-boundary case (identical labelling) and end to end, including a concurrent multi-config run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent b90e928 commit 7a8c84d

6 files changed

Lines changed: 153 additions & 35 deletions

File tree

src/patchworks/_merge.py

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -622,6 +622,21 @@ def zarr_native_merge(
622622
"zarr_native_merge: renumbered to 1..%d in the same LUT", n_objects
623623
)
624624

625+
# Relabelling straight back into the source array saves writing the whole
626+
# volume a second time. It is safe because the boundary scan has already
627+
# finished by this point, so nothing still needs the original ids, and
628+
# each worker owns a disjoint, chunk-aligned region.
629+
in_place = (staged_path, staged_component) == (out_path, out_component)
630+
if in_place:
631+
if output_chunks is not None and tuple(output_chunks) != tuple(
632+
chunk_shape
633+
):
634+
raise ValueError(
635+
"output_chunks cannot differ from the source chunking when "
636+
"merging in place (the array is rewritten, not recreated)"
637+
)
638+
logger.info("zarr_native_merge: relabeling in place, no second store")
639+
625640
out_root = zarr.open_group(out_path, mode="a")
626641
out_chunks = tuple(chunk_shape)
627642
if output_chunks is not None:
@@ -636,11 +651,14 @@ def zarr_native_merge(
636651
"output_chunks must divide the staged chunk shape so workers "
637652
f"write whole chunks; axis/staged/output mismatches: {bad}"
638653
)
639-
# Match the staged dtype: ids are already compact (dense by construction,
640-
# and compacted again above when sequential), so nothing needs a wider one.
641-
_create_zarr_label_array(
642-
out_root, out_component, shape, out_chunks, dtype=arr.dtype
643-
)
654+
if not in_place:
655+
# Match the staged dtype: ids are already compact (dense by
656+
# construction, and compacted again above when sequential), so nothing
657+
# needs a wider one. Creating this in place would delete the very
658+
# array we are about to read.
659+
_create_zarr_label_array(
660+
out_root, out_component, shape, out_chunks, dtype=arr.dtype
661+
)
644662

645663
# Row-major, matching spatial_tiles' order -- so chunk i is tile i and the
646664
# offsets line up with the per-tile counts.
@@ -669,7 +687,7 @@ def zarr_native_merge(
669687
n_w = max(1, min(n_workers, max(1, len(tasks))))
670688
logger.info(
671689
"zarr_native_merge: relabeling %d chunks with %d worker(s)…",
672-
n_chunks,
690+
len(tasks),
673691
n_w,
674692
)
675693

tests/test_distributed.py

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,59 @@ def test_empty_chunks_are_skipped_not_just_short_circuited(tmp_path):
192192
assert written == ["0"], f"only the occupied chunk should exist: {written}"
193193

194194

195+
def test_in_place_merge_matches_the_two_store_merge(tmp_path):
196+
"""Relabelling back into the source must give the same labelling.
197+
198+
It is safe because the boundary scan finishes before any chunk is
199+
rewritten, so nothing still needs the original ids -- and it saves writing
200+
the whole volume a second time.
201+
"""
202+
img = np.zeros((8, 48), "uint16")
203+
img[1:4, 4:12] = 500 # inside tile 0
204+
img[2:6, 14:26] = 500 # straddles the x=16 boundary
205+
tile = (8, 16)
206+
207+
results = {}
208+
for name in ("two_store", "in_place"):
209+
stage = str(tmp_path / f"stage_{name}.zarr")
210+
create_stage(stage, img.shape, tile)
211+
counts = {
212+
i: stage_tile(img, _fn, stage, i, tile_shape=tile, overlap=2)
213+
for i in range(len(spatial_tiles(img.shape, tile)))
214+
}
215+
# in place = the merge's output *is* its input
216+
out = (
217+
stage if name == "in_place" else str(tmp_path / f"out_{name}.zarr")
218+
)
219+
component = "staged" if name == "in_place" else "labels"
220+
results[name] = merge_tile_labels(
221+
stage,
222+
write_to=out,
223+
input_component="staged",
224+
output_component=component,
225+
sequential_labels=True,
226+
label_counts=counts,
227+
).compute()
228+
229+
assert np.array_equal(results["two_store"], results["in_place"]), (
230+
"in-place relabelling changed the result"
231+
)
232+
assert set(np.unique(results["in_place"]).tolist()) == {0, 1, 2}
233+
234+
# And it must refuse a rechunk it cannot honour, rather than silently
235+
# ignoring it: in place the array is rewritten, never recreated.
236+
stage = str(tmp_path / "stage_guard.zarr")
237+
create_stage(stage, img.shape, tile)
238+
with pytest.raises(ValueError, match="cannot differ"):
239+
merge_tile_labels(
240+
stage,
241+
write_to=stage,
242+
input_component="staged",
243+
output_component="staged",
244+
output_chunks=(8, 8),
245+
)
246+
247+
195248
def test_stage_tile_returns_dense_label_count(tmp_path):
196249
"""stage_tile reports how many labels it wrote, and writes exactly 1..n."""
197250
img = np.zeros((8, 16), "uint16")

workflow/rules/merge.smk

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@
22

33
rule merge:
44
input:
5-
batch_done,
5+
markers=batch_done,
6+
tiles=TILES,
67
output:
78
touch(f"{RUN}/labels.done"),
89
log:

workflow/scripts/merge.py

Lines changed: 34 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
from patchworks._chunks import _get_available_memory
2121
from patchworks.plugins.ome_zarr import register_labels
2222

23-
from _pw import stage_path, start_log
23+
from _pw import load_tiles_json, stage_path, start_log
2424

2525
start_log(snakemake.log[0]) # noqa: F821
2626
cfg = snakemake.config # noqa: F821
@@ -29,7 +29,14 @@
2929
image_store = str(Path(work_dir) / "image.zarr")
3030
label_group = f"{image_store}/labels/{label_name}"
3131

32-
staged = zarr.open_group(stage_path(work_dir, label_name), mode="r")["staged"]
32+
# prepare recorded where the segment jobs wrote: the label group's level 0
33+
# (merged in place) or a scratch stage store.
34+
manifest = load_tiles_json(snakemake.input.tiles) # noqa: F821
35+
target_path = manifest["target_path"]
36+
target_component = manifest.get("target_component", "staged")
37+
in_place = bool(manifest.get("in_place", False))
38+
39+
staged = zarr.open_group(target_path, mode="r")[target_component]
3340

3441
# Size the relabel pool against what this job was actually granted, not the
3542
# node. Each worker holds roughly a few copies of one chunk, so the RAM budget
@@ -49,29 +56,30 @@
4956
# counts in lets the merge compute global id ranges by a cumulative sum,
5057
# replacing a full read+write of the store that existed only to renumber it.
5158
label_counts = {}
52-
for marker in snakemake.input: # noqa: F821
59+
for marker in snakemake.input.markers: # noqa: F821
5360
for index, n in json.loads(Path(marker).read_text())["counts"].items():
5461
label_counts[int(index)] = int(n)
5562

56-
# Merge straight into the label group's level 0. Writing to a scratch
57-
# _merged.zarr and letting write_labels copy it across cost a full extra
58-
# read+write of the volume plus the scratch store's disk; register_labels
59-
# already expects level 0 to exist and only adds the pyramid and metadata.
60-
root = zarr.open_group(image_store, mode="a")
61-
parent = root.require_group("labels")
62-
if label_name in parent:
63-
del parent[label_name]
64-
parent.require_group(label_name)
65-
66-
# Level 0 keeps napari-friendly chunks even when tiles are much larger; the
67-
# cap has to divide the tile shape so merge workers still write whole chunks.
68-
out_chunks = capped_output_chunks(staged.chunks, (16, 1024, 1024))
63+
if in_place:
64+
# The tiles already sit in labels/<name>/0, so the merge rewrites them
65+
# where they are: no scratch store, and one full write of the volume less.
66+
# Safe because the boundary scan finishes before any chunk is rewritten.
67+
out_chunks = None
68+
else:
69+
# Level 0 keeps napari-friendly chunks even when tiles are much larger;
70+
# the cap must divide the tile so workers still write whole chunks.
71+
root = zarr.open_group(image_store, mode="a")
72+
parent = root.require_group("labels")
73+
if label_name in parent:
74+
del parent[label_name]
75+
parent.require_group(label_name)
76+
out_chunks = capped_output_chunks(staged.chunks, (16, 1024, 1024))
6977

7078
_, n_objects = merge_tile_labels(
71-
stage_path(work_dir, label_name),
72-
write_to=label_group,
73-
input_component="staged",
74-
output_component="0",
79+
target_path,
80+
write_to=label_group if not in_place else target_path,
81+
input_component=target_component,
82+
output_component="0" if not in_place else target_component,
7583
output_chunks=out_chunks,
7684
sequential_labels=cfg.get("sequential_labels", True),
7785
n_workers=cfg.get("merge_workers") or default_workers,
@@ -88,11 +96,12 @@
8896
n_objects=n_objects,
8997
)
9098

91-
shutil.rmtree(stage_path(work_dir, label_name), ignore_errors=True)
92-
# Also drop the checkpoint's completion sentinel (stage.zarr.done): the
93-
# "prepare" rule's stage=touch(STAGE_OK) output must not outlive the store it
94-
# claims exists, or a future rerun (e.g. re-segmenting for new labels) skips
95-
# "prepare" and "segment" tries to open a stage.zarr that's already gone.
99+
if not in_place:
100+
shutil.rmtree(stage_path(work_dir, label_name), ignore_errors=True)
101+
# Drop the checkpoint's completion sentinel (stage.zarr.done): the "prepare"
102+
# rule's stage=touch(STAGE_OK) output must not outlive what it claims exists,
103+
# or a future rerun (e.g. re-segmenting for new labels) skips "prepare" and
104+
# "segment" writes into a target that is already gone or already merged.
96105
Path(f"{stage_path(work_dir, label_name)}.done").unlink(missing_ok=True)
97106
print(f"[patchworks] labels written to {group}")
98107
open(snakemake.output[0], "w").close() # noqa: F821

workflow/scripts/prepare_tiles.py

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,15 @@
55
from pathlib import Path
66

77
import numpy as np
8+
import zarr
89

910
from patchworks import (
1011
auto_empty_threshold,
1112
auto_tile_shape,
1213
auto_tile_shape_cellpose,
1314
block_for_tile,
1415
build_occupancy_map,
16+
capped_output_chunks,
1517
create_stage,
1618
normalize_overlap,
1719
spatial_tiles,
@@ -20,6 +22,9 @@
2022

2123
from _pw import open_image, stage_path, start_log, validate_config
2224

25+
# Chunking ceiling for the written labels, so a viewer can page them lazily.
26+
LABEL_CHUNK_CAP = (16, 1024, 1024)
27+
2328
start_log(snakemake.log[0]) # noqa: F821
2429
cfg = snakemake.config # noqa: F821
2530
work_dir = cfg["work_dir"]
@@ -134,7 +139,31 @@
134139
for i in range(0, len(occupied), tiles_per_job)
135140
]
136141

137-
create_stage(stage_path(work_dir, label_name), image.shape, tile_shape)
142+
# Where the segment jobs write. When the tile is already within the label
143+
# chunk cap, they can write straight into the label group's level 0 and the
144+
# merge relabels it in place -- one full write of the volume less than
145+
# staging to a scratch store and copying it across. An oversized tile keeps
146+
# the scratch store, so level 0 can still be chunked for lazy viewing.
147+
image_store = str(Path(work_dir) / "image.zarr")
148+
in_place = capped_output_chunks(tile_shape, LABEL_CHUNK_CAP) == tuple(
149+
tile_shape
150+
)
151+
if in_place:
152+
target_path, target_component = f"{image_store}/labels/{label_name}", "0"
153+
root = zarr.open_group(image_store, mode="a")
154+
parent = root.require_group("labels")
155+
if label_name in parent:
156+
del parent[label_name]
157+
parent.require_group(label_name)
158+
print("[patchworks] segmenting straight into the label group (in place)")
159+
else:
160+
target_path, target_component = stage_path(work_dir, label_name), "staged"
161+
print(
162+
f"[patchworks] tile {tuple(tile_shape)} exceeds the label chunk cap "
163+
f"{LABEL_CHUNK_CAP}; staging to a scratch store so level 0 stays "
164+
"chunked for viewing"
165+
)
166+
create_stage(target_path, image.shape, tile_shape, component=target_component)
138167

139168
Path(work_dir, label_name, "tiles.json").write_text(
140169
json.dumps(
@@ -145,6 +174,9 @@
145174
"occupied": occupied,
146175
"tiles_per_job": tiles_per_job,
147176
"batches": batches,
177+
"target_path": target_path,
178+
"target_component": target_component,
179+
"in_place": in_place,
148180
},
149181
indent=2,
150182
)

workflow/scripts/segment_tile.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111

1212
from patchworks import stage_tile
1313

14-
from _pw import build_fn, load_tiles_json, open_image, stage_path, start_log
14+
from _pw import build_fn, load_tiles_json, open_image, start_log
1515

1616
start_log(snakemake.log[0]) # noqa: F821
1717
cfg = snakemake.config # noqa: F821
@@ -25,7 +25,11 @@
2525

2626
# Built once for the whole batch: this is what makes the model load amortize.
2727
fn = build_fn(cfg)
28-
stage = stage_path(work_dir, label_name)
28+
# prepare decides where tiles land: the label group's level 0 directly when
29+
# the tile fits the chunk cap (the merge then relabels it in place), else a
30+
# scratch stage store.
31+
stage = manifest["target_path"]
32+
component = manifest.get("target_component", "staged")
2933
tile_shape = tuple(manifest["tile_shape"])
3034

3135
counts = {}
@@ -38,6 +42,7 @@
3842
tile_shape=tile_shape,
3943
# Scalar (older manifests) or per-axis list; stage_tile normalizes both.
4044
overlap=manifest["overlap"],
45+
component=component,
4146
)
4247
print(f"[patchworks] segmented tile {index}: {counts[index]} label(s)")
4348

0 commit comments

Comments
 (0)