From f7af9ffb39461e09e0b785e21a15ad51f7d3b08e Mon Sep 17 00:00:00 2001 From: Amanda Richardson Date: Wed, 24 Jan 2024 19:02:16 -0600 Subject: [PATCH 01/19] pushing typehints --- smartsim/_core/control/controller.py | 2 +- smartsim/_core/launcher/step/step.py | 2 +- smartsim/entity/model.py | 57 ++++++++++++++++++++++------ smartsim/settings/base.py | 14 ++++++- smartsim/settings/lsfSettings.py | 3 +- 5 files changed, 63 insertions(+), 15 deletions(-) diff --git a/smartsim/_core/control/controller.py b/smartsim/_core/control/controller.py index e3e463c51..b7a4f572e 100644 --- a/smartsim/_core/control/controller.py +++ b/smartsim/_core/control/controller.py @@ -631,7 +631,7 @@ def _prep_entity_client_env(self, entity: Model) -> None: # Set address to local if it's a colocated model if entity.colocated and entity.run_settings.colocated_db_settings is not None: db_name_colo = entity.run_settings.colocated_db_settings["db_identifier"] - + assert isinstance(db_name_colo, str) for key in address_dict: _, db_id = unpack_db_identifier(key, "_") if db_name_colo == db_id: diff --git a/smartsim/_core/launcher/step/step.py b/smartsim/_core/launcher/step/step.py index ebbdd074e..74b0f4fdf 100644 --- a/smartsim/_core/launcher/step/step.py +++ b/smartsim/_core/launcher/step/step.py @@ -93,7 +93,7 @@ def get_colocated_launch_script(self) -> str: ) makedirs(osp.dirname(script_path), exist_ok=True) - db_settings: t.Dict[str, str] = {} + db_settings = {} if isinstance(self.step_settings, RunSettings): db_settings = self.step_settings.colocated_db_settings or {} diff --git a/smartsim/entity/model.py b/smartsim/entity/model.py index 6b97cbf2e..3dd0c750d 100644 --- a/smartsim/entity/model.py +++ b/smartsim/entity/model.py @@ -258,13 +258,15 @@ def colocate_db_uds( f"Invalid name for unix socket: {unix_socket}. Must only " "contain alphanumeric characters or . : _ - /" ) - + assert isinstance(unix_socket, str) and not isinstance(unix_socket, object) + assert isinstance(socket_permissions, int) and not isinstance(socket_permissions, object) uds_options = { "unix_socket": unix_socket, "socket_permissions": socket_permissions, "port": 0, # This is hardcoded to 0 as recommended by redis for UDS } - + assert isinstance(uds_options.get("port"), int) and not isinstance(uds_options.get("port"), object) + common_options = { "cpus": db_cpus, "custom_pinning": custom_pinning, @@ -329,12 +331,23 @@ def colocate_db_tcp( "db_identifier": db_identifier, } self._set_colocated_db_settings(tcp_options, common_options, **kwargs) - + def _set_colocated_db_settings( self, - connection_options: t.Dict[str, t.Any], - common_options: t.Dict[str, t.Any], - **kwargs: t.Any, + connection_options: t.Dict[str, + t.Union[ + int, + list[str], + str]], + common_options: t.Dict[str, + t.Union[ + t.Iterable[ + t.Union[int, t.Iterable[int]]], + bool, + int, + str, + None]], + **kwargs: t.Union[int, None], ) -> None: """ Ingest the connection-specific options (UDS/TCP) and set the final settings @@ -357,21 +370,43 @@ def _set_colocated_db_settings( ) # TODO list which db settings can be extras + x = common_options.get("custom_pinning") + assert isinstance(x, t.Iterable) and not isinstance(x, str) + e = common_options.get("cpus") + assert isinstance(e, int) common_options["custom_pinning"] = self._create_pinning_string( - common_options["custom_pinning"], common_options["cpus"] + x, e ) - colo_db_config = {} + # set this to the type of colocated_db_settings + # colo_db_config : t.Dict[str,str] = {} + colo_db_config : t.Dict[str, + t.Union[ + bool, + int, + str, + None, + list[str], + t.Iterable[ + t.Union[int, t.Iterable[int]]], + list[DBModel], + list[DBScript], + t.Dict[str, + t.Union[int, None]], + t.Dict[str, str] + ]]= {} colo_db_config.update(connection_options) colo_db_config.update(common_options) - # redisai arguments for inference settings - colo_db_config["rai_args"] = { + + redis_ai_temp = { "threads_per_queue": kwargs.get("threads_per_queue", None), "inter_op_parallelism": kwargs.get("inter_op_parallelism", None), "intra_op_parallelism": kwargs.get("intra_op_parallelism", None), } + # redisai arguments for inference settings + colo_db_config["rai_args"] = redis_ai_temp colo_db_config["extra_db_args"] = { - k: str(v) for k, v in kwargs.items() if k not in colo_db_config["rai_args"] + k: str(v) for k, v in kwargs.items() if k not in redis_ai_temp } self._check_db_objects_colo() diff --git a/smartsim/settings/base.py b/smartsim/settings/base.py index a6df4eed4..d9bb820d0 100644 --- a/smartsim/settings/base.py +++ b/smartsim/settings/base.py @@ -31,6 +31,7 @@ from .._core.utils.helpers import expand_exe_path, fmt_dict, is_valid_cmd from ..log import get_logger +from ..entity.dbobject import DBModel, DBScript logger = get_logger(__name__) @@ -96,7 +97,18 @@ def __init__( self.container = container self._run_command = run_command self.in_batch = False - self.colocated_db_settings: t.Optional[t.Dict[str, str]] = None + self.colocated_db_settings: t.Optional[t.Dict[str, + t.Union[ + bool, + int, + str, + None, + list[str], + t.Iterable[t.Union[int, t.Iterable[int]]], + list[DBModel], + list[DBScript], + t.Dict[str, t.Union[int, None]], + t.Dict[str, str]]]] = None @property def exe_args(self) -> t.Union[str, t.List[str]]: diff --git a/smartsim/settings/lsfSettings.py b/smartsim/settings/lsfSettings.py index 47fe91802..a06845c5b 100644 --- a/smartsim/settings/lsfSettings.py +++ b/smartsim/settings/lsfSettings.py @@ -97,7 +97,8 @@ def set_cpus_per_rs(self, cpus_per_rs: int) -> None: :type cpus_per_rs: int or str """ if self.colocated_db_settings: - db_cpus = int(self.colocated_db_settings.get("db_cpus", 0)) + db_cpus = self.colocated_db_settings.get("db_cpus", 0) + assert isinstance(db_cpus,int) if not db_cpus: raise ValueError("db_cpus must be configured on colocated_db_settings") From 6ca7995ed39f8dcc70707a49d313532266e60c47 Mon Sep 17 00:00:00 2001 From: Amanda Richardson Date: Thu, 25 Jan 2024 14:14:53 -0600 Subject: [PATCH 02/19] fixing trailing white spaces --- smartsim/entity/model.py | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/smartsim/entity/model.py b/smartsim/entity/model.py index 3dd0c750d..0646d98a1 100644 --- a/smartsim/entity/model.py +++ b/smartsim/entity/model.py @@ -258,14 +258,17 @@ def colocate_db_uds( f"Invalid name for unix socket: {unix_socket}. Must only " "contain alphanumeric characters or . : _ - /" ) - assert isinstance(unix_socket, str) and not isinstance(unix_socket, object) - assert isinstance(socket_permissions, int) and not isinstance(socket_permissions, object) + assert isinstance(unix_socket, str) + assert not isinstance(unix_socket, object) + assert isinstance(socket_permissions, int) + assert not isinstance(socket_permissions, object) uds_options = { "unix_socket": unix_socket, "socket_permissions": socket_permissions, - "port": 0, # This is hardcoded to 0 as recommended by redis for UDS + "port": 0, # 0 as recommended by redis for UDS } - assert isinstance(uds_options.get("port"), int) and not isinstance(uds_options.get("port"), object) + assert isinstance(uds_options.get("port"), int) + assert isinstance(uds_options.get("port"), object) common_options = { "cpus": db_cpus, @@ -334,18 +337,18 @@ def colocate_db_tcp( def _set_colocated_db_settings( self, - connection_options: t.Dict[str, + connection_options: t.Dict[str, t.Union[ - int, - list[str], + int, + list[str], str]], common_options: t.Dict[str, t.Union[ t.Iterable[ - t.Union[int, t.Iterable[int]]], - bool, - int, - str, + t.Union[int, t.Iterable[int]]], + bool, + int, + str, None]], **kwargs: t.Union[int, None], ) -> None: @@ -378,20 +381,18 @@ def _set_colocated_db_settings( x, e ) - # set this to the type of colocated_db_settings - # colo_db_config : t.Dict[str,str] = {} - colo_db_config : t.Dict[str, + colo_db_config : t.Dict[str, t.Union[ - bool, - int, - str, + bool, + int, + str, None, list[str], t.Iterable[ t.Union[int, t.Iterable[int]]], list[DBModel], list[DBScript], - t.Dict[str, + t.Dict[str, t.Union[int, None]], t.Dict[str, str] ]]= {} From 3590d2acf5c78b852084b16b741ce027d02f7a8e Mon Sep 17 00:00:00 2001 From: Amanda Richardson Date: Thu, 25 Jan 2024 14:33:25 -0600 Subject: [PATCH 03/19] attempting to fix pylint --- smartsim/entity/model.py | 6 +++--- smartsim/settings/base.py | 18 ++++++++++++++---- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/smartsim/entity/model.py b/smartsim/entity/model.py index 0646d98a1..4d1d2416f 100644 --- a/smartsim/entity/model.py +++ b/smartsim/entity/model.py @@ -269,7 +269,7 @@ def colocate_db_uds( } assert isinstance(uds_options.get("port"), int) assert isinstance(uds_options.get("port"), object) - + common_options = { "cpus": db_cpus, "custom_pinning": custom_pinning, @@ -334,7 +334,7 @@ def colocate_db_tcp( "db_identifier": db_identifier, } self._set_colocated_db_settings(tcp_options, common_options, **kwargs) - + def _set_colocated_db_settings( self, connection_options: t.Dict[str, @@ -398,7 +398,7 @@ def _set_colocated_db_settings( ]]= {} colo_db_config.update(connection_options) colo_db_config.update(common_options) - + redis_ai_temp = { "threads_per_queue": kwargs.get("threads_per_queue", None), "inter_op_parallelism": kwargs.get("inter_op_parallelism", None), diff --git a/smartsim/settings/base.py b/smartsim/settings/base.py index d9bb820d0..f81c0f568 100644 --- a/smartsim/settings/base.py +++ b/smartsim/settings/base.py @@ -97,18 +97,28 @@ def __init__( self.container = container self._run_command = run_command self.in_batch = False - self.colocated_db_settings: t.Optional[t.Dict[str, + self.colocated_db_settings: t.Optional[t.Dict[str, t.Union[ bool, int, str, None, list[str], - t.Iterable[t.Union[int, t.Iterable[int]]], + t.Iterable[ + t.Union[ + int, + t.Iterable[ + int]]], list[DBModel], list[DBScript], - t.Dict[str, t.Union[int, None]], - t.Dict[str, str]]]] = None + t.Dict[ + str, + t.Union[ + int, + None]], + t.Dict[ + str, + str]]]] = None @property def exe_args(self) -> t.Union[str, t.List[str]]: From c2b51a300a34a8f69be366209e6648b7c612bdb6 Mon Sep 17 00:00:00 2001 From: Amanda Richardson Date: Thu, 25 Jan 2024 14:46:57 -0600 Subject: [PATCH 04/19] space --- smartsim/settings/base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/smartsim/settings/base.py b/smartsim/settings/base.py index f81c0f568..d7def15b1 100644 --- a/smartsim/settings/base.py +++ b/smartsim/settings/base.py @@ -117,7 +117,7 @@ def __init__( int, None]], t.Dict[ - str, + str, str]]]] = None @property From 1f1ebf225e883bb3605781811d34f0cf641777db Mon Sep 17 00:00:00 2001 From: Amanda Richardson Date: Fri, 26 Jan 2024 11:39:49 -0600 Subject: [PATCH 05/19] test --- smartsim/entity/model.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/smartsim/entity/model.py b/smartsim/entity/model.py index 4d1d2416f..4db589543 100644 --- a/smartsim/entity/model.py +++ b/smartsim/entity/model.py @@ -344,8 +344,8 @@ def _set_colocated_db_settings( str]], common_options: t.Dict[str, t.Union[ - t.Iterable[ - t.Union[int, t.Iterable[int]]], + t.Union[t.Iterable[ + t.Union[int, t.Iterable[int]]], None], bool, int, str, @@ -374,7 +374,9 @@ def _set_colocated_db_settings( # TODO list which db settings can be extras x = common_options.get("custom_pinning") + reveal_type(x) assert isinstance(x, t.Iterable) and not isinstance(x, str) + reveal_type(x) e = common_options.get("cpus") assert isinstance(e, int) common_options["custom_pinning"] = self._create_pinning_string( From 86e1f39cc3105b3a9692fdf25493dd3877f09465 Mon Sep 17 00:00:00 2001 From: Amanda Richardson Date: Fri, 26 Jan 2024 11:42:09 -0600 Subject: [PATCH 06/19] test --- smartsim/entity/model.py | 1 + 1 file changed, 1 insertion(+) diff --git a/smartsim/entity/model.py b/smartsim/entity/model.py index 4db589543..a75509831 100644 --- a/smartsim/entity/model.py +++ b/smartsim/entity/model.py @@ -333,6 +333,7 @@ def colocate_db_tcp( "debug": debug, "db_identifier": db_identifier, } + #test self._set_colocated_db_settings(tcp_options, common_options, **kwargs) def _set_colocated_db_settings( From b0ec4a5325c35970af76324e5feb3bd435a9b8e8 Mon Sep 17 00:00:00 2001 From: Amanda Richardson Date: Fri, 26 Jan 2024 11:51:46 -0600 Subject: [PATCH 07/19] pylint correction --- smartsim/entity/model.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/smartsim/entity/model.py b/smartsim/entity/model.py index a75509831..d3f024362 100644 --- a/smartsim/entity/model.py +++ b/smartsim/entity/model.py @@ -375,9 +375,7 @@ def _set_colocated_db_settings( # TODO list which db settings can be extras x = common_options.get("custom_pinning") - reveal_type(x) assert isinstance(x, t.Iterable) and not isinstance(x, str) - reveal_type(x) e = common_options.get("cpus") assert isinstance(e, int) common_options["custom_pinning"] = self._create_pinning_string( From 3ecec9f98f698fa012e42079be77a6869252cc90 Mon Sep 17 00:00:00 2001 From: Amanda Richardson Date: Fri, 26 Jan 2024 11:59:52 -0600 Subject: [PATCH 08/19] fixing isort issues --- smartsim/_core/_cli/build.py | 2 +- smartsim/entity/model.py | 62 +++++++++++++++----------------- smartsim/settings/base.py | 41 ++++++++++----------- smartsim/settings/lsfSettings.py | 2 +- tests/install/test_builder.py | 4 +-- 5 files changed, 51 insertions(+), 60 deletions(-) diff --git a/smartsim/_core/_cli/build.py b/smartsim/_core/_cli/build.py index 5f1e7f051..ecad43fd0 100644 --- a/smartsim/_core/_cli/build.py +++ b/smartsim/_core/_cli/build.py @@ -253,7 +253,7 @@ def check_py_torch_version(versions: Versioner, device_in: _TDeviceStr = "cpu") "Torch version not found in python environment. " "Attempting to install via `pip`" ) - wheel_device = device if device == "cpu" else device_suffix.replace("+","") + wheel_device = device if device == "cpu" else device_suffix.replace("+", "") pip( "install", "--extra-index-url", diff --git a/smartsim/entity/model.py b/smartsim/entity/model.py index d3f024362..c398ec136 100644 --- a/smartsim/entity/model.py +++ b/smartsim/entity/model.py @@ -333,24 +333,22 @@ def colocate_db_tcp( "debug": debug, "db_identifier": db_identifier, } - #test + # test self._set_colocated_db_settings(tcp_options, common_options, **kwargs) def _set_colocated_db_settings( self, - connection_options: t.Dict[str, - t.Union[ - int, - list[str], - str]], - common_options: t.Dict[str, - t.Union[ - t.Union[t.Iterable[ - t.Union[int, t.Iterable[int]]], None], - bool, - int, - str, - None]], + connection_options: t.Dict[str, t.Union[int, list[str], str]], + common_options: t.Dict[ + str, + t.Union[ + t.Union[t.Iterable[t.Union[int, t.Iterable[int]]], None], + bool, + int, + str, + None, + ], + ], **kwargs: t.Union[int, None], ) -> None: """ @@ -378,25 +376,23 @@ def _set_colocated_db_settings( assert isinstance(x, t.Iterable) and not isinstance(x, str) e = common_options.get("cpus") assert isinstance(e, int) - common_options["custom_pinning"] = self._create_pinning_string( - x, e - ) - - colo_db_config : t.Dict[str, - t.Union[ - bool, - int, - str, - None, - list[str], - t.Iterable[ - t.Union[int, t.Iterable[int]]], - list[DBModel], - list[DBScript], - t.Dict[str, - t.Union[int, None]], - t.Dict[str, str] - ]]= {} + common_options["custom_pinning"] = self._create_pinning_string(x, e) + + colo_db_config: t.Dict[ + str, + t.Union[ + bool, + int, + str, + None, + list[str], + t.Iterable[t.Union[int, t.Iterable[int]]], + list[DBModel], + list[DBScript], + t.Dict[str, t.Union[int, None]], + t.Dict[str, str], + ], + ] = {} colo_db_config.update(connection_options) colo_db_config.update(common_options) diff --git a/smartsim/settings/base.py b/smartsim/settings/base.py index d7def15b1..8f7ea3af9 100644 --- a/smartsim/settings/base.py +++ b/smartsim/settings/base.py @@ -30,8 +30,8 @@ from smartsim.settings.containers import Container from .._core.utils.helpers import expand_exe_path, fmt_dict, is_valid_cmd -from ..log import get_logger from ..entity.dbobject import DBModel, DBScript +from ..log import get_logger logger = get_logger(__name__) @@ -97,28 +97,23 @@ def __init__( self.container = container self._run_command = run_command self.in_batch = False - self.colocated_db_settings: t.Optional[t.Dict[str, - t.Union[ - bool, - int, - str, - None, - list[str], - t.Iterable[ - t.Union[ - int, - t.Iterable[ - int]]], - list[DBModel], - list[DBScript], - t.Dict[ - str, - t.Union[ - int, - None]], - t.Dict[ - str, - str]]]] = None + self.colocated_db_settings: t.Optional[ + t.Dict[ + str, + t.Union[ + bool, + int, + str, + None, + list[str], + t.Iterable[t.Union[int, t.Iterable[int]]], + list[DBModel], + list[DBScript], + t.Dict[str, t.Union[int, None]], + t.Dict[str, str], + ], + ] + ] = None @property def exe_args(self) -> t.Union[str, t.List[str]]: diff --git a/smartsim/settings/lsfSettings.py b/smartsim/settings/lsfSettings.py index a06845c5b..08a0a6b80 100644 --- a/smartsim/settings/lsfSettings.py +++ b/smartsim/settings/lsfSettings.py @@ -98,7 +98,7 @@ def set_cpus_per_rs(self, cpus_per_rs: int) -> None: """ if self.colocated_db_settings: db_cpus = self.colocated_db_settings.get("db_cpus", 0) - assert isinstance(db_cpus,int) + assert isinstance(db_cpus, int) if not db_cpus: raise ValueError("db_cpus must be configured on colocated_db_settings") diff --git a/tests/install/test_builder.py b/tests/install/test_builder.py index e6d030133..446cdf776 100644 --- a/tests/install/test_builder.py +++ b/tests/install/test_builder.py @@ -25,14 +25,14 @@ # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -import pytest - import functools import pathlib import platform import threading import time +import pytest + import smartsim._core._install.builder as build # The tests in this file belong to the group_a group From 262751615bd6f7d3a6c0ef2f47db83907e1b2a03 Mon Sep 17 00:00:00 2001 From: Amanda Richardson Date: Fri, 26 Jan 2024 12:17:46 -0600 Subject: [PATCH 09/19] testing --- smartsim/entity/model.py | 1 - 1 file changed, 1 deletion(-) diff --git a/smartsim/entity/model.py b/smartsim/entity/model.py index c398ec136..182936f0a 100644 --- a/smartsim/entity/model.py +++ b/smartsim/entity/model.py @@ -259,7 +259,6 @@ def colocate_db_uds( "contain alphanumeric characters or . : _ - /" ) assert isinstance(unix_socket, str) - assert not isinstance(unix_socket, object) assert isinstance(socket_permissions, int) assert not isinstance(socket_permissions, object) uds_options = { From fad01ca334d9046721df99c7c9796e372546e6a5 Mon Sep 17 00:00:00 2001 From: Amanda Richardson Date: Fri, 26 Jan 2024 13:36:49 -0600 Subject: [PATCH 10/19] test --- smartsim/entity/model.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/smartsim/entity/model.py b/smartsim/entity/model.py index 182936f0a..dac83bdc9 100644 --- a/smartsim/entity/model.py +++ b/smartsim/entity/model.py @@ -260,14 +260,14 @@ def colocate_db_uds( ) assert isinstance(unix_socket, str) assert isinstance(socket_permissions, int) - assert not isinstance(socket_permissions, object) - uds_options = { + uds_options: t.Dict[str, t.Union[int, str]] = { "unix_socket": unix_socket, "socket_permissions": socket_permissions, "port": 0, # 0 as recommended by redis for UDS } - assert isinstance(uds_options.get("port"), int) - assert isinstance(uds_options.get("port"), object) + # assert isinstance(uds_options.get("port"), int) + # assert isinstance(uds_options.get("socket_permissions"), int) + # assert isinstance(uds_options.get("unix_socket"), str) common_options = { "cpus": db_cpus, @@ -337,7 +337,7 @@ def colocate_db_tcp( def _set_colocated_db_settings( self, - connection_options: t.Dict[str, t.Union[int, list[str], str]], + connection_options: t.Mapping[str, t.Union[int, list[str], str]], common_options: t.Dict[ str, t.Union[ From 857b8466f084e1e22f79c2538ce5e7890f0736cf Mon Sep 17 00:00:00 2001 From: Amanda Richardson Date: Fri, 26 Jan 2024 13:57:18 -0600 Subject: [PATCH 11/19] test --- smartsim/entity/model.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/smartsim/entity/model.py b/smartsim/entity/model.py index dac83bdc9..d41eb053a 100644 --- a/smartsim/entity/model.py +++ b/smartsim/entity/model.py @@ -371,8 +371,9 @@ def _set_colocated_db_settings( ) # TODO list which db settings can be extras - x = common_options.get("custom_pinning") - assert isinstance(x, t.Iterable) and not isinstance(x, str) + x = t.cast(t.Optional[t.Iterable[t.Union[int, t.Iterable[int]]]], common_options.get("custom_pinning")) + #assert isinstance(x, t.Iterable) and not isinstance(x, str) + reveal_type(x) e = common_options.get("cpus") assert isinstance(e, int) common_options["custom_pinning"] = self._create_pinning_string(x, e) From 28090a3d26eac972b14d5a5b1d1404b8357e6731 Mon Sep 17 00:00:00 2001 From: Amanda Richardson Date: Fri, 26 Jan 2024 13:58:02 -0600 Subject: [PATCH 12/19] test --- smartsim/entity/model.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/smartsim/entity/model.py b/smartsim/entity/model.py index d41eb053a..33e8ef34c 100644 --- a/smartsim/entity/model.py +++ b/smartsim/entity/model.py @@ -371,7 +371,8 @@ def _set_colocated_db_settings( ) # TODO list which db settings can be extras - x = t.cast(t.Optional[t.Iterable[t.Union[int, t.Iterable[int]]]], common_options.get("custom_pinning")) + x = t.cast(t.Optional[t.Iterable[t.Union[int, t.Iterable[int]]]], + common_options.get("custom_pinning")) #assert isinstance(x, t.Iterable) and not isinstance(x, str) reveal_type(x) e = common_options.get("cpus") From cbdbe92887c5a5a5da0f5eba636c14ca8a576548 Mon Sep 17 00:00:00 2001 From: Amanda Richardson Date: Fri, 26 Jan 2024 14:04:39 -0600 Subject: [PATCH 13/19] test --- smartsim/entity/model.py | 1 - 1 file changed, 1 deletion(-) diff --git a/smartsim/entity/model.py b/smartsim/entity/model.py index 33e8ef34c..95d9ff4e9 100644 --- a/smartsim/entity/model.py +++ b/smartsim/entity/model.py @@ -374,7 +374,6 @@ def _set_colocated_db_settings( x = t.cast(t.Optional[t.Iterable[t.Union[int, t.Iterable[int]]]], common_options.get("custom_pinning")) #assert isinstance(x, t.Iterable) and not isinstance(x, str) - reveal_type(x) e = common_options.get("cpus") assert isinstance(e, int) common_options["custom_pinning"] = self._create_pinning_string(x, e) From 683967697203438b6219ac71553716d14705f7f3 Mon Sep 17 00:00:00 2001 From: Amanda Richardson Date: Fri, 26 Jan 2024 14:12:18 -0600 Subject: [PATCH 14/19] pushing black --- smartsim/entity/model.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/smartsim/entity/model.py b/smartsim/entity/model.py index 95d9ff4e9..5de489e32 100644 --- a/smartsim/entity/model.py +++ b/smartsim/entity/model.py @@ -371,9 +371,11 @@ def _set_colocated_db_settings( ) # TODO list which db settings can be extras - x = t.cast(t.Optional[t.Iterable[t.Union[int, t.Iterable[int]]]], - common_options.get("custom_pinning")) - #assert isinstance(x, t.Iterable) and not isinstance(x, str) + x = t.cast( + t.Optional[t.Iterable[t.Union[int, t.Iterable[int]]]], + common_options.get("custom_pinning"), + ) + # assert isinstance(x, t.Iterable) and not isinstance(x, str) e = common_options.get("cpus") assert isinstance(e, int) common_options["custom_pinning"] = self._create_pinning_string(x, e) From e4146cd1e129a23d129fd17063e37cdbcea5da4c Mon Sep 17 00:00:00 2001 From: Amanda Richardson Date: Fri, 26 Jan 2024 14:31:57 -0600 Subject: [PATCH 15/19] remove assert --- smartsim/entity/model.py | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/smartsim/entity/model.py b/smartsim/entity/model.py index 5de489e32..11b850b9a 100644 --- a/smartsim/entity/model.py +++ b/smartsim/entity/model.py @@ -258,16 +258,13 @@ def colocate_db_uds( f"Invalid name for unix socket: {unix_socket}. Must only " "contain alphanumeric characters or . : _ - /" ) - assert isinstance(unix_socket, str) - assert isinstance(socket_permissions, int) + t.cast(str, unix_socket) + t.cast(int, socket_permissions) uds_options: t.Dict[str, t.Union[int, str]] = { "unix_socket": unix_socket, "socket_permissions": socket_permissions, "port": 0, # 0 as recommended by redis for UDS } - # assert isinstance(uds_options.get("port"), int) - # assert isinstance(uds_options.get("socket_permissions"), int) - # assert isinstance(uds_options.get("unix_socket"), str) common_options = { "cpus": db_cpus, @@ -375,9 +372,7 @@ def _set_colocated_db_settings( t.Optional[t.Iterable[t.Union[int, t.Iterable[int]]]], common_options.get("custom_pinning"), ) - # assert isinstance(x, t.Iterable) and not isinstance(x, str) - e = common_options.get("cpus") - assert isinstance(e, int) + e = t.cast(int, common_options.get("cpus")) common_options["custom_pinning"] = self._create_pinning_string(x, e) colo_db_config: t.Dict[ From c3e32ffb2669c1bcc32d4da4046c11b295a99f91 Mon Sep 17 00:00:00 2001 From: Amanda Richardson Date: Fri, 26 Jan 2024 14:36:02 -0600 Subject: [PATCH 16/19] edits --- smartsim/_core/control/controller.py | 2 +- smartsim/settings/lsfSettings.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/smartsim/_core/control/controller.py b/smartsim/_core/control/controller.py index b7a4f572e..c11f0b4a8 100644 --- a/smartsim/_core/control/controller.py +++ b/smartsim/_core/control/controller.py @@ -631,7 +631,7 @@ def _prep_entity_client_env(self, entity: Model) -> None: # Set address to local if it's a colocated model if entity.colocated and entity.run_settings.colocated_db_settings is not None: db_name_colo = entity.run_settings.colocated_db_settings["db_identifier"] - assert isinstance(db_name_colo, str) + t.cast(str, db_name_colo) for key in address_dict: _, db_id = unpack_db_identifier(key, "_") if db_name_colo == db_id: diff --git a/smartsim/settings/lsfSettings.py b/smartsim/settings/lsfSettings.py index 08a0a6b80..53822b8e8 100644 --- a/smartsim/settings/lsfSettings.py +++ b/smartsim/settings/lsfSettings.py @@ -98,7 +98,7 @@ def set_cpus_per_rs(self, cpus_per_rs: int) -> None: """ if self.colocated_db_settings: db_cpus = self.colocated_db_settings.get("db_cpus", 0) - assert isinstance(db_cpus, int) + t.cast(int, db_cpus) if not db_cpus: raise ValueError("db_cpus must be configured on colocated_db_settings") From dfc4e36849e85d49b8fa819f9e0b31da010b51d1 Mon Sep 17 00:00:00 2001 From: Amanda Richardson Date: Fri, 26 Jan 2024 14:37:44 -0600 Subject: [PATCH 17/19] pushing for review --- smartsim/entity/model.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/smartsim/entity/model.py b/smartsim/entity/model.py index 11b850b9a..6ccb7160e 100644 --- a/smartsim/entity/model.py +++ b/smartsim/entity/model.py @@ -335,7 +335,7 @@ def colocate_db_tcp( def _set_colocated_db_settings( self, connection_options: t.Mapping[str, t.Union[int, list[str], str]], - common_options: t.Dict[ + common_options: t.Mapping[ str, t.Union[ t.Union[t.Iterable[t.Union[int, t.Iterable[int]]], None], From 242f31acd3de71dce136467cc90d321aca46aa61 Mon Sep 17 00:00:00 2001 From: Amanda Richardson Date: Fri, 26 Jan 2024 14:48:21 -0600 Subject: [PATCH 18/19] fixing errors --- smartsim/_core/control/controller.py | 2 +- smartsim/entity/model.py | 6 +++--- smartsim/settings/lsfSettings.py | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/smartsim/_core/control/controller.py b/smartsim/_core/control/controller.py index c11f0b4a8..b7a4f572e 100644 --- a/smartsim/_core/control/controller.py +++ b/smartsim/_core/control/controller.py @@ -631,7 +631,7 @@ def _prep_entity_client_env(self, entity: Model) -> None: # Set address to local if it's a colocated model if entity.colocated and entity.run_settings.colocated_db_settings is not None: db_name_colo = entity.run_settings.colocated_db_settings["db_identifier"] - t.cast(str, db_name_colo) + assert isinstance(db_name_colo, str) for key in address_dict: _, db_id = unpack_db_identifier(key, "_") if db_name_colo == db_id: diff --git a/smartsim/entity/model.py b/smartsim/entity/model.py index 6ccb7160e..fe21362c5 100644 --- a/smartsim/entity/model.py +++ b/smartsim/entity/model.py @@ -258,8 +258,8 @@ def colocate_db_uds( f"Invalid name for unix socket: {unix_socket}. Must only " "contain alphanumeric characters or . : _ - /" ) - t.cast(str, unix_socket) - t.cast(int, socket_permissions) + assert isinstance(unix_socket, str) + assert isinstance(socket_permissions, int) uds_options: t.Dict[str, t.Union[int, str]] = { "unix_socket": unix_socket, "socket_permissions": socket_permissions, @@ -335,7 +335,7 @@ def colocate_db_tcp( def _set_colocated_db_settings( self, connection_options: t.Mapping[str, t.Union[int, list[str], str]], - common_options: t.Mapping[ + common_options: t.Dict[ str, t.Union[ t.Union[t.Iterable[t.Union[int, t.Iterable[int]]], None], diff --git a/smartsim/settings/lsfSettings.py b/smartsim/settings/lsfSettings.py index 53822b8e8..08a0a6b80 100644 --- a/smartsim/settings/lsfSettings.py +++ b/smartsim/settings/lsfSettings.py @@ -98,7 +98,7 @@ def set_cpus_per_rs(self, cpus_per_rs: int) -> None: """ if self.colocated_db_settings: db_cpus = self.colocated_db_settings.get("db_cpus", 0) - t.cast(int, db_cpus) + assert isinstance(db_cpus, int) if not db_cpus: raise ValueError("db_cpus must be configured on colocated_db_settings") From 73495abf17a1f16e3a18864cd4e99717f1be95fd Mon Sep 17 00:00:00 2001 From: Amanda Richardson Date: Mon, 29 Jan 2024 14:37:33 -0600 Subject: [PATCH 19/19] addressing matt comments --- smartsim/entity/model.py | 22 +++++++++++----------- smartsim/settings/base.py | 6 +++--- smartsim/settings/lsfSettings.py | 3 +-- 3 files changed, 15 insertions(+), 16 deletions(-) diff --git a/smartsim/entity/model.py b/smartsim/entity/model.py index fe21362c5..01d9173f9 100644 --- a/smartsim/entity/model.py +++ b/smartsim/entity/model.py @@ -258,12 +258,11 @@ def colocate_db_uds( f"Invalid name for unix socket: {unix_socket}. Must only " "contain alphanumeric characters or . : _ - /" ) - assert isinstance(unix_socket, str) - assert isinstance(socket_permissions, int) uds_options: t.Dict[str, t.Union[int, str]] = { "unix_socket": unix_socket, "socket_permissions": socket_permissions, - "port": 0, # 0 as recommended by redis for UDS + # This is hardcoded to 0 as recommended by redis for UDS + "port": 0, } common_options = { @@ -329,12 +328,11 @@ def colocate_db_tcp( "debug": debug, "db_identifier": db_identifier, } - # test self._set_colocated_db_settings(tcp_options, common_options, **kwargs) def _set_colocated_db_settings( self, - connection_options: t.Mapping[str, t.Union[int, list[str], str]], + connection_options: t.Mapping[str, t.Union[int, t.List[str], str]], common_options: t.Dict[ str, t.Union[ @@ -368,12 +366,14 @@ def _set_colocated_db_settings( ) # TODO list which db settings can be extras - x = t.cast( + custom_pinning_ = t.cast( t.Optional[t.Iterable[t.Union[int, t.Iterable[int]]]], common_options.get("custom_pinning"), ) - e = t.cast(int, common_options.get("cpus")) - common_options["custom_pinning"] = self._create_pinning_string(x, e) + cpus_ = t.cast(int, common_options.get("cpus")) + common_options["custom_pinning"] = self._create_pinning_string( + custom_pinning_, cpus_ + ) colo_db_config: t.Dict[ str, @@ -382,10 +382,10 @@ def _set_colocated_db_settings( int, str, None, - list[str], + t.List[str], t.Iterable[t.Union[int, t.Iterable[int]]], - list[DBModel], - list[DBScript], + t.List[DBModel], + t.List[DBScript], t.Dict[str, t.Union[int, None]], t.Dict[str, str], ], diff --git a/smartsim/settings/base.py b/smartsim/settings/base.py index 8f7ea3af9..c4eefc780 100644 --- a/smartsim/settings/base.py +++ b/smartsim/settings/base.py @@ -105,10 +105,10 @@ def __init__( int, str, None, - list[str], + t.List[str], t.Iterable[t.Union[int, t.Iterable[int]]], - list[DBModel], - list[DBScript], + t.List[DBModel], + t.List[DBScript], t.Dict[str, t.Union[int, None]], t.Dict[str, str], ], diff --git a/smartsim/settings/lsfSettings.py b/smartsim/settings/lsfSettings.py index 08a0a6b80..11785f7b7 100644 --- a/smartsim/settings/lsfSettings.py +++ b/smartsim/settings/lsfSettings.py @@ -97,8 +97,7 @@ def set_cpus_per_rs(self, cpus_per_rs: int) -> None: :type cpus_per_rs: int or str """ if self.colocated_db_settings: - db_cpus = self.colocated_db_settings.get("db_cpus", 0) - assert isinstance(db_cpus, int) + db_cpus = int(t.cast(int, self.colocated_db_settings.get("db_cpus", 0))) if not db_cpus: raise ValueError("db_cpus must be configured on colocated_db_settings")