Merge branch 'master' into fix-binary-executable-mode-change

Makefile

@@ -0,0 +1,44 @@
+# Define the Python interpreter and MATLAB command
+PYTHON = python
+MATLAB = matlab
+
+# Define the directories and files
+EXAMPLES_DIR = examples
+TESTS_DIR = tests
+MATLAB_SCRIPT = tests/matlab_test_data_collectors/run_all_collectors.m
+KWAVE_MATLAB_PATH = $(abspath ../k-wave) # Get absolute path of k-wave directory
+
+# Define the artifact directory
+COLLECTED_VALUES_DIR = $(abspath tests/matlab_test_data_collectors/python_testers/collectedValues)
+
+# Default target
+all: run-examples test
+
+# Target to run all examples
+run-examples:
+	@echo "Running all examples..."
+	@MPLBACKEND=Agg $(PYTHON) run_examples.py
+
+# Target to run pytest, which depends on running the MATLAB script first
+test: $(COLLECTED_VALUES_DIR)
+	@echo "Running pytest..."
+	@pytest $(TESTS_DIR)
+
+# Target to run the MATLAB script and create the artifact directory
+$(COLLECTED_VALUES_DIR): 
+	@echo "Running MATLAB script to collect values..."; \
+	$(MATLAB) -batch "run('$(MATLAB_SCRIPT)');"; \
+# Clean target (optional) - cleans Python caches and collected values
+clean: clean-python clean-collected_values
+
+clean-python:
+	@echo "Cleaning Python cache files..."
+	@find . -name '*.pyc' -delete
+	@find . -name '__pycache__' -delete
+
+# Clean collected values directory
+clean-collected_values:
+	@echo "Cleaning collected values directory..."
+	@rm -rf $(COLLECTED_VALUES_DIR)
+
+.PHONY: all run-examples run-tests clean-python clean-collected_values clean

docs/development/development_environment.rst

@@ -21,6 +21,46 @@ Ensure pre-commit is configured by running the following command:
 
     pre-commit install
 
+Running Tests
+=======================
+Assuming matlab is installed locally, and `k-wave <https://github.com/ucl-bug/k-wave>` is installed in a parallel directory, testing can be performed using the make file located in the project root.
+.. code-block:: bash
+
+    make test
+
+This process will first generate refernce files in matlab and run the complete python test suite against them.
+
+To run the tests manually after reference generation, use the following command:    
+
+.. code-block:: bash
+
+    pytest 
+
+To run the tests with coverage, use the following command:
+
+.. code-block:: bash
+
+    coverage run
+
+To run all examples, to ensure they still run after changes use the following command:
+.. code-block:: bash
+    make run-examples
+
+or
+
+.. code-block:: bash
+
+    MPLBACKEND=Agg python run_examples.py
+
+
+
+If you want to force the examples to run on the cpu:
+
+.. code-block:: bash
+
+    MPLBACKEND=Agg KWAVE_FORCE_CPU=1 python run_examples.py
+
+
 Test References
 =======================
 
@@ -42,7 +82,7 @@ These tests are located in the ``tests`` directory. The comparison between ``mat
   These ``.json`` files are stored in the code repository and do not need to be regenerated.
   Since these files are generated from the original k-Wave package, they only need to be updated when a new release of k-Wave is made.
 
-**Matlab reference file generation** is a bit involved process. Below are the steps that describe the process.
+**Matlab reference file generation** can be described in the following steps.
 
 #. Open desired example in matlab, e.g. `example_pr_2D_TR_directional_sensors.m <https://github.com/ucl-bug/k-wave/blob/main/k-Wave/examples/example_pr_2D_TR_directional_sensors.m>`_
 #. Find the lines where the call to one of the `kSpaceFirstOrder-family` function is made. For example,
@@ -63,14 +103,3 @@ These tests are located in the ``tests`` directory. The comparison between ``mat
         https://github.com/waltsims/k-wave-python/blob/1f9df5d987d0b3edb1a8a43fad0885d3d6079029/tests/h5_summary.py#L97-L106
 
 
-To run the tests, use the following command:    
-
-.. code-block:: bash
-
-    pytest 
-
-To run the tests with coverage, use the following command:
-
-.. code-block:: bash
-
-    coverage run

kwave/executor.py

@@ -1,3 +1,4 @@
+import os
 import stat
 import subprocess
 import sys
@@ -14,6 +15,10 @@ def __init__(self, execution_options: SimulationExecutionOptions, simulation_opt
         self.execution_options = execution_options
         self.simulation_options = simulation_options
 
+        if os.environ.get("KWAVE_FORCE_CPU") == "1":
+            self.execution_options.is_gpu_simulation = False
+            self.execution_options.binary_name = "kspaceFirstOrder-OMP"
+            self.execution_options.binary_path = kwave.BINARY_PATH / self.execution_options.binary_name
         self._make_binary_executable()
 
     def _make_binary_executable(self):
@@ -28,13 +33,12 @@ def _make_binary_executable(self):
         binary_path.chmod(binary_path.stat().st_mode | stat.S_IEXEC)
 
     def run_simulation(self, input_filename: str, output_filename: str, options: str):
-        command = (
-            f"{self.execution_options.system_string}"
-            f'"{self.execution_options.binary_path}" -i {input_filename} -o {output_filename} {options}'
-        )
+        command = [str(self.execution_options.binary_path), "-i", input_filename, "-o", output_filename, options]
 
         try:
-            with subprocess.Popen(command, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=True, text=True) as proc:
+            with subprocess.Popen(
+                command, env=self.execution_options.env_vars, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True
+            ) as proc:
                 stdout, stderr = "", ""
                 if self.execution_options.show_sim_log:
                     # Stream stdout in real-time

kwave/options/simulation_execution_options.py

@@ -1,9 +1,9 @@
 from dataclasses import dataclass
+import os
 from typing import Optional, Union
 
 from kwave import PLATFORM, BINARY_PATH
 from kwave.ksensor import kSensor
-from kwave.utils.checks import is_unix
 
 
 @dataclass
@@ -117,34 +117,22 @@ def get_options_string(self, sensor: kSensor) -> str:
         return options_string
 
     @property
-    def system_string(self):
-        # set OS string for setting environment variables
-        if is_unix():
-            env_set_str = ""
-            sys_sep_str = " "
-        else:
-            env_set_str = "set "
-            sys_sep_str = " & "
+    def env_vars(self) -> dict:
+        env = os.environ
 
-        # set system string to define domain for thread migration
-        system_string = env_set_str
         if PLATFORM != "darwin":
-            system_string += "OMP_PLACES=cores" + sys_sep_str
+            env.update({"OMP_PLACES": "cores"})
 
         if self.thread_binding is not None:
             if PLATFORM == "darwin":
                 raise ValueError("Thread binding is not supported in MacOS.")
             # read the parameters and update the system options
             if self.thread_binding:
-                system_string = system_string + " " + env_set_str + "OMP_PROC_BIND=SPREAD" + sys_sep_str
+                env.update({"OMP_PROC_BIND": "SPREAD"})
             else:
-                system_string = system_string + " " + env_set_str + "OMP_PROC_BIND=CLOSE" + sys_sep_str
+                env.update({"OMP_PROC_BIND": "CLOSE"})
         else:
             if PLATFORM != "darwin":
-                # set to round-robin over places
-                system_string = system_string + " " + env_set_str + "OMP_PROC_BIND=SPREAD" + sys_sep_str
-
-        if self.system_call:
-            system_string = system_string + " " + self.system_call + sys_sep_str
+                env.update({"OMP_PROC_BIND": "SPREAD"})
 
-        return system_string
+        return env

run_examples.py

@@ -0,0 +1,26 @@
+import os
+from pathlib import Path
+import subprocess
+
+
+def run_python_files(directory):
+    # Recursively walk through the directory
+    for root, _, files in os.walk(directory):
+        for file in files:
+            # Check if it's a Python file
+            if file.endswith(".py"):
+                file_path = os.path.join(root, file)
+                print(f"Running: {file_path}")
+                try:
+                    # Use subprocess to run the Python file
+                    result = subprocess.run(["python", file_path], capture_output=True, text=True)
+                    print(f"Output:\n{result.stdout}")
+                    if result.stderr:
+                        print(f"Errors:\n{result.stderr}")
+                except Exception as e:
+                    print(f"Failed to run {file_path}: {e}")
+
+
+if __name__ == "__main__":
+    directory = Path("examples/")
+    run_python_files(directory)

tests/matlab_test_data_collectors/run_all_collectors.m

@@ -1,5 +1,6 @@
 clear all;
-
+% ensure k-wave is on the path
+addpath(genpath('../../../k-wave'));
 directory = pwd + "/matlab_collectors";
 files = getListOfFiles(directory);
 % remove this file.

tests/test_executor.py

@@ -99,9 +99,11 @@ def test_run_simulation_success(self):
             sensor_data = executor.run_simulation("input.h5", "output.h5", "options")
 
         normalized_path = os.path.normpath(self.execution_options.binary_path)
-        expected_command = f"{self.execution_options.system_string}" f'"{normalized_path}" -i input.h5 ' f"-o output.h5 " f"options"
+        expected_command = [normalized_path, "-i", "input.h5", "-o", "output.h5", "options"]
 
-        self.mock_popen.assert_called_once_with(expected_command, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=True, text=True)
+        self.mock_popen.assert_called_once_with(
+            expected_command, env=self.execution_options.env_vars, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True
+        )
         self.mock_proc.communicate.assert_called_once()
         self.assertEqual(sensor_data, dotdict())
 
@@ -149,38 +151,57 @@ def test_parse_executable_output(self):
         self.assertIn("data", result)
         self.assertTrue(np.all(result["data"] == np.ones((10, 10))))
 
-
-def test_executor_file_not_found_on_non_darwin():
-    # Configure the mock path object
-    mock_binary_path = MagicMock(spec=Path)
-    mock_binary_path.chmod.side_effect = FileNotFoundError
-
-    # Mock the execution options to use the mocked path
-    mock_execution_options = MagicMock()
-    mock_execution_options.binary_path = mock_binary_path
-    mock_execution_options.is_gpu_simulation = False
-
-    with patch("kwave.PLATFORM", "windows"):
-        with pytest.raises(FileNotFoundError):
-            _ = Executor(execution_options=mock_execution_options, simulation_options=MagicMock())
-
-
-def test_executor_gpu_cuda_failure_darwin():
-    expected_error_msg = (
-        "GPU simulations are currently not supported on MacOS. Try running the simulation on CPU by setting is_gpu_simulation=False."
-    )
-    # Configure the mock path object
-    mock_binary_path = MagicMock(spec=Path)
-    mock_binary_path.chmod.side_effect = FileNotFoundError
-
-    # Mock the execution options to use the mocked path
-    mock_execution_options = MagicMock()
-    mock_execution_options.binary_path = mock_binary_path
-    mock_execution_options.is_gpu_simulation = True
-
-    with patch("kwave.PLATFORM", "darwin"):
-        with pytest.raises(ValueError, match=expected_error_msg):
-            _ = Executor(execution_options=mock_execution_options, simulation_options=MagicMock())
+    def test_executor_file_not_found_on_non_darwin(self):
+        # Configure the mock path object
+        mock_binary_path = MagicMock(spec=Path)
+        mock_binary_path.chmod.side_effect = FileNotFoundError
+
+        # Mock the execution options to use the mocked path
+        mock_execution_options = MagicMock()
+        mock_execution_options.binary_path = mock_binary_path
+        mock_execution_options.is_gpu_simulation = False
+
+        with patch("kwave.PLATFORM", "windows"):
+            with pytest.raises(FileNotFoundError):
+                _ = Executor(execution_options=mock_execution_options, simulation_options=MagicMock())
+
+    def test_cpu_environment_variable(self):
+        """Test that environment variable KWAVE_FORCE_CPU sets CPU simulation."""
+        # Set the environment variable
+        os.environ["KWAVE_FORCE_CPU"] = "1"
+
+        # Create mock execution options with default GPU simulation
+        mock_execution_options = MagicMock()
+        mock_execution_options.is_gpu_simulation = True
+        mock_execution_options.binary_name = "kspaceFirstOrder-CUDA"
+        mock_execution_options.binary_path = MagicMock()
+
+        # Create the Executor instance
+        executor = Executor(execution_options=mock_execution_options, simulation_options=MagicMock())
+
+        # Assert that the environment variable has changed the simulation to CPU
+        self.assertFalse(executor.execution_options.is_gpu_simulation)
+        self.assertEqual(executor.execution_options.binary_name, "kspaceFirstOrder-OMP")
+
+        # Cleanup environment variable
+        del os.environ["KWAVE_FORCE_CPU"]
+
+    def test_executor_gpu_cuda_failure_darwin(self):
+        expected_error_msg = (
+            "GPU simulations are currently not supported on MacOS. Try running the simulation on CPU by setting is_gpu_simulation=False."
+        )
+        # Configure the mock path object
+        mock_binary_path = MagicMock(spec=Path)
+        mock_binary_path.chmod.side_effect = FileNotFoundError
+
+        # Mock the execution options to use the mocked path
+        mock_execution_options = MagicMock()
+        mock_execution_options.binary_path = mock_binary_path
+        mock_execution_options.is_gpu_simulation = True
+
+        with patch("kwave.PLATFORM", "darwin"):
+            with pytest.raises(ValueError, match=expected_error_msg):
+                _ = Executor(execution_options=mock_execution_options, simulation_options=MagicMock())
 
 
 if __name__ == "__main__":