diff --git a/doc/changes/dev/14213.bugfix.rst b/doc/changes/dev/14213.bugfix.rst new file mode 100644 index 00000000000..c3920c2c003 --- /dev/null +++ b/doc/changes/dev/14213.bugfix.rst @@ -0,0 +1 @@ +Preserve caller-owned memory-mapped preload files after Raw objects are destroyed, by `Bruno Aristimunha`_. diff --git a/mne/io/array/_array.py b/mne/io/array/_array.py index ccb3062ca2b..c41853133db 100644 --- a/mne/io/array/_array.py +++ b/mne/io/array/_array.py @@ -31,6 +31,9 @@ class RawArray(BaseRaw): Determines what gets copied on instantiation. "auto" (default) will copy info, and copy "data" only if necessary to get to double floating point precision. + If ``data`` is a memory-mapped array and is not copied, the caller + retains ownership of its backing file and is responsible for removing + it after the Raw object is no longer in use. .. versionadded:: 0.18 %(verbose)s diff --git a/mne/io/base.py b/mne/io/base.py index 0019a3cdf6b..7d41bb20be6 100644 --- a/mne/io/base.py +++ b/mne/io/base.py @@ -2,7 +2,6 @@ # License: BSD-3-Clause # Copyright the MNE-Python contributors. -import os import shutil from collections import defaultdict from collections.abc import Callable, Sequence @@ -140,11 +139,12 @@ class BaseRaw( preload : bool | str | ndarray Preload data into memory for data manipulation and faster indexing. If True, the data will be preloaded into memory (fast, requires - large amount of memory). If preload is a string, preload is the - file name of a memory-mapped file which is used to store the data - on the hard drive (slower, requires less memory). If preload is an - ndarray, the data are taken from that array. If False, data are not - read until save. + large amount of memory). If preload is a string, it is the name of a + freshly created memory-mapped file used to store the data on the hard + drive (slower, requires less memory). An existing file is overwritten. + The caller owns the file and is responsible for removing it after the + Raw object is no longer in use. If preload is an ndarray, the data are + taken from that array. If False, data are not read until save. first_samps : sequence Sequence of the first sample number from each raw file. For unsplit raw files this should be a length-one list or tuple. @@ -602,8 +602,10 @@ def load_data( Parameters ---------- memmap : path-like | None - If not ``None``, preload data into a memory-mapped file at this - path. If ``None`` (default), preload data into RAM. + If not ``None``, preload data into a freshly created memory-mapped file + at this path. An existing file is overwritten. The caller owns the file + and is responsible for removing it after the Raw object is no longer in + use. If ``None`` (default), preload data into RAM. .. versionadded:: 1.13 %(verbose)s @@ -812,18 +814,6 @@ def set_annotations( return self - def __del__(self): # noqa: D105 - # remove file for memmap - fname = getattr(getattr(self, "_data", None), "filename", None) - if fname is not None: - # First, close the file out; happens automatically on del - del self._data - # Now file can be removed - try: - os.remove(fname) - except OSError: - pass # ignore file that no longer exists - def __enter__(self): """Entering with block.""" return self diff --git a/mne/io/edf/tests/test_edf.py b/mne/io/edf/tests/test_edf.py index 211641dc726..1672a131fc0 100644 --- a/mne/io/edf/tests/test_edf.py +++ b/mne/io/edf/tests/test_edf.py @@ -3,6 +3,7 @@ # Copyright the MNE-Python contributors. import datetime +import gc from contextlib import nullcontext from functools import partial from io import BytesIO @@ -317,6 +318,28 @@ def test_edf_data_broken(tmp_path): assert_allclose(data, data_new) +@pytest.mark.parametrize("method", ("constructor", "load_data")) +def test_edf_preload_memmap_ownership(method, tmp_path): + """Test ownership of a populated EDF preload memmap.""" + memmap_fname = tmp_path / f"edf-{method}-memmap.dat" + if method == "constructor": + raw = read_raw_edf(edf_stim_channel_path, preload=memmap_fname) + else: + raw = read_raw_edf(edf_stim_channel_path, preload=False) + raw.load_data(memmap=memmap_fname) + shape = raw._data.shape + expected = raw.get_data(picks=[0], start=0, stop=10)[0] + assert expected.any() + assert memmap_fname.stat().st_size == raw._data.nbytes + del raw + gc.collect() + + assert memmap_fname.is_file() + data = np.memmap(memmap_fname, dtype=np.float64, mode="r", shape=shape) + assert_array_equal(data[0, :10], expected) + del data + + def test_duplicate_channel_labels_edf(): """Test reading edf file with duplicate channel names.""" EXPECTED_CHANNEL_NAMES = ["EEG F1-Ref-0", "EEG F2-Ref", "EEG F1-Ref-1"] diff --git a/mne/io/fiff/tests/test_raw_fiff.py b/mne/io/fiff/tests/test_raw_fiff.py index 23b47812ed3..c996f187015 100644 --- a/mne/io/fiff/tests/test_raw_fiff.py +++ b/mne/io/fiff/tests/test_raw_fiff.py @@ -3,6 +3,7 @@ # Copyright the MNE-Python contributors. import datetime +import gc import os import pathlib import pickle @@ -315,7 +316,8 @@ def test_multiple_files(tmp_path): ) _compare_combo(raw, raw_combo, times, n_times) raw_combo = concatenate_raws( - [read_raw_fif(f) for f in [fif_fname, fif_fname]], preload="memmap8.dat" + [read_raw_fif(f) for f in [fif_fname, fif_fname]], + preload=tmp_path / "memmap8.dat", ) _compare_combo(raw, raw_combo, times, n_times) assert raw[:, :][0].shape[1] * 2 == raw_combo0[:, :][0].shape[1] @@ -349,13 +351,13 @@ def test_multiple_files(tmp_path): raw_combo = concatenate_raws( [read_raw_fif(fif_fname, preload=False), read_raw_fif(fif_fname, preload=True)], - preload="memmap3.dat", + preload=tmp_path / "memmap3.dat", ) _compare_combo(raw, raw_combo, times, n_times) raw_combo = concatenate_raws( [read_raw_fif(fif_fname, preload=True), read_raw_fif(fif_fname, preload=True)], - preload="memmap4.dat", + preload=tmp_path / "memmap4.dat", ) _compare_combo(raw, raw_combo, times, n_times) @@ -364,7 +366,7 @@ def test_multiple_files(tmp_path): read_raw_fif(fif_fname, preload=False), read_raw_fif(fif_fname, preload=False), ], - preload="memmap5.dat", + preload=tmp_path / "memmap5.dat", ) _compare_combo(raw, raw_combo, times, n_times) @@ -957,9 +959,9 @@ def test_io_complex(tmp_path, dtype): @testing.requires_testing_data -def test_getitem(): +def test_getitem(tmp_path): """Test getitem/indexing of Raw.""" - for preload in [False, True, "memmap1.dat"]: + for preload in [False, True, tmp_path / "memmap1.dat"]: raw = read_raw_fif(fif_fname, preload=preload) data, times = raw[0, :] data1, times1 = raw[0] @@ -1078,9 +1080,11 @@ def test_proj(tmp_path): @testing.requires_testing_data -@pytest.mark.parametrize("preload", [False, True, "memmap2.dat"]) +@pytest.mark.parametrize("preload", [False, True, "memmap"]) def test_preload_modify(preload, tmp_path): """Test preloading and modifying data.""" + if preload == "memmap": + preload = tmp_path / "memmap2.dat" rng = np.random.default_rng(0) raw = read_raw_fif(fif_fname, preload=preload) @@ -2030,6 +2034,9 @@ def test_memmap(tmp_path): raw_0._data[:] = 0.0 assert not raw_0._data.any() assert raw_1._data[:1, 3:5].all() + del raw_0, raw_1 + gc.collect() + assert Path(memmaps[3]).is_file() # other things like drop_channels and crop work but do not use memmapping, # eventually we might want to add support for some of these as users # require them. diff --git a/mne/io/tests/test_raw.py b/mne/io/tests/test_raw.py index 909ff9504ae..0f4b47d6e80 100644 --- a/mne/io/tests/test_raw.py +++ b/mne/io/tests/test_raw.py @@ -4,6 +4,7 @@ # License: BSD-3-Clause # Copyright the MNE-Python contributors. +import gc import math import re from contextlib import chdir, redirect_stdout @@ -838,16 +839,62 @@ def _read_raw_arange(preload=False, verbose=None): return _RawArange(preload, verbose) -def test_load_data_memmap(tmp_path): - """Test loading raw data into a memmap via load_data.""" - raw = _read_raw_arange(preload=False) - memmap_fname = tmp_path / "raw-load-data-memmap.dat" - raw.load_data(memmap=memmap_fname) +@pytest.mark.parametrize("method", ("constructor", "load_data")) +def test_preload_memmap_ownership(method, tmp_path): + """Test that a caller owns an explicit preload memmap path.""" + memmap_fname = tmp_path / f"raw-{method}-memmap.dat" + memmap_fname.write_bytes(b"stale" * 20_000) + if method == "constructor": + raw = _read_raw_arange(preload=memmap_fname) + else: + raw = _read_raw_arange(preload=False) + raw.load_data(memmap=memmap_fname) assert raw.preload assert isinstance(raw._data, np.memmap) assert Path(raw._data.filename) == memmap_fname - assert_array_equal(raw._data[:, 0], np.arange(1, 9)) + assert memmap_fname.stat().st_size == raw._data.nbytes + assert_array_equal(raw.get_data()[:, 0], np.arange(1, 9)) + raw.close() + assert_array_equal(raw.get_data()[:, 0], np.arange(1, 9)) + del raw + gc.collect() + + assert memmap_fname.is_file() + data = np.memmap(memmap_fname, dtype=np.float64, mode="r", shape=(8, 1000)) + assert_array_equal(data[:, 0], np.arange(1, 9)) + del data + replacement = tmp_path / "replacement.dat" + replacement.write_bytes(b"replacement") + replacement.replace(memmap_fname) + assert memmap_fname.read_bytes() == b"replacement" + + +def test_append_memmap_ownership(tmp_path): + """Test ownership of a caller-named concatenation memmap.""" + memmap_fname = tmp_path / "raw-append-memmap.dat" + raw = _read_raw_arange() + other = _read_raw_arange() + raw.append(other, preload=memmap_fname) + assert isinstance(raw._data, np.memmap) + expected = np.repeat(np.arange(1, 9)[:, np.newaxis], 2, axis=1) + assert_array_equal(raw.get_data()[:, [0, -1]], expected) + copied = raw.copy() + assert_array_equal(copied.get_data(), raw.get_data()) + del copied, other, raw + gc.collect() + assert memmap_fname.is_file() + + +def test_raw_array_memmap_ownership(tmp_path): + """Test ownership of a memmap supplied directly to RawArray.""" + memmap_fname = tmp_path / "raw-array-memmap.dat" + source = np.memmap(memmap_fname, dtype=np.float64, mode="w+", shape=(2, 10)) + source[:] = np.arange(20).reshape(2, 10) + raw = RawArray(source, create_info(2, 10.0), copy=None) + del source, raw + gc.collect() + assert memmap_fname.is_file() def test_test_raw_reader(): diff --git a/mne/utils/docs.py b/mne/utils/docs.py index 851952400b7..111ff21a112 100644 --- a/mne/utils/docs.py +++ b/mne/utils/docs.py @@ -3697,19 +3697,22 @@ def _reflow_param_docstring(docstring, has_first_line=True, width=75): preload : bool | str Preload data into memory for data manipulation and faster indexing. If True, the data will be preloaded into memory (fast, requires - large amount of memory). If preload is a string, preload is the - file name of a memory-mapped file which is used to store the data - on the hard drive (slower, requires less memory).""" + large amount of memory). If preload is a string, it is the name of a + freshly created memory-mapped file used to store the data on the hard + drive (slower, requires less memory). An existing file is overwritten. + The caller owns the file and is responsible for removing it after the + Raw object is no longer in use.""" docdict["preload_concatenate"] = """ preload : bool | str | None Preload data into memory for data manipulation and faster indexing. If True, the data will be preloaded into memory (fast, requires - large amount of memory). If preload is a string, preload is the - file name of a memory-mapped file which is used to store the data - on the hard drive (slower, requires less memory). If preload is - None, preload=True or False is inferred using the preload status - of the instances passed in. + large amount of memory). If preload is a string, it is the name of a + freshly created memory-mapped file used to store the data on the hard + drive (slower, requires less memory). An existing file is overwritten. + The caller owns the file and is responsible for removing it after the + Raw object is no longer in use. If preload is None, preload=True or False + is inferred using the preload status of the instances passed in. """ docdict["proj_epochs"] = """