From de3adce4a2425fbb536d6eb5d44e175f14e395b7 Mon Sep 17 00:00:00 2001 From: Andrew Shao Date: Fri, 30 Jun 2023 22:30:32 +0000 Subject: [PATCH] Remove changing default parameter by type for MacOSX Throw in more tests for fun --- smartsim/_core/launcher/colocated.py | 6 ++-- smartsim/entity/model.py | 33 +++++++++++-------- tests/backends/test_dbscript.py | 8 ----- tests/test_colo_model_local.py | 47 ++++++++++++++++++++++++++++ 4 files changed, 71 insertions(+), 23 deletions(-) diff --git a/smartsim/_core/launcher/colocated.py b/smartsim/_core/launcher/colocated.py index 506c9a4593..9b9252289b 100644 --- a/smartsim/_core/launcher/colocated.py +++ b/smartsim/_core/launcher/colocated.py @@ -137,10 +137,12 @@ def _build_colocated_wrapper_cmd( db_cmd.extend([ 'taskset', '-c', custom_pinning ]) - db_cmd.extend([ + db_cmd.extend( + [ CONFIG.database_exe, CONFIG.database_conf, - "--loadmodule", CONFIG.redisai + "--loadmodule", + CONFIG.redisai ] ) diff --git a/smartsim/entity/model.py b/smartsim/entity/model.py index ada431f26d..06007523e1 100644 --- a/smartsim/entity/model.py +++ b/smartsim/entity/model.py @@ -40,7 +40,6 @@ from ..log import get_logger logger = get_logger(__name__) -_default_pinning = [] if sys.platform == 'darwin' else None class Model(SmartSimEntity): def __init__( @@ -170,7 +169,7 @@ def colocate_db_uds( unix_socket: str = "/tmp/redis.socket", socket_permissions: int = 755, db_cpus: int = 1, - custom_pinning: t.Optional[t.Iterable[t.Union(int, t.Iterable[int])]] = _default_pinning, + custom_pinning: t.Optional[t.Iterable[t.Union(int, t.Iterable[int])]] = None, debug: bool = False, **kwargs: t.Any, ) -> None: @@ -229,7 +228,7 @@ def colocate_db_tcp( port: int = 6379, ifname: t.Union[str, list[str]] = "lo", db_cpus: int = 1, - custom_pinning: t.Optional[t.Iterable[t.Union(int, t.Iterable[int])]] = _default_pinning, + custom_pinning: t.Optional[t.Iterable[t.Union(int, t.Iterable[int])]] = None, debug: bool = False, **kwargs: t.Any, ) -> None: @@ -298,7 +297,7 @@ def _set_colocated_db_settings( if hasattr(self.run_settings, "_prep_colocated_db"): self.run_settings._prep_colocated_db(common_options["cpus"]) - if "limit_app_cpus" in common_options: + if "limit_app_cpus" in kwargs: raise SSUnsupportedError( "Pinning of app CPUs via limit_app_cpus is no supported. Modify RunSettings " "instead using the correct binding option for your launcher." @@ -352,15 +351,25 @@ def _stringify_id(id: int) -> int: else: raise TypeError(f"Argument is of type '{type(id)}' not 'int'") - # Deal with MacOSX limitations first + _invalid_input_message = ( + "Expected a cpu pinning specification of type iterable of ints or " + f"iterables of ints. Instead got type `{type(pin_ids)}`" + ) + + # Deal with MacOSX limitations first. The "None" (default) disables pinning + # and is equivalent to []. The only invalid option is an iterable if "darwin" in sys.platform: - if (pin_ids is None) or isinstance(pin_ids, collections.abc.Iterable): - logger.warning( - "CPU pinning is not supported on MacOSX. Setting pin_ids = []. " - "To eliminate this message, set custom_pinning = [] when adding " - "the colocated db." + if not (pin_ids is None) or not pin_ids: + return None + elif isinstance(pin_ids, collections.abc.Iterable): + warnings.warn( + "CPU pinning is not supported on MacOSX. Ignoring pinning " + "specification.", + RuntimeWarning ) return None + else: + raise TypeError(_invalid_input_message) # Flatten the iterable into a list and check to make sure that the resulting # elements are all ints if pin_ids is None: @@ -376,9 +385,7 @@ def _stringify_id(id: int) -> int: pin_list.append(_stringify_id(pin_id)) return ','.join(sorted(set(pin_list))) else: - raise TypeError( - "Expected a cpu pinning spec of type iterable of ints or " - f"iterables of ints. Instead got type `{type(pin_ids)}`") + raise TypeError(_invalid_input_message) def params_to_args(self) -> None: """Convert parameters to command line arguments and update run settings.""" diff --git a/tests/backends/test_dbscript.py b/tests/backends/test_dbscript.py index 7c9cf3dc0d..2aa0118893 100644 --- a/tests/backends/test_dbscript.py +++ b/tests/backends/test_dbscript.py @@ -237,7 +237,6 @@ def test_colocated_db_script(fileutils, wlmutils, mlutils): colo_model.colocate_db_tcp( port=test_port, db_cpus=1, - limit_app_cpus=False, debug=True, ifname=test_interface, ) @@ -316,7 +315,6 @@ def test_colocated_db_script_ensemble(fileutils, wlmutils, mlutils): entity.colocate_db_tcp( port=test_port + i, db_cpus=1, - limit_app_cpus=False, debug=True, ifname=test_interface, ) @@ -332,7 +330,6 @@ def test_colocated_db_script_ensemble(fileutils, wlmutils, mlutils): colo_model.colocate_db_tcp( port=test_port + len(colo_ensemble), db_cpus=1, - limit_app_cpus=False, debug=True, ifname=test_interface, ) @@ -421,7 +418,6 @@ def test_colocated_db_script_ensemble_reordered(fileutils, wlmutils, mlutils): entity.colocate_db_tcp( port=test_port + i, db_cpus=1, - limit_app_cpus=False, debug=True, ifname=test_interface, ) @@ -437,7 +433,6 @@ def test_colocated_db_script_ensemble_reordered(fileutils, wlmutils, mlutils): colo_model.colocate_db_tcp( port=test_port + len(colo_ensemble), db_cpus=1, - limit_app_cpus=False, debug=True, ifname=test_interface ) @@ -495,7 +490,6 @@ def test_db_script_errors(fileutils, wlmutils, mlutils): colo_model.colocate_db_tcp( port=test_port, db_cpus=1, - limit_app_cpus=False, debug=True, ifname=test_interface, ) @@ -522,7 +516,6 @@ def test_db_script_errors(fileutils, wlmutils, mlutils): entity.colocate_db_tcp( port=test_port + i, db_cpus=1, - limit_app_cpus=False, debug=True, ifname=test_interface, ) @@ -560,7 +553,6 @@ def test_db_script_errors(fileutils, wlmutils, mlutils): entity.colocate_db_tcp( port=test_port + i, db_cpus=1, - limit_app_cpus=False, debug=True, ifname=test_interface, ) diff --git a/tests/test_colo_model_local.py b/tests/test_colo_model_local.py index 2eb8e100aa..e1254b76d3 100644 --- a/tests/test_colo_model_local.py +++ b/tests/test_colo_model_local.py @@ -29,6 +29,7 @@ import pytest from smartsim import Experiment, status +from smartsim.error import SSUnsupportedError from smartsim.entity import Model if sys.platform == "darwin": @@ -36,6 +37,52 @@ else: supported_dbs = ["uds", "tcp", "deprecated"] +@pytest.mark.skipif(not (sys.platform=='darwin'), reason='MacOS-only test') +def test_macosx_warning(fileutils, coloutils): + db_args = {"custom_pinning":[1]} + db_type = 'uds' # Test is insensitive to choice of db + + exp = Experiment("colocated_model_defaults", launcher="local") + with pytest.warns( + RuntimeError, + match="CPU pinning is not supported on MacOSX. Ignoring pinning specification." + ): + colo_model = coloutils.setup_test_colo( + fileutils, + db_type, + exp, + db_args, + ) + +def test_unsupported_limit_app(fileutils, coloutils): + db_args = {"limit_app_cpus":True} + db_type = 'uds' # Test is insensitive to choice of db + + exp = Experiment("colocated_model_defaults", launcher="local") + with pytest.raises(SSUnsupportedError): + colo_model = coloutils.setup_test_colo( + fileutils, + db_type, + exp, + db_args, + ) + +@pytest.mark.parametrize("custom_pinning", [1,"10","#",1.,['a'],[1.]]) +def test_unsupported_custom_pinning(fileutils, coloutils, custom_pinning): + db_type = "uds" # Test is insensitive to choice of db + db_args = {"custom_pinning": custom_pinning} + + exp = Experiment("colocated_model_defaults", launcher="local") + with pytest.raises(TypeError): + colo_model = coloutils.setup_test_colo( + fileutils, + db_type, + exp, + db_args, + ) + + + @pytest.mark.parametrize("pin_list, num_cpus, expected", [ pytest.param(None, 2, "0,1", id="Automatic creation of pinned cpu list"), pytest.param([1,2], 2, "1,2", id="Individual ids only"),