Skip to content

Commit b90e928

Browse files
lguerardclaude
andcommitted
perf(merge): ⚡ skip chunks that hold no labels, and size occupancy blocks
The per-tile counts already say which chunks are background, so the merge need not touch them: an empty chunk is now neither read nor written. Zarr leaves an unwritten chunk unmaterialised and reads it back as the fill value, so on a sparse image this saves I/O *and* disk. Likewise a boundary column next to an empty chunk cannot produce a touching pair, so the scan skips it. On a one-blob test image that is 1 of 16 chunks relabelled and 0 of 24 boundary columns read. Doing that exposed why skip_empty was doing nothing on smaller images: the occupancy block defaulted to (1, 128, 128), which on a 128x128 image is one block spanning the whole plane, so every tile over-covered it and tested occupied. The map was right; the resolution was useless. block_for_tile sizes the block to a quarter of the tile, and prepare/run_multi pass it. The same test image goes from 16/16 tiles segmented to 1/16. Two bugs in build_occupancy_map surfaced with it. overwrite=True built the new map and then threw it away: the guard for "a concurrent run finished first" fired unconditionally, so the stale map was returned. And a map built with a different block was reused forever, so changing tile_shape silently kept the old resolution. It now rebuilds when the block differs, and an explicit overwrite clears the old directory first. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 6617363 commit b90e928

7 files changed

Lines changed: 236 additions & 23 deletions

File tree

src/patchworks/__init__.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,12 @@
4646
)
4747
from ._io import auto_empty_threshold, estimate_empty_tiles, load_ome_zarr
4848
from ._merge import capped_output_chunks, merge_tile_labels
49-
from ._occupancy import build_occupancy_map, occupancy_path, tile_occupancy
49+
from ._occupancy import (
50+
block_for_tile,
51+
build_occupancy_map,
52+
occupancy_path,
53+
tile_occupancy,
54+
)
5055
from ._postprocess import dilate_labels
5156
from ._relabel import relabel_sequential_array, relabel_sequential_zarr
5257
from ._relations import label_relations
@@ -67,6 +72,7 @@
6772
"load_ome_zarr",
6873
"estimate_empty_tiles",
6974
"auto_empty_threshold",
75+
"block_for_tile",
7076
"build_occupancy_map",
7177
"occupancy_path",
7278
"tile_occupancy",

src/patchworks/_merge.py

Lines changed: 51 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,7 @@ def _scan_touching_pairs(
157157
chunk_shape: tuple[int, ...],
158158
label_offsets: "np.ndarray | None" = None,
159159
n_workers: int = 1,
160+
has_labels: "np.ndarray | None" = None,
160161
) -> np.ndarray:
161162
"""Scan chunk-boundary slabs; return (N, 2) int64 array of touching pairs.
162163
@@ -183,6 +184,11 @@ def _scan_touching_pairs(
183184
n_workers : int
184185
Threads to scan with. Each task reads its own slab, so this is
185186
embarrassingly parallel and was needlessly serial.
187+
has_labels : np.ndarray, optional
188+
Per-chunk boolean: does this chunk hold any label? A boundary next to
189+
an empty chunk can never produce a touching pair, so those columns are
190+
not read at all. On a sparse image (``skip_empty``) that is most of
191+
them.
186192
187193
Returns
188194
-------
@@ -197,15 +203,36 @@ def _scan_touching_pairs(
197203

198204
# One task per chunk-column of one boundary face. Each reads its own slab
199205
# and produces its own pairs, so they are independent.
200-
tasks: list[tuple[int, int, tuple[int, ...]]] = []
206+
tasks: list[tuple[int, int, tuple[int, ...], int, int]] = []
207+
skipped = 0
201208
for ax, pos in _boundary_face_specs(shape, chunk_shape):
202209
face_axes = [a for a in range(arr.ndim) if a != ax]
203210
face_ranges = [range(0, shape[a], chunk_shape[a]) for a in face_axes]
204211
for offsets in _iproduct(*face_ranges):
205-
tasks.append((ax, pos, offsets))
212+
grid = [0] * arr.ndim
213+
for a, off in zip(face_axes, offsets):
214+
grid[a] = off // chunk_shape[a]
215+
grid[ax] = pos // chunk_shape[ax]
216+
b_idx = int(np.ravel_multi_index(tuple(grid), n_per_dim))
217+
grid[ax] -= 1 # the chunk on the near side of the boundary
218+
a_idx = int(np.ravel_multi_index(tuple(grid), n_per_dim))
219+
# A pair needs a non-zero label on *both* sides, so a boundary
220+
# touching an empty chunk cannot produce one -- don't read it.
221+
if has_labels is not None and not (
222+
has_labels[a_idx] and has_labels[b_idx]
223+
):
224+
skipped += 1
225+
continue
226+
tasks.append((ax, pos, offsets, a_idx, b_idx))
227+
if skipped:
228+
logger.info(
229+
"boundary scan: skipping %d/%d columns that border an empty chunk",
230+
skipped,
231+
skipped + len(tasks),
232+
)
206233

207-
def _one(task: tuple[int, int, tuple[int, ...]]) -> "np.ndarray | None":
208-
ax, pos, offsets = task
234+
def _one(task: tuple) -> "np.ndarray | None":
235+
ax, pos, offsets, a_idx, b_idx = task
209236
face_axes = [a for a in range(arr.ndim) if a != ax]
210237
sl: list = [slice(None)] * arr.ndim
211238
sl[ax] = slice(pos - 1, pos + 1)
@@ -215,13 +242,6 @@ def _one(task: tuple[int, int, tuple[int, ...]]) -> "np.ndarray | None":
215242
a_vals = slab[0].ravel().astype(np.int64)
216243
b_vals = slab[1].ravel().astype(np.int64)
217244
if label_offsets is not None:
218-
grid = [0] * arr.ndim
219-
for a, off in zip(face_axes, offsets):
220-
grid[a] = off // chunk_shape[a]
221-
grid[ax] = pos // chunk_shape[ax]
222-
b_idx = int(np.ravel_multi_index(tuple(grid), n_per_dim))
223-
grid[ax] -= 1 # the chunk on the near side of the boundary
224-
a_idx = int(np.ravel_multi_index(tuple(grid), n_per_dim))
225245
a_vals[a_vals > 0] += label_offsets[a_idx]
226246
b_vals[b_vals > 0] += label_offsets[b_idx]
227247
mask = (a_vals > 0) & (b_vals > 0) & (a_vals != b_vals)
@@ -514,6 +534,11 @@ def zarr_native_merge(
514534
with ``offset`` an exclusive cumulative sum -- O(n_chunks) arithmetic
515535
instead of reading and rewriting the entire store to renumber it.
516536
``None`` falls back to the streaming renumber pass.
537+
538+
A count of 0 also means that chunk holds no labels, so the merge can
539+
skip it outright -- neither reading it nor writing zeros over it. Zarr
540+
leaves an unwritten chunk unmaterialised and reads it back as the
541+
fill value, so on a sparse image this saves both I/O and disk.
517542
sequential : bool
518543
Renumber the merged labels to a contiguous ``1..N``. Free here: the id
519544
domain is dense by construction, so the compaction is a ``np.unique``
@@ -554,7 +579,9 @@ def zarr_native_merge(
554579
max_label = int(offsets[-1] + counts_arr[-1]) if n_chunks else 0
555580
else:
556581
offsets = None
582+
counts_arr = None
557583
max_label = _make_globally_unique(arr, shape, chunk_shape)
584+
has_labels = None if counts_arr is None else counts_arr > 0
558585
logger.info(
559586
"zarr_native_merge: shape=%s chunks=%s max_label=%d (%s)",
560587
shape,
@@ -571,6 +598,7 @@ def zarr_native_merge(
571598
chunk_shape,
572599
label_offsets=offsets,
573600
n_workers=n_workers,
601+
has_labels=has_labels,
574602
)
575603
logger.info(
576604
"zarr_native_merge: %d touching pairs → building LUT", len(pairs)
@@ -623,11 +651,22 @@ def zarr_native_merge(
623651
)
624652
for idx in _iproduct(*[range(n) for n in n_per_dim])
625653
]
654+
# A chunk that wrote no labels is all background. Skipping it leaves the
655+
# output chunk unwritten, which zarr reads back as the fill value (0) and
656+
# never stores -- so an empty region costs neither I/O nor disk.
626657
tasks = [
627658
(sl, int(offsets[i]) if offsets is not None else 0)
628659
for i, sl in enumerate(chunk_slices)
660+
if has_labels is None or has_labels[i]
629661
]
630-
n_w = max(1, min(n_workers, n_chunks))
662+
if has_labels is not None and len(tasks) < n_chunks:
663+
logger.info(
664+
"zarr_native_merge: relabeling %d of %d chunks (%d hold no labels)",
665+
len(tasks),
666+
n_chunks,
667+
n_chunks - len(tasks),
668+
)
669+
n_w = max(1, min(n_workers, max(1, len(tasks))))
631670
logger.info(
632671
"zarr_native_merge: relabeling %d chunks with %d worker(s)…",
633672
n_chunks,

src/patchworks/_occupancy.py

Lines changed: 78 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030
from concurrent.futures import ThreadPoolExecutor
3131
from itertools import product as _iproduct
3232
from pathlib import Path
33-
from typing import Any, Union
33+
from typing import Any, Sequence, Union
3434

3535
import numpy as np
3636
import zarr
@@ -40,6 +40,47 @@
4040
logger = logging.getLogger(__name__)
4141

4242
DEFAULT_BLOCK = (1, 128, 128)
43+
# Blocks per tile edge. A block is the finest thing the map can resolve, so it
44+
# has to be well under the tile or every tile over-covers the same block and
45+
# comes back "occupied" -- correct, but useless as a skip list.
46+
_BLOCKS_PER_TILE = 4
47+
48+
49+
def block_for_tile(
50+
tile_shape: Sequence[int], cap: Sequence[int] = DEFAULT_BLOCK
51+
) -> tuple[int, ...]:
52+
"""Occupancy block sized so a tile spans several blocks.
53+
54+
The map can only resolve whole blocks, so a block as large as the tile
55+
makes every tile hit the same block and test occupied. Sizing it to a
56+
fraction of the tile keeps the answer discriminating, while the *cap*
57+
keeps the map small on big tiles.
58+
59+
Parameters
60+
----------
61+
tile_shape : sequence of int
62+
The tile shape the map will be queried with.
63+
cap : sequence of int, optional
64+
Largest block per axis.
65+
66+
Returns
67+
-------
68+
tuple of int
69+
Block shape, at least 1 per axis.
70+
71+
Examples
72+
--------
73+
>>> block_for_tile((16, 1024, 1024))
74+
(1, 128, 128)
75+
>>> block_for_tile((8, 32, 32))
76+
(1, 8, 8)
77+
"""
78+
return tuple(
79+
max(1, min(int(c), int(t) // _BLOCKS_PER_TILE or 1))
80+
for t, c in zip(tile_shape, cap)
81+
)
82+
83+
4384
# Bytes to pull per read while pooling; keeps peak memory flat regardless of
4485
# how large the image is.
4586
_READ_TARGET_BYTES = 128 * 1024**2
@@ -153,8 +194,25 @@ def build_occupancy_map(
153194
store = str(image_store)
154195
out_path = occupancy_path(store, level)
155196
if not overwrite and Path(out_path).exists():
156-
logger.info("occupancy map already present at %s", out_path)
157-
return out_path
197+
# Reuse only if it was built at the block we want. A map left over
198+
# from a run with a different tile_shape is coarser (or finer) than
199+
# this run needs, and silently reusing it would degrade every
200+
# occupancy answer that follows.
201+
try:
202+
existing = tuple(zarr.open_array(out_path, mode="r").attrs["block"])
203+
except Exception:
204+
existing = None
205+
if existing == tuple(block):
206+
logger.info("occupancy map already present at %s", out_path)
207+
return out_path
208+
logger.info(
209+
"rebuilding occupancy map at %s: it was built with block %s, "
210+
"this run needs %s",
211+
out_path,
212+
existing,
213+
tuple(block),
214+
)
215+
overwrite = True
158216

159217
root = zarr.open_group(store, mode="r")
160218
src = _level_array(root, level, store)
@@ -229,9 +287,13 @@ def _one(starts: tuple[int, ...]) -> None:
229287
raise
230288

231289
if Path(out_path).exists():
232-
# Another run finished first; theirs is as good as ours.
233-
shutil.rmtree(tmp_path, ignore_errors=True)
234-
return out_path
290+
if not overwrite:
291+
# A concurrent run finished first; theirs is as good as ours.
292+
shutil.rmtree(tmp_path, ignore_errors=True)
293+
return out_path
294+
# Explicit rebuild: clear the old one, or os.replace would fail on a
295+
# non-empty directory and we would keep serving the stale map.
296+
shutil.rmtree(out_path, ignore_errors=True)
235297
try:
236298
os.replace(tmp_path, out_path)
237299
except OSError:
@@ -294,6 +356,16 @@ def tile_occupancy(
294356
f"tile_shape is {len(tile_shape)}-D but the occupancy map is "
295357
f"{len(block)}-D"
296358
)
359+
coarse = [
360+
(ax, b, t) for ax, (b, t) in enumerate(zip(block, tile_shape)) if b >= t
361+
]
362+
if coarse:
363+
logger.warning(
364+
"occupancy blocks are as large as the tile on axes %s; every tile "
365+
"will over-cover the same block and test occupied. Rebuild the "
366+
"map with block=block_for_tile(tile_shape).",
367+
[ax for ax, _, _ in coarse],
368+
)
297369

298370
tile_grid = tuple(-(-s // t) for s, t in zip(sp_shape, tile_shape))
299371
occupancy = np.zeros(tile_grid, dtype=bool)

tests/test_distributed.py

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
"""Tests for the per-tile distributed building blocks."""
22

3+
from pathlib import Path
4+
35
import numpy as np
46
import pytest
57
import zarr
@@ -151,6 +153,45 @@ def test_offsets_match_the_renumber_pass(tmp_path):
151153
assert set(ids.tolist()) == {1, 2, 3}, f"expected 3 objects, got {ids}"
152154

153155

156+
def test_empty_chunks_are_skipped_not_just_short_circuited(tmp_path):
157+
"""Chunks with no labels must cost neither I/O nor disk.
158+
159+
A count of 0 already tells the merge the chunk is background, so it need
160+
not be read at all -- and leaving the output chunk unwritten means zarr
161+
never materialises it.
162+
"""
163+
img = np.zeros((8, 64), "uint16")
164+
img[1:4, 1:7] = 500 # wholly inside tile 0; the other seven are empty
165+
tile = (8, 8)
166+
167+
stage = str(tmp_path / "stage.zarr")
168+
create_stage(stage, img.shape, tile)
169+
counts = {
170+
i: stage_tile(img, _fn, stage, i, tile_shape=tile, overlap=0)
171+
for i in range(len(spatial_tiles(img.shape, tile)))
172+
}
173+
assert sum(1 for n in counts.values() if n == 0) == 7, "7 empty tiles"
174+
175+
out = str(tmp_path / "out.zarr")
176+
merged = merge_tile_labels(
177+
stage,
178+
write_to=out,
179+
input_component="staged",
180+
sequential_labels=True,
181+
label_counts=counts,
182+
).compute()
183+
184+
# Correctness first: the one real object survives, everything else is 0.
185+
assert set(np.unique(merged).tolist()) == {0, 1}
186+
assert merged[1:4, 1:7].min() == 1
187+
188+
# And the empty chunks were never written: zarr stores only the chunks it
189+
# was given, so the skipped ones leave no file behind.
190+
chunk_dir = Path(out) / "labels" / "c" / "0"
191+
written = sorted(p.name for p in chunk_dir.iterdir())
192+
assert written == ["0"], f"only the occupied chunk should exist: {written}"
193+
194+
154195
def test_stage_tile_returns_dense_label_count(tmp_path):
155196
"""stage_tile reports how many labels it wrote, and writes exactly 1..n."""
156197
img = np.zeros((8, 16), "uint16")

tests/test_occupancy.py

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import zarr
66

77
from patchworks import (
8+
block_for_tile,
89
build_occupancy_map,
910
estimate_empty_tiles,
1011
occupancy_path,
@@ -123,6 +124,16 @@ def test_build_is_idempotent_and_covers_ragged_edges(tmp_path):
123124

124125
assert build_occupancy_map(store, block=(1, 8, 8)) == path
125126

127+
# ...but a *different* block must rebuild rather than serve a stale map,
128+
# or changing tile_shape would silently keep the old resolution forever.
129+
build_occupancy_map(store, block=(1, 4, 4))
130+
assert tuple(zarr.open_array(path, mode="r").attrs["block"]) == (1, 4, 4)
131+
132+
# And an explicit overwrite must actually take effect: the guard against
133+
# a concurrent build used to discard the freshly built map.
134+
build_occupancy_map(store, block=(1, 8, 8), overwrite=True)
135+
assert tuple(zarr.open_array(path, mode="r").attrs["block"]) == (1, 8, 8)
136+
126137

127138
def test_map_lives_outside_the_image_store(tmp_path):
128139
"""The map must not make the image store an invalid zarr hierarchy.
@@ -188,6 +199,33 @@ def _counting(self, key):
188199
)
189200

190201

202+
def test_block_must_be_finer_than_the_tile(tmp_path):
203+
"""A block as coarse as the tile makes every tile test occupied.
204+
205+
The map can only resolve whole blocks, so one block spanning the tile is
206+
over-covered by every tile -- correct, but useless as a skip list. This is
207+
what silently disabled skip_empty on smaller images.
208+
"""
209+
assert block_for_tile((16, 1024, 1024)) == (1, 128, 128)
210+
assert block_for_tile((8, 32, 32)) == (1, 8, 8)
211+
assert block_for_tile((1, 4, 4)) == (1, 1, 1) # never below 1
212+
213+
# A tile-sized block marks everything occupied...
214+
data = np.zeros((1, 8, 128, 128), "uint16")
215+
data[0, 2:5, 4:20, 4:20] = 900 # one blob in one corner
216+
store = _write_store(tmp_path, data)
217+
tile = (8, 32, 32)
218+
219+
build_occupancy_map(store, block=(1, 128, 128))
220+
coarse = tile_occupancy(store, tile, channel=0, threshold=100)
221+
assert coarse["n_occupied"] == coarse["n_tiles"], "coarse blocks over-cover"
222+
223+
# ...while a tile-derived block finds only the tile that has signal.
224+
build_occupancy_map(store, block=block_for_tile(tile), overwrite=True)
225+
fine = tile_occupancy(store, tile, channel=0, threshold=100)
226+
assert fine["n_occupied"] == 1, f"expected 1 occupied, got {fine}"
227+
228+
191229
def test_tile_shape_dimensionality_is_checked(tmp_path):
192230
data = np.zeros((1, 2, 16, 16), "uint16")
193231
store = _write_store(tmp_path, data)

0 commit comments

Comments
 (0)