From 444e502a716eba92dc0b7e9c9f32f24c5937094a Mon Sep 17 00:00:00 2001 From: Julia Putko Date: Tue, 20 Aug 2024 18:00:10 -0500 Subject: [PATCH 01/20] removed unused attrs from application, in progress --- smartsim/_core/utils/helpers.py | 50 ++ smartsim/entity/ensemble.py | 3 - smartsim/entity/entity.py | 6 +- smartsim/entity/model.py | 564 +-------------------- tests/temp_tests/test_colocatedJobGroup.py | 6 +- tests/temp_tests/test_jobGroup.py | 24 +- 6 files changed, 69 insertions(+), 584 deletions(-) diff --git a/smartsim/_core/utils/helpers.py b/smartsim/_core/utils/helpers.py index 62d176259..d26c71664 100644 --- a/smartsim/_core/utils/helpers.py +++ b/smartsim/_core/utils/helpers.py @@ -31,11 +31,14 @@ import base64 import collections.abc +import itertools import os import signal import subprocess +import sys import typing as t import uuid +import warnings from datetime import datetime from functools import lru_cache from pathlib import Path @@ -564,3 +567,50 @@ def push_unique(self, fn: _TSignalHandlerFn) -> bool: if did_push := fn not in self: self.push(fn) return did_push + + def _create_pinning_string( + pin_ids: t.Optional[t.Iterable[t.Union[int, t.Iterable[int]]]], cpus: int + ) -> t.Optional[str]: + """Create a comma-separated string of CPU ids. By default, ``None`` + returns 0,1,...,cpus-1; an empty iterable will disable pinning + altogether, and an iterable constructs a comma separated string of + integers (e.g. ``[0, 2, 5]`` -> ``"0,2,5"``) + """ + + def _stringify_id(_id: int) -> str: + """Return the cPU id as a string if an int, otherwise raise a ValueError""" + if isinstance(_id, int): + if _id < 0: + raise ValueError("CPU id must be a nonnegative number") + return str(_id) + + raise TypeError(f"Argument is of type '{type(_id)}' not 'int'") + + try: + pin_ids = tuple(pin_ids) if pin_ids is not None else None + except TypeError: + raise TypeError( + "Expected a cpu pinning specification of type iterable of ints or " + f"iterables of ints. Instead got type `{type(pin_ids)}`" + ) from None + + # Deal with MacOSX limitations first. The "None" (default) disables pinning + # and is equivalent to []. The only invalid option is a non-empty pinning + if sys.platform == "darwin": + if pin_ids: + warnings.warn( + "CPU pinning is not supported on MacOSX. Ignoring pinning " + "specification.", + RuntimeWarning, + ) + return None + + # Flatten the iterable into a list and check to make sure that the resulting + # elements are all ints + if pin_ids is None: + return ",".join(_stringify_id(i) for i in range(cpus)) + if not pin_ids: + return None + pin_ids = ((x,) if isinstance(x, int) else x for x in pin_ids) + to_fmt = itertools.chain.from_iterable(pin_ids) + return ",".join(sorted({_stringify_id(x) for x in to_fmt})) diff --git a/smartsim/entity/ensemble.py b/smartsim/entity/ensemble.py index 07ebe25de..aff9d33aa 100644 --- a/smartsim/entity/ensemble.py +++ b/smartsim/entity/ensemble.py @@ -87,9 +87,6 @@ def _create_applications(self) -> tuple[Application, ...]: Application( name=f"{self.name}-{i}", exe=self.exe, - run_settings=_mock.Mock(), - # ^^^^^^^^^^^^^^^^^^^^^^^ - # FIXME: remove this constructor arg! It should not exist!! exe_args=self.exe_args, files=self.files, params=permutation.params, diff --git a/smartsim/entity/entity.py b/smartsim/entity/entity.py index 8c4bd4e4f..33c16657f 100644 --- a/smartsim/entity/entity.py +++ b/smartsim/entity/entity.py @@ -98,17 +98,15 @@ def _on_disable(self) -> None: class SmartSimEntity: - def __init__(self, name: str, run_settings: "RunSettings") -> None: + def __init__(self, name: str) -> None: """Initialize a SmartSim entity. - Each entity must have a name, path, and - run_settings. All entities within SmartSim + Each entity must have a name and path. All entities within SmartSim share these attributes. :param name: Name of the entity """ self.name = name - self.run_settings = run_settings @property def type(self) -> str: diff --git a/smartsim/entity/model.py b/smartsim/entity/model.py index a1186cedd..a5f8d55e8 100644 --- a/smartsim/entity/model.py +++ b/smartsim/entity/model.py @@ -61,11 +61,8 @@ def __init__( self, name: str, exe: str, - run_settings: "RunSettings", params: t.Optional[t.Dict[str, str]] = None, exe_args: t.Optional[t.List[str]] = None, - params_as_args: t.Optional[t.List[str]] = None, - batch_settings: t.Optional["BatchSettings"] = None, files: t.Optional[EntityFiles] = None, ): """Initialize a ``Application`` @@ -75,25 +72,14 @@ def __init__( :param exe_args: executable arguments :param params: application parameters for writing into configuration files or to be passed as command line arguments to executable. - :param run_settings: launcher settings specified in the experiment - :param params_as_args: list of parameters which have to be - interpreted as command line arguments to - be added to run_settings - :param batch_settings: Launcher settings for running the individual - application as a batch job :param files: Files to have available to the application """ - super().__init__(name, run_settings) + super().__init__(name) self.exe = [expand_exe_path(exe)] # self.exe = [exe] if run_settings.container else [expand_exe_path(exe)] self.exe_args = exe_args or [] self.params = params.copy() if params else {} - self.params_as_args = params_as_args self.incoming_entities: t.List[SmartSimEntity] = [] - self._key_prefixing_enabled = False - self.batch_settings = batch_settings - self._fs_models: t.List[FSModel] = [] - self._fs_scripts: t.List[FSScript] = [] self.files = copy.deepcopy(files) if files else None @property @@ -112,32 +98,6 @@ def exe_args(self, value: t.Union[str, t.List[str], None]) -> None: """ self._exe_args = self._build_exe_args(value) - @property - def fs_models(self) -> t.Iterable[FSModel]: - """Retrieve an immutable collection of attached models - - :return: Return an immutable collection of attached models - """ - return (model for model in self._fs_models) - - @property - def fs_scripts(self) -> t.Iterable[FSScript]: - """Retrieve an immutable collection attached of scripts - - :return: Return an immutable collection of attached scripts - """ - return (script for script in self._fs_scripts) - - @property - def colocated(self) -> bool: - """Return True if this Model will run with a colocated FeatureStore - - :return: Return True of the Model will run with a colocated FeatureStore - """ - if self.run_settings is None: - return False - return bool(self.run_settings.colocated_fs_settings) - def add_exe_args(self, args: t.Union[str, t.List[str]]) -> None: """Add executable arguments to executable @@ -146,43 +106,6 @@ def add_exe_args(self, args: t.Union[str, t.List[str]]) -> None: args = self._build_exe_args(args) self._exe_args.extend(args) - def register_incoming_entity(self, incoming_entity: SmartSimEntity) -> None: - """Register future communication between entities. - - Registers the named data sources that this entity - has access to by storing the key_prefix associated - with that entity - - :param incoming_entity: The entity that data will be received from - :raises SmartSimError: if incoming entity has already been registered - """ - if incoming_entity.name in [ - in_entity.name for in_entity in self.incoming_entities - ]: - raise EntityExistsError( - f"'{incoming_entity.name}' has already " - + "been registered as an incoming entity" - ) - - self.incoming_entities.append(incoming_entity) - - def enable_key_prefixing(self) -> None: - """If called, the entity will prefix its keys with its own application name""" - self._key_prefixing_enabled = True - - def disable_key_prefixing(self) -> None: - """If called, the entity will not prefix its keys with its own - application name - """ - self._key_prefixing_enabled = False - - def query_key_prefixing(self) -> bool: - """Inquire as to whether this entity will prefix its keys with its name - - :return: Return True if entity will prefix its keys with its name - """ - return self._key_prefixing_enabled - def attach_generator_files( self, to_copy: t.Optional[t.List[str]] = None, @@ -242,498 +165,15 @@ def print_attached_files(self) -> None: """Print a table of the attached files on std out""" print(self.attached_files_table) - def colocate_fs(self, *args: t.Any, **kwargs: t.Any) -> None: - """An alias for ``Application.colocate_fs_tcp``""" - warnings.warn( - ( - "`colocate_fs` has been deprecated and will be removed in a \n" - "future release. Please use `colocate_fs_tcp` or `colocate_fs_uds`." - ), - FutureWarning, - ) - self.colocate_fs_tcp(*args, **kwargs) - - def colocate_fs_uds( - self, - unix_socket: str = "/tmp/redis.socket", - socket_permissions: int = 755, - fs_cpus: int = 1, - custom_pinning: t.Optional[t.Iterable[t.Union[int, t.Iterable[int]]]] = None, - debug: bool = False, - fs_identifier: str = "", - **kwargs: t.Any, - ) -> None: - """Colocate an FeatureStore instance with this Application over UDS. - - This method will initialize settings which add an unsharded feature - store to this Application instance. Only this Application will be able - to communicate with this colocated feature store by using Unix Domain - sockets. - - Extra parameters for the fs can be passed through kwargs. This includes - many performance, caching and inference settings. - - .. highlight:: python - .. code-block:: python - - example_kwargs = { - "maxclients": 100000, - "threads_per_queue": 1, - "inter_op_threads": 1, - "intra_op_threads": 1, - "server_threads": 2 # keydb only - } - - Generally these don't need to be changed. - - :param unix_socket: path to where the socket file will be created - :param socket_permissions: permissions for the socketfile - :param fs_cpus: number of cpus to use for FeatureStore - :param custom_pinning: CPUs to pin the FeatureStore to. Passing an empty - iterable disables pinning - :param debug: launch Application with extra debug information about the - colocated fs - :param kwargs: additional keyword arguments to pass to the FeatureStore - feature store - """ - - if not re.match(r"^[a-zA-Z0-9.:\,_\-/]*$", unix_socket): - raise ValueError( - f"Invalid name for unix socket: {unix_socket}. Must only " - "contain alphanumeric characters or . : _ - /" - ) - uds_options: t.Dict[str, t.Union[int, str]] = { - "unix_socket": unix_socket, - "socket_permissions": socket_permissions, - # This is hardcoded to 0 as recommended by redis for UDS - "port": 0, - } - - common_options = { - "cpus": fs_cpus, - "custom_pinning": custom_pinning, - "debug": debug, - "fs_identifier": fs_identifier, - } - self._set_colocated_fs_settings(uds_options, common_options, **kwargs) - - def colocate_fs_tcp( - self, - port: int = 6379, - ifname: t.Union[str, list[str]] = "lo", - fs_cpus: int = 1, - custom_pinning: t.Optional[t.Iterable[t.Union[int, t.Iterable[int]]]] = None, - debug: bool = False, - fs_identifier: str = "", - **kwargs: t.Any, - ) -> None: - """Colocate an FeatureStore instance with this Application over TCP/IP. - - This method will initialize settings which add an unsharded feature - store to this Application instance. Only this Application will be able - to communicate with this colocated feature store by using the loopback - TCP interface. - - Extra parameters for the fs can be passed through kwargs. This includes - many performance, caching and inference settings. - - .. highlight:: python - .. code-block:: python - - ex. kwargs = { - maxclients: 100000, - threads_per_queue: 1, - inter_op_threads: 1, - intra_op_threads: 1, - server_threads: 2 # keydb only - } - - Generally these don't need to be changed. - - :param port: port to use for FeatureStore feature store - :param ifname: interface to use for FeatureStore - :param fs_cpus: number of cpus to use for FeatureStore - :param custom_pinning: CPUs to pin the FeatureStore to. Passing an empty - iterable disables pinning - :param debug: launch Application with extra debug information about the - colocated fs - :param kwargs: additional keyword arguments to pass to the FeatureStore - feature store - """ - - tcp_options = {"port": port, "ifname": ifname} - common_options = { - "cpus": fs_cpus, - "custom_pinning": custom_pinning, - "debug": debug, - "fs_identifier": fs_identifier, - } - self._set_colocated_fs_settings(tcp_options, common_options, **kwargs) - - def _set_colocated_fs_settings( - self, - connection_options: t.Mapping[str, t.Union[int, t.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: - """ - Ingest the connection-specific options (UDS/TCP) and set the final settings - for the colocated feature store - """ - - if hasattr(self.run_settings, "mpmd") and len(self.run_settings.mpmd) > 0: - raise SSUnsupportedError( - "Applications colocated with feature stores cannot be run as a " - "mpmd workload" - ) - - if hasattr(self.run_settings, "_prep_colocated_fs"): - # pylint: disable-next=protected-access - self.run_settings._prep_colocated_fs(common_options["cpus"]) - - if "limit_app_cpus" in kwargs: - raise SSUnsupportedError( - "Pinning app CPUs via limit_app_cpus is not supported. Modify " - "RunSettings using the correct binding option for your launcher." - ) - - # TODO list which fs settings can be extras - custom_pinning_ = t.cast( - t.Optional[t.Iterable[t.Union[int, t.Iterable[int]]]], - common_options.get("custom_pinning"), - ) - cpus_ = t.cast(int, common_options.get("cpus")) - common_options["custom_pinning"] = self._create_pinning_string( - custom_pinning_, cpus_ - ) - - colo_fs_config: t.Dict[ - str, - t.Union[ - bool, - int, - str, - None, - t.List[str], - t.Iterable[t.Union[int, t.Iterable[int]]], - t.List[FSModel], - t.List[FSScript], - t.Dict[str, t.Union[int, None]], - t.Dict[str, str], - ], - ] = {} - colo_fs_config.update(connection_options) - colo_fs_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), - "intra_op_parallelism": kwargs.get("intra_op_parallelism", None), - } - # redisai arguments for inference settings - colo_fs_config["rai_args"] = redis_ai_temp - colo_fs_config["extra_fs_args"] = { - k: str(v) for k, v in kwargs.items() if k not in redis_ai_temp - } - - self._check_fs_objects_colo() - colo_fs_config["fs_models"] = self._fs_models - colo_fs_config["fs_scripts"] = self._fs_scripts - - self.run_settings.colocated_fs_settings = colo_fs_config - - @staticmethod - def _create_pinning_string( - pin_ids: t.Optional[t.Iterable[t.Union[int, t.Iterable[int]]]], cpus: int - ) -> t.Optional[str]: - """Create a comma-separated string of CPU ids. By default, ``None`` - returns 0,1,...,cpus-1; an empty iterable will disable pinning - altogether, and an iterable constructs a comma separated string of - integers (e.g. ``[0, 2, 5]`` -> ``"0,2,5"``) - """ - - def _stringify_id(_id: int) -> str: - """Return the cPU id as a string if an int, otherwise raise a ValueError""" - if isinstance(_id, int): - if _id < 0: - raise ValueError("CPU id must be a nonnegative number") - return str(_id) - - raise TypeError(f"Argument is of type '{type(_id)}' not 'int'") - - try: - pin_ids = tuple(pin_ids) if pin_ids is not None else None - except TypeError: - raise TypeError( - "Expected a cpu pinning specification of type iterable of ints or " - f"iterables of ints. Instead got type `{type(pin_ids)}`" - ) from None - - # Deal with MacOSX limitations first. The "None" (default) disables pinning - # and is equivalent to []. The only invalid option is a non-empty pinning - if sys.platform == "darwin": - if pin_ids: - warnings.warn( - "CPU pinning is not supported on MacOSX. Ignoring pinning " - "specification.", - RuntimeWarning, - ) - return None - - # Flatten the iterable into a list and check to make sure that the resulting - # elements are all ints - if pin_ids is None: - return ",".join(_stringify_id(i) for i in range(cpus)) - if not pin_ids: - return None - pin_ids = ((x,) if isinstance(x, int) else x for x in pin_ids) - to_fmt = itertools.chain.from_iterable(pin_ids) - return ",".join(sorted({_stringify_id(x) for x in to_fmt})) - def params_to_args(self) -> None: """Convert parameters to command line arguments and update run settings.""" - if self.params_as_args is not None: - for param in self.params_as_args: - if not param in self.params: - raise ValueError( - f"Tried to convert {param} to command line argument for " - f"application {self.name}, but its value was not found " - "in application params" - ) - if self.run_settings is None: - raise ValueError( - "Tried to configure command line parameter for Application " - f"{self.name}, but no RunSettings are set." - ) - self.add_exe_args(cat_arg_and_value(param, self.params[param])) - - def add_ml_model( - self, - name: str, - backend: str, - model: t.Optional[bytes] = None, - model_path: t.Optional[str] = None, - device: str = Device.CPU.value.upper(), - devices_per_node: int = 1, - first_device: int = 0, - batch_size: int = 0, - min_batch_size: int = 0, - min_batch_timeout: int = 0, - tag: str = "", - inputs: t.Optional[t.List[str]] = None, - outputs: t.Optional[t.List[str]] = None, - ) -> None: - """A TF, TF-lite, PT, or ONNX model to load into the fs at runtime - - Each ML Model added will be loaded into an - FeatureStore (converged or not) prior to the execution - of this Model instance - - One of either model (in memory representation) or model_path (file) - must be provided - - :param name: key to store model under - :param backend: name of the backend (TORCH, TF, TFLITE, ONNX) - :param model: A model in memory (only supported for non-colocated - feature stores) - :param model_path: serialized model - :param device: name of device for execution - :param devices_per_node: The number of GPU devices available on the host. - This parameter only applies to GPU devices and will be ignored if device - is specified as CPU. - :param first_device: The first GPU device to use on the host. - This parameter only applies to GPU devices and will be ignored if device - is specified as CPU. - :param batch_size: batch size for execution - :param min_batch_size: minimum batch size for model execution - :param min_batch_timeout: time to wait for minimum batch size - :param tag: additional tag for model information - :param inputs: model inputs (TF only) - :param outputs: model outupts (TF only) - """ - fs_model = FSModel( - name=name, - backend=backend, - model=model, - model_file=model_path, - device=device, - devices_per_node=devices_per_node, - first_device=first_device, - batch_size=batch_size, - min_batch_size=min_batch_size, - min_batch_timeout=min_batch_timeout, - tag=tag, - inputs=inputs, - outputs=outputs, - ) - self.add_ml_model_object(fs_model) - - def add_script( - self, - name: str, - script: t.Optional[str] = None, - script_path: t.Optional[str] = None, - device: str = Device.CPU.value.upper(), - devices_per_node: int = 1, - first_device: int = 0, - ) -> None: - """TorchScript to launch with this Model instance - - Each script added to the application will be loaded into an - FeatureStore (converged or not) prior to the execution - of this Model instance - - Device selection is either "GPU" or "CPU". If many devices are - present, a number can be passed for specification e.g. "GPU:1". - - Setting ``devices_per_node=N``, with N greater than one will result - in the script being stored in the first N devices of type ``device``; - alternatively, setting ``first_device=M`` will result in the script - being stored on nodes M through M + N - 1. - - One of either script (in memory string representation) or script_path (file) - must be provided - - :param name: key to store script under - :param script: TorchScript code (only supported for non-colocated featurestores) - :param script_path: path to TorchScript code - :param device: device for script execution - :param devices_per_node: The number of GPU devices available on the host. - This parameter only applies to GPU devices and will be ignored if device - is specified as CPU. - :param first_device: The first GPU device to use on the host. - This parameter only applies to GPU devices and will be ignored if device - is specified as CPU. - """ - fs_script = FSScript( - name=name, - script=script, - script_path=script_path, - device=device, - devices_per_node=devices_per_node, - first_device=first_device, - ) - self.add_script_object(fs_script) - - def add_function( - self, - name: str, - function: t.Optional[str] = None, - device: str = Device.CPU.value.upper(), - devices_per_node: int = 1, - first_device: int = 0, - ) -> None: - """TorchScript function to launch with this Application instance - - Each script function to the application will be loaded into a - non-converged FeatureStore prior to the execution - of this Application instance. - - For converged featurestores, the :meth:`add_script` method should be used. - - Device selection is either "GPU" or "CPU". If many devices are - present, a number can be passed for specification e.g. "GPU:1". - - Setting ``devices_per_node=N``, with N greater than one will result - in the application being stored in the first N devices of type ``device``. - - :param name: key to store function under - :param function: TorchScript function code - :param device: device for script execution - :param devices_per_node: The number of GPU devices available on the host. - This parameter only applies to GPU devices and will be ignored if device - is specified as CPU. - :param first_device: The first GPU device to use on the host. - This parameter only applies to GPU devices and will be ignored if device - is specified as CPU. - """ - fs_script = FSScript( - name=name, - script=function, - device=device, - devices_per_node=devices_per_node, - first_device=first_device, - ) - self.add_script_object(fs_script) - - def __hash__(self) -> int: - return hash(self.name) - - def __eq__(self, other: object) -> bool: - if not isinstance(other, Application): - return False - - if self.name == other.name: - return True - return False + ... def __str__(self) -> str: # pragma: no cover entity_str = "Name: " + self.name + "\n" entity_str += "Type: " + self.type + "\n" - entity_str += str(self.run_settings) + "\n" - if self._fs_models: - entity_str += "FS Models: \n" + str(len(self._fs_models)) + "\n" - if self._fs_scripts: - entity_str += "FS Scripts: \n" + str(len(self._fs_scripts)) + "\n" return entity_str - def add_ml_model_object(self, fs_model: FSModel) -> None: - if not fs_model.is_file and self.colocated: - err_msg = ( - "ML model can not be set from memory for colocated feature stores.\n" - ) - err_msg += ( - f"Please store the ML model named {fs_model.name} in binary format " - ) - err_msg += "and add it to the SmartSim Application as file." - raise SSUnsupportedError(err_msg) - - self._fs_models.append(fs_model) - - def add_script_object(self, fs_script: FSScript) -> None: - if fs_script.func and self.colocated: - if not isinstance(fs_script.func, str): - err_msg = ( - "Functions can not be set from memory for colocated " - "feature stores.\n" - f"Please convert the function named {fs_script.name} " - "to a string or store it as a text file and add it to the " - "SmartSim Application with add_script." - ) - raise SSUnsupportedError(err_msg) - self._fs_scripts.append(fs_script) - - def _check_fs_objects_colo(self) -> None: - for fs_model in self._fs_models: - if not fs_model.is_file: - err_msg = ( - "ML model can not be set from memory for colocated " - "feature stores.\n" - f"Please store the ML model named {fs_model.name} in binary " - "format and add it to the SmartSim Application as file." - ) - raise SSUnsupportedError(err_msg) - - for fs_script in self._fs_scripts: - if fs_script.func: - if not isinstance(fs_script.func, str): - err_msg = ( - "Functions can not be set from memory for colocated " - "feature stores.\nPlease convert the function named " - f"{fs_script.name} to a string or store it as a text" - "file and add it to the SmartSim Application with add_script." - ) - raise SSUnsupportedError(err_msg) - @staticmethod def _build_exe_args(exe_args: t.Optional[t.Union[str, t.List[str]]]) -> t.List[str]: """Check and convert exe_args input to a desired collection format""" diff --git a/tests/temp_tests/test_colocatedJobGroup.py b/tests/temp_tests/test_colocatedJobGroup.py index e8852b58f..1e76c2f8d 100644 --- a/tests/temp_tests/test_colocatedJobGroup.py +++ b/tests/temp_tests/test_colocatedJobGroup.py @@ -35,9 +35,9 @@ pytestmark = pytest.mark.group_a # TODO replace with LaunchSettings -app_1 = Application("app_1", "python", run_settings=LaunchSettings("slurm")) -app_2 = Application("app_2", "python", run_settings=LaunchSettings("slurm")) -app_3 = Application("app_3", "python", run_settings=LaunchSettings("slurm")) +app_1 = Application("app_1", "python") +app_2 = Application("app_2", "python") +app_3 = Application("app_3", "python") class MockJob(BaseJob): diff --git a/tests/temp_tests/test_jobGroup.py b/tests/temp_tests/test_jobGroup.py index 20c25d36a..d870a340d 100644 --- a/tests/temp_tests/test_jobGroup.py +++ b/tests/temp_tests/test_jobGroup.py @@ -34,9 +34,9 @@ pytestmark = pytest.mark.group_a # TODO replace with LaunchSettings -app_1 = Application("app_1", "python", LaunchSettings("slurm")) -app_2 = Application("app_2", "python", LaunchSettings("slurm")) -app_3 = Application("app_3", "python", LaunchSettings("slurm")) +app_1 = Application("app_1", "python") +app_2 = Application("app_2", "python") +app_3 = Application("app_3", "python") class MockJob(BaseJob): @@ -45,8 +45,8 @@ def get_launch_steps(self): def test_invalid_job_name(wlmutils): - job_1 = Job(app_1, wlmutils.get_test_launcher()) - job_2 = Job(app_2, wlmutils.get_test_launcher()) + job_1 = Job(app_1, LaunchSettings("slurm")) + job_2 = Job(app_2, LaunchSettings("slurm")) with pytest.raises(ValueError): _ = JobGroup([job_1, job_2], name="name/not/allowed") @@ -58,26 +58,26 @@ def test_create_JobGroup(): def test_name_setter(wlmutils): - job_1 = Job(app_1, wlmutils.get_test_launcher()) - job_2 = Job(app_2, wlmutils.get_test_launcher()) + job_1 = Job(app_1, LaunchSettings("slurm")) + job_2 = Job(app_2, LaunchSettings("slurm")) job_group = JobGroup([job_1, job_2]) job_group.name = "new_name" assert job_group.name == "new_name" def test_getitem_JobGroup(wlmutils): - job_1 = Job(app_1, wlmutils.get_test_launcher()) - job_2 = Job(app_2, wlmutils.get_test_launcher()) + job_1 = Job(app_1, LaunchSettings("slurm")) + job_2 = Job(app_2, LaunchSettings("slurm")) job_group = JobGroup([job_1, job_2]) get_value = job_group[0].entity.name assert get_value == job_1.entity.name def test_setitem_JobGroup(wlmutils): - job_1 = Job(app_1, wlmutils.get_test_launcher()) - job_2 = Job(app_2, wlmutils.get_test_launcher()) + job_1 = Job(app_1, LaunchSettings("slurm")) + job_2 = Job(app_2, LaunchSettings("slurm")) job_group = JobGroup([job_1, job_2]) - job_3 = Job(app_3, wlmutils.get_test_launcher()) + job_3 = Job(app_3, LaunchSettings("slurm")) job_group[1] = job_3 assert len(job_group) == 2 get_value = job_group[1] From 71311ae227bd805350ff2dd523611e96d2817245 Mon Sep 17 00:00:00 2001 From: Julia Putko Date: Wed, 21 Aug 2024 17:56:49 -0500 Subject: [PATCH 02/20] in progress rm attrs --- smartsim/_core/generation/generator.py | 2 +- smartsim/entity/dbnode.py | 2 +- smartsim/entity/ensemble.py | 2 +- smartsim/entity/model.py | 117 ++++++++++++++++++++----- 4 files changed, 96 insertions(+), 27 deletions(-) diff --git a/smartsim/_core/generation/generator.py b/smartsim/_core/generation/generator.py index 9c58cceaa..229dae339 100644 --- a/smartsim/_core/generation/generator.py +++ b/smartsim/_core/generation/generator.py @@ -154,7 +154,7 @@ def _build_operations(cls, job: Job, job_path: pathlib.Path) -> None: app = t.cast(Application, job.entity) cls._copy_files(app.files, job_path) cls._symlink_files(app.files, job_path) - cls._write_tagged_files(app.files, app.params, job_path) + cls._write_tagged_files(app.files, app.file_parameters, job_path) @staticmethod def _copy_files(files: t.Union[EntityFiles, None], dest: pathlib.Path) -> None: diff --git a/smartsim/entity/dbnode.py b/smartsim/entity/dbnode.py index 54ec68e1a..fa6298183 100644 --- a/smartsim/entity/dbnode.py +++ b/smartsim/entity/dbnode.py @@ -64,7 +64,7 @@ def __init__( fs_identifier: str = "", ) -> None: """Initialize a feature store node within an feature store.""" - super().__init__(name, run_settings) + super().__init__(name) self.exe = [exe] if run_settings.container else [expand_exe_path(exe)] self.exe_args = exe_args or [] self.ports = ports diff --git a/smartsim/entity/ensemble.py b/smartsim/entity/ensemble.py index aff9d33aa..446f82b77 100644 --- a/smartsim/entity/ensemble.py +++ b/smartsim/entity/ensemble.py @@ -89,7 +89,7 @@ def _create_applications(self) -> tuple[Application, ...]: exe=self.exe, exe_args=self.exe_args, files=self.files, - params=permutation.params, + file_parameters=permutation.file_parameters, params_as_args=permutation.exe_args, # type: ignore[arg-type] # ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ # FIXME: this is the wrong type on Application! diff --git a/smartsim/entity/model.py b/smartsim/entity/model.py index a5f8d55e8..bcc91c737 100644 --- a/smartsim/entity/model.py +++ b/smartsim/entity/model.py @@ -27,19 +27,13 @@ from __future__ import annotations import copy -import itertools -import re -import sys import typing as t -import warnings from os import getcwd from os import path as osp from .._core._install.builder import Device -from .._core.utils.helpers import cat_arg_and_value, expand_exe_path -from ..error import EntityExistsError, SSUnsupportedError +from .._core.utils.helpers import expand_exe_path from ..log import get_logger -from .dbobject import FSModel, FSScript from .entity import SmartSimEntity from .files import EntityFiles @@ -56,49 +50,122 @@ # TODO: Remove this supression when we strip fileds/functionality # (run-settings/batch_settings/params_as_args/etc)! # pylint: disable-next=too-many-public-methods + class Application(SmartSimEntity): def __init__( self, name: str, exe: str, - params: t.Optional[t.Dict[str, str]] = None, - exe_args: t.Optional[t.List[str]] = None, + exe_args: t.Optional[t.Union[str,t.Sequence[str]]] = None, files: t.Optional[EntityFiles] = None, - ): - """Initialize a ``Application`` + file_parameters: t.Mapping[str, str] | None = None, + ) -> None: + """Initialize an ``Application`` :param name: name of the application :param exe: executable to run :param exe_args: executable arguments - :param params: application parameters for writing into configuration files or - to be passed as command line arguments to executable. - :param files: Files to have available to the application + :param files: files to be copied, symlinked, and/or configured prior to + execution + :param file_parameters: parameters and values to be used when configuring + files """ super().__init__(name) - self.exe = [expand_exe_path(exe)] - # self.exe = [exe] if run_settings.container else [expand_exe_path(exe)] - self.exe_args = exe_args or [] - self.params = params.copy() if params else {} - self.incoming_entities: t.List[SmartSimEntity] = [] - self.files = copy.deepcopy(files) if files else None + self._exe = [expand_exe_path(exe)] + self._exe_args = self._build_exe_args(exe_args) or [] + self._files = copy.deepcopy(files) if files else None + self._file_parameters = copy.deepcopy(file_parameters) if file_parameters else {} + self._incoming_entities: t.List[SmartSimEntity] = [] + self._key_prefixing_enabled = False + + #TODO Talk through as a group if _key_prefixing_enabled + # should have proeprty and setter decorators or do we stick with something of similar syntax. + # Bring this up to the group after the rest of the class is done so they see what a consistent + # API is currently being formed. + #TODO Discuss if the exe_args parameter should be set with a str in the construct + # and setter or should we stick to t.Sequence[str] only. This might require group discussion. + #TODO Discuss with the core team when/if properties should always be returned via reference + # or deep copy + #TODO Ticket says to remove prefixing, but I think that needs to stay + #TODO @property for _exe + #TODO @exe.setter for _exe + #TODO @property for _files + #TODO @pfiles.setter for _files + #-- added propert #TODO @property for _file_parameters + #-- added setter #TODO @file_parameters.setter for _file_parameters + #TODO @property for _incoming_entities + #TODO @incoming_entities.setter for _incoming_entites + #TODO Update __str__ + #TODO Should attached_files_table be deleted and replaced with @property? + #--moved #TODO Put create pinning string into a new ticket for finding a home for it + #TODO check consistency of variable names and constructor with Ensemble, where appropriate + #TODO Unit tests!! + #TODO Cleanup documentation + + + @property + def exe(self) -> str: + """Return executable to run. + + :returns: application executable to run + """ + return self._exe + + @exe.setter + def exe(self, value: str) -> None: + + self._exe = value + + @property + def files(self) -> t.Optional[EntityFiles]: + """Return + + :returns: + """ + return self._files + + @files.setter + def files(self, value: t.Optional[EntityFiles]) -> None: + + self._files = value + @property - def exe_args(self) -> t.Union[str, t.List[str]]: + def exe_args(self) -> t.List[str]: + # TODO why does this say immutable if it is not a deep copy? """Return an immutable list of attached executable arguments. - :returns: attached executable arguments + :returns: application executable arguments """ return self._exe_args @exe_args.setter def exe_args(self, value: t.Union[str, t.List[str], None]) -> None: + # TODO should we just make this a t.Sequence[str] if the + # constructor is just t.list[str] """Set the executable arguments. :param value: executable arguments """ self._exe_args = self._build_exe_args(value) - def add_exe_args(self, args: t.Union[str, t.List[str]]) -> None: + @property + def file_parameters(self) -> t.Mapping[str, str]: + """Return file parameters. + + :returns: application file parameters + """ + return self._file_parameters + + @file_parameters.setter + def file_parameters(self, value: t.Mapping[str, str] | None) -> None: + """Set the file parameters. + + :param value: file parameters + """ + self._file_parameters = value + + def add_exe_args(self, args: t.Union[str, t.List[str], None]) -> None: """Add executable arguments to executable :param args: executable arguments @@ -106,6 +173,7 @@ def add_exe_args(self, args: t.Union[str, t.List[str]]) -> None: args = self._build_exe_args(args) self._exe_args.extend(args) + def attach_generator_files( self, to_copy: t.Optional[t.List[str]] = None, @@ -169,13 +237,14 @@ def params_to_args(self) -> None: """Convert parameters to command line arguments and update run settings.""" ... + #TODO Update __str__ def __str__(self) -> str: # pragma: no cover entity_str = "Name: " + self.name + "\n" entity_str += "Type: " + self.type + "\n" return entity_str @staticmethod - def _build_exe_args(exe_args: t.Optional[t.Union[str, t.List[str]]]) -> t.List[str]: + def _build_exe_args(exe_args: t.Optional[t.Union[str, t.List[str], None]]) -> t.List[str]: """Check and convert exe_args input to a desired collection format""" if not exe_args: return [] From a8e852d3361a044c4e86fc6d4aadf84d8dc35f26 Mon Sep 17 00:00:00 2001 From: Julia Putko Date: Thu, 22 Aug 2024 14:20:22 -0500 Subject: [PATCH 03/20] in progress --- smartsim/entity/model.py | 26 ++++++++++++--- smartsim/experiment.py | 69 ++++++++++++++-------------------------- 2 files changed, 44 insertions(+), 51 deletions(-) diff --git a/smartsim/entity/model.py b/smartsim/entity/model.py index bcc91c737..22faa2f72 100644 --- a/smartsim/entity/model.py +++ b/smartsim/entity/model.py @@ -78,14 +78,24 @@ def __init__( self._incoming_entities: t.List[SmartSimEntity] = [] self._key_prefixing_enabled = False + + ## add setter fror key_prefixing + + ## talk about whether dragon has a way to namespace keys + + + #TODO Talk through as a group if _key_prefixing_enabled - # should have proeprty and setter decorators or do we stick with something of similar syntax. + # should have property and setter decorators or do we stick with something of similar syntax. # Bring this up to the group after the rest of the class is done so they see what a consistent # API is currently being formed. + + #TODO Discuss if the exe_args parameter should be set with a str in the construct # and setter or should we stick to t.Sequence[str] only. This might require group discussion. #TODO Discuss with the core team when/if properties should always be returned via reference # or deep copy + #TODO Ticket says to remove prefixing, but I think that needs to stay #TODO @property for _exe #TODO @exe.setter for _exe @@ -96,10 +106,14 @@ def __init__( #TODO @property for _incoming_entities #TODO @incoming_entities.setter for _incoming_entites #TODO Update __str__ - #TODO Should attached_files_table be deleted and replaced with @property? + + #--moved #TODO Put create pinning string into a new ticket for finding a home for it + #TODO check consistency of variable names and constructor with Ensemble, where appropriate + #TODO Unit tests!! + #TODO Cleanup documentation @@ -131,16 +145,18 @@ def files(self, value: t.Optional[EntityFiles]) -> None: @property - def exe_args(self) -> t.List[str]: + def exe_args(self) -> t.List[str]: # t.Sequence[str] use sequence of strings # TODO why does this say immutable if it is not a deep copy? + # TODO review whether this should be a deepcopy - are we relying of having this be a reference? """Return an immutable list of attached executable arguments. :returns: application executable arguments """ - return self._exe_args + # make this a deepcopy (( maybe discuss )) - al and andrew? + return self._exe_args @exe_args.setter - def exe_args(self, value: t.Union[str, t.List[str], None]) -> None: + def exe_args(self, value: t.Union[str, t.List[str], None]) -> None: # t.Sequence[str] ... # TODO should we just make this a t.Sequence[str] if the # constructor is just t.list[str] """Set the executable arguments. diff --git a/smartsim/experiment.py b/smartsim/experiment.py index 55ccea7b5..ef3725269 100644 --- a/smartsim/experiment.py +++ b/smartsim/experiment.py @@ -28,13 +28,10 @@ from __future__ import annotations -import collections import datetime import itertools -import os import os.path as osp import pathlib -import textwrap import typing as t from os import environ, getcwd @@ -46,15 +43,8 @@ from smartsim.error import errors from smartsim.status import InvalidJobStatus, JobStatus -from ._core import Controller, Generator, Manifest, previewrenderer -from .database import FeatureStore -from .entity import ( - Application, - Ensemble, - EntitySequence, - SmartSimEntity, - TelemetryConfiguration, -) +from ._core import Generator, Manifest, previewrenderer +from .entity import TelemetryConfiguration from .error import SmartSimError from .log import ctx_exp_path, get_logger, method_contextualizer @@ -94,64 +84,51 @@ def _on_disable(self) -> None: # pylint: disable=no-self-use class Experiment: - """Experiment is a factory class that creates stages of a workflow - and manages their execution. - - The instances created by an Experiment represent executable code - that is either user-specified, like the ``Application`` instance created - by ``Experiment.create_application``, or pre-configured, like the ``FeatureStore`` - instance created by ``Experiment.create_feature_store``. - - Experiment methods that accept a variable list of arguments, such as - ``Experiment.start`` or ``Experiment.stop``, accept any number of the - instances created by the Experiment. - - In general, the Experiment class is designed to be initialized once - and utilized throughout runtime. + """The Experiment class is used to schedule, launch, track, and manage + jobs and job groups. Also, it is the SmartSim class that manages + internal data structures, processes, and infrastructure for interactive + capabilties such as the SmartSim dashboard and historical lookback on + launched jobs and job groups. The Experiment class is designed to be + initialized once and utilized throughout the entirety of a workflow. """ def __init__(self, name: str, exp_path: str | None = None): """Initialize an Experiment instance. - With the default settings, the Experiment will use the - local launcher, which will start all Experiment created - instances on the localhost. Example of initializing an Experiment - .. highlight:: python - .. code-block:: python - - exp = Experiment(name="my_exp", launcher="local") - - SmartSim supports multiple launchers which also can be specified - based on the type of system you are running on. .. highlight:: python .. code-block:: python - exp = Experiment(name="my_exp", launcher="slurm") + exp = Experiment(name="my_exp") + + The name of a SmartSim ``Experiment`` will determine the + name of the ``Experiment`` directory that is created inside of the + current working directory. - If you want your Experiment driver script to be run across - multiple system with different schedulers (workload managers) - you can also use the `auto` argument to have the Experiment detect - which launcher to use based on system installed binaries and libraries. + If a different ``Experiment`` path is desired, the ``exp_path`` + parameter can be set as shown in the example below. .. highlight:: python .. code-block:: python - exp = Experiment(name="my_exp", launcher="auto") + exp = Experiment(name="my_exp", exp_path="/full/path/to/exp") - The Experiment path will default to the current working directory - and if the ``Experiment.generate`` method is called, a directory - with the Experiment name will be created to house the output - from the Experiment. + Note that the provided path must exist prior to ``Experiment`` + construction and that an experiment name subdirectory will not be + created inside of the provide path. :param name: name for the ``Experiment`` :param exp_path: path to location of ``Experiment`` directory """ + if not name: + raise TypeError("Experiment name must be non-empty string") + self.name = name + if exp_path: if not isinstance(exp_path, str): raise TypeError("exp_path argument was not of type str") From 40e782f74ae227a451768f2f89c6bfe46c8a4d27 Mon Sep 17 00:00:00 2001 From: Julia Putko Date: Fri, 23 Aug 2024 15:23:06 -0500 Subject: [PATCH 04/20] add files to ignore, refactor of application, fixing tests to accomodate change --- pyproject.toml | 2 + smartsim/entity/ensemble.py | 5 +- smartsim/entity/model.py | 149 ++++++++++++++++------------ tests/temp_tests/test_launchable.py | 31 +++--- tests/test_ensemble.py | 16 ++- tests/test_experiment.py | 2 +- tests/test_generator.py | 16 ++- 7 files changed, 122 insertions(+), 99 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 5df64aa97..767aa710e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -166,6 +166,8 @@ module = [ "smartsim._core.utils.telemetry.*", "smartsim.database.*", "smartsim.settings.sgeSettings", + "smartsim._core.control.controller_utils", + "smartsim.entity.dbnode", ] ignore_missing_imports = true ignore_errors = true diff --git a/smartsim/entity/ensemble.py b/smartsim/entity/ensemble.py index 446f82b77..9f9ec6e34 100644 --- a/smartsim/entity/ensemble.py +++ b/smartsim/entity/ensemble.py @@ -89,10 +89,7 @@ def _create_applications(self) -> tuple[Application, ...]: exe=self.exe, exe_args=self.exe_args, files=self.files, - file_parameters=permutation.file_parameters, - params_as_args=permutation.exe_args, # type: ignore[arg-type] - # ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - # FIXME: this is the wrong type on Application! + file_parameters=permutation.params, ) for i, permutation in enumerate(permutations_) ) diff --git a/smartsim/entity/model.py b/smartsim/entity/model.py index 22faa2f72..531f05888 100644 --- a/smartsim/entity/model.py +++ b/smartsim/entity/model.py @@ -28,10 +28,8 @@ import copy import typing as t -from os import getcwd from os import path as osp -from .._core._install.builder import Device from .._core.utils.helpers import expand_exe_path from ..log import get_logger from .entity import SmartSimEntity @@ -51,12 +49,13 @@ # (run-settings/batch_settings/params_as_args/etc)! # pylint: disable-next=too-many-public-methods + class Application(SmartSimEntity): def __init__( self, name: str, exe: str, - exe_args: t.Optional[t.Union[str,t.Sequence[str]]] = None, + exe_args: t.Optional[t.Union[str, t.Sequence[str]]] = None, files: t.Optional[EntityFiles] = None, file_parameters: t.Mapping[str, str] | None = None, ) -> None: @@ -65,7 +64,7 @@ def __init__( :param name: name of the application :param exe: executable to run :param exe_args: executable arguments - :param files: files to be copied, symlinked, and/or configured prior to + :param files: files to be copied, symlinked, and/or configured prior to execution :param file_parameters: parameters and values to be used when configuring files @@ -74,51 +73,38 @@ def __init__( self._exe = [expand_exe_path(exe)] self._exe_args = self._build_exe_args(exe_args) or [] self._files = copy.deepcopy(files) if files else None - self._file_parameters = copy.deepcopy(file_parameters) if file_parameters else {} + self._file_parameters = ( + copy.deepcopy(file_parameters) if file_parameters else {} + ) self._incoming_entities: t.List[SmartSimEntity] = [] self._key_prefixing_enabled = False - - ## add setter fror key_prefixing - - ## talk about whether dragon has a way to namespace keys - - - - #TODO Talk through as a group if _key_prefixing_enabled - # should have property and setter decorators or do we stick with something of similar syntax. + # TODO Talk through as a group if _key_prefixing_enabled + # should have proeprty and setter decorators or do we stick with something of similar syntax. # Bring this up to the group after the rest of the class is done so they see what a consistent # API is currently being formed. - - - #TODO Discuss if the exe_args parameter should be set with a str in the construct + # TODO Discuss if the exe_args parameter should be set with a str in the construct # and setter or should we stick to t.Sequence[str] only. This might require group discussion. - #TODO Discuss with the core team when/if properties should always be returned via reference - # or deep copy - - #TODO Ticket says to remove prefixing, but I think that needs to stay - #TODO @property for _exe - #TODO @exe.setter for _exe - #TODO @property for _files - #TODO @pfiles.setter for _files - #-- added propert #TODO @property for _file_parameters - #-- added setter #TODO @file_parameters.setter for _file_parameters - #TODO @property for _incoming_entities - #TODO @incoming_entities.setter for _incoming_entites - #TODO Update __str__ - - - #--moved #TODO Put create pinning string into a new ticket for finding a home for it - - #TODO check consistency of variable names and constructor with Ensemble, where appropriate - - #TODO Unit tests!! - - #TODO Cleanup documentation - - - @property - def exe(self) -> str: + # TODO Discuss with the core team when/if properties shoulda always be returned via reference + # or deep copy + # TODO Ticket says to remove prefixing, but I think that needs to stay + # TODO @property for _exe + # TODO @exe.setter for _exe + # TODO @property for _files + # TODO @pfiles.setter for _files + # TODO @property for _file_parameters + # TODO @file_parameters.setter for _file_parameters + # TODO @property for _incoming_entities + # TODO @incoming_entities.setter for _incoming_entites + # TODO Update __str__ + # TODO Should attached_files_table be deleted and replaced with @property? + # TODO Put create pinning string into a new ticket for finding a home for it + # TODO check consistency of variable names and constructor with Ensemble, where appropriate + # TODO Unit tests!!! + # TODO Cleanup documentation + + @property + def exe(self) -> t.List[str]: """Return executable to run. :returns: application executable to run @@ -126,39 +112,43 @@ def exe(self) -> str: return self._exe @exe.setter - def exe(self, value: str) -> None: - + def exe(self, value: t.List[str]) -> None: + """Set executable to run. + + :param value: executable to run + """ self._exe = value @property def files(self) -> t.Optional[EntityFiles]: - """Return + """Return files to be copied, symlinked, and/or configured prior to + execution. - :returns: + :returns: files """ return self._files @files.setter def files(self, value: t.Optional[EntityFiles]) -> None: - + """Set files to be copied, symlinked, and/or configured prior to + execution. + + :param value: files + """ self._files = value - @property - def exe_args(self) -> t.List[str]: # t.Sequence[str] use sequence of strings + def exe_args(self) -> t.Sequence[str]: # TODO why does this say immutable if it is not a deep copy? - # TODO review whether this should be a deepcopy - are we relying of having this be a reference? + # TODO review whether this should be a deepcopy - are we relying of having this be a reference? """Return an immutable list of attached executable arguments. :returns: application executable arguments """ - # make this a deepcopy (( maybe discuss )) - al and andrew? - return self._exe_args + return copy.deepcopy(self._exe_args) @exe_args.setter - def exe_args(self, value: t.Union[str, t.List[str], None]) -> None: # t.Sequence[str] ... - # TODO should we just make this a t.Sequence[str] if the - # constructor is just t.list[str] + def exe_args(self, value: t.Union[str, t.Sequence[str], None]) -> None: # """Set the executable arguments. :param value: executable arguments @@ -174,13 +164,37 @@ def file_parameters(self) -> t.Mapping[str, str]: return self._file_parameters @file_parameters.setter - def file_parameters(self, value: t.Mapping[str, str] | None) -> None: + def file_parameters(self, value: t.Mapping[str, str]) -> None: """Set the file parameters. :param value: file parameters """ self._file_parameters = value - + + @property + def incoming_entities(self) -> t.List[SmartSimEntity]: + """Return incoming entities. + + :returns: incoming entities + """ + return self._incoming_entities + + @incoming_entities.setter + def incoming_entities(self, value: t.List[SmartSimEntity]) -> None: + """Set the incoming entities. + + :param value: incoming entities + """ + self._incoming_entities = value + + @property + def key_prefixing_enabled(self) -> bool: + return self._key_prefixing_enabled + + @key_prefixing_enabled.setter + def key_prefixing_enabled(self, value: bool) -> None: + self.key_prefixing_enabled + def add_exe_args(self, args: t.Union[str, t.List[str], None]) -> None: """Add executable arguments to executable @@ -189,7 +203,6 @@ def add_exe_args(self, args: t.Union[str, t.List[str], None]) -> None: args = self._build_exe_args(args) self._exe_args.extend(args) - def attach_generator_files( self, to_copy: t.Optional[t.List[str]] = None, @@ -253,14 +266,28 @@ def params_to_args(self) -> None: """Convert parameters to command line arguments and update run settings.""" ... - #TODO Update __str__ def __str__(self) -> str: # pragma: no cover entity_str = "Name: " + self.name + "\n" entity_str += "Type: " + self.type + "\n" + entity_str += "Executable:\n" + for ex in self.exe: + entity_str += f"{ex}\n" + entity_str += "Executable Arguments:\n" + for ex_arg in self.exe_args: + entity_str += f"{str(ex_arg)}\n" + entity_str += "Entity Files: " + str(self.files) + "\n" + entity_str += "File Parameters: " + str(self.file_parameters) + "\n" + entity_str += "Incoming Entities:\n" + for entity in self.incoming_entities: + entity_str += f"{entity}\n" + entity_str += "Key Prefixing Enabled: " + str(self.key_prefixing_enabled) + "\n" + return entity_str @staticmethod - def _build_exe_args(exe_args: t.Optional[t.Union[str, t.List[str], None]]) -> t.List[str]: + def _build_exe_args( + exe_args: t.Optional[t.Union[str, t.Sequence[str], None]] + ) -> t.List[str]: """Check and convert exe_args input to a desired collection format""" if not exe_args: return [] diff --git a/tests/temp_tests/test_launchable.py b/tests/temp_tests/test_launchable.py index 16fba6cff..68c13c350 100644 --- a/tests/temp_tests/test_launchable.py +++ b/tests/temp_tests/test_launchable.py @@ -53,10 +53,10 @@ def test_launchable_init(): def test_invalid_job_name(wlmutils): entity = Application( "test_name", - run_settings="RunSettings", exe="echo", exe_args=["spam", "eggs"], ) # Mock RunSettings + settings = LaunchSettings(wlmutils.get_test_launcher()) with pytest.raises(ValueError): _ = Job(entity, settings, name="path/to/name") @@ -65,7 +65,6 @@ def test_invalid_job_name(wlmutils): def test_job_init(): entity = Application( "test_name", - run_settings=LaunchSettings("slurm"), exe="echo", exe_args=["spam", "eggs"], ) @@ -80,7 +79,6 @@ def test_job_init(): def test_name_setter(): entity = Application( "test_name", - run_settings=LaunchSettings("slurm"), exe="echo", exe_args=["spam", "eggs"], ) @@ -92,7 +90,6 @@ def test_name_setter(): def test_job_init_deepcopy(): entity = Application( "test_name", - run_settings=LaunchSettings("slurm"), exe="echo", exe_args=["spam", "eggs"], ) @@ -104,7 +101,7 @@ def test_job_init_deepcopy(): def test_add_mpmd_pair(): - entity = SmartSimEntity("test_name", LaunchSettings("slurm")) + entity = SmartSimEntity("test_name") mpmd_job = MPMDJob() mpmd_job.add_mpmd_pair(entity, LaunchSettings("slurm")) @@ -121,7 +118,6 @@ def test_mpmdpair_init(): "test_name", "echo", exe_args=["spam", "eggs"], - run_settings=LaunchSettings("slurm"), ) mpmd_pair = MPMDPair(entity, LaunchSettings("slurm")) assert isinstance(mpmd_pair, MPMDPair) @@ -136,7 +132,6 @@ def test_mpmdpair_init_deepcopy(): entity = Application( "test_name", "echo", - run_settings=LaunchSettings("slurm"), exe_args=["spam", "eggs"], ) settings = LaunchSettings("slurm") @@ -153,14 +148,12 @@ def test_check_launcher(): "entity1", "echo", exe_args=["hello", "world"], - run_settings=LaunchSettings("slurm"), ) launch_settings1 = LaunchSettings("slurm") entity2 = Application( "entity2", "echo", exe_args=["hello", "world"], - run_settings=LaunchSettings("slurm"), ) launch_settings2 = LaunchSettings("slurm") mpmd_pairs = [] @@ -179,10 +172,10 @@ def test_add_mpmd_pair_check_launcher_error(): """Test that an error is raised when a pairs is added to an mpmd job using add_mpmd_pair that does not have the same launcher type""" mpmd_pairs = [] - entity1 = SmartSimEntity("entity1", LaunchSettings("slurm")) + entity1 = SmartSimEntity("entity1") launch_settings1 = LaunchSettings("slurm") - entity2 = SmartSimEntity("entity2", LaunchSettings("pals")) + entity2 = SmartSimEntity("entity2") launch_settings2 = LaunchSettings("pals") pair1 = MPMDPair(entity1, launch_settings1) @@ -197,10 +190,10 @@ def test_add_mpmd_pair_check_launcher_error(): def test_add_mpmd_pair_check_entity(): """Test that mpmd pairs that have the same entity type can be added to an MPMD Job""" mpmd_pairs = [] - entity1 = Application("entity1", "python", LaunchSettings("slurm")) + entity1 = Application("entity1", "python") launch_settings1 = LaunchSettings("slurm") - entity2 = Application("entity2", "python", LaunchSettings("slurm")) + entity2 = Application("entity2", "python") launch_settings2 = LaunchSettings("slurm") pair1 = MPMDPair(entity1, launch_settings1) @@ -217,10 +210,10 @@ def test_add_mpmd_pair_check_entity_error(): """Test that an error is raised when a pairs is added to an mpmd job using add_mpmd_pair that does not have the same entity type""" mpmd_pairs = [] - entity1 = Application("entity1", "python", LaunchSettings("slurm")) + entity1 = Application("entity1", "python") launch_settings1 = LaunchSettings("slurm") - entity2 = Application("entity2", "python", LaunchSettings("pals")) + entity2 = Application("entity2", "python") launch_settings2 = LaunchSettings("pals") pair1 = MPMDPair(entity1, launch_settings1) @@ -237,10 +230,10 @@ def test_create_mpmdjob_invalid_mpmdpairs(): does not have the same launcher type""" mpmd_pairs = [] - entity1 = Application("entity1", "python", LaunchSettings("slurm")) + entity1 = Application("entity1", "python") launch_settings1 = LaunchSettings("slurm") - entity1 = Application("entity1", "python", LaunchSettings("pals")) + entity1 = Application("entity1", "python") launch_settings2 = LaunchSettings("pals") pair1 = MPMDPair(entity1, launch_settings1) @@ -258,9 +251,9 @@ def test_create_mpmdjob_valid_mpmdpairs(): """Test that all pairs have the same entity type is enforced when creating an MPMDJob""" mpmd_pairs = [] - entity1 = Application("entity1", "python", LaunchSettings("slurm")) + entity1 = Application("entity1", "python") launch_settings1 = LaunchSettings("slurm") - entity1 = Application("entity1", "python", LaunchSettings("slurm")) + entity1 = Application("entity1", "python") launch_settings2 = LaunchSettings("slurm") pair1 = MPMDPair(entity1, launch_settings1) diff --git a/tests/test_ensemble.py b/tests/test_ensemble.py index 4eb578a71..c22e0e0db 100644 --- a/tests/test_ensemble.py +++ b/tests/test_ensemble.py @@ -89,27 +89,33 @@ def test_strategy_error_raised_if_a_strategy_that_dne_is_requested(test_dir): @pytest.mark.parametrize( - "params", + "file_parameters", ( pytest.param({"SPAM": ["eggs"]}, id="Non-Empty Params"), pytest.param({}, id="Empty Params"), pytest.param(None, id="Nullish Params"), ), ) -def test_replicated_applications_have_eq_deep_copies_of_parameters(params, test_dir): +def test_replicated_applications_have_eq_deep_copies_of_parameters( + file_parameters, test_dir +): apps = list( Ensemble( "test_ensemble", "echo", ("hello",), replicas=4, - file_parameters=params, + file_parameters=file_parameters, )._create_applications() ) assert len(apps) >= 2 # Sanitiy check to make sure the test is valid - assert all(app_1.params == app_2.params for app_1 in apps for app_2 in apps) assert all( - app_1.params is not app_2.params + app_1.file_parameters == app_2.file_parameters + for app_1 in apps + for app_2 in apps + ) + assert all( + app_1.file_parameters is not app_2.file_parameters for app_1 in apps for app_2 in apps if app_1 is not app_2 diff --git a/tests/test_experiment.py b/tests/test_experiment.py index 8671bfedb..4ea22a1a0 100644 --- a/tests/test_experiment.py +++ b/tests/test_experiment.py @@ -185,7 +185,7 @@ class EchoHelloWorldEntity(entity.SmartSimEntity): """A simple smartsim entity that meets the `ExecutableProtocol` protocol""" def __init__(self): - super().__init__("test-entity", _mock.Mock()) + super().__init__("test-entity") def __eq__(self, other): if type(self) is not type(other): diff --git a/tests/test_generator.py b/tests/test_generator.py index 4ecda339b..e1b620e91 100644 --- a/tests/test_generator.py +++ b/tests/test_generator.py @@ -59,9 +59,7 @@ def test_generate_job_directory(test_dir, wlmutils, generator_instance): """Test Generator.generate_job""" # Create Job launch_settings = LaunchSettings(wlmutils.get_test_launcher()) - app = Application( - "app_name", exe="python", run_settings="RunSettings" - ) # Mock RunSettings + app = Application("app_name", exe="python") # Mock RunSettings job = Job(app, launch_settings) # Mock id run_id = "temp_id" @@ -98,7 +96,7 @@ def test_exp_private_generate_method(wlmutils, test_dir, generator_instance): # Create Experiment exp = Experiment(name="experiment_name", exp_path=test_dir) # Create Job - app = Application("name", "python", run_settings="RunSettings") # Mock RunSettings + app = Application("name", "python") # Mock RunSettings launch_settings = LaunchSettings(wlmutils.get_test_launcher()) job = Job(app, launch_settings) # Generate Job directory @@ -116,7 +114,7 @@ def test_generate_copy_file(generator_instance, fileutils, wlmutils): """Test that attached copy files are copied into Job directory""" # Create the Job and attach copy generator file launch_settings = LaunchSettings(wlmutils.get_test_launcher()) - app = Application("name", "python", run_settings="RunSettings") # Mock RunSettings + app = Application("name", "python") # Mock RunSettings script = fileutils.get_test_conf_path("sleep.py") app.attach_generator_files(to_copy=script) job = Job(app, launch_settings) @@ -130,7 +128,7 @@ def test_generate_copy_file(generator_instance, fileutils, wlmutils): def test_generate_copy_directory(wlmutils, get_gen_copy_dir, generator_instance): # Create the Job and attach generator file launch_settings = LaunchSettings(wlmutils.get_test_launcher()) - app = Application("name", "python", run_settings="RunSettings") # Mock RunSettings + app = Application("name", "python") # Mock RunSettings app.attach_generator_files(to_copy=get_gen_copy_dir) job = Job(app, launch_settings) @@ -143,7 +141,7 @@ def test_generate_copy_directory(wlmutils, get_gen_copy_dir, generator_instance) def test_generate_symlink_directory(wlmutils, generator_instance, get_gen_symlink_dir): # Create the Job and attach generator file launch_settings = LaunchSettings(wlmutils.get_test_launcher()) - app = Application("name", "python", run_settings="RunSettings") # Mock RunSettings + app = Application("name", "python") # Mock RunSettings # Attach directory to Application app.attach_generator_files(to_symlink=get_gen_symlink_dir) # Create Job @@ -166,7 +164,7 @@ def test_generate_symlink_directory(wlmutils, generator_instance, get_gen_symlin def test_generate_symlink_file(get_gen_symlink_dir, wlmutils, generator_instance): # Create the Job and attach generator file launch_settings = LaunchSettings(wlmutils.get_test_launcher()) - app = Application("name", "python", "RunSettings") + app = Application("name", "python") # Path of directory to symlink symlink_dir = get_gen_symlink_dir # Get a list of all files in the directory @@ -210,7 +208,7 @@ def test_generate_configure(fileutils, wlmutils, generator_instance): "1200": "120", "VALID": "valid", } - app = Application("name_1", "python", "RunSettings", params=param_dict) + app = Application("name_1", "python", file_parameters=param_dict) app.attach_generator_files(to_configure=tagged_files) job = Job(app, launch_settings) From fba48d3f7952a93873d4e04154d0362dd9cfc108 Mon Sep 17 00:00:00 2001 From: Julia Putko Date: Mon, 26 Aug 2024 18:07:43 -0500 Subject: [PATCH 05/20] update to model --- smartsim/entity/model.py | 47 ++++++++++++++++------------------------ 1 file changed, 19 insertions(+), 28 deletions(-) diff --git a/smartsim/entity/model.py b/smartsim/entity/model.py index 531f05888..1a28b12e6 100644 --- a/smartsim/entity/model.py +++ b/smartsim/entity/model.py @@ -88,15 +88,6 @@ def __init__( # TODO Discuss with the core team when/if properties shoulda always be returned via reference # or deep copy # TODO Ticket says to remove prefixing, but I think that needs to stay - # TODO @property for _exe - # TODO @exe.setter for _exe - # TODO @property for _files - # TODO @pfiles.setter for _files - # TODO @property for _file_parameters - # TODO @file_parameters.setter for _file_parameters - # TODO @property for _incoming_entities - # TODO @incoming_entities.setter for _incoming_entites - # TODO Update __str__ # TODO Should attached_files_table be deleted and replaced with @property? # TODO Put create pinning string into a new ticket for finding a home for it # TODO check consistency of variable names and constructor with Ensemble, where appropriate @@ -119,24 +110,6 @@ def exe(self, value: t.List[str]) -> None: """ self._exe = value - @property - def files(self) -> t.Optional[EntityFiles]: - """Return files to be copied, symlinked, and/or configured prior to - execution. - - :returns: files - """ - return self._files - - @files.setter - def files(self, value: t.Optional[EntityFiles]) -> None: - """Set files to be copied, symlinked, and/or configured prior to - execution. - - :param value: files - """ - self._files = value - @property def exe_args(self) -> t.Sequence[str]: # TODO why does this say immutable if it is not a deep copy? @@ -155,6 +128,24 @@ def exe_args(self, value: t.Union[str, t.Sequence[str], None]) -> None: # """ self._exe_args = self._build_exe_args(value) + @property + def files(self) -> t.Optional[EntityFiles]: + """Return files to be copied, symlinked, and/or configured prior to + execution. + + :returns: files + """ + return self._files + + @files.setter + def files(self, value: t.Optional[EntityFiles]) -> None: + """Set files to be copied, symlinked, and/or configured prior to + execution. + + :param value: files + """ + self._files = value + @property def file_parameters(self) -> t.Mapping[str, str]: """Return file parameters. @@ -193,7 +184,7 @@ def key_prefixing_enabled(self) -> bool: @key_prefixing_enabled.setter def key_prefixing_enabled(self, value: bool) -> None: - self.key_prefixing_enabled + self.key_prefixing_enabled = value def add_exe_args(self, args: t.Union[str, t.List[str], None]) -> None: """Add executable arguments to executable From a324c32b7168261886abd652db422b237bbe7647 Mon Sep 17 00:00:00 2001 From: Julia Putko Date: Mon, 26 Aug 2024 18:25:02 -0500 Subject: [PATCH 06/20] in progress ensemble refactor --- smartsim/entity/ensemble.py | 123 ++++++++++++++++++++++++++++++++++++ 1 file changed, 123 insertions(+) diff --git a/smartsim/entity/ensemble.py b/smartsim/entity/ensemble.py index 9f9ec6e34..8b36c4c81 100644 --- a/smartsim/entity/ensemble.py +++ b/smartsim/entity/ensemble.py @@ -70,6 +70,129 @@ def __init__( self.permutation_strategy = permutation_strategy self.max_permutations = max_permutations self.replicas = replicas + """Initialize an ``Ensemble`` of application instances + + :param name: name of the ensemble + :param exe: executable to run + :param exe_args: executable arguments + :param exe_arg_parameters: + :param files: files to be copied, symlinked, and/or configured prior to + execution + :param file_parameters: parameters and values to be used when configuring + files + :param permutation_strategy: + :param max_permutations: + :param replicas: number of ``Application`` replicas to create + :return: and ``Ensemble`` instance + """ + + + @property + def exe(self) -> str | os.PathLike[str]: + """Return executable to run. + + :returns: application executable to run + """ + return self.exe + + @exe.setter + def exe(self, value: str | os.PathLike[str]) -> None: + """Set executable to run. + + :param value: executable to run + """ + self.exe = value + + @property + def exe_args(self) -> t.Sequence[str] | None: + """Return an immutable list of attached executable arguments. + + :returns: application executable arguments + """ + return self.exe_args + + @exe_args.setter + def exe_args(self, value: t.Sequence[str] | None) -> None: + """Set the executable arguments. + + :param value: executable arguments + """ + self.exe_args = value + + @property + def exe_arg_parameters(self) -> t.Mapping[str, t.Sequence[t.Sequence[str]]] | None: + """Return the executable argument parameters + + :returns: executable arguments parameters + """ + return self.exe_arg_parameters + + @exe_arg_parameters.setter + def exe_arg_parameters(self, value: t.Mapping[str, t.Sequence[t.Sequence[str]]] | None) -> None: + """Set the executable arguments. + + :param value: executable arguments + """ + self.exe_arg_parameters = value + + @property + def files(self) -> EntityFiles | None: + """Return files to be copied, symlinked, and/or configured prior to + execution. + + :returns: files + """ + return self.files + + @files.setter + def files(self, value: EntityFiles | None) -> None: + """Set files to be copied, symlinked, and/or configured prior to + execution. + + :param value: files + """ + self.files = value + + @property + def file_parameters(self) -> t.Mapping[str, t.Sequence[str]] | None: + """Return file parameters. + + :returns: application file parameters + """ + return self.file_parameters + + @file_parameters.setter + def file_parameters(self, value: t.Mapping[str, t.Sequence[str]] | None) -> None: + """Set the file parameters. + + :param value: file parameters + """ + self.file_parameters = value + + @property + def permutation_strategy(self) -> str | strategies.PermutationStrategyType: + return self.permutation_strategy + + @permutation_strategy.setter + def permutation_strategy(self, value: str | strategies.PermutationStrategyType) -> None: + self.permutation_strategy = value + + @property + def max_permutations(self) -> int: + return self.max_permutations + + @max_permutations.setter + def max_permutations(self, value: int) -> None: + self.max_permutations = value + + @property + def replicas(self) -> int: + return self.replicas + + @replicas.setter + def replicas(self, value: int) -> None: + self.replicas = value + def _create_applications(self) -> tuple[Application, ...]: """Concretize the ensemble attributes into a collection of From e48541db806c74e96a0f11efd03d328c61c79af1 Mon Sep 17 00:00:00 2001 From: Julia Putko Date: Tue, 27 Aug 2024 17:25:54 -0500 Subject: [PATCH 07/20] pr review comments addressed --- smartsim/_core/utils/helpers.py | 19 +++---- smartsim/entity/__init__.py | 2 +- smartsim/entity/{model.py => application.py} | 53 ++++++++------------ smartsim/entity/dbnode.py | 1 + smartsim/entity/ensemble.py | 2 +- smartsim/experiment.py | 2 +- tests/_legacy/test_output_files.py | 2 +- tests/_legacy/test_symlinking.py | 2 +- tests/temp_tests/test_colocatedJobGroup.py | 3 +- tests/temp_tests/test_jobGroup.py | 4 +- tests/temp_tests/test_launchable.py | 5 +- tests/test_experiment.py | 2 +- tests/test_generator.py | 12 ++--- 13 files changed, 48 insertions(+), 61 deletions(-) rename smartsim/entity/{model.py => application.py} (82%) diff --git a/smartsim/_core/utils/helpers.py b/smartsim/_core/utils/helpers.py index d26c71664..344814250 100644 --- a/smartsim/_core/utils/helpers.py +++ b/smartsim/_core/utils/helpers.py @@ -336,6 +336,16 @@ def execute_platform_cmd(cmd: str) -> t.Tuple[str, int]: return process.stdout.decode("utf-8"), process.returncode +def _stringify_id(_id: int) -> str: + """Return the CPU id as a string if an int, otherwise raise a ValueError""" + if isinstance(_id, int): + if _id < 0: + raise ValueError("CPU id must be a nonnegative number") + return str(_id) + + raise TypeError(f"Argument is of type '{type(_id)}' not 'int'") + + class CrayExPlatformResult: locate_msg = "Unable to locate `{0}`." @@ -577,15 +587,6 @@ def _create_pinning_string( integers (e.g. ``[0, 2, 5]`` -> ``"0,2,5"``) """ - def _stringify_id(_id: int) -> str: - """Return the cPU id as a string if an int, otherwise raise a ValueError""" - if isinstance(_id, int): - if _id < 0: - raise ValueError("CPU id must be a nonnegative number") - return str(_id) - - raise TypeError(f"Argument is of type '{type(_id)}' not 'int'") - try: pin_ids = tuple(pin_ids) if pin_ids is not None else None except TypeError: diff --git a/smartsim/entity/__init__.py b/smartsim/entity/__init__.py index ce6140844..34e0e2178 100644 --- a/smartsim/entity/__init__.py +++ b/smartsim/entity/__init__.py @@ -30,4 +30,4 @@ from .entity import SmartSimEntity, TelemetryConfiguration from .entityList import EntityList, EntitySequence from .files import TaggedFilesHierarchy -from .model import Application +from .application import Application diff --git a/smartsim/entity/model.py b/smartsim/entity/application.py similarity index 82% rename from smartsim/entity/model.py rename to smartsim/entity/application.py index 1a28b12e6..8b5e1f379 100644 --- a/smartsim/entity/model.py +++ b/smartsim/entity/application.py @@ -70,7 +70,7 @@ def __init__( files """ super().__init__(name) - self._exe = [expand_exe_path(exe)] + self._exe = expand_exe_path(exe) self._exe_args = self._build_exe_args(exe_args) or [] self._files = copy.deepcopy(files) if files else None self._file_parameters = ( @@ -79,23 +79,8 @@ def __init__( self._incoming_entities: t.List[SmartSimEntity] = [] self._key_prefixing_enabled = False - # TODO Talk through as a group if _key_prefixing_enabled - # should have proeprty and setter decorators or do we stick with something of similar syntax. - # Bring this up to the group after the rest of the class is done so they see what a consistent - # API is currently being formed. - # TODO Discuss if the exe_args parameter should be set with a str in the construct - # and setter or should we stick to t.Sequence[str] only. This might require group discussion. - # TODO Discuss with the core team when/if properties shoulda always be returned via reference - # or deep copy - # TODO Ticket says to remove prefixing, but I think that needs to stay - # TODO Should attached_files_table be deleted and replaced with @property? - # TODO Put create pinning string into a new ticket for finding a home for it - # TODO check consistency of variable names and constructor with Ensemble, where appropriate - # TODO Unit tests!!! - # TODO Cleanup documentation - @property - def exe(self) -> t.List[str]: + def exe(self) -> str: """Return executable to run. :returns: application executable to run @@ -103,22 +88,20 @@ def exe(self) -> t.List[str]: return self._exe @exe.setter - def exe(self, value: t.List[str]) -> None: + def exe(self, value: str) -> None: """Set executable to run. :param value: executable to run """ - self._exe = value + self._exe = copy.deepcopy(value) @property def exe_args(self) -> t.Sequence[str]: - # TODO why does this say immutable if it is not a deep copy? - # TODO review whether this should be a deepcopy - are we relying of having this be a reference? """Return an immutable list of attached executable arguments. :returns: application executable arguments """ - return copy.deepcopy(self._exe_args) + return self._exe_args @exe_args.setter def exe_args(self, value: t.Union[str, t.Sequence[str], None]) -> None: # @@ -144,7 +127,7 @@ def files(self, value: t.Optional[EntityFiles]) -> None: :param value: files """ - self._files = value + self._files = copy.deepcopy(value) @property def file_parameters(self) -> t.Mapping[str, str]: @@ -160,7 +143,7 @@ def file_parameters(self, value: t.Mapping[str, str]) -> None: :param value: file parameters """ - self._file_parameters = value + self._file_parameters = copy.deepcopy(value) @property def incoming_entities(self) -> t.List[SmartSimEntity]: @@ -176,15 +159,23 @@ def incoming_entities(self, value: t.List[SmartSimEntity]) -> None: :param value: incoming entities """ - self._incoming_entities = value + self._incoming_entities = copy.deepcopy(value) @property def key_prefixing_enabled(self) -> bool: + """Return whether key prefixing is enabled for the application. + + :param value: key prefixing enabled + """ return self._key_prefixing_enabled @key_prefixing_enabled.setter def key_prefixing_enabled(self, value: bool) -> None: - self.key_prefixing_enabled = value + """Set whether key prefixing is enabled for the application. + + :param value: key prefixing enabled + """ + self.key_prefixing_enabled = copy.deepcopy(value) def add_exe_args(self, args: t.Union[str, t.List[str], None]) -> None: """Add executable arguments to executable @@ -253,10 +244,6 @@ def print_attached_files(self) -> None: """Print a table of the attached files on std out""" print(self.attached_files_table) - def params_to_args(self) -> None: - """Convert parameters to command line arguments and update run settings.""" - ... - def __str__(self) -> str: # pragma: no cover entity_str = "Name: " + self.name + "\n" entity_str += "Type: " + self.type + "\n" @@ -266,12 +253,12 @@ def __str__(self) -> str: # pragma: no cover entity_str += "Executable Arguments:\n" for ex_arg in self.exe_args: entity_str += f"{str(ex_arg)}\n" - entity_str += "Entity Files: " + str(self.files) + "\n" - entity_str += "File Parameters: " + str(self.file_parameters) + "\n" + entity_str += f"Entity Files: {self.files}\n" + entity_str += f"File Parameters: {self.file_parameters}\n" entity_str += "Incoming Entities:\n" for entity in self.incoming_entities: entity_str += f"{entity}\n" - entity_str += "Key Prefixing Enabled: " + str(self.key_prefixing_enabled) + "\n" + entity_str += f"Key Prefixing Enabled: {self.key_prefixing_enabled}\n" return entity_str diff --git a/smartsim/entity/dbnode.py b/smartsim/entity/dbnode.py index fa6298183..60a69b522 100644 --- a/smartsim/entity/dbnode.py +++ b/smartsim/entity/dbnode.py @@ -65,6 +65,7 @@ def __init__( ) -> None: """Initialize a feature store node within an feature store.""" super().__init__(name) + self.run_settings = run_settings self.exe = [exe] if run_settings.container else [expand_exe_path(exe)] self.exe_args = exe_args or [] self.ports = ports diff --git a/smartsim/entity/ensemble.py b/smartsim/entity/ensemble.py index 9f9ec6e34..fbae4080f 100644 --- a/smartsim/entity/ensemble.py +++ b/smartsim/entity/ensemble.py @@ -34,7 +34,7 @@ from smartsim.entity import _mock, entity, strategies from smartsim.entity.files import EntityFiles -from smartsim.entity.model import Application +from smartsim.entity.application import Application from smartsim.entity.strategies import ParamSet from smartsim.launchable.job import Job diff --git a/smartsim/experiment.py b/smartsim/experiment.py index ef3725269..0705fb47e 100644 --- a/smartsim/experiment.py +++ b/smartsim/experiment.py @@ -87,7 +87,7 @@ class Experiment: """The Experiment class is used to schedule, launch, track, and manage jobs and job groups. Also, it is the SmartSim class that manages internal data structures, processes, and infrastructure for interactive - capabilties such as the SmartSim dashboard and historical lookback on + capabilities such as the SmartSim dashboard and historical lookback on launched jobs and job groups. The Experiment class is designed to be initialized once and utilized throughout the entirety of a workflow. """ diff --git a/tests/_legacy/test_output_files.py b/tests/_legacy/test_output_files.py index 3b786548f..58bc056fe 100644 --- a/tests/_legacy/test_output_files.py +++ b/tests/_legacy/test_output_files.py @@ -35,7 +35,7 @@ from smartsim._core.launcher.step import Step from smartsim.database.orchestrator import FeatureStore from smartsim.entity.ensemble import Ensemble -from smartsim.entity.model import Application +from smartsim.entity.application import Application from smartsim.settings.base import RunSettings from smartsim.settings.slurmSettings import SbatchSettings, SrunSettings diff --git a/tests/_legacy/test_symlinking.py b/tests/_legacy/test_symlinking.py index 11219a81b..a2c331ab6 100644 --- a/tests/_legacy/test_symlinking.py +++ b/tests/_legacy/test_symlinking.py @@ -34,7 +34,7 @@ from smartsim._core.control.controller import Controller, _AnonymousBatchJob from smartsim.database.orchestrator import FeatureStore from smartsim.entity.ensemble import Ensemble -from smartsim.entity.model import Application +from smartsim.entity.application import Application from smartsim.settings.base import RunSettings from smartsim.settings.slurmSettings import SbatchSettings, SrunSettings diff --git a/tests/temp_tests/test_colocatedJobGroup.py b/tests/temp_tests/test_colocatedJobGroup.py index 1e76c2f8d..90a5e254c 100644 --- a/tests/temp_tests/test_colocatedJobGroup.py +++ b/tests/temp_tests/test_colocatedJobGroup.py @@ -26,7 +26,7 @@ import pytest -from smartsim.entity.model import Application +from smartsim.entity.application import Application from smartsim.launchable.basejob import BaseJob from smartsim.launchable.colocatedJobGroup import ColocatedJobGroup from smartsim.launchable.job import Job @@ -34,7 +34,6 @@ pytestmark = pytest.mark.group_a -# TODO replace with LaunchSettings app_1 = Application("app_1", "python") app_2 = Application("app_2", "python") app_3 = Application("app_3", "python") diff --git a/tests/temp_tests/test_jobGroup.py b/tests/temp_tests/test_jobGroup.py index d870a340d..5f27199b5 100644 --- a/tests/temp_tests/test_jobGroup.py +++ b/tests/temp_tests/test_jobGroup.py @@ -26,14 +26,14 @@ import pytest -from smartsim.entity.model import Application +from smartsim.entity.application import Application from smartsim.launchable.basejob import BaseJob from smartsim.launchable.job import Job from smartsim.launchable.jobGroup import JobGroup from smartsim.settings.launchSettings import LaunchSettings pytestmark = pytest.mark.group_a -# TODO replace with LaunchSettings + app_1 = Application("app_1", "python") app_2 = Application("app_2", "python") app_3 = Application("app_3", "python") diff --git a/tests/temp_tests/test_launchable.py b/tests/temp_tests/test_launchable.py index 68c13c350..9dbe32c5b 100644 --- a/tests/temp_tests/test_launchable.py +++ b/tests/temp_tests/test_launchable.py @@ -28,7 +28,7 @@ from smartsim.entity.ensemble import Ensemble from smartsim.entity.entity import SmartSimEntity -from smartsim.entity.model import Application +from smartsim.entity.application import Application from smartsim.error.errors import SSUnsupportedError from smartsim.launchable import Job, Launchable from smartsim.launchable.launchable import SmartSimObject @@ -37,7 +37,6 @@ from smartsim.settings import LaunchSettings pytestmark = pytest.mark.group_a -# TODO replace with LaunchSettings def test_smartsimobject_init(): @@ -55,7 +54,7 @@ def test_invalid_job_name(wlmutils): "test_name", exe="echo", exe_args=["spam", "eggs"], - ) # Mock RunSettings + ) settings = LaunchSettings(wlmutils.get_test_launcher()) with pytest.raises(ValueError): diff --git a/tests/test_experiment.py b/tests/test_experiment.py index 4ea22a1a0..d29f14f87 100644 --- a/tests/test_experiment.py +++ b/tests/test_experiment.py @@ -37,7 +37,7 @@ from smartsim._core import dispatch from smartsim._core.control.launch_history import LaunchHistory -from smartsim.entity import _mock, entity +from smartsim.entity import entity from smartsim.experiment import Experiment from smartsim.launchable import job from smartsim.settings import launchSettings diff --git a/tests/test_generator.py b/tests/test_generator.py index e1b620e91..6a60403e4 100644 --- a/tests/test_generator.py +++ b/tests/test_generator.py @@ -12,7 +12,7 @@ from smartsim import Experiment from smartsim._core import dispatch from smartsim._core.generation.generator import Generator -from smartsim.entity import Application, Ensemble, SmartSimEntity, _mock +from smartsim.entity import Application, Ensemble, SmartSimEntity from smartsim.entity.files import EntityFiles from smartsim.launchable import Job from smartsim.settings import LaunchSettings @@ -59,7 +59,7 @@ def test_generate_job_directory(test_dir, wlmutils, generator_instance): """Test Generator.generate_job""" # Create Job launch_settings = LaunchSettings(wlmutils.get_test_launcher()) - app = Application("app_name", exe="python") # Mock RunSettings + app = Application("app_name", exe="python") job = Job(app, launch_settings) # Mock id run_id = "temp_id" @@ -96,7 +96,7 @@ def test_exp_private_generate_method(wlmutils, test_dir, generator_instance): # Create Experiment exp = Experiment(name="experiment_name", exp_path=test_dir) # Create Job - app = Application("name", "python") # Mock RunSettings + app = Application("name", "python") launch_settings = LaunchSettings(wlmutils.get_test_launcher()) job = Job(app, launch_settings) # Generate Job directory @@ -114,7 +114,7 @@ def test_generate_copy_file(generator_instance, fileutils, wlmutils): """Test that attached copy files are copied into Job directory""" # Create the Job and attach copy generator file launch_settings = LaunchSettings(wlmutils.get_test_launcher()) - app = Application("name", "python") # Mock RunSettings + app = Application("name", "python") script = fileutils.get_test_conf_path("sleep.py") app.attach_generator_files(to_copy=script) job = Job(app, launch_settings) @@ -128,7 +128,7 @@ def test_generate_copy_file(generator_instance, fileutils, wlmutils): def test_generate_copy_directory(wlmutils, get_gen_copy_dir, generator_instance): # Create the Job and attach generator file launch_settings = LaunchSettings(wlmutils.get_test_launcher()) - app = Application("name", "python") # Mock RunSettings + app = Application("name", "python") app.attach_generator_files(to_copy=get_gen_copy_dir) job = Job(app, launch_settings) @@ -141,7 +141,7 @@ def test_generate_copy_directory(wlmutils, get_gen_copy_dir, generator_instance) def test_generate_symlink_directory(wlmutils, generator_instance, get_gen_symlink_dir): # Create the Job and attach generator file launch_settings = LaunchSettings(wlmutils.get_test_launcher()) - app = Application("name", "python") # Mock RunSettings + app = Application("name", "python") # Attach directory to Application app.attach_generator_files(to_symlink=get_gen_symlink_dir) # Create Job From cc22b54182fff6c32348a40d073ee06e87f86c09 Mon Sep 17 00:00:00 2001 From: Julia Putko Date: Wed, 28 Aug 2024 14:39:17 -0500 Subject: [PATCH 08/20] doc strings and isort fixes --- smartsim/_core/utils/helpers.py | 11 ++++++++++- smartsim/entity/__init__.py | 2 +- smartsim/entity/application.py | 20 ++++++++++++++++---- smartsim/entity/ensemble.py | 2 +- tests/_legacy/test_output_files.py | 2 +- tests/_legacy/test_symlinking.py | 2 +- tests/temp_tests/test_launchable.py | 2 +- 7 files changed, 31 insertions(+), 10 deletions(-) diff --git a/smartsim/_core/utils/helpers.py b/smartsim/_core/utils/helpers.py index ceea20bb8..96dbe074a 100644 --- a/smartsim/_core/utils/helpers.py +++ b/smartsim/_core/utils/helpers.py @@ -288,7 +288,11 @@ def execute_platform_cmd(cmd: str) -> t.Tuple[str, int]: def _stringify_id(_id: int) -> str: - """Return the CPU id as a string if an int, otherwise raise a ValueError""" + """Return the CPU id as a string if an int, otherwise raise a ValueError + + :params _id: the CPU id as an int + :returns: the CPU as a string + """ if isinstance(_id, int): if _id < 0: raise ValueError("CPU id must be a nonnegative number") @@ -536,6 +540,11 @@ def _create_pinning_string( returns 0,1,...,cpus-1; an empty iterable will disable pinning altogether, and an iterable constructs a comma separated string of integers (e.g. ``[0, 2, 5]`` -> ``"0,2,5"``) + + :params pin_ids: CPU ids + :params cpu: number of CPUs + :raises TypeError: if pin id is not an iterable of ints + :returns: a comma separated string of CPU ids """ try: diff --git a/smartsim/entity/__init__.py b/smartsim/entity/__init__.py index 34e0e2178..7ffa290b2 100644 --- a/smartsim/entity/__init__.py +++ b/smartsim/entity/__init__.py @@ -24,10 +24,10 @@ # OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +from .application import Application from .dbnode import FSNode from .dbobject import * from .ensemble import Ensemble from .entity import SmartSimEntity, TelemetryConfiguration from .entityList import EntityList, EntitySequence from .files import TaggedFilesHierarchy -from .application import Application diff --git a/smartsim/entity/application.py b/smartsim/entity/application.py index 8b5e1f379..e416dadd6 100644 --- a/smartsim/entity/application.py +++ b/smartsim/entity/application.py @@ -70,14 +70,21 @@ def __init__( files """ super().__init__(name) + """The name of the application""" self._exe = expand_exe_path(exe) + """The executable to run""" self._exe_args = self._build_exe_args(exe_args) or [] + """The executable arguments""" self._files = copy.deepcopy(files) if files else None + """Files to be copied, symlinked, and/or configured prior to execution""" self._file_parameters = ( copy.deepcopy(file_parameters) if file_parameters else {} ) + """Parameters and values to be used when configuring files""" self._incoming_entities: t.List[SmartSimEntity] = [] + """Entities for which the prefix will have to be known by other entities""" self._key_prefixing_enabled = False + """Unique prefix to avoid key collisions""" @property def exe(self) -> str: @@ -97,7 +104,7 @@ def exe(self, value: str) -> None: @property def exe_args(self) -> t.Sequence[str]: - """Return an immutable list of attached executable arguments. + """Return a list of attached executable arguments. :returns: application executable arguments """ @@ -212,6 +219,7 @@ def attach_generator_files( :param to_copy: files to copy :param to_symlink: files to symlink :param to_configure: input files with tagged parameters + :raises ValueError: if the generator file already exists """ to_copy = to_copy or [] to_symlink = to_symlink or [] @@ -266,12 +274,16 @@ def __str__(self) -> str: # pragma: no cover def _build_exe_args( exe_args: t.Optional[t.Union[str, t.Sequence[str], None]] ) -> t.List[str]: - """Check and convert exe_args input to a desired collection format""" + """Check and convert exe_args input to a desired collection format + + :param exe_args: + :raises TypeError: if exe_args is not a list of str or str + """ if not exe_args: return [] if isinstance(exe_args, list): - exe_args = copy.deepcopy(exe_args) + exe_args = exe_args if not ( isinstance(exe_args, str) @@ -283,6 +295,6 @@ def _build_exe_args( raise TypeError("Executable arguments were not a list of str or a str.") if isinstance(exe_args, str): - return exe_args.split() + return copy.deepcopy(exe_args.split()) return exe_args diff --git a/smartsim/entity/ensemble.py b/smartsim/entity/ensemble.py index fbae4080f..bf35a46a9 100644 --- a/smartsim/entity/ensemble.py +++ b/smartsim/entity/ensemble.py @@ -33,8 +33,8 @@ import typing as t from smartsim.entity import _mock, entity, strategies -from smartsim.entity.files import EntityFiles from smartsim.entity.application import Application +from smartsim.entity.files import EntityFiles from smartsim.entity.strategies import ParamSet from smartsim.launchable.job import Job diff --git a/tests/_legacy/test_output_files.py b/tests/_legacy/test_output_files.py index 58bc056fe..713001feb 100644 --- a/tests/_legacy/test_output_files.py +++ b/tests/_legacy/test_output_files.py @@ -34,8 +34,8 @@ from smartsim._core.control.controller import Controller, _AnonymousBatchJob from smartsim._core.launcher.step import Step from smartsim.database.orchestrator import FeatureStore -from smartsim.entity.ensemble import Ensemble from smartsim.entity.application import Application +from smartsim.entity.ensemble import Ensemble from smartsim.settings.base import RunSettings from smartsim.settings.slurmSettings import SbatchSettings, SrunSettings diff --git a/tests/_legacy/test_symlinking.py b/tests/_legacy/test_symlinking.py index a2c331ab6..4447a49d1 100644 --- a/tests/_legacy/test_symlinking.py +++ b/tests/_legacy/test_symlinking.py @@ -33,8 +33,8 @@ from smartsim._core.config import CONFIG from smartsim._core.control.controller import Controller, _AnonymousBatchJob from smartsim.database.orchestrator import FeatureStore -from smartsim.entity.ensemble import Ensemble from smartsim.entity.application import Application +from smartsim.entity.ensemble import Ensemble from smartsim.settings.base import RunSettings from smartsim.settings.slurmSettings import SbatchSettings, SrunSettings diff --git a/tests/temp_tests/test_launchable.py b/tests/temp_tests/test_launchable.py index 9dbe32c5b..b4da5564d 100644 --- a/tests/temp_tests/test_launchable.py +++ b/tests/temp_tests/test_launchable.py @@ -26,9 +26,9 @@ import pytest +from smartsim.entity.application import Application from smartsim.entity.ensemble import Ensemble from smartsim.entity.entity import SmartSimEntity -from smartsim.entity.application import Application from smartsim.error.errors import SSUnsupportedError from smartsim.launchable import Job, Launchable from smartsim.launchable.launchable import SmartSimObject From 8b72051b6ce88d564a575fe0475bb2484979823a Mon Sep 17 00:00:00 2001 From: Julia Putko Date: Wed, 28 Aug 2024 16:25:36 -0500 Subject: [PATCH 09/20] __str refactor; deepycopy changes; clean --- smartsim/_core/utils/helpers.py | 2 +- smartsim/entity/application.py | 51 ++++++++++++++------------------- 2 files changed, 22 insertions(+), 31 deletions(-) diff --git a/smartsim/_core/utils/helpers.py b/smartsim/_core/utils/helpers.py index 96dbe074a..56eaa98d3 100644 --- a/smartsim/_core/utils/helpers.py +++ b/smartsim/_core/utils/helpers.py @@ -289,7 +289,7 @@ def execute_platform_cmd(cmd: str) -> t.Tuple[str, int]: def _stringify_id(_id: int) -> str: """Return the CPU id as a string if an int, otherwise raise a ValueError - + :params _id: the CPU id as an int :returns: the CPU as a string """ diff --git a/smartsim/entity/application.py b/smartsim/entity/application.py index e416dadd6..c5369bc31 100644 --- a/smartsim/entity/application.py +++ b/smartsim/entity/application.py @@ -27,6 +27,7 @@ from __future__ import annotations import copy +import textwrap import typing as t from os import path as osp @@ -35,13 +36,6 @@ from .entity import SmartSimEntity from .files import EntityFiles -if t.TYPE_CHECKING: - from smartsim.types import TODO - - RunSettings = TODO - BatchSettings = TODO - - logger = get_logger(__name__) @@ -166,7 +160,7 @@ def incoming_entities(self, value: t.List[SmartSimEntity]) -> None: :param value: incoming entities """ - self._incoming_entities = copy.deepcopy(value) + self._incoming_entities = copy.copy(value) @property def key_prefixing_enabled(self) -> bool: @@ -253,29 +247,26 @@ def print_attached_files(self) -> None: print(self.attached_files_table) def __str__(self) -> str: # pragma: no cover - entity_str = "Name: " + self.name + "\n" - entity_str += "Type: " + self.type + "\n" - entity_str += "Executable:\n" - for ex in self.exe: - entity_str += f"{ex}\n" - entity_str += "Executable Arguments:\n" - for ex_arg in self.exe_args: - entity_str += f"{str(ex_arg)}\n" - entity_str += f"Entity Files: {self.files}\n" - entity_str += f"File Parameters: {self.file_parameters}\n" - entity_str += "Incoming Entities:\n" - for entity in self.incoming_entities: - entity_str += f"{entity}\n" - entity_str += f"Key Prefixing Enabled: {self.key_prefixing_enabled}\n" - - return entity_str + exe_args_str = "\n".join(self.exe_args) + entities_str = "\n".join(str(entity) for entity in self.incoming_entities) + return textwrap.dedent(f"""\ + Name: {self.name} + Type: {self.type} + Executable: + {self.exe} + Executable Arguments: + {exe_args_str} + Entity Files: {self.files} + File Parameters: {self.file_parameters} + Incoming Entities: + {entities_str} + Key Prefixing Enabled: {self.key_prefixing_enabled} + """) @staticmethod - def _build_exe_args( - exe_args: t.Optional[t.Union[str, t.Sequence[str], None]] - ) -> t.List[str]: + def _build_exe_args(exe_args: str | list[str] | None) -> t.List[str]: """Check and convert exe_args input to a desired collection format - + :param exe_args: :raises TypeError: if exe_args is not a list of str or str """ @@ -283,7 +274,7 @@ def _build_exe_args( return [] if isinstance(exe_args, list): - exe_args = exe_args + exe_args = copy.deepcopy(exe_args) if not ( isinstance(exe_args, str) @@ -295,6 +286,6 @@ def _build_exe_args( raise TypeError("Executable arguments were not a list of str or a str.") if isinstance(exe_args, str): - return copy.deepcopy(exe_args.split()) + return exe_args.split() return exe_args From 2330f7a25f63160b6e71dd09e0d26c6c1896a7a6 Mon Sep 17 00:00:00 2001 From: Julia Putko Date: Wed, 28 Aug 2024 17:15:49 -0500 Subject: [PATCH 10/20] ensemble update and mypy changes --- smartsim/entity/application.py | 7 +- smartsim/entity/ensemble.py | 251 +++++++++++++++++++-------------- 2 files changed, 143 insertions(+), 115 deletions(-) diff --git a/smartsim/entity/application.py b/smartsim/entity/application.py index e416dadd6..a2c8c992e 100644 --- a/smartsim/entity/application.py +++ b/smartsim/entity/application.py @@ -43,8 +43,6 @@ logger = get_logger(__name__) - - # TODO: Remove this supression when we strip fileds/functionality # (run-settings/batch_settings/params_as_args/etc)! # pylint: disable-next=too-many-public-methods @@ -282,9 +280,6 @@ def _build_exe_args( if not exe_args: return [] - if isinstance(exe_args, list): - exe_args = exe_args - if not ( isinstance(exe_args, str) or ( @@ -297,4 +292,4 @@ def _build_exe_args( if isinstance(exe_args, str): return copy.deepcopy(exe_args.split()) - return exe_args + return copy.deepcopy(exe_args) diff --git a/smartsim/entity/ensemble.py b/smartsim/entity/ensemble.py index d9c064c57..4066958e7 100644 --- a/smartsim/entity/ensemble.py +++ b/smartsim/entity/ensemble.py @@ -32,7 +32,7 @@ import os.path import typing as t -from smartsim.entity import _mock, entity, strategies +from smartsim.entity import entity, strategies from smartsim.entity.application import Application from smartsim.entity.files import EntityFiles from smartsim.entity.strategies import ParamSet @@ -59,139 +59,171 @@ def __init__( max_permutations: int = -1, replicas: int = 1, ) -> None: - self.name = name - self.exe = os.fspath(exe) - self.exe_args = list(exe_args) if exe_args else [] - self.exe_arg_parameters = ( - copy.deepcopy(exe_arg_parameters) if exe_arg_parameters else {} - ) - self.files = copy.deepcopy(files) if files else EntityFiles() - self.file_parameters = dict(file_parameters) if file_parameters else {} - self.permutation_strategy = permutation_strategy - self.max_permutations = max_permutations - self.replicas = replicas """Initialize an ``Ensemble`` of application instances :param name: name of the ensemble :param exe: executable to run :param exe_args: executable arguments - :param exe_arg_parameters: + :param exe_arg_parameters: parameters and values to be used when configuring entities :param files: files to be copied, symlinked, and/or configured prior to execution :param file_parameters: parameters and values to be used when configuring files - :param permutation_strategy: - :param max_permutations: - :param replicas: number of ``Application`` replicas to create + :param permutation_strategy: strategy to control how the param values are applied to the Ensemble + :param max_permutations: max parameter permutations to set for the ensemble + :param replicas: number of identical entities to create within an Ensemble :return: and ``Ensemble`` instance """ + self.name = name + """The name of the ensemble """ + self._exe = os.fspath(exe) + """The executable to run """ + self._exe_args = list(exe_args) if exe_args else [] + """The executable arguments """ + self._exe_arg_parameters = ( + copy.deepcopy(exe_arg_parameters) if exe_arg_parameters else {} + ) + """The parameters and values to be used when configuring entities""" + self._files = copy.deepcopy(files) if files else EntityFiles() + """The files to be copied, symlinked, and/or configured prior to execution""" + self._file_parameters = dict(file_parameters) if file_parameters else {} + """The parameters and values to be used when configuring files""" + self._permutation_strategy = permutation_strategy + """The strategy to control how the param values are applied to the Ensemble""" + self._max_permutations = max_permutations + """The maximum number of entities to come out of the permutation strategy""" + self._replicas = replicas + """How many identical entities to create within an Ensemble""" + + @property + def exe(self) -> str: + """Return executable to run. + :returns: application executable to run + """ + return self._exe + + @exe.setter + def exe(self, value: str | os.PathLike[str]) -> None: + """Set executable to run. - @property - def exe(self) -> str | os.PathLike[str]: - """Return executable to run. + :param value: executable to run + """ + self._exe = os.fspath(value) - :returns: application executable to run - """ - return self.exe - - @exe.setter - def exe(self, value: str | os.PathLike[str]) -> None: - """Set executable to run. + @property + def exe_args(self) -> t.Sequence[str]: + """Return an immutable list of attached executable arguments. - :param value: executable to run - """ - self.exe = value + :returns: application executable arguments + """ + return self._exe_args + + @exe_args.setter + def exe_args(self, value: t.Sequence[str]) -> None: + """Set the executable arguments. - @property - def exe_args(self) -> t.Sequence[str] | None: - """Return an immutable list of attached executable arguments. + :param value: executable arguments + """ + self._exe_args = list(value) - :returns: application executable arguments - """ - return self.exe_args - - @exe_args.setter - def exe_args(self, value: t.Sequence[str] | None) -> None: - """Set the executable arguments. + @property + def exe_arg_parameters(self) -> t.Mapping[str, t.Sequence[t.Sequence[str]]]: + """Return the executable argument parameters - :param value: executable arguments - """ - self.exe_args = value + :returns: executable arguments parameters + """ + return self._exe_arg_parameters + + @exe_arg_parameters.setter + def exe_arg_parameters(self, value: t.Mapping[str, t.Sequence[t.Sequence[str]]]) -> None: + """Set the executable arguments. - @property - def exe_arg_parameters(self) -> t.Mapping[str, t.Sequence[t.Sequence[str]]] | None: - """Return the executable argument parameters + :param value: executable arguments + """ + self._exe_arg_parameters = copy.deepcopy(value) - :returns: executable arguments parameters - """ - return self.exe_arg_parameters - - @exe_arg_parameters.setter - def exe_arg_parameters(self, value: t.Mapping[str, t.Sequence[t.Sequence[str]]] | None) -> None: - """Set the executable arguments. - - :param value: executable arguments - """ - self.exe_arg_parameters = value - - @property - def files(self) -> EntityFiles | None: - """Return files to be copied, symlinked, and/or configured prior to - execution. - - :returns: files - """ - return self.files - - @files.setter - def files(self, value: EntityFiles | None) -> None: - """Set files to be copied, symlinked, and/or configured prior to - execution. - - :param value: files - """ - self.files = value - - @property - def file_parameters(self) -> t.Mapping[str, t.Sequence[str]] | None: - """Return file parameters. + @property + def files(self) -> EntityFiles: + """Return files to be copied, symlinked, and/or configured prior to + execution. - :returns: application file parameters - """ - return self.file_parameters - - @file_parameters.setter - def file_parameters(self, value: t.Mapping[str, t.Sequence[str]] | None) -> None: - """Set the file parameters. + :returns: files + """ + return self._files + + @files.setter + def files(self, value: EntityFiles) -> None: + """Set files to be copied, symlinked, and/or configured prior to + execution. - :param value: file parameters - """ - self.file_parameters = value + :param value: files + """ + self._files = copy.deepcopy(value) + + @property + def file_parameters(self) -> t.Mapping[str, t.Sequence[str]]: + """Return file parameters. - @property - def permutation_strategy(self) -> str | strategies.PermutationStrategyType: - return self.permutation_strategy - - @permutation_strategy.setter - def permutation_strategy(self, value: str | strategies.PermutationStrategyType) -> None: - self.permutation_strategy = value - - @property - def max_permutations(self) -> int: - return self.max_permutations - - @max_permutations.setter - def max_permutations(self, value: int) -> None: - self.max_permutations = value + :returns: application file parameters + """ + return self._file_parameters + + @file_parameters.setter + def file_parameters(self, value: t.Mapping[str, t.Sequence[str]]) -> None: + """Set the file parameters. + + :param value: file parameters + """ + self._file_parameters = dict(value) + + @property + def permutation_strategy(self) -> str | strategies.PermutationStrategyType: + """Return the permutation strategy + + :return: permutation strategy + """ + return self._permutation_strategy + + @permutation_strategy.setter + def permutation_strategy(self, value: str | strategies.PermutationStrategyType) -> None: + """Set the permutation strategy - @property - def replicas(self) -> int: - return self.replicas + :param value: permutation strategy + """ + self._permutation_strategy = value + + @property + def max_permutations(self) -> int: + """Return the maximum permutations + + :return: max permutations + """ + return self._max_permutations + + @max_permutations.setter + def max_permutations(self, value: int) -> None: + """Set the maximum permutations + + :param value: the maxpermutations + """ + self._max_permutations = value + + @property + def replicas(self) -> int: + """Return the number of replicas - @replicas.setter - def replicas(self, value: int) -> None: - self.replicas = value + :return: number of replicas + """ + return self._replicas + + @replicas.setter + def replicas(self, value: int) -> None: + """Set the number of replicas + + :return: the number of replicas + """ + self._replicas = value def _create_applications(self) -> tuple[Application, ...]: @@ -199,6 +231,7 @@ def _create_applications(self) -> tuple[Application, ...]: application instances. """ permutation_strategy = strategies.resolve(self.permutation_strategy) + combinations = permutation_strategy( self.file_parameters, self.exe_arg_parameters, self.max_permutations ) From bb93bde0f14616fe98fbacaad14f421c62fbaebe Mon Sep 17 00:00:00 2001 From: Julia Putko Date: Wed, 28 Aug 2024 18:01:33 -0500 Subject: [PATCH 11/20] mypy error fix --- smartsim/entity/application.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/smartsim/entity/application.py b/smartsim/entity/application.py index c5369bc31..525fa4b18 100644 --- a/smartsim/entity/application.py +++ b/smartsim/entity/application.py @@ -97,7 +97,7 @@ def exe(self, value: str) -> None: self._exe = copy.deepcopy(value) @property - def exe_args(self) -> t.Sequence[str]: + def exe_args(self) -> t.MutableSequence[str]: """Return a list of attached executable arguments. :returns: application executable arguments @@ -105,7 +105,7 @@ def exe_args(self) -> t.Sequence[str]: return self._exe_args @exe_args.setter - def exe_args(self, value: t.Union[str, t.Sequence[str], None]) -> None: # + def exe_args(self, value: t.Union[str, t.Sequence[str], None]) -> None: """Set the executable arguments. :param value: executable arguments @@ -264,7 +264,7 @@ def __str__(self) -> str: # pragma: no cover """) @staticmethod - def _build_exe_args(exe_args: str | list[str] | None) -> t.List[str]: + def _build_exe_args(exe_args: t.Union[str, t.Sequence[str], None]) -> t.List[str]: """Check and convert exe_args input to a desired collection format :param exe_args: From 4f3d187cceff83264142aeffda3441f5adc36236 Mon Sep 17 00:00:00 2001 From: Julia Putko Date: Wed, 28 Aug 2024 18:11:09 -0500 Subject: [PATCH 12/20] fixing tests --- tests/temp_tests/test_launchable.py | 4 ++-- tests/test_shell_launcher.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/temp_tests/test_launchable.py b/tests/temp_tests/test_launchable.py index b4da5564d..b74190f3c 100644 --- a/tests/temp_tests/test_launchable.py +++ b/tests/temp_tests/test_launchable.py @@ -70,7 +70,7 @@ def test_job_init(): job = Job(entity, LaunchSettings("slurm")) assert isinstance(job, Job) assert job.entity.name == "test_name" - assert "echo" in job.entity.exe[0] + assert "echo" in job.entity.exe assert "spam" in job.entity.exe_args assert "eggs" in job.entity.exe_args @@ -121,7 +121,7 @@ def test_mpmdpair_init(): mpmd_pair = MPMDPair(entity, LaunchSettings("slurm")) assert isinstance(mpmd_pair, MPMDPair) assert mpmd_pair.entity.name == "test_name" - assert "echo" in mpmd_pair.entity.exe[0] + assert "echo" in mpmd_pair.entity.exe assert "spam" in mpmd_pair.entity.exe_args assert "eggs" in mpmd_pair.entity.exe_args diff --git a/tests/test_shell_launcher.py b/tests/test_shell_launcher.py index 6b03f8501..b626fbcc6 100644 --- a/tests/test_shell_launcher.py +++ b/tests/test_shell_launcher.py @@ -35,7 +35,7 @@ from smartsim._core.shell.shellLauncher import ShellLauncher, ShellLauncherCommand, sp from smartsim._core.utils import helpers from smartsim._core.utils.shell import * -from smartsim.entity import _mock, entity +from smartsim.entity import entity from smartsim.error.errors import LauncherJobNotFound from smartsim.status import JobStatus @@ -46,7 +46,7 @@ class EchoHelloWorldEntity(entity.SmartSimEntity): """A simple smartsim entity that meets the `ExecutableProtocol` protocol""" def __init__(self): - super().__init__("test-entity", _mock.Mock()) + super().__init__("test-entity") def __eq__(self, other): if type(self) is not type(other): From 0f0b2636f025ae6a3fdd90a6661617b4fc53152b Mon Sep 17 00:00:00 2001 From: Julia Putko Date: Wed, 28 Aug 2024 18:20:51 -0500 Subject: [PATCH 13/20] remove unnecessary lines --- smartsim/entity/application.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/smartsim/entity/application.py b/smartsim/entity/application.py index 525fa4b18..78e89025d 100644 --- a/smartsim/entity/application.py +++ b/smartsim/entity/application.py @@ -273,9 +273,6 @@ def _build_exe_args(exe_args: t.Union[str, t.Sequence[str], None]) -> t.List[str if not exe_args: return [] - if isinstance(exe_args, list): - exe_args = copy.deepcopy(exe_args) - if not ( isinstance(exe_args, str) or ( From 202cb3228f180961d1ce6c59c6acdb7aa47e4786 Mon Sep 17 00:00:00 2001 From: Julia Putko Date: Wed, 28 Aug 2024 18:36:41 -0500 Subject: [PATCH 14/20] formatting fix --- smartsim/entity/ensemble.py | 39 ++++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 18 deletions(-) diff --git a/smartsim/entity/ensemble.py b/smartsim/entity/ensemble.py index 4066958e7..a75735575 100644 --- a/smartsim/entity/ensemble.py +++ b/smartsim/entity/ensemble.py @@ -94,7 +94,7 @@ def __init__( """The maximum number of entities to come out of the permutation strategy""" self._replicas = replicas """How many identical entities to create within an Ensemble""" - + @property def exe(self) -> str: """Return executable to run. @@ -102,7 +102,7 @@ def exe(self) -> str: :returns: application executable to run """ return self._exe - + @exe.setter def exe(self, value: str | os.PathLike[str]) -> None: """Set executable to run. @@ -118,7 +118,7 @@ def exe_args(self) -> t.Sequence[str]: :returns: application executable arguments """ return self._exe_args - + @exe_args.setter def exe_args(self, value: t.Sequence[str]) -> None: """Set the executable arguments. @@ -127,16 +127,18 @@ def exe_args(self, value: t.Sequence[str]) -> None: """ self._exe_args = list(value) - @property + @property def exe_arg_parameters(self) -> t.Mapping[str, t.Sequence[t.Sequence[str]]]: """Return the executable argument parameters :returns: executable arguments parameters """ return self._exe_arg_parameters - + @exe_arg_parameters.setter - def exe_arg_parameters(self, value: t.Mapping[str, t.Sequence[t.Sequence[str]]]) -> None: + def exe_arg_parameters( + self, value: t.Mapping[str, t.Sequence[t.Sequence[str]]] + ) -> None: """Set the executable arguments. :param value: executable arguments @@ -151,7 +153,7 @@ def files(self) -> EntityFiles: :returns: files """ return self._files - + @files.setter def files(self, value: EntityFiles) -> None: """Set files to be copied, symlinked, and/or configured prior to @@ -160,7 +162,7 @@ def files(self, value: EntityFiles) -> None: :param value: files """ self._files = copy.deepcopy(value) - + @property def file_parameters(self) -> t.Mapping[str, t.Sequence[str]]: """Return file parameters. @@ -168,7 +170,7 @@ def file_parameters(self) -> t.Mapping[str, t.Sequence[str]]: :returns: application file parameters """ return self._file_parameters - + @file_parameters.setter def file_parameters(self, value: t.Mapping[str, t.Sequence[str]]) -> None: """Set the file parameters. @@ -184,15 +186,17 @@ def permutation_strategy(self) -> str | strategies.PermutationStrategyType: :return: permutation strategy """ return self._permutation_strategy - + @permutation_strategy.setter - def permutation_strategy(self, value: str | strategies.PermutationStrategyType) -> None: + def permutation_strategy( + self, value: str | strategies.PermutationStrategyType + ) -> None: """Set the permutation strategy - + :param value: permutation strategy """ self._permutation_strategy = value - + @property def max_permutations(self) -> int: """Return the maximum permutations @@ -200,7 +204,7 @@ def max_permutations(self) -> int: :return: max permutations """ return self._max_permutations - + @max_permutations.setter def max_permutations(self, value: int) -> None: """Set the maximum permutations @@ -208,15 +212,15 @@ def max_permutations(self, value: int) -> None: :param value: the maxpermutations """ self._max_permutations = value - + @property def replicas(self) -> int: """Return the number of replicas - + :return: number of replicas """ return self._replicas - + @replicas.setter def replicas(self, value: int) -> None: """Set the number of replicas @@ -225,7 +229,6 @@ def replicas(self, value: int) -> None: """ self._replicas = value - def _create_applications(self) -> tuple[Application, ...]: """Concretize the ensemble attributes into a collection of application instances. From 6378593a68c02a69b6ccec2d78eb534ea42daf84 Mon Sep 17 00:00:00 2001 From: Julia Putko Date: Thu, 29 Aug 2024 19:21:55 -0500 Subject: [PATCH 15/20] added tests;format and type changes --- smartsim/entity/ensemble.py | 11 ++++---- tests/test_ensemble.py | 52 ++++++++++++++++++++++++++++++++++++- 2 files changed, 56 insertions(+), 7 deletions(-) diff --git a/smartsim/entity/ensemble.py b/smartsim/entity/ensemble.py index a75735575..204ca76fa 100644 --- a/smartsim/entity/ensemble.py +++ b/smartsim/entity/ensemble.py @@ -72,14 +72,13 @@ def __init__( :param permutation_strategy: strategy to control how the param values are applied to the Ensemble :param max_permutations: max parameter permutations to set for the ensemble :param replicas: number of identical entities to create within an Ensemble - :return: and ``Ensemble`` instance """ self.name = name - """The name of the ensemble """ + """The name of the ensemble""" self._exe = os.fspath(exe) - """The executable to run """ + """The executable to run""" self._exe_args = list(exe_args) if exe_args else [] - """The executable arguments """ + """The executable arguments""" self._exe_arg_parameters = ( copy.deepcopy(exe_arg_parameters) if exe_arg_parameters else {} ) @@ -112,8 +111,8 @@ def exe(self, value: str | os.PathLike[str]) -> None: self._exe = os.fspath(value) @property - def exe_args(self) -> t.Sequence[str]: - """Return an immutable list of attached executable arguments. + def exe_args(self) -> t.MutableSequence[str]: + """Return a list of attached executable arguments. :returns: application executable arguments """ diff --git a/tests/test_ensemble.py b/tests/test_ensemble.py index c22e0e0db..5198681fe 100644 --- a/tests/test_ensemble.py +++ b/tests/test_ensemble.py @@ -26,11 +26,13 @@ import itertools import typing as t +from glob import glob +from os import path as osp import pytest -from smartsim.entity import _mock from smartsim.entity.ensemble import Ensemble +from smartsim.entity.files import EntityFiles from smartsim.entity.strategies import ParamSet from smartsim.settings.launchSettings import LaunchSettings @@ -40,6 +42,54 @@ _2x2_EXE_ARG = {"EXE": [["a"], ["b", "c"]], "ARGS": [["d"], ["e", "f"]]} +@pytest.fixture +def get_gen_configure_dir(fileutils): + yield fileutils.get_test_conf_path(osp.join("generator_files", "tag_dir_template")) + + +def test_exe_property(): + e = Ensemble(name="test", exe="path/to/example_simulation_program") + exe = e.exe + assert exe == e.exe + + +def test_exe_args_property(): + e = Ensemble("test", exe="path/to/example_simulation_program", exe_args="sleepy.py") + exe_args = e.exe_args + assert exe_args == e.exe_args + + +def test_exe_arg_parameters_property(): + exe_arg_parameters = {"-N": 2} + e = Ensemble( + "test", + exe="path/to/example_simulation_program", + exe_arg_parameters=exe_arg_parameters, + ) + exe_arg_parameters = e.exe_arg_parameters + assert exe_arg_parameters == e.exe_arg_parameters + + +def test_files_property(get_gen_configure_dir): + tagged_files = sorted(glob(get_gen_configure_dir + "/*")) + files = EntityFiles(tagged=tagged_files) + e = Ensemble("test", exe="path/to/example_simulation_program", files=files) + files = e.files + assert files == e.files + + +def test_file_parameters_property(): + file_parameters = {"h": [5, 6, 7, 8]} + e = Ensemble( + "test", + exe="path/to/example_simulation_program", + file_parameters=file_parameters, + ) + file_parameters = e.file_parameters + + assert file_parameters == e.file_parameters + + def user_created_function( file_params: t.Mapping[str, t.Sequence[str]], exe_arg_params: t.Mapping[str, t.Sequence[t.Sequence[str]]], From 1a44b33ac716f0227d3f74e9a2991b14079fc51d Mon Sep 17 00:00:00 2001 From: Julia Putko Date: Tue, 3 Sep 2024 16:57:25 -0500 Subject: [PATCH 16/20] file_params and exe_args are deepcopies --- smartsim/entity/ensemble.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/smartsim/entity/ensemble.py b/smartsim/entity/ensemble.py index 204ca76fa..db0e90c48 100644 --- a/smartsim/entity/ensemble.py +++ b/smartsim/entity/ensemble.py @@ -51,7 +51,7 @@ def __init__( self, name: str, exe: str | os.PathLike[str], - exe_args: t.Sequence[str] | None = None, + exe_args: t.MutableSequence[str] | None = None, exe_arg_parameters: t.Mapping[str, t.Sequence[t.Sequence[str]]] | None = None, files: EntityFiles | None = None, file_parameters: t.Mapping[str, t.Sequence[str]] | None = None, @@ -77,7 +77,7 @@ def __init__( """The name of the ensemble""" self._exe = os.fspath(exe) """The executable to run""" - self._exe_args = list(exe_args) if exe_args else [] + self._exe_args = copy.deepcopy(exe_args) if exe_args else [] """The executable arguments""" self._exe_arg_parameters = ( copy.deepcopy(exe_arg_parameters) if exe_arg_parameters else {} @@ -85,7 +85,7 @@ def __init__( """The parameters and values to be used when configuring entities""" self._files = copy.deepcopy(files) if files else EntityFiles() """The files to be copied, symlinked, and/or configured prior to execution""" - self._file_parameters = dict(file_parameters) if file_parameters else {} + self._file_parameters = copy.deepcopy(file_parameters) if file_parameters else {} """The parameters and values to be used when configuring files""" self._permutation_strategy = permutation_strategy """The strategy to control how the param values are applied to the Ensemble""" From b618b9a09918e921a2d61f790f90dd97d0922f11 Mon Sep 17 00:00:00 2001 From: Julia Putko Date: Tue, 3 Sep 2024 17:03:47 -0500 Subject: [PATCH 17/20] formatting fix --- smartsim/entity/ensemble.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/smartsim/entity/ensemble.py b/smartsim/entity/ensemble.py index db0e90c48..f1f305193 100644 --- a/smartsim/entity/ensemble.py +++ b/smartsim/entity/ensemble.py @@ -85,7 +85,9 @@ def __init__( """The parameters and values to be used when configuring entities""" self._files = copy.deepcopy(files) if files else EntityFiles() """The files to be copied, symlinked, and/or configured prior to execution""" - self._file_parameters = copy.deepcopy(file_parameters) if file_parameters else {} + self._file_parameters = ( + copy.deepcopy(file_parameters) if file_parameters else {} + ) """The parameters and values to be used when configuring files""" self._permutation_strategy = permutation_strategy """The strategy to control how the param values are applied to the Ensemble""" From f913ce03239a67762edafdbfd6316b118aed375c Mon Sep 17 00:00:00 2001 From: Julia Putko Date: Wed, 4 Sep 2024 15:54:18 -0500 Subject: [PATCH 18/20] changed typing --- smartsim/entity/application.py | 2 +- smartsim/entity/ensemble.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/smartsim/entity/application.py b/smartsim/entity/application.py index 09cddd3a8..12764845d 100644 --- a/smartsim/entity/application.py +++ b/smartsim/entity/application.py @@ -271,7 +271,7 @@ def _build_exe_args(exe_args: t.Union[str, t.Sequence[str], None]) -> t.List[str if isinstance(exe_args, str): return exe_args.split() - return copy.deepcopy(exe_args) + return list(exe_args) def print_attached_files(self) -> None: """Print a table of the attached files on std out""" diff --git a/smartsim/entity/ensemble.py b/smartsim/entity/ensemble.py index f1f305193..f228c4a8a 100644 --- a/smartsim/entity/ensemble.py +++ b/smartsim/entity/ensemble.py @@ -51,7 +51,7 @@ def __init__( self, name: str, exe: str | os.PathLike[str], - exe_args: t.MutableSequence[str] | None = None, + exe_args: t.Sequence[str] | None = None, exe_arg_parameters: t.Mapping[str, t.Sequence[t.Sequence[str]]] | None = None, files: EntityFiles | None = None, file_parameters: t.Mapping[str, t.Sequence[str]] | None = None, @@ -77,7 +77,7 @@ def __init__( """The name of the ensemble""" self._exe = os.fspath(exe) """The executable to run""" - self._exe_args = copy.deepcopy(exe_args) if exe_args else [] + self.exe_args = list(exe_args) if exe_args else [] """The executable arguments""" self._exe_arg_parameters = ( copy.deepcopy(exe_arg_parameters) if exe_arg_parameters else {} @@ -113,7 +113,7 @@ def exe(self, value: str | os.PathLike[str]) -> None: self._exe = os.fspath(value) @property - def exe_args(self) -> t.MutableSequence[str]: + def exe_args(self) -> t.List[str]: """Return a list of attached executable arguments. :returns: application executable arguments From b1431a772efdd3c2769e5000d1608997adf443e2 Mon Sep 17 00:00:00 2001 From: Julia Putko Date: Wed, 4 Sep 2024 18:12:07 -0500 Subject: [PATCH 19/20] type fix --- smartsim/entity/application.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/smartsim/entity/application.py b/smartsim/entity/application.py index 12764845d..364e87e04 100644 --- a/smartsim/entity/application.py +++ b/smartsim/entity/application.py @@ -26,6 +26,7 @@ from __future__ import annotations +import collections import copy import textwrap import typing as t @@ -262,7 +263,7 @@ def _build_exe_args(exe_args: t.Union[str, t.Sequence[str], None]) -> t.List[str if not ( isinstance(exe_args, str) or ( - isinstance(exe_args, list) + isinstance(exe_args, collections.abc.Sequence) and all(isinstance(arg, str) for arg in exe_args) ) ): From ca8654f5c81c350d7c3cd8ffb8ee226fb27c46fa Mon Sep 17 00:00:00 2001 From: Julia Putko Date: Thu, 5 Sep 2024 11:13:09 -0500 Subject: [PATCH 20/20] formatting fix --- smartsim/entity/application.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/smartsim/entity/application.py b/smartsim/entity/application.py index 364e87e04..a8302fc1f 100644 --- a/smartsim/entity/application.py +++ b/smartsim/entity/application.py @@ -263,7 +263,7 @@ def _build_exe_args(exe_args: t.Union[str, t.Sequence[str], None]) -> t.List[str if not ( isinstance(exe_args, str) or ( - isinstance(exe_args, collections.abc.Sequence) + isinstance(exe_args, collections.abc.Sequence) and all(isinstance(arg, str) for arg in exe_args) ) ):