You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
#206 fixed eval test by routing its loader through resolve_dataloader_workers(). The validation loader inside the training loop was not included, so the same starvation still happens once per epoch.
Observed
Watching an A30 during leech model train (binary charging dataset, 6,668,328 train / 1,176,763 val chunks, --num-workers 2, 15 epochs):
fully fed for a while, then dropped to 30% for 5 minutes, now back
That cadence matches a validation pass exactly. The eval reference point from #205 — 7,835,334 chunks at 8% GPU took ~35 min — scales to ~5.3 min for 1,176,763 val chunks, which is the observed drop.
Cost: ~5 min of near-idle GPU × 15 epochs ≈ 75 minutes per training arm. Our current sweep has 6 arms, so ≈ 7 GPU-hours of idle accelerator.
Cause
training.py:1490 resolves workers for the train loader:
training.py:1583-1600 then builds the val loader without it:
val_loader=Noneifval_datasetisnotNone:
# Validation __getitem__ is trivially fast (no augmentation, pre-tensorized# lookups only), so workers add memory overhead without benefit. Dropping# val workers halves the total process count and avoids OOM-triggered# segfaults on large multiclass datasets.val_loader_kwargs: dict= {"collate_fn": collate_fn, "num_workers": 0}
The reasoning has two halves that have aged differently:
"avoids OOM-triggered segfaults on large multiclass datasets" — a real constraint, and the reason a blanket num_workers > 0 is not the right fix.
"__getitem__ is trivially fast, so workers add no benefit" — this is the same assumption that produced eval: DataLoader num_workers is hardcoded to 0, leaving the GPU at 8% #205. __getitem__ being cheap does not mean one process can saturate a GPU: collate, pin, host-to-device and the forward pass all serialize onto that core. Measured, it cannot.
Note also that resolve_dataloader_workers's own docstring states "Every caller that builds a loader goes through this function, so that guard lives in one place." That invariant is currently false — which is presumably how this was missed.
Suggested fix
Route the val loader through resolve_dataloader_workers(num_workers, device) like the train and test loaders. It already handles both concerns in the comment above:
caps the auto count by sched_getaffinity, so a 2-core GPU job does not fork 8 workers
returns 0 in daemonic processes, so grid search is unaffected
If the multiclass OOM risk is specific to that path, gating on it explicitly would be clearer than a blanket 0 — the current code penalises every binary run to protect a multiclass case.
Other call sites that still bypass it
Asked to check for the same pattern elsewhere:
file
line
note
training.py
1595
the val loader above
calibration.py
193, 543
num_workers: int = 0 passed straight through; on CUDA that is literally 0, not AUTO
commands/benchmark.py
63
builds its own loader_kwargs
training.py
261, 265
the older SignalCNN path — no num_workers at all
gridsearch.py
340
probably fine — runs in daemonic pool workers, where the resolver returns 0 anyway
calibration.py is the one most likely to matter after the val loader, since it runs on CUDA with real data.
Caveat
I inferred the val pass from timing and GPU utilisation rather than instrumenting the loop directly, so the attribution is strong but circumstantial. The 5-minute period, the once-per-epoch cadence, and the val-set size all agree, but a debug! line timing the val pass would confirm it outright.
#206 fixed
eval testby routing its loader throughresolve_dataloader_workers(). The validation loader inside the training loop was not included, so the same starvation still happens once per epoch.Observed
Watching an A30 during
leech model train(binary charging dataset, 6,668,328 train / 1,176,763 val chunks,--num-workers 2, 15 epochs):That cadence matches a validation pass exactly. The eval reference point from #205 — 7,835,334 chunks at 8% GPU took ~35 min — scales to ~5.3 min for 1,176,763 val chunks, which is the observed drop.
Cost: ~5 min of near-idle GPU × 15 epochs ≈ 75 minutes per training arm. Our current sweep has 6 arms, so ≈ 7 GPU-hours of idle accelerator.
Cause
training.py:1490resolves workers for the train loader:training.py:1583-1600then builds the val loader without it:The reasoning has two halves that have aged differently:
num_workers > 0is not the right fix.__getitem__is trivially fast, so workers add no benefit" — this is the same assumption that produced eval: DataLoader num_workers is hardcoded to 0, leaving the GPU at 8% #205.__getitem__being cheap does not mean one process can saturate a GPU: collate, pin, host-to-device and the forward pass all serialize onto that core. Measured, it cannot.Note also that
resolve_dataloader_workers's own docstring states "Every caller that builds a loader goes through this function, so that guard lives in one place." That invariant is currently false — which is presumably how this was missed.Suggested fix
Route the val loader through
resolve_dataloader_workers(num_workers, device)like the train and test loaders. It already handles both concerns in the comment above:sched_getaffinity, so a 2-core GPU job does not fork 8 workersIf the multiclass OOM risk is specific to that path, gating on it explicitly would be clearer than a blanket 0 — the current code penalises every binary run to protect a multiclass case.
Other call sites that still bypass it
Asked to check for the same pattern elsewhere:
training.pycalibration.pynum_workers: int = 0passed straight through; on CUDA that is literally 0, not AUTOcommands/benchmark.pyloader_kwargstraining.pySignalCNNpath — nonum_workersat allgridsearch.pycalibration.pyis the one most likely to matter after the val loader, since it runs on CUDA with real data.Caveat
I inferred the val pass from timing and GPU utilisation rather than instrumenting the loop directly, so the attribution is strong but circumstantial. The 5-minute period, the once-per-epoch cadence, and the val-set size all agree, but a
debug!line timing the val pass would confirm it outright.