Merge dev: recoverable locks and a straight answer for glob inputs (v2.1.1)

Two failures that cost a cluster round-trip each to diagnose, both because
the message pointed somewhere other than the cause.

A glob input without sequence_pattern fell through to bioio, which reads
single files and could only report "BioImage does not support the image:
'.../*tif'" -- naming the path rather than the missing setting. to_ome_zarr
now says which key is absent and shows an example regex, and run_multi checks
it in its pre-flight so the run fails before submitting anything.

Snakemake releases its lock only on a clean exit, so a killed run leaves the
directory locked. run_multi made that harder than it should be by giving each
phase its own state directory, including a `.snakemake_convert` that is never
named anywhere -- so clearing a lock meant guessing paths. `--unlock` now
covers every directory it manages, and the conversion failure prints that
exact command.

Author: lguerard

docs/guide/snakemake.md

@@ -346,6 +346,19 @@ the GPU partition stays busy instead of idling through every config's
 `prepare` and multi-hour `merge` in turn. A config that fails does **not**
 abort the others; you get a per-config status and a non-zero exit.
 
+!!! tip "After a killed run"
+    Snakemake only releases its lock on a clean exit, so a run that was killed
+    (Ctrl-C, an SSH drop, an OOM) leaves the directory locked. Each phase has
+    its own state directory, so releasing them by hand means reconstructing
+    several paths — use the flag instead:
+
+    ```bash
+    pixi run multi -- --unlock     # then re-run normally
+    ```
+
+    Running the orchestrator under `tmux` avoids most of these in the first
+    place: it survives a dropped connection.
+
 !!! warning "`jobs:` is per config"
     The profile's `jobs:` caps one Snakemake process. Running three configs
     concurrently can therefore have 3× that many jobs in flight — lower it if
@@ -450,6 +463,8 @@ prologue. The simplest path is a single shared env that the compute nodes see.
 | Symptom | Fix |
 |---------|-----|
 | `snakemake: command not found` | use `python -m snakemake` |
+| `Directory cannot be locked` | a previous run was killed instead of exiting cleanly. For a multi-run: `pixi run multi -- --unlock` (it covers every state directory, including the conversion one). For a single config: add `--unlock --directory <the same one you ran with>` |
+| `BioImage does not support the image: '.../*tif'` | a glob input needs `sequence_pattern` — see [A folder of single-plane TIFFs](ome_zarr_napari.md#a-folder-of-single-plane-tiffs) |
 | Segment jobs pend forever | wrong `slurm_partition`/GPU request; on scicore use `gres: "gpu:1"` |
 | Segment dies, `Network is unreachable` | offline GPU nodes — the `fetch_model` localrule caches the model on the submit host first; if it still fails, your submit host has no network either (pre-download manually) |
 | `cellpose is not installed` in a job | the job's env lacks `patchworks[cellpose]` |

src/patchworks/plugins/ome_zarr.py

@@ -45,6 +45,7 @@
 
 from __future__ import annotations
 
+import glob
 import logging
 import math
 from concurrent.futures import ThreadPoolExecutor
@@ -1201,6 +1202,21 @@ def _to_dask(
     if sequence_pattern is not None:
         arr, detected, ps = _open_tiff_sequence(path, sequence_pattern)
         return arr, axes or detected, ps
+    if any(ch in path for ch in "*?[") or (
+        not Path(path).exists() and glob.glob(path)
+    ):
+        # Without sequence_pattern this would be handed to bioio, which reads
+        # single files and reports only that it cannot handle the "format" --
+        # pointing at the glob rather than the missing setting.
+        raise ValueError(
+            f"{path!r} looks like a glob over several files, but "
+            "sequence_pattern is not set, so there is no way to tell which "
+            "part of each filename is Z, C or T.\n"
+            "Set it to a regex with named groups, e.g.\n"
+            "    sequence_pattern: '_Z(?P<Z>\\d+)_C(?P<C>\\d+)_V\\d+'\n"
+            "for files like sample_Z001_C0_V0.tif. Pass a single file "
+            "instead if you did not mean a sequence."
+        )
     if path.endswith(".zarr"):
         arr = load_ome_zarr(source, channel=None)
         ax = axes or _default_axes(arr.ndim)

tests/test_ome_zarr.py

@@ -399,3 +399,19 @@ def test_sharding(tmp_path):
     assert (
         getattr(_zarr.open_array(f"{out3}/0", mode="r"), "shards", None) is None
     )
+
+
+def test_glob_without_sequence_pattern_says_so(tmp_path):
+    """A glob input must name the missing setting, not blame the format.
+
+    Without sequence_pattern the glob fell through to bioio, which reads one
+    file and could only report "does not support the image" -- pointing at the
+    path instead of at the config key that was missing.
+    """
+    with pytest.raises(ValueError, match="sequence_pattern is not set"):
+        to_ome_zarr(str(tmp_path / "*.tif"), tmp_path / "out.zarr")
+
+    # A real single file must still reach the normal readers.
+    with pytest.raises(Exception) as exc:
+        to_ome_zarr(str(tmp_path / "scan.ims"), tmp_path / "o2.zarr")
+    assert "sequence_pattern" not in str(exc.value)

workflow/scripts/run_multi.py

@@ -46,6 +46,7 @@ def _snakemake_cmd(
     dry_run: bool,
     state_dir: Path | None = None,
     targets: list[str] | None = None,
+    extra: list[str] | None = None,
 ) -> list[str]:
     """Build one snakemake invocation.
 
@@ -69,6 +70,8 @@ def _snakemake_cmd(
         cmd += ["--cores", str(cores), "--rerun-triggers", "mtime"]
     if dry_run:
         cmd += ["-n", "-p"]
+    if extra:
+        cmd += extra
     if targets:
         # "--" ends option parsing: --rerun-triggers takes a variable number
         # of values and would otherwise swallow the target path.
@@ -114,6 +117,18 @@ def _spread(key):
                 f"share a chunk layout; got {_spread(key)}"
             )
 
+    for path, cfg in zip(paths, cfgs):
+        source = str(cfg.get("input", ""))
+        if any(ch in source for ch in "*?[") and not cfg.get(
+            "sequence_pattern"
+        ):
+            problems.append(
+                f"{path.name}: input {source!r} is a glob over several files "
+                "but sequence_pattern is unset, so nothing says which part of "
+                "each filename is Z/C/T. Set e.g. "
+                r"sequence_pattern: '_Z(?P<Z>\d+)_C(?P<C>\d+)_V\d+'"
+            )
+
     names = [cfg.get("label_name") for cfg in cfgs]
     duplicates = {n for n in names if names.count(n) > 1}
     if duplicates:
@@ -160,6 +175,15 @@ def main() -> None:
         action="store_true",
         help="pass -n -p to every Snakemake run; skips relations",
     )
+    parser.add_argument(
+        "--unlock",
+        action="store_true",
+        help=(
+            "release stale Snakemake locks in every state directory this "
+            "script manages, then exit. Needed after a run was killed or "
+            "died: the lock is only released on a clean exit."
+        ),
+    )
     args = parser.parse_args()
 
     workflow_dir = Path(__file__).resolve().parent.parent
@@ -173,6 +197,32 @@ def main() -> None:
     work_dir = _validate_configs(seg_config_paths, seg_cfgs)
     image_store = f"{work_dir}/image.zarr"
 
+    # Each phase gets its own Snakemake state directory (the lock lives in the
+    # working directory, not the config), so unlocking has to cover all of
+    # them -- and nobody should have to reconstruct these paths by hand.
+    state_dirs = [Path(work_dir) / ".snakemake_convert"] + [
+        Path(cfg["work_dir"]) / cfg["label_name"] / ".snakemake"
+        for cfg in seg_cfgs
+    ]
+    if args.unlock:
+        for state_dir in state_dirs:
+            if not state_dir.exists():
+                continue
+            _run(
+                _snakemake_cmd(
+                    seg_config_paths[0],
+                    workflow_dir=workflow_dir,
+                    profile=args.profile,
+                    cores=args.cores,
+                    dry_run=False,
+                    state_dir=state_dir,
+                    extra=["--unlock"],
+                ),
+                workflow_dir,
+            )
+        print("[run_multi] unlocked; re-run without --unlock", flush=True)
+        return
+
     # Phase A: convert exactly once. The three runs are about to go concurrent
     # and `convert` writes with overwrite=True, so letting them race on it
     # would have them clobbering one store. Ask for its marker explicitly.
@@ -189,7 +239,13 @@ def main() -> None:
         workflow_dir,
     )
     if rc != 0:
-        print("[run_multi] ERROR: conversion failed", file=sys.stderr)
+        print(
+            "[run_multi] ERROR: conversion failed.\n"
+            "  If the log says the directory cannot be locked, a previous run "
+            "was killed rather than exiting cleanly; release it with:\n"
+            f"      {Path(sys.argv[0]).name} --config {args.config} --unlock",
+            file=sys.stderr,
+        )
         sys.exit(rc)
 
     # Still phase A: build the occupancy map here too. Every config's `prepare`