Skip to content

Commit ab889be

Browse files
lguerardclaude
andcommitted
fix(workflow): šŸ› read merge's label counts by path, not from snakemake.input
merge.py started reading its inputs when it began consuming the per-tile label counts, and that surfaced a latent problem: it was handed tiles.json where it expected the seg markers. Cause: merge.py deletes stage.zarr.done, which is an output of the `prepare` checkpoint. With that gone the checkpoint can never be resolved again, so on any later DAG evaluation Snakemake cannot expand batch_done and substitutes a placeholder input instead -- batch_done is not even called. The old merge.py ignored snakemake.input entirely, so this went unnoticed. The marker paths are deterministic, so they are now derived from the manifest rather than from snakemake.input, which is immune to the substitution. And the sentinel is only removed on the scratch-store route, which is the only one that deletes the store the sentinel refers to; in place there is nothing to invalidate. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 0ae7f05 commit ab889be

1 file changed

Lines changed: 20 additions & 7 deletions

File tree

ā€Žworkflow/scripts/merge.pyā€Ž

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,16 @@
5656
# counts in lets the merge compute global id ranges by a cumulative sum,
5757
# replacing a full read+write of the store that existed only to renumber it.
5858
label_counts = {}
59-
for marker in snakemake.input.markers: # noqa: F821
60-
for index, n in json.loads(Path(marker).read_text())["counts"].items():
59+
# Derive the marker paths from the manifest rather than reading
60+
# snakemake.input: when the `prepare` checkpoint cannot be resolved (see the
61+
# STAGE_OK note at the end of this file), Snakemake substitutes a placeholder
62+
# for a checkpoint-dependent input function, and `markers` then points at
63+
# tiles.json instead of the seg markers. The paths are deterministic, so
64+
# building them here is both simpler and immune to that.
65+
seg_dir = Path(work_dir) / label_name / "seg"
66+
for batch in range(len(manifest["batches"])):
67+
marker = seg_dir / f"{batch}.done"
68+
for index, n in json.loads(marker.read_text())["counts"].items():
6169
label_counts[int(index)] = int(n)
6270

6371
if in_place:
@@ -97,11 +105,16 @@
97105
)
98106

99107
if not in_place:
108+
# Only the scratch route creates a store to clean up -- and only it has to
109+
# drop the checkpoint's completion sentinel, because that sentinel would
110+
# otherwise outlive the store it claims exists and a rerun would skip
111+
# "prepare" and segment into something already deleted.
112+
#
113+
# Deleting a checkpoint output is not free: it leaves `prepare`
114+
# permanently unresolvable, so a later DAG evaluation cannot expand
115+
# batch_done and hands dependent rules a placeholder input instead. That
116+
# is why the label counts above are read by path, not from snakemake.input.
100117
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.
105-
Path(f"{stage_path(work_dir, label_name)}.done").unlink(missing_ok=True)
118+
Path(f"{stage_path(work_dir, label_name)}.done").unlink(missing_ok=True)
106119
print(f"[patchworks] labels written to {group}")
107120
open(snakemake.output[0], "w").close() # noqa: F821

0 commit comments

Comments
Ā (0)