Skip to content

Commit 1e747fc

Browse files
lguerardclaude
andcommitted
fix(gpu): 🐛 catch cupy OOM, read the granted GPU, validate configs in prepare
Five defects around running Cellpose and DoG on shared cluster GPUs. _is_cuda_oom matched a substring on RuntimeError only, but cupy.cuda.memory.OutOfMemoryError derives from Exception -- so the DoG path and dilate_labels(use_gpu=True) had no retry at all while Cellpose had four. The retry now lives in patchworks._gpu, classifies by exception class first (cupy says "Out of memory allocating", torch says "CUDA out of memory"), and both plugins use it. The Cellpose retry also slept up to 300s while keeping its model pinned in VRAM -- exactly the memory the contending job needed. It now evicts the model cache before backing off and reloads on retry, seconds against a backoff measured in minutes. _get_gpu_memory asked NVML for index 0 unconditionally. NVML enumerates every GPU on the node regardless of --gres=gpu:1, so on a multi-GPU node this read a different device's free memory -- and auto tile sizing was built on that number. It now resolves the device from CUDA_VISIBLE_DEVICES (index or UUID) and keeps 20% headroom instead of sizing tiles to fill every free byte of a device we don't own outright. The DoG plugin never released cupy's pool, which does not return blocks to the driver on its own, so a long-lived worker's footprint only grew. It now drops its references and frees the pool per tile. Config mistakes now fail in prepare, a cheap CPU job, instead of in the first GPU job after convert and prepare have already run: gpu_memory_gb: "auto" was truthy so "auto" * 1024**3 built a ~4 GB string; a non-"auto" tile_shape string became a character tuple; and unknown cellpose:/custom.kwargs keys were forwarded blind. prepare also stopped reading cfg["cellpose"] unconditionally for tile_shape: "auto", which raised KeyError for any DoG or threshold config. Also: the Cellpose version parse blew up at import on a suffixed release like "4.0rc1" (only ImportError was caught), and do_3D: false with a z>1 tile silently handed Cellpose a stack it would read as channels -- now an error. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 6c99dda commit 1e747fc

7 files changed

Lines changed: 518 additions & 75 deletions

File tree

src/patchworks/_chunks.py

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -213,23 +213,40 @@ def safe_worker_count(
213213
return max(1, min(cpu_cap, mem_cap))
214214

215215

216+
# Leave room for a co-tenant: info.free is a point-in-time reading of a device
217+
# we usually do not own outright, and sizing a tile to fill all of it is what
218+
# turns another job's growth into our OOM.
219+
_GPU_HEADROOM = 0.8
220+
221+
216222
def _get_gpu_memory() -> int:
217-
"""Return free GPU VRAM in bytes.
223+
"""Return usable GPU VRAM in bytes for the device we were granted.
224+
225+
NVML enumerates every GPU on the node regardless of ``--gres=gpu:1``, so
226+
asking for index 0 unconditionally reads the *wrong* device's free memory
227+
whenever SLURM granted anything but the first one -- and tile sizing was
228+
built on that number. Resolve the device from ``CUDA_VISIBLE_DEVICES``,
229+
then keep a headroom fraction rather than claiming every free byte.
218230
219231
Returns
220232
-------
221233
int
222-
Free VRAM of GPU 0 via ``nvidia-ml-py``, or an 8 GiB fallback if the
223-
query fails.
234+
Usable VRAM, or an 8 GiB fallback if the query fails.
224235
"""
225236
try:
226237
import pynvml
227238

239+
from ._gpu import visible_device_index, visible_device_uuid
240+
228241
pynvml.nvmlInit()
229-
handle = pynvml.nvmlDeviceGetHandleByIndex(0)
242+
uuid = visible_device_uuid()
243+
if uuid is not None:
244+
handle = pynvml.nvmlDeviceGetHandleByUUID(uuid.encode())
245+
else:
246+
handle = pynvml.nvmlDeviceGetHandleByIndex(visible_device_index())
230247
info = pynvml.nvmlDeviceGetMemoryInfo(handle)
231248
pynvml.nvmlShutdown()
232-
return int(info.free)
249+
return int(int(info.free) * _GPU_HEADROOM)
233250
except Exception:
234251
logger.warning(
235252
"GPU memory query failed (nvidia-ml-py not installed?); "

src/patchworks/_gpu.py

Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,158 @@
1+
"""Shared GPU helpers: device selection and surviving transient OOM.
2+
3+
Cluster GPUs are shared. A co-tenant job's footprint can grow mid-run and push
4+
an otherwise-fine tile over the edge, so an out-of-memory error is often
5+
transient rather than a sign the work does not fit. Retrying on the GPU beats
6+
falling back to the CPU, where a single tile can take well over an hour.
7+
"""
8+
9+
from __future__ import annotations
10+
11+
import logging
12+
import os
13+
import time
14+
from typing import Any, Callable
15+
16+
logger = logging.getLogger(__name__)
17+
18+
OOM_RETRIES = 4
19+
OOM_BACKOFF_SECONDS = 30
20+
21+
22+
def is_oom(exc: BaseException) -> bool:
23+
"""Return whether *exc* is an out-of-memory error from any GPU stack.
24+
25+
Matching only ``RuntimeError`` missed cupy entirely:
26+
``cupy.cuda.memory.OutOfMemoryError`` derives from ``Exception``, not
27+
``RuntimeError``, so the DoG plugin and ``dilate_labels(use_gpu=True)``
28+
got no retry at all. Class name is checked first because cupy's message
29+
reads "Out of memory allocating ..." while torch's reads "CUDA out of
30+
memory", and a future backend may word it differently again.
31+
32+
Parameters
33+
----------
34+
exc : BaseException
35+
The exception to classify.
36+
37+
Returns
38+
-------
39+
bool
40+
True when the exception reports exhausted device memory.
41+
"""
42+
if type(exc).__name__ in ("OutOfMemoryError", "CUDAOutOfMemoryError"):
43+
return True
44+
return "out of memory" in str(exc).lower()
45+
46+
47+
def free_gpu_caches() -> None:
48+
"""Release cached (but unused) device memory from torch and cupy.
49+
50+
Both keep their own allocator pool and neither releases it on its own, so
51+
a process that has used either holds that memory for its lifetime. Freeing
52+
both also defragments, which is often what actually lets a retry succeed.
53+
Each is optional and skipped when not imported.
54+
"""
55+
torch = __import__("sys").modules.get("torch")
56+
if torch is not None and getattr(torch, "cuda", None) is not None:
57+
try:
58+
torch.cuda.empty_cache()
59+
except Exception: # pragma: no cover - defensive
60+
logger.debug("torch.cuda.empty_cache() failed", exc_info=True)
61+
62+
cupy = __import__("sys").modules.get("cupy")
63+
if cupy is not None:
64+
try:
65+
cupy.get_default_memory_pool().free_all_blocks()
66+
cupy.get_default_pinned_memory_pool().free_all_blocks()
67+
except Exception: # pragma: no cover - defensive
68+
logger.debug("cupy pool release failed", exc_info=True)
69+
70+
71+
def retry_on_oom(
72+
call: Callable[[], Any],
73+
*,
74+
enabled: bool = True,
75+
on_release: Callable[[], None] | None = None,
76+
retries: int = OOM_RETRIES,
77+
backoff: int = OOM_BACKOFF_SECONDS,
78+
) -> Any:
79+
"""Run *call*, retrying with a backoff while the GPU is out of memory.
80+
81+
Parameters
82+
----------
83+
call : callable
84+
Zero-argument callable doing the GPU work.
85+
enabled : bool
86+
Set False on a CPU path so real errors surface immediately.
87+
on_release : callable, optional
88+
Called before each backoff, to drop anything big this process is
89+
holding on the device. Sleeping while still pinning a cached model is
90+
self-defeating -- that memory is exactly what the co-tenant needs.
91+
retries : int
92+
Retries after the first attempt.
93+
backoff : int
94+
Base seconds; the wait grows linearly with the attempt number.
95+
96+
Returns
97+
-------
98+
Any
99+
Whatever *call* returns.
100+
"""
101+
for attempt in range(retries + 1):
102+
try:
103+
return call()
104+
except Exception as exc:
105+
if not enabled or not is_oom(exc):
106+
raise
107+
if on_release is not None:
108+
on_release()
109+
free_gpu_caches()
110+
if attempt == retries:
111+
logger.error(
112+
"GPU OOM persisted after %d retries; giving up.", retries
113+
)
114+
raise
115+
wait = backoff * (attempt + 1)
116+
logger.warning(
117+
"GPU OOM (likely contention on a shared device); released "
118+
"caches, retrying in %ds (attempt %d/%d).",
119+
wait,
120+
attempt + 1,
121+
retries,
122+
)
123+
time.sleep(wait)
124+
125+
126+
def visible_device_index() -> int:
127+
"""NVML index of the device this process is actually allowed to use.
128+
129+
``CUDA_VISIBLE_DEVICES`` remaps indices for the CUDA runtime but not for
130+
NVML, which always enumerates every GPU on the node. Querying NVML index 0
131+
unconditionally therefore reads a *different* GPU's free memory whenever
132+
SLURM granted anything other than the first one -- and tile sizing was
133+
built on that number.
134+
135+
Returns
136+
-------
137+
int
138+
NVML device index, defaulting to 0 when nothing constrains us.
139+
"""
140+
visible = os.environ.get("CUDA_VISIBLE_DEVICES", "").strip()
141+
if not visible:
142+
return 0
143+
first = visible.split(",")[0].strip()
144+
if not first or first.startswith("GPU-") or first.startswith("MIG-"):
145+
# A UUID form: NVML can resolve it directly, so leave index lookup
146+
# alone and let the caller fall back.
147+
return 0
148+
try:
149+
return max(0, int(first))
150+
except ValueError:
151+
return 0
152+
153+
154+
def visible_device_uuid() -> "str | None":
155+
"""UUID of the allowed device, when ``CUDA_VISIBLE_DEVICES`` uses one."""
156+
visible = os.environ.get("CUDA_VISIBLE_DEVICES", "").strip()
157+
first = visible.split(",")[0].strip() if visible else ""
158+
return first if first.startswith(("GPU-", "MIG-")) else None

src/patchworks/plugins/cellpose.py

Lines changed: 53 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -21,16 +21,36 @@
2121

2222
import numpy as np
2323

24+
from .._gpu import retry_on_oom
25+
2426
logger = logging.getLogger(__name__)
2527

28+
29+
def _parse_version(raw: str) -> tuple[int, ...]:
30+
"""Leading numeric components of a version string.
31+
32+
``int(x)`` over the raw segments blows up on any suffixed release
33+
(``"4.0rc1"``, ``"4.0.1.dev0"``) — and at *import* time, where only
34+
ImportError was being caught, so the package became unimportable rather
35+
than degrading.
36+
"""
37+
parts = []
38+
for segment in raw.split(".")[:2]:
39+
digits = "".join(c for c in segment if c.isdigit())
40+
if not digits:
41+
break
42+
parts.append(int(digits))
43+
return tuple(parts) or (0,)
44+
45+
2646
try:
2747
from cellpose import models as _cellpose_models
2848

29-
_CELLPOSE_VERSION: tuple[int, ...] = tuple(
30-
int(x) for x in importlib.metadata.version("cellpose").split(".")[:2]
49+
_CELLPOSE_VERSION: tuple[int, ...] = _parse_version(
50+
importlib.metadata.version("cellpose")
3151
)
3252
_CELLPOSE_V4 = _CELLPOSE_VERSION[0] >= 4
33-
except ImportError as _e:
53+
except ImportError:
3454
_cellpose_models = None # type: ignore[assignment]
3555
_CELLPOSE_VERSION = (0, 0)
3656
_CELLPOSE_V4 = False
@@ -196,28 +216,23 @@ def _get_model(cellpose_dict: dict[str, Any]) -> Any:
196216
return _model_cache[key]
197217

198218

199-
def _is_cuda_oom(exc: Exception) -> bool:
200-
"""Return whether *exc* looks like a CUDA out-of-memory error.
201-
202-
Parameters
203-
----------
204-
exc : Exception
205-
Exception raised by ``model.eval``.
219+
def _drop_cached_models() -> None:
220+
"""Evict the per-process model cache so its VRAM can be reclaimed.
206221
207-
Returns
208-
-------
209-
bool
210-
True if the exception is a CUDA OOM.
222+
Waiting out an OOM while still holding a loaded model pinned on the device
223+
is self-defeating: that memory is exactly what the contending job needs.
224+
The model is reloaded from the (already downloaded) weights on the next
225+
call, which costs seconds against a backoff measured in minutes.
211226
"""
212-
return "out of memory" in str(exc).lower()
213-
214-
215-
_OOM_RETRIES = 4
216-
_OOM_BACKOFF_SECONDS = 30
227+
if _model_cache:
228+
logger.info(
229+
"releasing %d cached Cellpose model(s) before backing off",
230+
len(_model_cache),
231+
)
232+
_model_cache.clear()
217233

218234

219235
def _eval_with_oom_fallback(
220-
model: Any,
221236
img: np.ndarray,
222237
kwargs: dict[str, Any],
223238
cellpose_dict: dict[str, Any],
@@ -227,14 +242,13 @@ def _eval_with_oom_fallback(
227242
Cluster GPUs are often shared: another job's memory footprint can grow
228243
mid-run and push an otherwise-fine tile size over the edge. Staying on
229244
GPU matters — this tile can take well over an hour, so falling back to
230-
CPU would be far worse than waiting. Instead this clears the allocator
231-
cache (fixes fragmentation) and retries with a backoff, giving the
232-
cotenant job time to free memory, before finally giving up.
245+
CPU would be far worse than waiting. See :func:`patchworks._gpu.retry_on_oom`.
246+
247+
The model is fetched inside the retry, so an eviction during backoff is
248+
followed by a reload rather than a use-after-free of the cached handle.
233249
234250
Parameters
235251
----------
236-
model : Any
237-
Cellpose model instance.
238252
img : np.ndarray
239253
Image to segment.
240254
kwargs : dict
@@ -247,32 +261,11 @@ def _eval_with_oom_fallback(
247261
np.ndarray
248262
Label array from ``model.eval``.
249263
"""
250-
import time
251-
252-
for attempt in range(_OOM_RETRIES + 1):
253-
try:
254-
return model.eval(img, **kwargs)[0]
255-
except RuntimeError as exc:
256-
if not cellpose_dict.get("gpu", False) or not _is_cuda_oom(exc):
257-
raise
258-
import torch
259-
260-
torch.cuda.empty_cache()
261-
if attempt == _OOM_RETRIES:
262-
logger.error(
263-
"CUDA OOM persisted after %d retries; giving up on tile.",
264-
_OOM_RETRIES,
265-
)
266-
raise
267-
wait = _OOM_BACKOFF_SECONDS * (attempt + 1)
268-
logger.warning(
269-
"CUDA OOM on tile (likely GPU contention); cleared cache, "
270-
"retrying in %ds (attempt %d/%d).",
271-
wait,
272-
attempt + 1,
273-
_OOM_RETRIES,
274-
)
275-
time.sleep(wait)
264+
return retry_on_oom(
265+
lambda: _get_model(cellpose_dict).eval(img, **kwargs)[0],
266+
enabled=bool(cellpose_dict.get("gpu", False)),
267+
on_release=_drop_cached_models,
268+
)
276269

277270

278271
def _run(block: np.ndarray, cellpose_dict: dict[str, Any]) -> np.ndarray:
@@ -290,7 +283,6 @@ def _run(block: np.ndarray, cellpose_dict: dict[str, Any]) -> np.ndarray:
290283
np.ndarray
291284
Integer (``int32``) label array of the same spatial shape.
292285
"""
293-
model = _get_model(cellpose_dict)
294286
do_3D = cellpose_dict["do_3D"]
295287

296288
if _CELLPOSE_V4:
@@ -310,13 +302,20 @@ def _run(block: np.ndarray, cellpose_dict: dict[str, Any]) -> np.ndarray:
310302

311303
if do_3D:
312304
kwargs["z_axis"] = 0
313-
masks = _eval_with_oom_fallback(model, block, kwargs, cellpose_dict)
305+
masks = _eval_with_oom_fallback(block, kwargs, cellpose_dict)
314306
return masks.astype("int32")
315307
else:
316308
# Squeeze singleton z so Cellpose gets a clean 2-D image
317309
squeeze = block.ndim == 3 and block.shape[0] == 1
310+
if block.ndim == 3 and not squeeze:
311+
raise ValueError(
312+
f"do_3D is False but this tile has {block.shape[0]} z-planes. "
313+
"Cellpose would receive the stack with no z_axis and treat "
314+
"the leading axis as channels. Set do_3D: true, or tile with "
315+
"z=1 to segment plane by plane."
316+
)
318317
img = block[0] if squeeze else block
319-
masks = _eval_with_oom_fallback(model, img, kwargs, cellpose_dict)
318+
masks = _eval_with_oom_fallback(img, kwargs, cellpose_dict)
320319
masks = masks.astype("int32")
321320
return masks[np.newaxis] if squeeze else masks
322321

0 commit comments

Comments
 (0)