Skip to content

Commit

Permalink
Remove changing default parameter by type for MacOSX
Browse files Browse the repository at this point in the history
Throw in more tests for fun
  • Loading branch information
ashao committed Jun 30, 2023
1 parent fba514e commit de3adce
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 23 deletions.
6 changes: 4 additions & 2 deletions smartsim/_core/launcher/colocated.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
]
)

Expand Down
33 changes: 20 additions & 13 deletions smartsim/entity/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__(
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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."
Expand Down Expand Up @@ -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:
Expand All @@ -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."""
Expand Down
8 changes: 0 additions & 8 deletions tests/backends/test_dbscript.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down Expand Up @@ -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,
)
Expand All @@ -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,
)
Expand Down Expand Up @@ -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,
)
Expand All @@ -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
)
Expand Down Expand Up @@ -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,
)
Expand All @@ -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,
)
Expand Down Expand Up @@ -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,
)
Expand Down
47 changes: 47 additions & 0 deletions tests/test_colo_model_local.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,60 @@
import pytest

from smartsim import Experiment, status
from smartsim.error import SSUnsupportedError
from smartsim.entity import Model

if sys.platform == "darwin":
supported_dbs = ["tcp", "deprecated"]
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"),
Expand Down

0 comments on commit de3adce

Please sign in to comment.