Skip to content

Commit

Permalink
extend coverage for negative device number
Browse files Browse the repository at this point in the history
  • Loading branch information
waltsims committed Dec 14, 2024
1 parent 99cc1fb commit 61d1901
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 11 deletions.
20 changes: 14 additions & 6 deletions kwave/options/simulation_execution_options.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ def __init__(
self._binary_dir = binary_dir
self.kwave_function_name = kwave_function_name
self.delete_data = delete_data
self.device_num = device_num
self.num_threads = num_threads
self._device_num = device_num
self._num_threads = num_threads
self.thread_binding = thread_binding
self.system_call = system_call
self.verbose_level = verbose_level
Expand Down Expand Up @@ -149,19 +149,27 @@ def binary_dir(self, value: str):
f"{value} is not a directory. If you are trying to set the `binary_path`, use the `binary_path` attribute instead."
)
self._binary_dir = Path(value)

@property
def device_num(self) -> Optional[int]:
return self._device_num

@device_num.setter
def device_num(self, value: Optional[int]):
if value is not None and value < 0:
raise ValueError("Device number must be non-negative")
self._device_num = value

def as_list(self, sensor: kSensor) -> list[str]:
options_list = []

if self.device_num is not None:
if self.device_num < 0:
raise ValueError("Device number must be non-negative")
options_list.append("-g")
options_list.append(str(self.device_num))

if self.num_threads is not None and PLATFORM != "windows":
if self._num_threads is not None and PLATFORM != "windows":
options_list.append("-t")
options_list.append(str(self.num_threads))
options_list.append(str(self._num_threads))

if self.verbose_level > 0:
options_list.append("--verbose")
Expand Down
15 changes: 10 additions & 5 deletions tests/test_simulation_execution_options.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ def test_default_initialization(self):
self.assertEqual(options.kwave_function_name, "kspaceFirstOrder3D")
self.assertTrue(options.delete_data)
self.assertIsNone(options.device_num)
self.assertEqual(options.num_threads, os.cpu_count()) # "all" should default to CPU count
self.assertEqual(options.num_threads, "all") # "all" should default to CPU count
self.assertIsNone(options.thread_binding)
self.assertEqual(options.verbose_level, 0)
self.assertTrue(options.auto_chunking)
Expand Down Expand Up @@ -75,6 +75,11 @@ def test_is_gpu_simulation_setter(self):
self.assertFalse(options.is_gpu_simulation)
self.assertEqual(options.binary_name, OMP_BINARY_NAME)

def test_device_num_setter_invalid(self):
"""Test setting an invalid device number."""
options = self.default_options


def test_binary_name_custom(self):
"""Test setting a custom binary name."""
options = self.default_options
Expand Down Expand Up @@ -170,7 +175,7 @@ def test_as_list_custom_record(self):
"-g",
f"{options.device_num}",
"-t",
"8",
f"{os.cpu_count()}",
"--verbose",
"1",
"--p_max",
Expand All @@ -183,9 +188,9 @@ def test_as_list_custom_record(self):
def test_as_list_with_invalid_values(self):
"""Test the behavior of as_list when there are invalid values."""
options = self.default_options
options.device_num = -1 # Invalid device_num should raise an exception when set
with self.assertRaises(ValueError):
options.as_list(self.mock_sensor)
options.device_num = -1


def test_as_list_no_record(self):
"""Test the list representation when there is no record."""
Expand All @@ -200,7 +205,7 @@ def test_as_list_no_record(self):
"-g",
f"{options.device_num}",
"-t",
"4",
f"{os.cpu_count()}",
"--p_raw", # Default value
]
for element in expected_elements:
Expand Down

0 comments on commit 61d1901

Please sign in to comment.