From 7e9d47c6feb26a834f35facf0358558b895a3c94 Mon Sep 17 00:00:00 2001 From: aconesac Date: Mon, 18 May 2026 19:26:11 +0200 Subject: [PATCH 1/3] test: add failing tests for issue #744 (data_path collision) Demonstrates the two bugs before the fix: - options_to_kwargs() always forwarded data_path='/tmp', causing every run to target /tmp/kwave_input.h5 and crash on the second call - prepare() gave a cryptic h5py ValueError instead of a clear FileExistsError when the input file already existed --- tests/test_compat.py | 12 ++++++++++++ tests/test_cpp_simulation.py | 10 ++++++++++ 2 files changed, 22 insertions(+) diff --git a/tests/test_compat.py b/tests/test_compat.py index 498b2001..9995af89 100644 --- a/tests/test_compat.py +++ b/tests/test_compat.py @@ -91,3 +91,15 @@ def test_both_options(self): def test_none_options(self): kwargs = options_to_kwargs() assert kwargs == {} + + def test_default_data_path_not_forwarded(self): + # Before fix: data_path defaulted to gettempdir() and was always forwarded, + # so every run targeted /tmp/kwave_input.h5 and crashed on the second call. + kwargs = options_to_kwargs(simulation_options=SimulationOptions()) + assert "data_path" not in kwargs + + def test_custom_data_path_is_forwarded(self, tmp_path): + opts = SimulationOptions() + opts.data_path = str(tmp_path) + kwargs = options_to_kwargs(simulation_options=opts) + assert kwargs["data_path"] == str(tmp_path) diff --git a/tests/test_cpp_simulation.py b/tests/test_cpp_simulation.py index c2171028..b99a0062 100644 --- a/tests/test_cpp_simulation.py +++ b/tests/test_cpp_simulation.py @@ -10,6 +10,16 @@ from kwave.solvers.cpp_simulation import CppSimulation +class TestPrepare: + def test_raises_when_input_already_exists(self, tmp_path): + # Before fix: h5py raised a cryptic ValueError('name already exists') + # instead of a clear FileExistsError. + (tmp_path / "kwave_input.h5").write_bytes(b"") + sim = CppSimulation.__new__(CppSimulation) + with pytest.raises(FileExistsError, match="already exists"): + sim.prepare(data_path=str(tmp_path)) + + class TestResolveBinaryPath: """Tests for CppSimulation._resolve_binary_path(). From 0a38e98383fc03274e85c246e7cd30971ac163e6 Mon Sep 17 00:00:00 2001 From: aconesac Date: Mon, 18 May 2026 11:58:33 +0200 Subject: [PATCH 2/3] fix: prevent HDF5 dataset collision when data_path is reused options_to_kwargs() was always forwarding data_path='/tmp' (the SimulationOptions default), bypassing the safe mkdtemp path in prepare() and causing h5py to crash with 'name already exists' on any second run. - compat.py: skip forwarding data_path when it equals gettempdir() - cpp_simulation.py: raise FileExistsError with a clear message when an explicit data_path already contains kwave_input.h5 Fixes #744 --- kwave/compat.py | 5 ++++- kwave/solvers/cpp_simulation.py | 17 ++++++++++++++++- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/kwave/compat.py b/kwave/compat.py index a077c8fc..1b7cbff9 100644 --- a/kwave/compat.py +++ b/kwave/compat.py @@ -39,7 +39,10 @@ def options_to_kwargs(simulation_options=None, execution_options=None): kwargs["use_kspace"] = opts.use_kspace kwargs["smooth_p0"] = opts.smooth_p0 if opts.data_path is not None: - kwargs["data_path"] = opts.data_path + from tempfile import gettempdir + + if opts.data_path != gettempdir(): + kwargs["data_path"] = opts.data_path if opts.save_to_disk_exit: kwargs["save_only"] = True diff --git a/kwave/solvers/cpp_simulation.py b/kwave/solvers/cpp_simulation.py index fe465c7c..2f17f747 100644 --- a/kwave/solvers/cpp_simulation.py +++ b/kwave/solvers/cpp_simulation.py @@ -4,6 +4,7 @@ Handles HDF5 serialization and C++ binary execution without depending on kWaveSimulation or the legacy options classes. """ + import os import shutil import stat @@ -46,6 +47,11 @@ def prepare(self, data_path=None): input_file = os.path.join(data_path, "kwave_input.h5") output_file = os.path.join(data_path, "kwave_output.h5") + if os.path.exists(input_file): + raise FileExistsError( + f"{input_file!r} already exists. Delete it or choose a different data_path to avoid overwriting previous simulation inputs." + ) + self._write_hdf5(input_file) return input_file, output_file @@ -56,7 +62,16 @@ def run(self, *, device="cpu", num_threads=None, device_num=None, quiet=False, d input_file, output_file = self.prepare(data_path=data_path) data_dir = os.path.dirname(input_file) try: - self._execute(input_file, output_file, device=device, num_threads=num_threads, device_num=device_num, quiet=quiet, debug=debug, binary_path=binary_path) + self._execute( + input_file, + output_file, + device=device, + num_threads=num_threads, + device_num=device_num, + quiet=quiet, + debug=debug, + binary_path=binary_path, + ) result = self._parse_output(output_file) result = self._fix_output_order(result) return result From edc928af07077920d837624c398b9e0a6d662292 Mon Sep 17 00:00:00 2001 From: Agustin Conesa-Celdran <146850214+aconesac@users.noreply.github.com> Date: Mon, 18 May 2026 19:49:05 +0200 Subject: [PATCH 3/3] Potential fix for pull request finding normalize both paths before comparing Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- kwave/compat.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/kwave/compat.py b/kwave/compat.py index 1b7cbff9..70f39391 100644 --- a/kwave/compat.py +++ b/kwave/compat.py @@ -39,9 +39,12 @@ def options_to_kwargs(simulation_options=None, execution_options=None): kwargs["use_kspace"] = opts.use_kspace kwargs["smooth_p0"] = opts.smooth_p0 if opts.data_path is not None: + import os from tempfile import gettempdir - if opts.data_path != gettempdir(): + normalized_data_path = os.path.realpath(os.path.normpath(os.fspath(opts.data_path))) + normalized_tempdir = os.path.realpath(os.path.normpath(os.fspath(gettempdir()))) + if normalized_data_path != normalized_tempdir: kwargs["data_path"] = opts.data_path if opts.save_to_disk_exit: kwargs["save_only"] = True