Skip to content

Commit e3d4f24

Browse files
Guido Gagliardiclaude
andcommitted
fix: cap the memmap LRU below the fd limit, and let callers subset cases
Two failures found by running the loader over the real archive. The memmap LRU holds an open file descriptor per entry and defaults to 1000, which sits just under the usual 1024 soft RLIMIT_NOFILE. Past that, caching fails with a warning rather than an exception and every read falls back to recomputing from the source, so a cached dataset silently becomes an uncached one -- 50,176 such warnings in one run here. The size is now capped at 75 % of the actual limit, with a warning naming ulimit -n. VitalDBDataset also had no way to restrict membership: `target` decides labels only, so a caller holding a 40-case cohort or a train split still loaded all 5,269 cases. Adds a `cases` whitelist -- needed for training on a split at all, not just for probes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent eeac9f9 commit e3d4f24

3 files changed

Lines changed: 81 additions & 2 deletions

File tree

physioex/data/base.py

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ def __init__(
162162
# which are expensive on NFS (~0.16ms per call on network filesystems).
163163
# Cache key is (subject_id, physical_channel, pipeline_hash).
164164
self._memmap_cache: OrderedDict[str, np.ndarray] = OrderedDict()
165-
self._memmap_cache_size = int(memmap_cache_size)
165+
self._memmap_cache_size = self._safe_memmap_cache_size(memmap_cache_size)
166166
self._memmap_cache_hits = 0
167167
self._memmap_cache_misses = 0
168168

@@ -203,6 +203,40 @@ def __init__(
203203
# Subclass hooks
204204
# ------------------------------------------------------------------
205205

206+
@staticmethod
207+
def _safe_memmap_cache_size(requested: int) -> int:
208+
"""Cap the memmap LRU so it cannot exhaust the process file limit.
209+
210+
Every cached memmap holds an open file descriptor. The default of 1000
211+
sits just under the usual 1024 soft ``RLIMIT_NOFILE``, so a dataset with
212+
more than ~1000 (subject, channel) pairs runs the process out of
213+
descriptors. The symptom is not an exception: caching fails with a
214+
warning and every read falls back to recomputing from the source file,
215+
turning a cached dataset into an uncached one at a fraction of the speed,
216+
silently.
217+
218+
Leaves at least a quarter of the budget for everything else the process
219+
needs to open -- source files, the cache writes themselves, sockets.
220+
"""
221+
try:
222+
import resource
223+
224+
soft, _hard = resource.getrlimit(resource.RLIMIT_NOFILE)
225+
except Exception: # pragma: no cover - non-POSIX
226+
return int(requested)
227+
if soft in (resource.RLIM_INFINITY, -1):
228+
return int(requested)
229+
230+
budget = max(16, int(soft * 0.75))
231+
if requested > budget:
232+
logger.warning(
233+
f"memmap_cache_size={requested} exceeds the safe budget for a "
234+
f"file-descriptor limit of {soft}; capping at {budget}. Raise "
235+
f"the limit (ulimit -n) to cache more."
236+
)
237+
return budget
238+
return int(requested)
239+
206240
@abstractmethod
207241
def _list_subjects(self) -> List[SubjectSpec]:
208242
"""Return the subject manifest. Lightweight: must NOT load signal data."""

physioex/data/datasets/vitaldb.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@
3939
import csv
4040
import logging
4141
from pathlib import Path
42-
from typing import Any, Callable, Dict, List, Optional, Union
42+
from typing import Any, Callable, Dict, Iterable, List, Optional, Union
4343

4444
import numpy as np
4545

@@ -68,6 +68,10 @@ class VitalDBDataset(BasePhysioDataset):
6868
(the sentinel ignored by ``CrossEntropyLoss(ignore_index=-1)``).
6969
ane_type: keep only cases with this ``ane_type`` in ``cases.csv``.
7070
Defaults to ``"General"``; pass ``None`` to keep every case.
71+
cases: restrict the dataset to these case ids. Without it every case
72+
on disk is listed, so a caller holding a cohort or a split would
73+
silently load the whole archive; ``target`` only decides labels,
74+
not membership.
7175
require_tracks: cases must carry all of these tracks (checked against
7276
``meta/trks.csv``, no file opening). Defaults to the EEG channel.
7377
epoch_length_sec: defaults to 120 s -- the 2-minute window used by the
@@ -106,6 +110,7 @@ def __init__(
106110
root: Optional[str] = None,
107111
target: TargetSpec = None,
108112
ane_type: Optional[str] = "General",
113+
cases: Optional[Iterable[str]] = None,
109114
require_tracks: tuple = (EEG1,),
110115
**kwargs,
111116
):
@@ -116,6 +121,7 @@ def __init__(
116121
# _list_subjects() and then _read_subject_labels().
117122
self._target = target
118123
self._ane_type = ane_type
124+
self._case_filter = {str(c) for c in cases} if cases is not None else None
119125
self._require_tracks = tuple(require_tracks)
120126
self._cases: Dict[str, Dict[str, Any]] = {}
121127
# caseid -> resolved target, filled eagerly in _list_subjects
@@ -186,6 +192,8 @@ def _list_subjects(self) -> List[SubjectSpec]:
186192

187193
subjects: List[SubjectSpec] = []
188194
for caseid, row in self._cases.items():
195+
if self._case_filter is not None and caseid not in self._case_filter:
196+
continue
189197
if self._ane_type is not None and row.get("ane_type") != self._ane_type:
190198
continue
191199
if with_tracks is not None and caseid not in with_tracks:

tests/test_vitaldb_dataset.py

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -298,3 +298,40 @@ def test_real_vitaldb_smoke():
298298
assert ds.get_n_subjects() > 5_000
299299
item = ds[0]
300300
assert torch.is_tensor(item["labels"])
301+
302+
303+
def test_cases_whitelist_restricts_membership(vitaldb_root, tmp_path):
304+
"""Without this, holding a cohort or a split still loads every case.
305+
306+
``target`` only decides labels: a caller that passed a 40-case cohort and
307+
expected a 40-case dataset would quietly get the whole archive.
308+
"""
309+
ds = make_dataset(vitaldb_root, tmp_path / "c1", cases=["1", "3"])
310+
assert ds.get_subjects() == ["1", "3"]
311+
assert make_dataset(vitaldb_root, tmp_path / "c2").get_subjects() == ["1", "2", "3"]
312+
313+
314+
def test_cases_whitelist_ignores_ids_not_on_disk(vitaldb_root, tmp_path):
315+
ds = make_dataset(vitaldb_root, tmp_path / "cache", cases=["1", "9999"])
316+
assert ds.get_subjects() == ["1"]
317+
318+
319+
def test_memmap_cache_is_capped_below_the_file_descriptor_limit(vitaldb_root, tmp_path):
320+
"""Every cached memmap holds an fd; the default 1000 sits under a 1024 limit.
321+
322+
Exhausting them does not raise -- caching fails with a warning and every
323+
read recomputes from source, so a cached dataset silently becomes an
324+
uncached one.
325+
"""
326+
import resource
327+
328+
soft, _ = resource.getrlimit(resource.RLIMIT_NOFILE)
329+
ds = make_dataset(vitaldb_root, tmp_path / "cache", memmap_cache_size=1_000_000)
330+
if soft not in (resource.RLIM_INFINITY, -1):
331+
assert ds._memmap_cache_size < soft
332+
assert ds._memmap_cache_size == max(16, int(soft * 0.75))
333+
334+
335+
def test_a_modest_memmap_cache_size_is_left_alone(vitaldb_root, tmp_path):
336+
ds = make_dataset(vitaldb_root, tmp_path / "cache", memmap_cache_size=8)
337+
assert ds._memmap_cache_size == 8

0 commit comments

Comments
 (0)