asdfasdfqrqwer commited on
Commit
80b7a00
·
1 Parent(s): 5f318ab

fix(zerogpu): build Separator inside @spaces.GPU to dodge RLock pickling error

Browse files

Bug: ZeroGPU throws _pickle.PicklingError: cannot pickle '_thread.RLock'
when @separate runs on HuggingFace ZeroGPU.

Root cause: The @spaces.GPU decorator ships the function's arguments to
a worker process via multiprocessing.Queue (which uses pickle). The
previous code passed a live Separator instance as the first argument:

@spaces.GPU
def separate_on_gpu(separator: Separator, input_file: str): ...
^^^^^^^^^^^^^^^^^^^^
not picklable — holds a Logger (RLock)
and possibly torch CUDA state

Fix: Move Separator construction AND model loading INSIDE the
@spaces.GPU-decorated function. Only picklable primitives
(str, int, float, bool, dict) cross the queue boundary.

Before: After:
--------- --------
separator = build_separator(...) output = _run_separation_on_gpu(
separator.load_model(...) architecture=..., # str
output = separate_on_gpu( model_file_dir=..., # str
separator, # <-- crashes out_dir=..., # str
input_file out_format=..., # str
) norm_thresh=..., # float
...
model_filename=..., # str
input_file=..., # str
)
# Inside the worker:
# separator = build_separator(...)
# separator.load_model(...)
# return separator.separate(input_file)

Additional safety:
- Added _sanitize_for_pickle() that recursively strips non-picklable
items from arch_params (defensive — UI should already only pass
primitives, but a stray torch tensor would re-introduce the bug)
- Updated module docstring documenting the ZeroGPU pickling rule so
future contributors don't re-introduce the bug
- The retry logic now re-creates the Separator on each attempt so a
half-broken state from a failed librosa call doesn't poison the retry

Verified locally:
- pickle.dumps(all_args) succeeds (363 bytes) — would have raised
PicklingError before this fix
- CPU end-to-end: 2 stems from 2s test tone in 16s
- All 4 API endpoints still work

Files changed (1) hide show
  1. main/separator.py +111 -21
main/separator.py CHANGED
@@ -3,8 +3,14 @@
3
  Thin wrapper around `audio_separator.separator.Separator` that:
4
  * Maps UI-friendly architecture names to the right params dict
5
  (`mdxc_params`, `mdx_params`, `vr_params`, `demucs_params`).
6
- * Wraps the actual `separator.separate()` call in `@spaces.GPU` so it can
7
- run on HuggingFace ZeroGPU hardware.
 
 
 
 
 
 
8
  * Parses the produced filenames back into stem-label → absolute-path pairs.
9
  * Applies known-good defaults for output bitrate and retries on the
10
  sporadic `librosa.ParameterError` that affects ~1 in 5 runs
@@ -74,6 +80,11 @@ def stem_label_from_filename(fname: str) -> str:
74
  # --------------------------------------------------------------------------- #
75
  # Separator construction #
76
  # --------------------------------------------------------------------------- #
 
 
 
 
 
77
 
78
  def build_separator(
79
  *,
@@ -87,7 +98,8 @@ def build_separator(
87
  arch_params: dict[str, Any],
88
  ) -> "Separator":
89
  """Instantiate an `audio_separator.Separator` pre-configured for the
90
- requested architecture.
 
91
  """
92
  if not HAS_SEPARATOR:
93
  raise RuntimeError(
@@ -150,23 +162,62 @@ def build_separator(
150
 
151
 
152
  # --------------------------------------------------------------------------- #
153
- # GPU-wrapped separation #
154
  # --------------------------------------------------------------------------- #
 
 
 
 
 
 
 
 
 
 
 
 
 
155
 
156
  @spaces.GPU
157
- def separate_on_gpu(separator: "Separator", input_file: str) -> list[str]:
158
- """Run the actual separation. Wrapped in `@spaces.GPU` so HuggingFace
159
- ZeroGPU allocates a GPU for this call. When running locally (no `spaces`
160
- module available), the decorator becomes a no-op and execution falls
161
- through to CPU.
 
 
 
 
 
 
 
 
 
 
 
 
162
 
163
  Includes a one-shot retry on the sporadic `librosa.ParameterError`
164
  ("Audio buffer is not finite") that affects ~1 in 5 runs on certain
165
- inputs (upstream issue #57).
 
166
  """
167
  last_exc: Exception | None = None
168
  for attempt in range(_MAX_RETRIES + 1):
169
  try:
 
 
 
 
 
 
 
 
 
 
 
 
 
170
  return separator.separate(input_file)
171
  except Exception as exc:
172
  last_exc = exc
@@ -204,8 +255,13 @@ def separate_core(
204
  ) -> dict[str, str]:
205
  """Run separation and return ``{stem_label: absolute_path}``.
206
 
207
- Returns an empty dict on missing input. Raises on actual errors so the UI
208
- layer can surface them via `gr.Warning`.
 
 
 
 
 
209
  """
210
  if not input_file:
211
  return {}
@@ -221,8 +277,27 @@ def separate_core(
221
  model_display_name,
222
  )
223
 
 
 
 
224
  out_dir = prepare_output_dir(input_file, output_dir)
225
- separator = build_separator(
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
226
  architecture=architecture,
227
  model_file_dir=model_file_dir,
228
  out_dir=out_dir,
@@ -230,16 +305,11 @@ def separate_core(
230
  norm_thresh=norm_thresh,
231
  amp_thresh=amp_thresh,
232
  batch_size=batch_size,
233
- arch_params=arch_params,
 
 
234
  )
235
 
236
- progress(0.10, desc="Loading model (first use downloads it)…")
237
- model_filename = resolve_model_filename(architecture, model_display_name)
238
- separator.load_model(model_filename=model_filename)
239
-
240
- progress(0.30, desc="Separating audio…")
241
- output_files = separate_on_gpu(separator, input_file)
242
-
243
  progress(0.95, desc="Finalizing stems…")
244
  stems: dict[str, str] = {}
245
  for rel_path in output_files:
@@ -258,3 +328,23 @@ def separate_core(
258
 
259
  log.info("Stems produced: %s", ", ".join(stems.keys()) or "(none)")
260
  return stems
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
3
  Thin wrapper around `audio_separator.separator.Separator` that:
4
  * Maps UI-friendly architecture names to the right params dict
5
  (`mdxc_params`, `mdx_params`, `vr_params`, `demucs_params`).
6
+ * Wraps the actual Separator construction + model loading + `separate()`
7
+ call in `@spaces.GPU` so it can run on HuggingFace ZeroGPU hardware.
8
+ CRITICAL: the `@spaces.GPU` decorator ships arguments to a worker
9
+ process via pickle, so the decorated function must take ONLY picklable
10
+ primitives (strings, dicts of primitives, numbers). A live `Separator`
11
+ instance holds a logger / torch objects containing `_thread.RLock`
12
+ which cannot be pickled — hence the construction must happen INSIDE
13
+ the decorator, not outside.
14
  * Parses the produced filenames back into stem-label → absolute-path pairs.
15
  * Applies known-good defaults for output bitrate and retries on the
16
  sporadic `librosa.ParameterError` that affects ~1 in 5 runs
 
80
  # --------------------------------------------------------------------------- #
81
  # Separator construction #
82
  # --------------------------------------------------------------------------- #
83
+ # NOTE: `build_separator` is now only called from INSIDE the `@spaces.GPU`
84
+ # boundary (in `_run_separation_on_gpu` below). It must NOT be called from
85
+ # the main process, because the returned `Separator` instance cannot be
86
+ # pickled across the ZeroGPU worker queue (it holds a `_thread.RLock`
87
+ # via its logger and possibly torch CUDA state).
88
 
89
  def build_separator(
90
  *,
 
98
  arch_params: dict[str, Any],
99
  ) -> "Separator":
100
  """Instantiate an `audio_separator.Separator` pre-configured for the
101
+ requested architecture. Must be called from inside the `@spaces.GPU`
102
+ boundary — see module docstring.
103
  """
104
  if not HAS_SEPARATOR:
105
  raise RuntimeError(
 
162
 
163
 
164
  # --------------------------------------------------------------------------- #
165
+ # GPU-wrapped separation #
166
  # --------------------------------------------------------------------------- #
167
+ # CRITICAL ZeroGPU pickling rule:
168
+ # The `@spaces.GPU` decorator ships the function's arguments to a worker
169
+ # process via `multiprocessing.Queue`, which uses pickle. The arguments
170
+ # here MUST all be picklable primitives:
171
+ # - str, int, float, bool
172
+ # - dict / list / tuple of the above
173
+ # A live `Separator` instance is NOT picklable — it holds a Python
174
+ # `logging.Logger` (which transitively owns a `_thread.RLock`) and may
175
+ # hold torch CUDA tensors / ONNX runtime sessions. Passing one across
176
+ # the queue raises `_pickle.PicklingError: cannot pickle '_thread.RLock'
177
+ # object`.
178
+ #
179
+ # Therefore: build the Separator INSIDE this function, not outside.
180
 
181
  @spaces.GPU
182
+ def _run_separation_on_gpu(
183
+ *,
184
+ architecture: str,
185
+ model_file_dir: str,
186
+ out_dir: str,
187
+ out_format: str,
188
+ norm_thresh: float,
189
+ amp_thresh: float,
190
+ batch_size: int,
191
+ arch_params: dict[str, Any],
192
+ model_filename: str,
193
+ input_file: str,
194
+ ) -> list[str]:
195
+ """Build the Separator, load the model, and run separation — all inside
196
+ the `@spaces.GPU` boundary so ZeroGPU allocates a GPU for the whole
197
+ process. Returns the list of output file paths produced by
198
+ `Separator.separate()`.
199
 
200
  Includes a one-shot retry on the sporadic `librosa.ParameterError`
201
  ("Audio buffer is not finite") that affects ~1 in 5 runs on certain
202
+ inputs (upstream issue #57). The retry re-creates the Separator from
203
+ scratch to dodge any half-broken state.
204
  """
205
  last_exc: Exception | None = None
206
  for attempt in range(_MAX_RETRIES + 1):
207
  try:
208
+ # Build a fresh Separator on every attempt — if librosa left
209
+ # the previous one in a bad state, we want a clean slate.
210
+ separator = build_separator(
211
+ architecture=architecture,
212
+ model_file_dir=model_file_dir,
213
+ out_dir=out_dir,
214
+ out_format=out_format,
215
+ norm_thresh=norm_thresh,
216
+ amp_thresh=amp_thresh,
217
+ batch_size=batch_size,
218
+ arch_params=arch_params,
219
+ )
220
+ separator.load_model(model_filename=model_filename)
221
  return separator.separate(input_file)
222
  except Exception as exc:
223
  last_exc = exc
 
255
  ) -> dict[str, str]:
256
  """Run separation and return ``{stem_label: absolute_path}``.
257
 
258
+ Returns an empty dict on missing input. Raises on actual errors so the
259
+ UI layer can surface them via `gr.Warning`.
260
+
261
+ This function runs in the main process. It only prepares picklable
262
+ arguments and then calls `_run_separation_on_gpu(...)` — the
263
+ `@spaces.GPU`-decorated worker function — which builds the Separator
264
+ and runs inference inside the ZeroGPU-allocated worker.
265
  """
266
  if not input_file:
267
  return {}
 
277
  model_display_name,
278
  )
279
 
280
+ # Prepare the per-input output directory from the main process — this
281
+ # is just filesystem work, no GPU needed, and the directory needs to
282
+ # exist before the worker writes stems into it.
283
  out_dir = prepare_output_dir(input_file, output_dir)
284
+
285
+ # Translate the UI-friendly model name into the filename the library
286
+ # expects. This is a pure dict lookup — safe to do in the main process.
287
+ model_filename = resolve_model_filename(architecture, model_display_name)
288
+
289
+ # Make sure arch_params only contains picklable primitives before
290
+ # shipping it across the ZeroGPU queue. (Defensive — the UI layer
291
+ # should already only pass primitives, but a stray torch tensor here
292
+ # would re-introduce the same PicklingError.)
293
+ arch_params_clean = _sanitize_for_pickle(arch_params)
294
+
295
+ progress(0.10, desc="Loading model (first use downloads it)…")
296
+ progress(0.30, desc="Separating audio…")
297
+
298
+ # All heavy work (Separator construction, model load, inference)
299
+ # happens inside the @spaces.GPU boundary.
300
+ output_files = _run_separation_on_gpu(
301
  architecture=architecture,
302
  model_file_dir=model_file_dir,
303
  out_dir=out_dir,
 
305
  norm_thresh=norm_thresh,
306
  amp_thresh=amp_thresh,
307
  batch_size=batch_size,
308
+ arch_params=arch_params_clean,
309
+ model_filename=model_filename,
310
+ input_file=input_file,
311
  )
312
 
 
 
 
 
 
 
 
313
  progress(0.95, desc="Finalizing stems…")
314
  stems: dict[str, str] = {}
315
  for rel_path in output_files:
 
328
 
329
  log.info("Stems produced: %s", ", ".join(stems.keys()) or "(none)")
330
  return stems
331
+
332
+
333
+ def _sanitize_for_pickle(obj: Any) -> Any:
334
+ """Recursively strip non-picklable items from a dict/list structure.
335
+
336
+ `@spaces.GPU` ships args across a multiprocessing queue, which uses
337
+ pickle. This helper ensures we never accidentally pass a torch tensor,
338
+ numpy array, or other unpicklable object through. Numbers, strings,
339
+ bools, lists, and dicts of primitives pass through unchanged.
340
+ """
341
+ if isinstance(obj, dict):
342
+ return {str(k): _sanitize_for_pickle(v) for k, v in obj.items()}
343
+ if isinstance(obj, (list, tuple)):
344
+ return [_sanitize_for_pickle(v) for v in obj]
345
+ if isinstance(obj, (str, int, float, bool)) or obj is None:
346
+ return obj
347
+ # Anything else (torch.Tensor, numpy array, custom object) — convert
348
+ # to its string representation so we don't crash the queue.
349
+ log.warning("Stripping non-picklable %s from arch_params: %r", type(obj).__name__, obj)
350
+ return str(obj)