From 55eb6b4b529cbc6be660d8d738a12d11cc64bfff Mon Sep 17 00:00:00 2001 From: Chris McBride Date: Thu, 1 Jun 2023 00:47:01 -0400 Subject: [PATCH 01/44] add core mypy support to makefile/reqs --- Makefile | 6 ++++++ pyproject.toml | 38 ++++++++++++++++++++++++++++++++++++++ requirements-dev.txt | 2 ++ requirements-mypy.txt | 7 +++++++ 4 files changed, 53 insertions(+) create mode 100644 requirements-mypy.txt diff --git a/Makefile b/Makefile index 4ceab6afd..e3b7745db 100644 --- a/Makefile +++ b/Makefile @@ -107,6 +107,12 @@ check-lint: @pylint --rcfile=.pylintrc ./smartsim +# help: check-mypy - run static type check +.PHONY: check-mypy +check-mypy: + @mypy --config-file=./pyproject.toml + + # help: # help: Documentation # help: ------- diff --git a/pyproject.toml b/pyproject.toml index 2517c3d7b..8cec5ded3 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -65,3 +65,41 @@ ignore_errors = true [tool.coverage.html] directory = "htmlcov" + +[tool.mypy] +python_version = "3.9" +namespace_packages = true +files = [ + "smartsim" +] +plugins = [] +ignore_errors = true + +[[tool.mypy.overrides]] +# Ignore packages that are not used or not typed +module = [ + "coloredlogs", + "smartredis", + "smartredis.error" +] +ignore_missing_imports = true +ignore_errors = true + +[[tool.mypy.overrides]] +module = [ + "smartsim.database.*", + "smartsim.entity.*", + "smartsim.experiment" +] + +ignore_errors=false + +# Strict fn defs +disallow_untyped_calls = true +disallow_untyped_defs = true +disallow_incomplete_defs = true +disallow_untyped_decorators = true + +# Safety/Upgrading Mypy +warn_unused_ignores = true +# warn_redundant_casts = true # not a per-module setting? diff --git a/requirements-dev.txt b/requirements-dev.txt index a1f7d4486..044ebfb10 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -4,3 +4,5 @@ pylint>=2.6.0 pytest>=6.0.0 pytest-cov>=2.10.1 click==8.0.2 +mypy>=1.3.0 + diff --git a/requirements-mypy.txt b/requirements-mypy.txt new file mode 100644 index 000000000..238ef3e7c --- /dev/null +++ b/requirements-mypy.txt @@ -0,0 +1,7 @@ +# From typeshed +types-psutil +types-redis +types-tabulate +types-tqdm + +# Not from typeshed From 858b77e448021810829c7c9cf9a5b666e89b4b86 Mon Sep 17 00:00:00 2001 From: Chris McBride Date: Thu, 1 Jun 2023 12:51:58 -0400 Subject: [PATCH 02/44] log.py typehints --- smartsim/log.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/smartsim/log.py b/smartsim/log.py index 5d6304b25..a08a7182d 100644 --- a/smartsim/log.py +++ b/smartsim/log.py @@ -23,6 +23,7 @@ # CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, # 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. +import typing as t import logging import os import sys @@ -38,7 +39,7 @@ # coloredlogs.DEFAULT_LOG_FORMAT = '%(asctime)s [%(threadName)s] %(hostname)s %(name)s[%(process)d] %(levelname)s %(message)s' -def _get_log_level(): +def _get_log_level() -> str: """Get the logging level based on environment variable SMARTSIM_LOG_LEVEL. If not set, default to info. @@ -65,7 +66,9 @@ def _get_log_level(): return "info" -def get_logger(name, log_level=None, fmt=None): +def get_logger( + name: str, log_level: t.Optional[str] = None, fmt: t.Optional[str] = None +) -> logging.Logger: """Return a logger instance levels: @@ -107,7 +110,7 @@ def get_logger(name, log_level=None, fmt=None): return logger -def log_to_file(filename, log_level="debug"): +def log_to_file(filename: str, log_level: str = "debug") -> None: """Installs a second filestream handler to the root logger, allowing subsequent logging calls to be sent to filename. From 9a329a44f589c55c8811f0ff90ab39c26e9abd6c Mon Sep 17 00:00:00 2001 From: Chris McBride Date: Thu, 1 Jun 2023 12:54:18 -0400 Subject: [PATCH 03/44] job.py typehints --- smartsim/_core/control/job.py | 36 +++++++++++++++++++++++++---------- 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/smartsim/_core/control/job.py b/smartsim/_core/control/job.py index d01d0dac6..c47dfefa0 100644 --- a/smartsim/_core/control/job.py +++ b/smartsim/_core/control/job.py @@ -25,8 +25,10 @@ # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. import time +import typing as t from ...status import STATUS_NEW +from ...entity import SmartSimEntity class Job: @@ -36,7 +38,14 @@ class Job: the controller class. """ - def __init__(self, job_name, job_id, entity, launcher, is_task): + def __init__( + self, + job_name: str, + job_id: str, + entity: SmartSimEntity, + launcher: str, + is_task: bool, + ) -> None: """Initialize a Job. :param job_name: Name of the job step @@ -65,11 +74,18 @@ def __init__(self, job_name, job_id, entity, launcher, is_task): self.history = History() @property - def ename(self): + def ename(self) -> str: """Return the name of the entity this job was created from""" return self.entity.name - def set_status(self, new_status, raw_status, returncode, error=None, output=None): + def set_status( + self, + new_status: str, + raw_status: str, + returncode: str, + error: str = None, + output: str = None, + ) -> None: """Set the status of a job. :param new_status: The new status of the job @@ -83,12 +99,12 @@ def set_status(self, new_status, raw_status, returncode, error=None, output=None self.error = error self.output = output - def record_history(self): + def record_history(self) -> None: """Record the launching history of a job.""" job_time = time.time() - self.start_time self.history.record(self.jid, self.status, self.returncode, job_time) - def reset(self, new_job_name, new_job_id, is_task): + def reset(self, new_job_name: str, new_job_id: str, is_task: bool) -> None: """Reset the job in order to be able to restart it. :param new_job_name: name of the new job step @@ -109,7 +125,7 @@ def reset(self, new_job_name, new_job_id, is_task): self.start_time = time.time() self.history.new_run() - def error_report(self): + def error_report(self) -> str: """A descriptive error report based on job fields :return: error report for display in terminal @@ -129,7 +145,7 @@ def error_report(self): warning += f"Error and output file located at: {self.entity.path}" return warning - def __str__(self): + def __str__(self) -> str: """Return user-readable string of the Job :returns: A user-readable string of the Job @@ -148,7 +164,7 @@ class History: on the previous launches of a job. """ - def __init__(self, runs=0): + def __init__(self, runs: int = 0) -> None: """Init a history object for a job :param runs: number of runs so far, defaults to 0 @@ -160,13 +176,13 @@ def __init__(self, runs=0): self.returns = dict() self.job_times = dict() - def record(self, job_id, status, returncode, job_time): + def record(self, job_id: str, status: str, returncode: str, job_time: str): """record the history of a job""" self.jids[self.runs] = job_id self.statuses[self.runs] = status self.returns[self.runs] = returncode self.job_times[self.runs] = job_time - def new_run(self): + def new_run(self) -> None: """increment run total""" self.runs += 1 From bebfab5e78fe5517fbfe67eeeb3ddfe0c6729a37 Mon Sep 17 00:00:00 2001 From: Chris McBride Date: Thu, 1 Jun 2023 13:10:51 -0400 Subject: [PATCH 04/44] containers.py typehints --- smartsim/settings/containers.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/smartsim/settings/containers.py b/smartsim/settings/containers.py index 44857cd06..3d0f66358 100644 --- a/smartsim/settings/containers.py +++ b/smartsim/settings/containers.py @@ -25,6 +25,7 @@ # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. import shutil +import typing as t from ..log import get_logger @@ -47,7 +48,9 @@ class Container: :type working_directory: str """ - def __init__(self, image, args="", mount="", working_directory=""): + def __init__( + self, image: str, args: str = "", mount: str = "", working_directory: str = "" + ) -> None: # Validate types if not isinstance(image, str): raise TypeError("image must be a str") @@ -63,7 +66,7 @@ def __init__(self, image, args="", mount="", working_directory=""): self.mount = mount self.working_directory = working_directory - def _containerized_run_command(self, run_command: str): + def _containerized_run_command(self, run_command: str) -> str: """Return modified run_command with container commands prepended. :param run_command: run command from a RunSettings class @@ -97,10 +100,10 @@ class Singularity(Container): :type mount: str | list[str] | dict[str, str], optional """ - def __init__(self, *args, **kwargs): + def __init__(self, *args: t.Any, **kwargs: t.Any) -> None: super().__init__(*args, **kwargs) - def _container_cmds(self, default_working_directory=""): + def _container_cmds(self, default_working_directory: str = "") -> t.List[str]: """Return list of container commands to be inserted before exe. Container members are validated during this call. From fe61a8a2a4a6b9a10b9c2c07834b4129b4f7eac9 Mon Sep 17 00:00:00 2001 From: Chris McBride Date: Thu, 1 Jun 2023 13:13:20 -0400 Subject: [PATCH 05/44] parser typehints --- smartsim/_core/launcher/pbs/pbsParser.py | 15 ++++++++------- smartsim/_core/launcher/slurm/slurmParser.py | 15 ++++++++------- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/smartsim/_core/launcher/pbs/pbsParser.py b/smartsim/_core/launcher/pbs/pbsParser.py index 2df89b73f..5e295bf81 100644 --- a/smartsim/_core/launcher/pbs/pbsParser.py +++ b/smartsim/_core/launcher/pbs/pbsParser.py @@ -25,9 +25,10 @@ # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. import json +import typing as t -def parse_qsub(output): +def parse_qsub(output: str) -> str: """Parse qsub output and return job id. For PBS, the output is the job id itself. @@ -39,7 +40,7 @@ def parse_qsub(output): return output -def parse_qsub_error(output): +def parse_qsub_error(output: str) -> str: """Parse and return error output of a failed qsub command. :param output: stderr of qsub command @@ -60,7 +61,7 @@ def parse_qsub_error(output): return base_err -def parse_qstat_jobid(output, job_id): +def parse_qstat_jobid(output: str, job_id: str) -> str: """Parse and return output of the qstat command run with options to obtain job status. @@ -82,7 +83,7 @@ def parse_qstat_jobid(output, job_id): return result -def parse_qstat_nodes(output): +def parse_qstat_nodes(output: str) -> t.List[str]: """Parse and return the qstat command run with options to obtain node list. @@ -96,7 +97,7 @@ def parse_qstat_nodes(output): :return: compute nodes of the allocation or job :rtype: list of str """ - nodes = [] + nodes: t.List[str] = [] out_json = load_and_clean_json(output) if "Jobs" not in out_json: return nodes @@ -111,7 +112,7 @@ def parse_qstat_nodes(output): return list(sorted(set(nodes))) -def parse_step_id_from_qstat(output, step_name): +def parse_step_id_from_qstat(output: str, step_name: str) -> t.Optional[str]: """Parse and return the step id from a qstat command :param output: output qstat @@ -135,7 +136,7 @@ def parse_step_id_from_qstat(output, step_name): return step_id -def load_and_clean_json(out): +def load_and_clean_json(out: str) -> t.Any: if len(out.strip()) == 0: return "" try: diff --git a/smartsim/_core/launcher/slurm/slurmParser.py b/smartsim/_core/launcher/slurm/slurmParser.py index e06dd3588..e7fa19306 100644 --- a/smartsim/_core/launcher/slurm/slurmParser.py +++ b/smartsim/_core/launcher/slurm/slurmParser.py @@ -24,6 +24,7 @@ # 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. +import typing as t from shutil import which """ @@ -31,13 +32,13 @@ """ -def parse_salloc(output): +def parse_salloc(output: str) -> t.Optional[str]: for line in output.split("\n"): if line.startswith("salloc: Granted job allocation"): return line.split()[-1] -def parse_salloc_error(output): +def parse_salloc_error(output: str) -> t.Optional[str]: """Parse and return error output of a failed salloc command :param output: stderr output of salloc command @@ -66,7 +67,7 @@ def parse_salloc_error(output): return None -def jobid_exact_match(parsed_id, job_id): +def jobid_exact_match(parsed_id: str, job_id: str) -> bool: """Check that job_id is an exact match and not the prefix of another job_id, like 1 and 11 or 1.1 and 1.10. Works with job id or step @@ -82,7 +83,7 @@ def jobid_exact_match(parsed_id, job_id): return parsed_id.split(".")[0] == job_id -def parse_sacct(output, job_id): +def parse_sacct(output: str, job_id: str) -> t.Tuple[str, t.Optional[str]]: """Parse and return output of the sacct command :param output: output of the sacct command @@ -92,7 +93,7 @@ def parse_sacct(output, job_id): :return: status and returncode :rtype: tuple """ - result = ("PENDING", None) + result: t.Tuple[str, t.Optional[str]] = ("PENDING", None) for line in output.split("\n"): line = line.split("|") if len(line) >= 3: @@ -104,7 +105,7 @@ def parse_sacct(output, job_id): return result -def parse_sstat_nodes(output, job_id): +def parse_sstat_nodes(output: str, job_id: str) -> t.List[str]: """Parse and return the sstat command This function parses and returns the nodes of @@ -127,7 +128,7 @@ def parse_sstat_nodes(output, job_id): return list(set(nodes)) -def parse_step_id_from_sacct(output, step_name): +def parse_step_id_from_sacct(output: str, step_name: str) -> t.Optional[str]: """Parse and return the step id from a sacct command :param output: output of sacct --noheader -p From 3a03328624bd7a188bd4fe7ffaab8e526e8b8a86 Mon Sep 17 00:00:00 2001 From: Chris McBride Date: Thu, 1 Jun 2023 13:19:04 -0400 Subject: [PATCH 06/44] step class typehints --- smartsim/_core/launcher/step/alpsStep.py | 12 +++++++----- smartsim/_core/launcher/step/cobaltStep.py | 10 ++++++---- smartsim/_core/launcher/step/localStep.py | 5 +++-- smartsim/_core/launcher/step/lsfStep.py | 22 ++++++++++++---------- smartsim/_core/launcher/step/mpiStep.py | 18 ++++++++++-------- smartsim/_core/launcher/step/pbsStep.py | 11 +++++++---- smartsim/_core/launcher/step/slurmStep.py | 20 +++++++++++--------- smartsim/_core/launcher/step/step.py | 13 +++++++------ 8 files changed, 63 insertions(+), 48 deletions(-) diff --git a/smartsim/_core/launcher/step/alpsStep.py b/smartsim/_core/launcher/step/alpsStep.py index 14832973f..1eef73680 100644 --- a/smartsim/_core/launcher/step/alpsStep.py +++ b/smartsim/_core/launcher/step/alpsStep.py @@ -26,17 +26,19 @@ import os import shutil +import typing as t from shlex import split as sh_split from ....error import AllocationError from ....log import get_logger from .step import Step +from ....settings import RunSettings logger = get_logger(__name__) class AprunStep(Step): - def __init__(self, name, cwd, run_settings): + def __init__(self, name: str, cwd: str, run_settings: RunSettings): """Initialize a ALPS aprun job step :param name: name of the entity to be launched @@ -52,7 +54,7 @@ def __init__(self, name, cwd, run_settings): if not self.run_settings.in_batch: self._set_alloc() - def get_launch_cmd(self): + def get_launch_cmd(self) -> t.List[str]: """Get the command to launch this step :return: launch command @@ -87,7 +89,7 @@ def get_launch_cmd(self): aprun_cmd.extend([">", output]) return aprun_cmd - def _set_alloc(self): + def _set_alloc(self) -> None: """Set the id of the allocation :raises AllocationError: allocation not listed or found @@ -107,7 +109,7 @@ def _set_alloc(self): "No allocation specified or found and not running in batch" ) - def _build_exe(self): + def _build_exe(self) -> t.List[str]: """Build the executable for this step :return: executable list @@ -120,7 +122,7 @@ def _build_exe(self): args = self.run_settings.exe_args return exe + args - def _make_mpmd(self): + def _make_mpmd(self) -> t.List[str]: """Build Aprun (MPMD) executable""" exe = self.run_settings.exe diff --git a/smartsim/_core/launcher/step/cobaltStep.py b/smartsim/_core/launcher/step/cobaltStep.py index 8a1653a11..daf818d93 100644 --- a/smartsim/_core/launcher/step/cobaltStep.py +++ b/smartsim/_core/launcher/step/cobaltStep.py @@ -26,15 +26,17 @@ import os import stat +import typing as t from ....log import get_logger +from ....settings.base import BatchSettings from .step import Step logger = get_logger(__name__) class CobaltBatchStep(Step): - def __init__(self, name, cwd, batch_settings): + def __init__(self, name: str, cwd: str, batch_settings: BatchSettings): """Initialize a Cobalt qsub step :param name: name of the entity to launch @@ -49,7 +51,7 @@ def __init__(self, name, cwd, batch_settings): self.step_cmds = [] self.managed = True - def get_launch_cmd(self): + def get_launch_cmd(self) -> t.List[str]: """Get the launch command for the batch :return: launch command for the batch @@ -58,7 +60,7 @@ def get_launch_cmd(self): script = self._write_script() return [self.batch_settings.batch_cmd, script] - def add_to_batch(self, step): + def add_to_batch(self, step) -> None: """Add a job step to this batch :param step: a job step instance e.g. SrunStep @@ -68,7 +70,7 @@ def add_to_batch(self, step): self.step_cmds.append(launch_cmd) logger.debug(f"Added step command to batch for {step.name}") - def _write_script(self): + def _write_script(self) -> str: """Write the batch script :return: batch script path after writing diff --git a/smartsim/_core/launcher/step/localStep.py b/smartsim/_core/launcher/step/localStep.py index b2080052c..31dcee483 100644 --- a/smartsim/_core/launcher/step/localStep.py +++ b/smartsim/_core/launcher/step/localStep.py @@ -26,6 +26,7 @@ import os import shutil +import typing as t from .step import Step @@ -36,7 +37,7 @@ def __init__(self, name, cwd, run_settings): self.run_settings = run_settings self.env = self._set_env() - def get_launch_cmd(self): + def get_launch_cmd(self) -> t.List[str]: cmd = [] # Add run command and args if user specified @@ -62,7 +63,7 @@ def get_launch_cmd(self): cmd.extend(self.run_settings.exe_args) return cmd - def _set_env(self): + def _set_env(self) -> t.Dict[str, str]: env = os.environ.copy() if self.run_settings.env_vars: for k, v in self.run_settings.env_vars.items(): diff --git a/smartsim/_core/launcher/step/lsfStep.py b/smartsim/_core/launcher/step/lsfStep.py index 280a1ca6a..03f862736 100644 --- a/smartsim/_core/launcher/step/lsfStep.py +++ b/smartsim/_core/launcher/step/lsfStep.py @@ -26,10 +26,12 @@ import os import shutil +import typing as t from ....error import AllocationError from ....log import get_logger from .step import Step +from ....settings.base import RunSettings logger = get_logger(__name__) @@ -50,7 +52,7 @@ def __init__(self, name, cwd, batch_settings): self.step_cmds = [] self.managed = True - def get_launch_cmd(self): + def get_launch_cmd(self) -> t.List[str]: """Get the launch command for the batch :return: launch command for the batch @@ -59,7 +61,7 @@ def get_launch_cmd(self): script = self._write_script() return [self.batch_settings.batch_cmd, script] - def add_to_batch(self, step): + def add_to_batch(self, step: Step) -> None: """Add a job step to this batch :param step: a job step instance e.g. SrunStep @@ -69,7 +71,7 @@ def add_to_batch(self, step): self.step_cmds.append(launch_cmd) logger.debug(f"Added step command to batch for {step.name}") - def _write_script(self): + def _write_script(self) -> str: """Write the batch script :return: batch script path after writing @@ -106,7 +108,7 @@ def _write_script(self): class JsrunStep(Step): - def __init__(self, name, cwd, run_settings): + def __init__(self, name: str, cwd: str, run_settings: RunSettings): """Initialize a LSF jsrun job step :param name: name of the entity to be launched @@ -123,7 +125,7 @@ def __init__(self, name, cwd, run_settings): if not self.run_settings.in_batch: self._set_alloc() - def get_output_files(self): + def get_output_files(self) -> t.Tuple[str, str]: """Return two paths to error and output files based on cwd""" output = self.get_step_file(ending=".out") error = self.get_step_file(ending=".err") @@ -147,7 +149,7 @@ def get_output_files(self): return output, error - def get_launch_cmd(self): + def get_launch_cmd(self) -> t.List[str]: """Get the command to launch this step :return: launch command @@ -187,7 +189,7 @@ def get_launch_cmd(self): return jsrun_cmd - def _set_alloc(self): + def _set_alloc(self) -> None: """Set the id of the allocation :raises AllocationError: allocation not listed or found @@ -202,7 +204,7 @@ def _set_alloc(self): "No allocation specified or found and not running in batch" ) - def _build_exe(self): + def _build_exe(self) -> t.List[str]: """Build the executable for this step :return: executable list @@ -212,14 +214,14 @@ def _build_exe(self): args = self.run_settings.exe_args if self.run_settings.mpmd: erf_file = self.get_step_file(ending=".mpmd") - cmd = self._make_mpmd() + _ = self._make_mpmd() mp_cmd = ["--erf_input", erf_file] return mp_cmd else: cmd = exe + args return cmd - def _make_mpmd(self): + def _make_mpmd(self) -> None: """Build LSF's Explicit Resource File""" erf_file = self.get_step_file(ending=".mpmd") diff --git a/smartsim/_core/launcher/step/mpiStep.py b/smartsim/_core/launcher/step/mpiStep.py index 0a97dd1fc..06dca6c8a 100644 --- a/smartsim/_core/launcher/step/mpiStep.py +++ b/smartsim/_core/launcher/step/mpiStep.py @@ -27,16 +27,18 @@ import os import shutil from shlex import split as sh_split +import typing as t from ....error import AllocationError from ....log import get_logger from .step import Step +from ....settings.base import RunSettings logger = get_logger(__name__) class _BaseMPIStep(Step): - def __init__(self, name, cwd, run_settings): + def __init__(self, name: str, cwd: str, run_settings: RunSettings) -> None: """Initialize a job step conforming to the MPI standard :param name: name of the entity to be launched @@ -61,7 +63,7 @@ def __init__(self, name, cwd, run_settings): def _run_command(self): return self.run_settings._run_command - def get_launch_cmd(self): + def get_launch_cmd(self) -> t.List[str]: """Get the command to launch this step :return: launch command @@ -93,7 +95,7 @@ def get_launch_cmd(self): mpi_cmd += [">", output] return mpi_cmd - def _set_alloc(self): + def _set_alloc(self) -> None: """Set the id of the allocation :raises AllocationError: allocation not listed or found @@ -112,7 +114,7 @@ def _set_alloc(self): "No allocation specified or found and not running in batch" ) - def _build_exe(self): + def _build_exe(self) -> t.List[str]: """Build the executable for this step :return: executable list @@ -125,7 +127,7 @@ def _build_exe(self): args = self.run_settings.exe_args return exe + args - def _make_mpmd(self): + def _make_mpmd(self) -> t.List[str]: """Build mpiexec (MPMD) executable""" exe = self.run_settings.exe args = self.run_settings.exe_args @@ -142,7 +144,7 @@ def _make_mpmd(self): class MpiexecStep(_BaseMPIStep): - def __init__(self, name, cwd, run_settings): + def __init__(self, name: str, cwd: str, run_settings: RunSettings) -> None: """Initialize an mpiexec job step :param name: name of the entity to be launched @@ -160,7 +162,7 @@ def __init__(self, name, cwd, run_settings): class MpirunStep(_BaseMPIStep): - def __init__(self, name, cwd, run_settings): + def __init__(self, name: str, cwd: str, run_settings: RunSettings) -> None: """Initialize an mpirun job step :param name: name of the entity to be launched @@ -178,7 +180,7 @@ def __init__(self, name, cwd, run_settings): class OrterunStep(_BaseMPIStep): - def __init__(self, name, cwd, run_settings): + def __init__(self, name: str, cwd: str, run_settings: RunSettings) -> None: """Initialize an orterun job step :param name: name of the entity to be launched diff --git a/smartsim/_core/launcher/step/pbsStep.py b/smartsim/_core/launcher/step/pbsStep.py index e70c0fd0b..40da0837a 100644 --- a/smartsim/_core/launcher/step/pbsStep.py +++ b/smartsim/_core/launcher/step/pbsStep.py @@ -24,14 +24,17 @@ # 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. +import typing as t + from ....log import get_logger from .step import Step +from ....settings.base import BatchSettings logger = get_logger(__name__) class QsubBatchStep(Step): - def __init__(self, name, cwd, batch_settings): + def __init__(self, name: str, cwd: str, batch_settings: BatchSettings) -> None: """Initialize a PBSpro qsub step :param name: name of the entity to launch @@ -46,7 +49,7 @@ def __init__(self, name, cwd, batch_settings): self.step_cmds = [] self.managed = True - def get_launch_cmd(self): + def get_launch_cmd(self) -> t.List[str]: """Get the launch command for the batch :return: launch command for the batch @@ -55,7 +58,7 @@ def get_launch_cmd(self): script = self._write_script() return [self.batch_settings.batch_cmd, script] - def add_to_batch(self, step): + def add_to_batch(self, step: Step) -> None: """Add a job step to this batch :param step: a job step instance e.g. SrunStep @@ -65,7 +68,7 @@ def add_to_batch(self, step): self.step_cmds.append(launch_cmd) logger.debug(f"Added step command to batch for {step.name}") - def _write_script(self): + def _write_script(self) -> str: """Write the batch script :return: batch script path after writing diff --git a/smartsim/_core/launcher/step/slurmStep.py b/smartsim/_core/launcher/step/slurmStep.py index fe59eb91e..c4ed81d83 100644 --- a/smartsim/_core/launcher/step/slurmStep.py +++ b/smartsim/_core/launcher/step/slurmStep.py @@ -27,16 +27,18 @@ import os import shutil from shlex import split as sh_split +import typing as t from ....error import AllocationError from ....log import get_logger from .step import Step +from ....settings.base import BatchSettings, RunSettings logger = get_logger(__name__) class SbatchStep(Step): - def __init__(self, name, cwd, batch_settings): + def __init__(self, name: str, cwd: str, batch_settings: BatchSettings): """Initialize a Slurm Sbatch step :param name: name of the entity to launch @@ -51,7 +53,7 @@ def __init__(self, name, cwd, batch_settings): self.step_cmds = [] self.managed = True - def get_launch_cmd(self): + def get_launch_cmd(self) -> t.List[str]: """Get the launch command for the batch :return: launch command for the batch @@ -60,7 +62,7 @@ def get_launch_cmd(self): script = self._write_script() return [self.batch_settings.batch_cmd, "--parsable", script] - def add_to_batch(self, step): + def add_to_batch(self, step: Step) -> None: """Add a job step to this batch :param step: a job step instance e.g. SrunStep @@ -71,7 +73,7 @@ def add_to_batch(self, step): self.step_cmds.append(launch_cmd) logger.debug(f"Added step command to batch for {step.name}") - def _write_script(self): + def _write_script(self) -> str: """Write the batch script :return: batch script path after writing @@ -102,7 +104,7 @@ def _write_script(self): class SrunStep(Step): - def __init__(self, name, cwd, run_settings): + def __init__(self, name: str, cwd: str, run_settings: RunSettings): """Initialize a srun job step :param name: name of the entity to be launched @@ -119,7 +121,7 @@ def __init__(self, name, cwd, run_settings): if not self.run_settings.in_batch: self._set_alloc() - def get_launch_cmd(self): + def get_launch_cmd(self) -> t.List[str]: """Get the command to launch this step :return: launch command @@ -168,7 +170,7 @@ def get_launch_cmd(self): return srun_cmd - def _set_alloc(self): + def _set_alloc(self) -> None: """Set the id of the allocation :raises AllocationError: allocation not listed or found @@ -186,7 +188,7 @@ def _set_alloc(self): "No allocation specified or found and not running in batch" ) - def _build_exe(self): + def _build_exe(self) -> None: """Build the executable for this step :return: executable list @@ -199,7 +201,7 @@ def _build_exe(self): args = self.run_settings.exe_args return exe + args - def _make_mpmd(self): + def _make_mpmd(self) -> t.List[str]: """Build Slurm multi-prog (MPMD) executable""" exe = self.run_settings.exe args = self.run_settings.exe_args diff --git a/smartsim/_core/launcher/step/step.py b/smartsim/_core/launcher/step/step.py index 108b8c8d6..9fe469223 100644 --- a/smartsim/_core/launcher/step/step.py +++ b/smartsim/_core/launcher/step/step.py @@ -26,6 +26,7 @@ import os.path as osp import time +import typing as t from ....log import get_logger from ...utils.helpers import get_base_36_repr @@ -35,26 +36,26 @@ class Step: - def __init__(self, name, cwd): + def __init__(self, name: str, cwd: str) -> None: self.name = self._create_unique_name(name) self.entity_name = name self.cwd = cwd self.managed = False - def get_launch_cmd(self): + def get_launch_cmd(self) -> t.List[str]: raise NotImplementedError - def _create_unique_name(self, entity_name): + def _create_unique_name(self, entity_name: str) -> str: step_name = entity_name + "-" + get_base_36_repr(time.time_ns()) return step_name - def get_output_files(self): + def get_output_files(self) -> t.Tuple[str, str]: """Return two paths to error and output files based on cwd""" output = self.get_step_file(ending=".out") error = self.get_step_file(ending=".err") return output, error - def get_step_file(self, ending=".sh", script_name=None): + def get_step_file(self, ending: str = ".sh", script_name: t.Optional[str] = None): """Get the name for a file/script created by the step class Used for Batch scripts, mpmd scripts, etc""" @@ -63,7 +64,7 @@ def get_step_file(self, ending=".sh", script_name=None): return osp.join(self.cwd, script_name) return osp.join(self.cwd, self.entity_name + ending) - def get_colocated_launch_script(self): + def get_colocated_launch_script(self) -> str: # prep step for colocated launch if specifed in run settings script_path = self.get_step_file(script_name=".colocated_launcher.sh") From 504aec273f75305bea44fccce4af300a46e7a057 Mon Sep 17 00:00:00 2001 From: Chris McBride Date: Thu, 1 Jun 2023 13:23:56 -0400 Subject: [PATCH 07/44] errors.py typehints --- smartsim/error/errors.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/smartsim/error/errors.py b/smartsim/error/errors.py index ac4730c5c..4869aa360 100644 --- a/smartsim/error/errors.py +++ b/smartsim/error/errors.py @@ -24,6 +24,7 @@ # 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. +import typing as t # Exceptions @@ -45,11 +46,11 @@ class UserStrategyError(SmartSimError): """Raised when there is an error with model creation inside an ensemble that is from a user provided permutation strategy""" - def __init__(self, perm_strat): + def __init__(self, perm_strat: str) -> None: message = self.create_message(perm_strat) super().__init__(message) - def create_message(self, perm_strat): + def create_message(self, perm_strat: str) -> str: prefix = "User provided ensemble generation strategy" message = "failed to generate valid parameter names and values" return " ".join((prefix, str(perm_strat), message)) @@ -60,11 +61,11 @@ class ParameterWriterError(SmartSimError): could not be written. """ - def __init__(self, file_path, read=True): + def __init__(self, file_path: str, read: bool = True) -> None: message = self.create_message(file_path, read) super().__init__(message) - def create_message(self, fp, read): + def create_message(self, fp: str, read: bool) -> str: if read: msg = f"Failed to read configuration file to write at {fp}" else: @@ -80,8 +81,6 @@ class SSInternalError(Exception): SSInternalError is raised when an internal error is encountered. """ - pass - class SSConfigError(SSInternalError): """Raised when there is an error in the configuration of SmartSim""" @@ -99,11 +98,15 @@ class ShellError(LauncherError): """Raised when error arises from function within launcher.shell Closely related to error from subprocess(Popen) commands""" - def __init__(self, message, shell_error, command_list): + def __init__( + self, message: str, shell_error: bool, command_list: t.Union[str, t.List[str]] + ) -> None: msg = self.create_message(message, shell_error, command_list) super().__init__(msg) - def create_message(self, message, shell_error, command_list): + def create_message( + self, message: str, shell_error: bool, command_list: t.Union[str, t.List[str]] + ) -> str: if isinstance(command_list, list): command_list = " ".join(command_list) msg = message + "\n" From 47fc11389e6d1f9c4af7d2a0f317c3edb2ef549c Mon Sep 17 00:00:00 2001 From: Chris McBride Date: Thu, 1 Jun 2023 13:53:23 -0400 Subject: [PATCH 08/44] utils typehints --- smartsim/_core/launcher/util/launcherUtil.py | 20 +++++++++++--------- smartsim/_core/launcher/util/shell.py | 17 +++++++++++++---- smartsim/_core/utils/network.py | 8 ++++---- 3 files changed, 28 insertions(+), 17 deletions(-) diff --git a/smartsim/_core/launcher/util/launcherUtil.py b/smartsim/_core/launcher/util/launcherUtil.py index b53ebc6f0..7b1961daa 100644 --- a/smartsim/_core/launcher/util/launcherUtil.py +++ b/smartsim/_core/launcher/util/launcherUtil.py @@ -24,13 +24,15 @@ # 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. +import typing as t + class ComputeNode: # cov-slurm """The ComputeNode class holds resource information about a physical compute node """ - def __init__(self, node_name=None, node_ppn=None): + def __init__(self, node_name: t.Optional[str] = None, node_ppn: t.Optional[int] =None) -> None: """Initialize a ComputeNode :param node_name: the name of the node @@ -38,10 +40,10 @@ def __init__(self, node_name=None, node_ppn=None): :param node_ppn: the number of ppn :type node_ppn: int """ - self.name = node_name - self.ppn = node_ppn + self.name: t.Optional[str] = node_name + self.ppn: t.Optional[int] = node_ppn - def _is_valid_node(self): + def _is_valid_node(self) -> bool: """Check if the node is complete Currently, validity is judged by name @@ -63,13 +65,13 @@ class Partition: # cov-slurm a system partition. """ - def __init__(self): + def __init__(self) -> None: """Initialize a system partition""" - self.name = None - self.min_ppn = None - self.nodes = set() + self.name: t.Optional[str] = None + self.min_ppn: t.Optional[int] = None + self.nodes: t.Set[ComputeNode] = set() - def _is_valid_partition(self): + def _is_valid_partition(self) -> bool: """Check if the partition is valid Currently, validity is judged by name diff --git a/smartsim/_core/launcher/util/shell.py b/smartsim/_core/launcher/util/shell.py index e16561aed..034abfb18 100644 --- a/smartsim/_core/launcher/util/shell.py +++ b/smartsim/_core/launcher/util/shell.py @@ -24,11 +24,11 @@ # 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. +import psutil import time +import typing as t from subprocess import PIPE, TimeoutExpired -import psutil - from ....error import ShellError from ....log import get_logger from ...utils.helpers import check_dev_log_level @@ -37,7 +37,14 @@ verbose_shell = check_dev_log_level() -def execute_cmd(cmd_list, shell=False, cwd=None, env=None, proc_input="", timeout=None): +def execute_cmd( + cmd_list: t.List[str], + shell: bool = False, + cwd: t.Optional[str] = None, + env: t.Dict = None, + proc_input: str = "", + timeout: t.Optional[int] = None, +) -> t.Tuple[int, str, str]: """Execute a command locally :param cmd_list: list of command with arguments @@ -86,7 +93,9 @@ def execute_cmd(cmd_list, shell=False, cwd=None, env=None, proc_input="", timeou return proc.returncode, out.decode("utf-8"), err.decode("utf-8") -def execute_async_cmd(cmd_list, cwd, env=None, out=PIPE, err=PIPE): +def execute_async_cmd( + cmd_list: t.List[str], cwd: str, env: t.Dict = None, out=PIPE, err=PIPE +) -> psutil.Popen: """Execute an asynchronous command This function executes an asynchronous command and returns a diff --git a/smartsim/_core/utils/network.py b/smartsim/_core/utils/network.py index 47e39673d..539cfb167 100644 --- a/smartsim/_core/utils/network.py +++ b/smartsim/_core/utils/network.py @@ -33,7 +33,7 @@ """ -def get_ip_from_host(host): +def get_ip_from_host(host: str) -> str: """Return the IP address for the interconnect. :param host: hostname of the compute node e.g. nid00004 @@ -46,7 +46,7 @@ def get_ip_from_host(host): # impossible to cover as it's only used in entrypoints -def get_ip_from_interface(interface): # pragma: no cover +def get_ip_from_interface(interface: str) -> str: # pragma: no cover """Get IPV4 address of a network interface :param interface: interface name @@ -72,7 +72,7 @@ def get_ip_from_interface(interface): # pragma: no cover # impossible to cover as it's only used in entrypoints -def get_lb_interface_name(): # pragma: no cover +def get_lb_interface_name() -> str: # pragma: no cover """Use psutil to get loopback interface name""" net_if_addrs = list(psutil.net_if_addrs()) for interface in net_if_addrs: @@ -81,7 +81,7 @@ def get_lb_interface_name(): # pragma: no cover raise OSError("Could not find loopback interface name") -def current_ip(interface="lo"): # pragma: no cover +def current_ip(interface: str = "lo") -> str: # pragma: no cover if interface == "lo": loopback = get_lb_interface_name() return get_ip_from_interface(loopback) From b918dc197d193bf9859ea34fffba3097b26fbbec Mon Sep 17 00:00:00 2001 From: Chris McBride Date: Thu, 1 Jun 2023 14:16:16 -0400 Subject: [PATCH 09/44] manifest.py typehints --- smartsim/_core/control/manifest.py | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/smartsim/_core/control/manifest.py b/smartsim/_core/control/manifest.py index dce554538..725992506 100644 --- a/smartsim/_core/control/manifest.py +++ b/smartsim/_core/control/manifest.py @@ -24,6 +24,8 @@ # 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. +import typing as t + from ...database import Orchestrator from ...entity import EntityList, SmartSimEntity from ...error import SmartSimError @@ -44,14 +46,14 @@ class Manifest: can all be passed as arguments """ - def __init__(self, *args): + def __init__(self, *args: SmartSimEntity) -> None: self._deployables = list(args) self._check_types(self._deployables) self._check_names(self._deployables) self._check_entity_lists_nonempty() @property - def db(self): + def db(self) -> Orchestrator: """Return Orchestrator instances in Manifest :raises SmartSimError: if user added to databases to manifest @@ -69,7 +71,7 @@ def db(self): return _db @property - def models(self): + def models(self) -> t.List[SmartSimEntity]: """Return Model instances in Manifest :return: model instances @@ -82,13 +84,13 @@ def models(self): return _models @property - def ensembles(self): + def ensembles(self) -> t.List[EntityList]: """Return Ensemble instances in Manifest :return: list of ensembles :rtype: List[Ensemble] """ - _ensembles = [] + _ensembles: t.List[EntityList] = [] for deployable in self._deployables: if isinstance(deployable, EntityList): is_exceptional_type = False @@ -101,7 +103,7 @@ def ensembles(self): return _ensembles @property - def all_entity_lists(self): + def all_entity_lists(self) -> t.List[EntityList]: """All entity lists, including ensembles and exceptional ones like Orchestrator @@ -115,7 +117,7 @@ def all_entity_lists(self): return _all_entity_lists - def _check_names(self, deployables): + def _check_names(self, deployables: t.List[t.Any]) -> None: used = [] for deployable in deployables: name = getattr(deployable, "name", None) @@ -125,7 +127,7 @@ def _check_names(self, deployables): raise SmartSimError("User provided two entities with the same name") used.append(name) - def _check_types(self, deployables): + def _check_types(self, deployables: t.List[t.Any]) -> None: for deployable in deployables: if not ( isinstance(deployable, SmartSimEntity) @@ -135,14 +137,14 @@ def _check_types(self, deployables): f"Entity has type {type(deployable)}, not SmartSimEntity or EntityList" ) - def _check_entity_lists_nonempty(self): + def _check_entity_lists_nonempty(self) -> None: """Check deployables for sanity before launching""" for entity_list in self.all_entity_lists: if len(entity_list) < 1: raise ValueError(f"{entity_list.name} is empty. Nothing to launch.") - def __str__(self): + def __str__(self) -> str: s = "" e_header = "=== Ensembles ===\n" m_header = "=== Models ===\n" @@ -183,14 +185,14 @@ def __str__(self): return s @property - def has_db_objects(self): + def has_db_objects(self) -> bool: """Check if any entity has DBObjects to set""" - def has_db_models(entity): + def has_db_models(entity: SmartSimEntity) -> bool: if hasattr(entity, "_db_models"): return len(entity._db_models) > 0 - def has_db_scripts(entity): + def has_db_scripts(entity: SmartSimEntity) -> bool: if hasattr(entity, "_db_scripts"): return len(entity._db_scripts) > 0 From d74a531671359cb064fca11e33fa9b1447697296 Mon Sep 17 00:00:00 2001 From: Chris McBride Date: Thu, 1 Jun 2023 14:36:16 -0400 Subject: [PATCH 10/44] entity typehints 1 --- smartsim/entity/entity.py | 10 +++++----- smartsim/entity/files.py | 37 ++++++++++++++++++++--------------- smartsim/entity/strategies.py | 33 +++++++++++-------------------- 3 files changed, 37 insertions(+), 43 deletions(-) diff --git a/smartsim/entity/entity.py b/smartsim/entity/entity.py index b80543b4e..39374277c 100644 --- a/smartsim/entity/entity.py +++ b/smartsim/entity/entity.py @@ -23,10 +23,10 @@ # CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, # 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. - +import smartsim.settings.base class SmartSimEntity: - def __init__(self, name, path, run_settings): + def __init__(self, name: str, path: str, run_settings: smartsim.settings.base.RunSettings) -> None: """Initialize a SmartSim entity. Each entity must have a name, path, and @@ -46,14 +46,14 @@ def __init__(self, name, path, run_settings): self.path = path @property - def type(self): + def type(self) -> str: """Return the name of the class""" return type(self).__name__ - def set_path(self, path): + def set_path(self, path: str) -> None: if not isinstance(path, str): raise TypeError("path argument must be a string") self.path = path - def __repr__(self): + def __repr__(self) -> str: return self.name diff --git a/smartsim/entity/files.py b/smartsim/entity/files.py index a8275667d..cb36e7829 100644 --- a/smartsim/entity/files.py +++ b/smartsim/entity/files.py @@ -23,8 +23,9 @@ # CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, # 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. - import os +import typing as t + from os import path @@ -47,7 +48,9 @@ class EntityFiles: without necessary having to copy the entire file. """ - def __init__(self, tagged, copy, symlink): + def __init__( + self, tagged: t.List[str], copy: t.List[str], symlink: t.List[str] + ) -> None: """Initialize an EntityFiles instance :param tagged: tagged files for model configuration @@ -65,7 +68,7 @@ def __init__(self, tagged, copy, symlink): self.tagged_hierarchy = None self._check_files() - def _check_files(self): + def _check_files(self) -> None: """Ensure the files provided by the user are of the correct type and actually exist somewhere on the filesystem. @@ -87,7 +90,7 @@ def _check_files(self): for i in range(len(self.link)): self.link[i] = self._check_path(self.link[i]) - def _type_check_files(self, file_list, file_type): + def _type_check_files(self, file_list: t.List[str], file_type: str) -> t.List[str]: """Check the type of the files provided by the user. :param file_list: either tagged, copy, or symlink files @@ -107,12 +110,12 @@ def _type_check_files(self, file_list, file_type): f"{file_type} files given were not of type list or str" ) else: - if not all(isinstance(f, str) for f in file_list): + if not all([isinstance(f, str) for f in file_list]): raise TypeError(f"Not all {file_type} files were of type str") return file_list @staticmethod - def _check_path(file_path): + def _check_path(file_path: str) -> str: """Given a user provided path-like str, find the actual path to the directory or file and create a full path. @@ -150,7 +153,7 @@ class TaggedFilesHierarchy: tagged file directory structure can be replicated """ - def __init__(self, parent=None, subdir_name=""): + def __init__(self, parent: t.Optional[t.Any] = None, subdir_name: str = "") -> None: """Initialize a TaggedFilesHierarchy :param parent: The parent hierarchy of the new hierarchy, @@ -184,18 +187,20 @@ def __init__(self, parent=None, subdir_name=""): if parent: parent.dirs.add(self) - self._base = path.join(parent.base, subdir_name) if parent else "" - self.parent = parent - self.files = set() - self.dirs = set() + self._base: str = path.join(parent.base, subdir_name) if parent else "" + self.parent: t.Any = parent + self.files: t.Set[str] = set() + self.dirs: t.Set[str] = set() @property - def base(self): + def base(self) -> str: """Property to ensure that self.base is read-only""" return self._base @classmethod - def from_list_paths(cls, path_list, dir_contents_to_base=False): + def from_list_paths( + cls, path_list: t.List[str], dir_contents_to_base: bool = False + ) -> t.Any: """Given a list of absolute paths to files and dirs, create and return a TaggedFilesHierarchy instance representing the file hierarchy of tagged files. All files in the path list will be placed in the base of @@ -225,7 +230,7 @@ def from_list_paths(cls, path_list, dir_contents_to_base=False): tagged_file_hierarchy._add_paths(path_list) return tagged_file_hierarchy - def _add_file(self, file): + def _add_file(self, file: str) -> None: """Add a file to the current level in the file hierarchy :param file: absoute path to a file to add to the hierarchy @@ -233,7 +238,7 @@ def _add_file(self, file): """ self.files.add(file) - def _add_dir(self, dir): + def _add_dir(self, dir: str) -> None: """Add a dir contianing tagged files by creating a new sub level in the tagged file hierarchy. All paths within the directroy are added to the the new level sub level tagged file hierarchy @@ -246,7 +251,7 @@ def _add_dir(self, dir): [path.join(dir, file) for file in os.listdir(dir)] ) - def _add_paths(self, paths): + def _add_paths(self, paths: t.List[str]) -> None: """Takes a list of paths and iterates over it, determining if each path is to a file or a dir and then appropriatly adding it to the TaggedFilesHierarchy. diff --git a/smartsim/entity/strategies.py b/smartsim/entity/strategies.py index d6236ba9c..a9c8ca01b 100644 --- a/smartsim/entity/strategies.py +++ b/smartsim/entity/strategies.py @@ -25,13 +25,15 @@ # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. # Generation Strategies +import random +import typing as t from itertools import product # create permutations of all parameters # single model if parameters only have one value -def create_all_permutations(param_names, param_values): +def create_all_permutations(param_names: t.List[str], param_values: t.List[t.List[str]], _n_models: int = 0) -> t.List[t.Dict[str, str]]: perms = list(product(*param_values)) all_permutations = [] for p in perms: @@ -40,31 +42,18 @@ def create_all_permutations(param_names, param_values): return all_permutations -def step_values(param_names, param_values): +def step_values(param_names: t.List[str], param_values: t.List[t.List[str]], _n_models: int = 0) -> t.List[t.Dict[str, str]]: permutations = [] for p in zip(*param_values): permutations.append(dict(zip(param_names, p))) return permutations -def random_permutations(param_names, param_values, n_models): - import random - +def random_permutations(param_names: t.List[str], param_values: t.List[t.List[str]], n_models: int = 0) -> t.List[t.Dict[str, str]]: # first, check if we've requested more values than possible. - perms = list(product(*param_values)) - if n_models >= len(perms): - return create_all_permutations(param_names, param_values) - else: - permutations = [] - permutation_strings = set() - while len(permutations) < n_models: - model_dict = dict( - zip( - param_names, - map(lambda x: x[random.randint(0, len(x) - 1)], param_values), - ) - ) - if str(model_dict) not in permutation_strings: - permutation_strings.add(str(model_dict)) - permutations.append(model_dict) - return permutations + permutations = create_all_permutations(param_names, param_values) + + if n_models and n_models < len(permutations): + permutations = random.sample(permutations, n_models) + + return permutations From 171834b46df86216e0b60bf4485ac624b980610a Mon Sep 17 00:00:00 2001 From: Chris McBride Date: Thu, 1 Jun 2023 14:59:29 -0400 Subject: [PATCH 11/44] controller.py typehints --- smartsim/_core/control/controller.py | 64 +++++++++++++++++----------- 1 file changed, 39 insertions(+), 25 deletions(-) diff --git a/smartsim/_core/control/controller.py b/smartsim/_core/control/controller.py index 80dfc68ca..b339c4926 100644 --- a/smartsim/_core/control/controller.py +++ b/smartsim/_core/control/controller.py @@ -29,13 +29,13 @@ import signal import threading import time +import typing as t from smartredis import Client -from smartredis.error import RedisConnectionError, RedisReplyError from ..._core.utils.redis import db_is_active, set_ml_model, set_script from ...database import Orchestrator -from ...entity import DBModel, DBNode, DBObject, DBScript, EntityList, SmartSimEntity +from ...entity import DBNode, EntityList, SmartSimEntity from ...error import LauncherError, SmartSimError, SSInternalError, SSUnsupportedError from ...log import get_logger from ...status import STATUS_RUNNING, TERMINAL_STATUSES @@ -43,6 +43,10 @@ from ..launcher import * from ..utils import check_cluster_status, create_cluster from .jobmanager import JobManager +from .manifest import Manifest +from .job import Job +from ...settings.base import BatchSettings + logger = get_logger(__name__) @@ -56,7 +60,7 @@ class Controller: underlying workload manager or run framework. """ - def __init__(self, launcher="local"): + def __init__(self, launcher: str="local") -> None: """Initialize a Controller :param launcher: the type of launcher being used @@ -65,7 +69,10 @@ def __init__(self, launcher="local"): self._jobs = JobManager(JM_LOCK) self.init_launcher(launcher) - def start(self, manifest, block=True, kill_on_interrupt=True): + def start(self, + manifest: Manifest, + block: bool = True, + kill_on_interrupt: bool = True) -> None: """Start the passed SmartSim entities This function should not be called directly, but rather @@ -90,7 +97,7 @@ def start(self, manifest, block=True, kill_on_interrupt=True): self.poll(5, True, kill_on_interrupt=kill_on_interrupt) @property - def orchestrator_active(self): + def orchestrator_active(self) -> bool: JM_LOCK.acquire() try: if len(self._jobs.db_jobs) > 0: @@ -99,7 +106,10 @@ def orchestrator_active(self): finally: JM_LOCK.release() - def poll(self, interval, verbose, kill_on_interrupt=True): + def poll(self, + interval: int, + verbose: bool, + kill_on_interrupt: bool = True) -> None: """Poll running jobs and receive logging output of job status :param interval: number of seconds to wait before polling again @@ -124,7 +134,7 @@ def poll(self, interval, verbose, kill_on_interrupt=True): finally: JM_LOCK.release() - def finished(self, entity): + def finished(self, entity: SmartSimEntity) -> bool: """Return a boolean indicating wether a job has finished or not :param entity: object launched by SmartSim. @@ -149,7 +159,7 @@ def finished(self, entity): f"Entity {entity.name} has not been launched in this experiment" ) from None - def stop_entity(self, entity): + def stop_entity(self, entity: SmartSimEntity) -> None: """Stop an instance of an entity This function will also update the status of the job in @@ -180,7 +190,7 @@ def stop_entity(self, entity): finally: JM_LOCK.release() - def stop_entity_list(self, entity_list): + def stop_entity_list(self, entity_list: EntityList) -> None: """Stop an instance of an entity list :param entity_list: entity list to be stopped @@ -192,7 +202,7 @@ def stop_entity_list(self, entity_list): for entity in entity_list.entities: self.stop_entity(entity) - def get_jobs(self): + def get_jobs(self) -> t.Dict[str, Job]: """Return a dictionary of completed job data :returns: dict[str, Job] @@ -203,7 +213,7 @@ def get_jobs(self): finally: JM_LOCK.release() - def get_entity_status(self, entity): + def get_entity_status(self, entity: SmartSimEntity) -> str: """Get the status of an entity :param entity: entity to get status of @@ -218,7 +228,7 @@ def get_entity_status(self, entity): ) return self._jobs.get_status(entity) - def get_entity_list_status(self, entity_list): + def get_entity_list_status(self, entity_list: EntityList) -> t.List[str]: """Get the statuses of an entity list :param entity_list: entity list containing entities to @@ -237,7 +247,7 @@ def get_entity_list_status(self, entity_list): statuses.append(self.get_entity_status(entity)) return statuses - def init_launcher(self, launcher): + def init_launcher(self, launcher: str) -> None: """Initialize the controller with a specific type of launcher. SmartSim currently supports slurm, pbs(pro), cobalt, lsf, and local launching @@ -267,7 +277,7 @@ def init_launcher(self, launcher): else: raise TypeError("Must provide a 'launcher' argument") - def _launch(self, manifest): + def _launch(self, manifest: Manifest) -> None: """Main launching function of the controller Orchestrators are always launched first so that the @@ -323,7 +333,7 @@ def _launch(self, manifest): for step, entity in steps: self._launch_step(step, entity) - def _launch_orchestrator(self, orchestrator): + def _launch_orchestrator(self, orchestrator: Orchestrator) -> None: """Launch an Orchestrator instance This function will launch the Orchestrator instance and @@ -378,7 +388,7 @@ def _launch_orchestrator(self, orchestrator): self._save_orchestrator(orchestrator) logger.debug(f"Orchestrator launched on nodes: {orchestrator.hosts}") - def _launch_step(self, job_step, entity): + def _launch_step(self, job_step, entity: SmartSimEntity) -> None: """Use the launcher to launch a job stop :param job_step: a job step instance @@ -408,7 +418,7 @@ def _launch_step(self, job_step, entity): logger.debug(f"Launching {entity.name}") self._jobs.add_job(job_step.name, job_id, entity, is_task) - def _create_batch_job_step(self, entity_list): + def _create_batch_job_step(self, entity_list: EntityList) -> t.Any: """Use launcher to create batch job step :param entity_list: EntityList to launch as batch @@ -426,7 +436,7 @@ def _create_batch_job_step(self, entity_list): batch_step.add_to_batch(step) return batch_step - def _create_job_step(self, entity): + def _create_job_step(self, entity: SmartSimEntity) -> t.Any: """Create job steps for all entities with the launcher :param entities: list of all entities to create steps for @@ -441,7 +451,7 @@ def _create_job_step(self, entity): step = self._launcher.create_step(entity.name, entity.path, entity.run_settings) return step - def _prep_entity_client_env(self, entity): + def _prep_entity_client_env(self, entity: SmartSimEntity) -> None: """Retrieve all connections registered to this entity :param entity: The entity to retrieve connections from @@ -482,7 +492,7 @@ def _prep_entity_client_env(self, entity): ) entity.run_settings.update_env(client_env) - def _save_orchestrator(self, orchestrator): + def _save_orchestrator(self, orchestrator: Orchestrator) -> None: """Save the orchestrator object via pickle This function saves the orchestrator information to a pickle @@ -503,7 +513,7 @@ def _save_orchestrator(self, orchestrator): with open(dat_file, "wb") as pickle_file: pickle.dump(orc_data, pickle_file) - def _orchestrator_launch_wait(self, orchestrator): + def _orchestrator_launch_wait(self, orchestrator: Orchestrator) -> None: """Wait for the orchestrator instances to run In the case where the orchestrator is launched as a batch @@ -552,7 +562,7 @@ def _orchestrator_launch_wait(self, orchestrator): # launch explicitly raise - def reload_saved_db(self, checkpoint_file): + def reload_saved_db(self, checkpoint_file: str) -> Orchestrator: JM_LOCK.acquire() try: if self.orchestrator_active: @@ -609,7 +619,7 @@ def reload_saved_db(self, checkpoint_file): finally: JM_LOCK.release() - def _set_dbobjects(self, manifest): + def _set_dbobjects(self, manifest: Manifest) -> None: if not manifest.has_db_objects: return @@ -649,9 +659,13 @@ def _set_dbobjects(self, manifest): class _AnonymousBatchJob(EntityList): - def __init__(self, name, path, batch_settings, **kwargs): + def __init__(self, + name: str, + path: str, + batch_settings: BatchSettings, + **kwargs: t.Any) -> None: super().__init__(name, path) self.batch_settings = batch_settings - def _initialize_entities(self, **kwargs): + def _initialize_entities(self, **kwargs: t.Any) -> None: ... From 53f6221ce194c9557e647d2d605ff1ec3ca0174c Mon Sep 17 00:00:00 2001 From: Chris McBride Date: Thu, 1 Jun 2023 15:18:45 -0400 Subject: [PATCH 12/44] redis.py typehints --- smartsim/_core/utils/redis.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/smartsim/_core/utils/redis.py b/smartsim/_core/utils/redis.py index 7755bc1d1..4cb635a67 100644 --- a/smartsim/_core/utils/redis.py +++ b/smartsim/_core/utils/redis.py @@ -25,17 +25,16 @@ # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. import logging +import redis import time -from itertools import product +import typing as t -import redis +from itertools import product from redis.cluster import RedisCluster, ClusterNode from redis.exceptions import ClusterDownError, RedisClusterException from smartredis import Client from smartredis.error import RedisReplyError -logging.getLogger("rediscluster").setLevel(logging.WARNING) - from ...entity import DBModel, DBScript from ...error import SSInternalError from ...log import get_logger @@ -43,10 +42,11 @@ from ..launcher.util.shell import execute_cmd from .network import get_ip_from_host +logging.getLogger("rediscluster").setLevel(logging.WARNING) logger = get_logger(__name__) -def create_cluster(hosts, ports): # cov-wlm +def create_cluster(hosts: t.List[str], ports: t.List[int]): # cov-wlm """Connect launched cluster instances. Should only be used in the case where cluster initialization @@ -79,7 +79,7 @@ def create_cluster(hosts, ports): # cov-wlm logger.debug(out) -def check_cluster_status(hosts, ports, trials=10): # cov-wlm +def check_cluster_status(hosts: t.List[str], ports: t.List[int], trials: int = 10): # cov-wlm """Check that a Redis/KeyDB cluster is up and running :param hosts: List of hostnames to connect to @@ -114,7 +114,7 @@ def check_cluster_status(hosts, ports, trials=10): # cov-wlm raise SSInternalError("Cluster setup could not be verified") -def db_is_active(hosts, ports, num_shards): +def db_is_active(hosts: t.List[str], ports: t.List[int], num_shards: int) -> bool: """Check if a DB is running if the DB is clustered, check cluster status, otherwise @@ -150,7 +150,7 @@ def db_is_active(hosts, ports, num_shards): return False -def set_ml_model(db_model: DBModel, client: Client): +def set_ml_model(db_model: DBModel, client: Client) -> None: logger.debug(f"Adding DBModel named {db_model.name}") devices = db_model._enumerate_devices() @@ -185,7 +185,7 @@ def set_ml_model(db_model: DBModel, client: Client): raise error -def set_script(db_script: DBScript, client: Client): +def set_script(db_script: DBScript, client: Client) -> None: logger.debug(f"Adding DBScript named {db_script.name}") devices = db_script._enumerate_devices() From 3569ac08a2f304393bd7c0f8d46e9390845bb2d0 Mon Sep 17 00:00:00 2001 From: Chris McBride Date: Thu, 1 Jun 2023 16:17:32 -0400 Subject: [PATCH 13/44] wlm settings typehints --- smartsim/settings/alpsSettings.py | 40 +++++++---- smartsim/settings/base.py | 104 +++++++++++++++------------- smartsim/settings/cobaltSettings.py | 25 ++++--- smartsim/settings/lsfSettings.py | 86 +++++++++++++---------- smartsim/settings/mpiSettings.py | 74 +++++++++++++------- smartsim/settings/palsSettings.py | 38 +++++----- smartsim/settings/pbsSettings.py | 42 +++++------ smartsim/settings/settings.py | 33 +++++---- smartsim/settings/slurmSettings.py | 77 +++++++++++--------- 9 files changed, 302 insertions(+), 217 deletions(-) diff --git a/smartsim/settings/alpsSettings.py b/smartsim/settings/alpsSettings.py index 2ce591ac4..f86fb0d10 100644 --- a/smartsim/settings/alpsSettings.py +++ b/smartsim/settings/alpsSettings.py @@ -24,12 +24,22 @@ # 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 __future__ import annotations +import typing as t + from ..error import SSUnsupportedError from .base import RunSettings class AprunSettings(RunSettings): - def __init__(self, exe, exe_args=None, run_args=None, env_vars=None, **kwargs): + def __init__( + self, + exe: str, + exe_args: t.Optional[t.Union[str, t.List[str]]] = None, + run_args: t.Optional[t.Dict[str, str]] = None, + env_vars: t.Optional[t.Dict[str, str]] = None, + **kwargs: t.Any, + ): """Settings to run job with ``aprun`` command ``AprunSettings`` can be used for both the `pbs` and `cobalt` @@ -54,7 +64,7 @@ def __init__(self, exe, exe_args=None, run_args=None, env_vars=None, **kwargs): ) self.mpmd = [] - def make_mpmd(self, aprun_settings): + def make_mpmd(self, aprun_settings: AprunSettings) -> None: """Make job an MPMD job This method combines two ``AprunSettings`` @@ -73,7 +83,7 @@ def make_mpmd(self, aprun_settings): ) self.mpmd.append(aprun_settings) - def set_cpus_per_task(self, cpus_per_task): + def set_cpus_per_task(self, cpus_per_task: int) -> None: """Set the number of cpus to use per task This sets ``--cpus-per-pe`` @@ -83,7 +93,7 @@ def set_cpus_per_task(self, cpus_per_task): """ self.run_args["cpus-per-pe"] = int(cpus_per_task) - def set_tasks(self, tasks): + def set_tasks(self, tasks: int) -> None: """Set the number of tasks for this job This sets ``--pes`` @@ -93,7 +103,7 @@ def set_tasks(self, tasks): """ self.run_args["pes"] = int(tasks) - def set_tasks_per_node(self, tasks_per_node): + def set_tasks_per_node(self, tasks_per_node: int) -> None: """Set the number of tasks for this job This sets ``--pes-per-node`` @@ -103,7 +113,7 @@ def set_tasks_per_node(self, tasks_per_node): """ self.run_args["pes-per-node"] = int(tasks_per_node) - def set_hostlist(self, host_list): + def set_hostlist(self, host_list: t.Union[str, t.List[str]]) -> None: """Specify the hostlist for this job :param host_list: hosts to launch on @@ -118,7 +128,7 @@ def set_hostlist(self, host_list): raise TypeError("host_list argument must be list of strings") self.run_args["node-list"] = ",".join(host_list) - def set_hostlist_from_file(self, file_path): + def set_hostlist_from_file(self, file_path: str) -> None: """Use the contents of a file to set the node list This sets ``--node-list-file`` @@ -128,7 +138,7 @@ def set_hostlist_from_file(self, file_path): """ self.run_args["node-list-file"] = str(file_path) - def set_excluded_hosts(self, host_list): + def set_excluded_hosts(self, host_list: t.Union[str, t.List[str]]) -> None: """Specify a list of hosts to exclude for launching this job :param host_list: hosts to exclude @@ -143,7 +153,7 @@ def set_excluded_hosts(self, host_list): raise TypeError("host_list argument must be list of strings") self.run_args["exclude-node-list"] = ",".join(host_list) - def set_cpu_bindings(self, bindings): + def set_cpu_bindings(self, bindings: t.Union[int, t.List[int]]) -> None: """Specifies the cores to which MPI processes are bound This sets ``--cpu-binding`` @@ -155,7 +165,7 @@ def set_cpu_bindings(self, bindings): bindings = [bindings] self.run_args["cpu-binding"] = ",".join(str(int(num)) for num in bindings) - def set_memory_per_node(self, memory_per_node): + def set_memory_per_node(self, memory_per_node: int) -> None: """Specify the real memory required per node This sets ``--memory-per-pe`` in megabytes @@ -165,7 +175,7 @@ def set_memory_per_node(self, memory_per_node): """ self.run_args["memory-per-pe"] = int(memory_per_node) - def set_verbose_launch(self, verbose): + def set_verbose_launch(self, verbose: bool) -> None: """Set the job to run in verbose mode This sets ``--debug`` arg to the highest level @@ -178,7 +188,7 @@ def set_verbose_launch(self, verbose): else: self.run_args.pop("debug", None) - def set_quiet_launch(self, quiet): + def set_quiet_launch(self, quiet: bool) -> None: """Set the job to run in quiet mode This sets ``--quiet`` @@ -191,7 +201,7 @@ def set_quiet_launch(self, quiet): else: self.run_args.pop("quiet", None) - def format_run_args(self): + def format_run_args(self) -> t.List[str]: """Return a list of ALPS formatted run arguments :return: list of ALPS arguments for these settings @@ -214,7 +224,7 @@ def format_run_args(self): args += ["=".join((prefix + opt, str(value)))] return args - def format_env_vars(self): + def format_env_vars(self) -> t.List[str]: """Format the environment variables for aprun :return: list of env vars @@ -226,7 +236,7 @@ def format_env_vars(self): formatted += ["-e", name + "=" + str(value)] return formatted - def set_walltime(self, walltime): + def set_walltime(self, walltime: str) -> None: """Set the walltime of the job Walltime is given in total number of seconds diff --git a/smartsim/settings/base.py b/smartsim/settings/base.py index b1ef66643..90e086912 100644 --- a/smartsim/settings/base.py +++ b/smartsim/settings/base.py @@ -22,6 +22,9 @@ # CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, # 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 __future__ import annotations + +import typing as t from .._core.utils.helpers import expand_exe_path, fmt_dict, init_default, is_valid_cmd from ..log import get_logger @@ -32,14 +35,16 @@ class RunSettings: def __init__( self, - exe, - exe_args=None, - run_command="", - run_args=None, - env_vars=None, - container=None, - **kwargs, - ): + exe: str, + exe_args: t.Optional[t.Union[str, t.List[str]]] = None, + run_command: str = "", + run_args: t.Optional[t.Dict[str, str]] = None, + env_vars: t.Optional[t.Dict[str, str]] = None, + # mcb + # container: bool = False, + container: bool = None, + **kwargs: t.Any, + ) -> None: """Run parameters for a ``Model`` The base ``RunSettings`` class should only be used with the `local` @@ -89,7 +94,7 @@ def __init__( # To be overwritten by subclasses. Set of reserved args a user cannot change reserved_run_args = set() # type: set[str] - def set_nodes(self, nodes): + def set_nodes(self, nodes: int) -> None: """Set the number of nodes :param nodes: number of nodes to run with @@ -102,7 +107,7 @@ def set_nodes(self, nodes): ) ) - def set_tasks(self, tasks): + def set_tasks(self, tasks: int) -> None: """Set the number of tasks to launch :param tasks: number of tasks to launch @@ -115,7 +120,7 @@ def set_tasks(self, tasks): ) ) - def set_tasks_per_node(self, tasks_per_node): + def set_tasks_per_node(self, tasks_per_node: int) -> None: """Set the number of tasks per node :param tasks_per_node: number of tasks to launch per node @@ -128,7 +133,7 @@ def set_tasks_per_node(self, tasks_per_node): ) ) - def set_task_map(self, task_mapping): + def set_task_map(self, task_mapping: str) -> None: """Set a task mapping :param task_mapping: task mapping @@ -141,7 +146,7 @@ def set_task_map(self, task_mapping): ) ) - def set_cpus_per_task(self, cpus_per_task): + def set_cpus_per_task(self, cpus_per_task: int) -> None: """Set the number of cpus per task :param cpus_per_task: number of cpus per task @@ -154,7 +159,7 @@ def set_cpus_per_task(self, cpus_per_task): ) ) - def set_hostlist(self, host_list): + def set_hostlist(self, host_list: t.Union[str, t.List[str]]) -> None: """Specify the hostlist for this job :param host_list: hosts to launch on @@ -167,7 +172,7 @@ def set_hostlist(self, host_list): ) ) - def set_hostlist_from_file(self, file_path): + def set_hostlist_from_file(self, file_path: str) -> None: """Use the contents of a file to specify the hostlist for this job :param file_path: Path to the hostlist file @@ -180,7 +185,7 @@ def set_hostlist_from_file(self, file_path): ) ) - def set_excluded_hosts(self, host_list): + def set_excluded_hosts(self, host_list: t.Union[str, t.List[str]]): """Specify a list of hosts to exclude for launching this job :param host_list: hosts to exclude @@ -193,7 +198,7 @@ def set_excluded_hosts(self, host_list): ) ) - def set_cpu_bindings(self, bindings): + def set_cpu_bindings(self, bindings: t.Union[int, t.List[int]]) -> None: """Set the cores to which MPI processes are bound :param bindings: List specifing the cores to which MPI processes are bound @@ -206,7 +211,7 @@ def set_cpu_bindings(self, bindings): ) ) - def set_memory_per_node(self, memory_per_node): + def set_memory_per_node(self, memory_per_node: int) -> None: """Set the amount of memory required per node in megabytes :param memory_per_node: Number of megabytes per node @@ -219,7 +224,7 @@ def set_memory_per_node(self, memory_per_node): ) ) - def set_verbose_launch(self, verbose): + def set_verbose_launch(self, verbose: bool) -> None: """Set the job to run in verbose mode :param verbose: Whether the job should be run verbosely @@ -232,7 +237,7 @@ def set_verbose_launch(self, verbose): ) ) - def set_quiet_launch(self, quiet): + def set_quiet_launch(self, quiet: bool) -> None: """Set the job to run in quiet mode :param quiet: Whether the job should be run quietly @@ -245,7 +250,7 @@ def set_quiet_launch(self, quiet): ) ) - def set_broadcast(self, dest_path=None): + def set_broadcast(self, dest_path: t.Optional[str] = None) -> None: """Copy executable file to allocated compute nodes :param dest_path: Path to copy an executable file @@ -258,7 +263,7 @@ def set_broadcast(self, dest_path=None): ) ) - def set_time(self, hours=0, minutes=0, seconds=0): + def set_time(self, hours: int = 0, minutes: int = 0, seconds: int = 0) -> None: """Automatically format and set wall time :param hours: number of hours to run job @@ -272,7 +277,7 @@ def set_time(self, hours=0, minutes=0, seconds=0): self._fmt_walltime(int(hours), int(minutes), int(seconds)) ) - def _fmt_walltime(self, hours, minutes, seconds): + def _fmt_walltime(self, hours: int, minutes: int, seconds: int) -> str: """Convert hours, minutes, and seconds into valid walltime format By defualt the formatted wall time is the total number of seconds. @@ -291,7 +296,7 @@ def _fmt_walltime(self, hours, minutes, seconds): time_ += seconds return str(time_) - def set_walltime(self, walltime): + def set_walltime(self, walltime: str) -> None: """Set the formatted walltime :param walltime: Time in format required by launcher`` @@ -304,7 +309,7 @@ def set_walltime(self, walltime): ) ) - def set_binding(self, binding): + def set_binding(self, binding: str) -> None: """Set binding :param binding: Binding @@ -317,7 +322,7 @@ def set_binding(self, binding): ) ) - def set_mpmd_preamble(self, preamble_lines): + def set_mpmd_preamble(self, preamble_lines: t.List[str]) -> None: """Set preamble to a file to make a job MPMD :param preamble_lines: lines to put at the beginning of a file. @@ -330,7 +335,7 @@ def set_mpmd_preamble(self, preamble_lines): ) ) - def make_mpmd(self, settings): + def make_mpmd(self, settings: RunSettings) -> None: """Make job an MPMD job :param settings: ``RunSettings`` instance @@ -344,7 +349,7 @@ def make_mpmd(self, settings): ) @property - def run_command(self): + def run_command(self) -> t.Optional[str]: """Return the launch binary used to launch the executable Attempt to expand the path to the executable if possible @@ -364,7 +369,7 @@ def run_command(self): # run without run command return None - def update_env(self, env_vars): + def update_env(self, env_vars: t.Dict[str, t.Union[str, int, float, bool]]) -> None: """Update the job environment variables To fully inherit the current user environment, add the @@ -387,7 +392,7 @@ def update_env(self, env_vars): else: self.env_vars[env] = val - def add_exe_args(self, args): + def add_exe_args(self, args: t.Union[str, t.List[str]]) -> None: """Add executable arguments to executable :param args: executable arguments @@ -401,7 +406,7 @@ def add_exe_args(self, args): raise TypeError("Executable arguments should be a list of str") self.exe_args.append(arg) - def set(self, arg, value=None, condition=True): + def set(self, arg: str, value: t.Optional[str] = None, condition: bool = True) -> None: """Allows users to set individual run arguments. A method that allows users to set run arguments after object @@ -472,7 +477,7 @@ def set(self, arg, value=None, condition=True): logger.warning(f"Overwritting argument '{arg}' with value '{value}'") self.run_args[arg] = value - def _set_exe_args(self, exe_args): + def _set_exe_args(self, exe_args: t.Optional[t.Union[str, t.List[str]]]) -> t.List[str]: if exe_args: if isinstance(exe_args, str): return exe_args.split() @@ -496,7 +501,7 @@ def _set_exe_args(self, exe_args): else: return [] - def format_run_args(self): + def format_run_args(self) -> t.List[str]: """Return formatted run arguments For ``RunSettings``, the run arguments are passed @@ -511,7 +516,7 @@ def format_run_args(self): formatted.append(str(value)) return formatted - def format_env_vars(self): + def format_env_vars(self) -> t.List[str]: """Build environment variable string :returns: formatted list of strings to export variables @@ -525,7 +530,7 @@ def format_env_vars(self): formatted.append(f"{key}={val}") return formatted - def __str__(self): # pragma: no-cover + def __str__(self) -> str: # pragma: no-cover string = f"Executable: {self.exe[0]}\n" string += f"Executable Arguments: {' '.join((self.exe_args))}" if self.run_command: @@ -538,17 +543,22 @@ def __str__(self): # pragma: no-cover class BatchSettings: - def __init__(self, batch_cmd, batch_args=None, **kwargs): + def __init__( + self, + batch_cmd: str, + batch_args: t.Optional[t.Dict[str, t.Any]] = None, + **kwargs: t.Any, + ) -> None: self._batch_cmd = batch_cmd self.batch_args = init_default({}, batch_args, dict) - self._preamble = [] + self._preamble: t.List[str] = [] self.set_nodes(kwargs.get("nodes", None)) self.set_walltime(kwargs.get("time", None)) self.set_queue(kwargs.get("queue", None)) self.set_account(kwargs.get("account", None)) @property - def batch_cmd(self): + def batch_cmd(self) -> str: """Return the batch command Tests to see if we can expand the batch command @@ -563,25 +573,25 @@ def batch_cmd(self): else: return self._batch_cmd - def set_nodes(self, num_nodes): + def set_nodes(self, num_nodes: int) -> None: raise NotImplementedError - def set_hostlist(self, host_list): + def set_hostlist(self, host_list: t.Union[str, t.List[str]]) -> None: raise NotImplementedError - def set_queue(self, queue): + def set_queue(self, queue: str) -> None: raise NotImplementedError - def set_walltime(self, walltime): + def set_walltime(self, walltime: str) -> None: raise NotImplementedError - def set_account(self, account): + def set_account(self, account: str) -> None: raise NotImplementedError - def format_batch_args(self): + def format_batch_args(self) -> t.List[str]: raise NotImplementedError - def set_batch_command(self, command): + def set_batch_command(self, command: str) -> None: """Set the command used to launch the batch e.g. ``sbatch`` :param command: batch command @@ -589,7 +599,7 @@ def set_batch_command(self, command): """ self._batch_cmd = command - def add_preamble(self, lines): + def add_preamble(self, lines: t.List[str]) -> None: """Add lines to the batch file preamble. The lines are just written (unmodified) at the beginning of the batch file (after the WLM directives) and can be used to e.g. @@ -605,7 +615,7 @@ def add_preamble(self, lines): else: raise TypeError("Expected str or List[str] for lines argument") - def __str__(self): # pragma: no-cover + def __str__(self) -> str: # pragma: no-cover string = f"Batch Command: {self._batch_cmd}" if self.batch_args: string += f"\nBatch arguments:\n{fmt_dict(self.batch_args)}" diff --git a/smartsim/settings/cobaltSettings.py b/smartsim/settings/cobaltSettings.py index 8e89410cd..769f98694 100644 --- a/smartsim/settings/cobaltSettings.py +++ b/smartsim/settings/cobaltSettings.py @@ -24,13 +24,20 @@ # 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. +import typing as t from .base import BatchSettings class CobaltBatchSettings(BatchSettings): def __init__( - self, nodes=None, time="", queue=None, account=None, batch_args=None, **kwargs - ): + self, + nodes: int = None, + time: str = "", + queue: t.Optional[str] = None, + account: t.Optional[str] = None, + batch_args: t.Optional[t.Dict[str, str]] = None, + **kwargs, + ) -> None: """Specify settings for a Cobalt ``qsub`` batch launch If the argument doesn't have a parameter, put None @@ -60,7 +67,7 @@ def __init__( **kwargs, ) - def set_walltime(self, walltime): + def set_walltime(self, walltime: str) -> None: """Set the walltime of the job format = "HH:MM:SS" @@ -76,7 +83,7 @@ def set_walltime(self, walltime): if walltime: self.batch_args["time"] = walltime - def set_nodes(self, num_nodes): + def set_nodes(self, num_nodes: int) -> None: """Set the number of nodes for this batch job :param num_nodes: number of nodes @@ -86,7 +93,7 @@ def set_nodes(self, num_nodes): if num_nodes: self.batch_args["nodecount"] = int(num_nodes) - def set_hostlist(self, host_list): + def set_hostlist(self, host_list: t.Union[str, t.List[str]]) -> None: """Specify the hostlist for this job :param host_list: hosts to launch on @@ -102,7 +109,7 @@ def set_hostlist(self, host_list): hosts = ",".join(host_list) self.batch_args["attrs"] = f"location={hosts}" - def set_tasks(self, num_tasks): + def set_tasks(self, num_tasks: int) -> None: """Set total number of processes to start :param num_tasks: number of processes @@ -110,7 +117,7 @@ def set_tasks(self, num_tasks): """ self.batch_args["proccount"] = int(num_tasks) - def set_queue(self, queue): + def set_queue(self, queue: str) -> None: """Set the queue for the batch job :param queue: queue name @@ -120,7 +127,7 @@ def set_queue(self, queue): if queue: self.batch_args["queue"] = str(queue) - def set_account(self, account): + def set_account(self, account: str) -> None: """Set the account for this batch job :param acct: account id @@ -130,7 +137,7 @@ def set_account(self, account): if account: self.batch_args["project"] = account - def format_batch_args(self): + def format_batch_args(self) -> t.List[str]: """Get the formatted batch arguments for a preview :return: list of batch arguments for Sbatch diff --git a/smartsim/settings/lsfSettings.py b/smartsim/settings/lsfSettings.py index 49373c2b5..41a92e0fc 100644 --- a/smartsim/settings/lsfSettings.py +++ b/smartsim/settings/lsfSettings.py @@ -24,6 +24,9 @@ # 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 __future__ import annotations + +import typing as t from pprint import pformat from ..error import SSUnsupportedError @@ -34,7 +37,14 @@ class JsrunSettings(RunSettings): - def __init__(self, exe, exe_args=None, run_args=None, env_vars=None, **kwargs): + def __init__( + self, + exe: str, + exe_args: t.Optional[t.Union[str, t.List[str]]] = None, + run_args: t.Optional[t.Dict[str, str]] = None, + env_vars: t.Optional[t.Dict[str, str]] = None, + **kwargs, + ) -> None: """Settings to run job with ``jsrun`` command ``JsrunSettings`` should only be used on LSF-based systems. @@ -64,7 +74,7 @@ def __init__(self, exe, exe_args=None, run_args=None, env_vars=None, **kwargs): reserved_run_args = {"chdir", "h"} - def set_num_rs(self, num_rs): + def set_num_rs(self, num_rs: t.Union[str, int]) -> None: """Set the number of resource sets to use This sets ``--nrs``. @@ -77,7 +87,7 @@ def set_num_rs(self, num_rs): else: self.run_args["nrs"] = int(num_rs) - def set_cpus_per_rs(self, cpus_per_rs): + def set_cpus_per_rs(self, cpus_per_rs: int) -> None: """Set the number of cpus to use per resource set This sets ``--cpu_per_rs`` @@ -96,7 +106,7 @@ def set_cpus_per_rs(self, cpus_per_rs): else: self.run_args["cpu_per_rs"] = int(cpus_per_rs) - def set_gpus_per_rs(self, gpus_per_rs): + def set_gpus_per_rs(self, gpus_per_rs: int) -> None: """Set the number of gpus to use per resource set This sets ``--gpu_per_rs`` @@ -109,7 +119,7 @@ def set_gpus_per_rs(self, gpus_per_rs): else: self.run_args["gpu_per_rs"] = int(gpus_per_rs) - def set_rs_per_host(self, rs_per_host): + def set_rs_per_host(self, rs_per_host: int) -> None: """Set the number of resource sets to use per host This sets ``--rs_per_host`` @@ -119,7 +129,7 @@ def set_rs_per_host(self, rs_per_host): """ self.run_args["rs_per_host"] = int(rs_per_host) - def set_tasks(self, tasks): + def set_tasks(self, tasks: int) -> None: """Set the number of tasks for this job This sets ``--np`` @@ -129,7 +139,7 @@ def set_tasks(self, tasks): """ self.run_args["np"] = int(tasks) - def set_tasks_per_rs(self, tasks_per_rs): + def set_tasks_per_rs(self, tasks_per_rs: int) -> None: """Set the number of tasks per resource set This sets ``--tasks_per_rs`` @@ -139,7 +149,7 @@ def set_tasks_per_rs(self, tasks_per_rs): """ self.run_args["tasks_per_rs"] = int(tasks_per_rs) - def set_tasks_per_node(self, tasks_per_node): + def set_tasks_per_node(self, tasks_per_node: int) -> None: """Set the number of tasks per resource set. This function is an alias for `set_tasks_per_rs`. @@ -149,7 +159,7 @@ def set_tasks_per_node(self, tasks_per_node): """ self.set_tasks_per_rs(tasks_per_node) - def set_cpus_per_task(self, cpus_per_task): + def set_cpus_per_task(self, cpus_per_task: int) -> None: """Set the number of cpus per tasks. This function is an alias for `set_cpus_per_rs`. @@ -159,7 +169,7 @@ def set_cpus_per_task(self, cpus_per_task): """ self.set_cpus_per_rs(int(cpus_per_task)) - def set_memory_per_rs(self, memory_per_rs): + def set_memory_per_rs(self, memory_per_rs: int) -> None: """Specify the number of megabytes of memory to assign to a resource set This sets ``--memory_per_rs`` @@ -169,7 +179,7 @@ def set_memory_per_rs(self, memory_per_rs): """ self.run_args["memory_per_rs"] = int(memory_per_rs) - def set_memory_per_node(self, memory_per_node): + def set_memory_per_node(self, memory_per_node: int) -> None: """Specify the number of megabytes of memory to assign to a resource set Alias for `set_memory_per_rs`. @@ -179,7 +189,7 @@ def set_memory_per_node(self, memory_per_node): """ self.set_memory_per_rs(memory_per_node) - def set_binding(self, binding): + def set_binding(self, binding: str) -> None: """Set binding This sets ``--bind`` @@ -189,7 +199,7 @@ def set_binding(self, binding): """ self.run_args["bind"] = binding - def make_mpmd(self, jsrun_settings=None): + def make_mpmd(self, jsrun_settings: t.Optional[JsrunSettings] = None): """Make step an MPMD (or SPMD) job. This method will activate job execution through an ERF file. @@ -210,7 +220,7 @@ def make_mpmd(self, jsrun_settings=None): if jsrun_settings: self.mpmd.append(jsrun_settings) - def set_mpmd_preamble(self, preamble_lines): + def set_mpmd_preamble(self, preamble_lines: t.List[str]) -> None: """Set preamble used in ERF file. Typical lines include `oversubscribe-cpu : allow` or `overlapping-rs : allow`. Can be used to set `launch_distribution`. If it is not present, @@ -223,7 +233,7 @@ def set_mpmd_preamble(self, preamble_lines): """ self.mpmd_preamble_lines = preamble_lines - def set_erf_sets(self, erf_sets): + def set_erf_sets(self, erf_sets: t.Dict[str, str]) -> None: """Set resource sets used for ERF (SPMD or MPMD) steps. ``erf_sets`` is a dictionary used to fill the ERF @@ -241,7 +251,7 @@ def set_erf_sets(self, erf_sets): """ self.erf_sets = erf_sets - def format_env_vars(self): + def format_env_vars(self) -> t.List[str]: """Format environment variables. Each variable needs to be passed with ``--env``. If a variable is set to ``None``, its value is propagated from the current environment. @@ -257,7 +267,7 @@ def format_env_vars(self): format_str += ["-E", f"{k}"] return format_str - def set_individual_output(self, suffix=None): + def set_individual_output(self, suffix: t.Optional[str] = None) -> None: """Set individual std output. This sets ``--stdio_mode individual`` @@ -274,7 +284,7 @@ def set_individual_output(self, suffix=None): if suffix: self.individual_suffix = suffix - def format_run_args(self): + def format_run_args(self) -> t.List[str]: """Return a list of LSF formatted run arguments :return: list of LSF arguments for these settings @@ -333,13 +343,13 @@ def format_run_args(self): args += ["=".join((prefix + opt, str(value)))] return args - def __str__(self): + def __str__(self) -> str: string = super().__str__() if self.mpmd: string += "\nERF settings: " + pformat(self.erf_sets) return string - def _prep_colocated_db(self, db_cpus): + def _prep_colocated_db(self, db_cpus: int) -> None: cpus_per_flag_set = False for cpu_per_rs_flag in ["cpu_per_rs", "c"]: if cpu_per_rs_flag in self.run_args: @@ -380,13 +390,13 @@ def _prep_colocated_db(self, db_cpus): class BsubBatchSettings(BatchSettings): def __init__( self, - nodes=None, - time=None, - project=None, - batch_args=None, - smts=None, - **kwargs, - ): + nodes: t.Optional[int] = None, + time: t.Optional[str] = None, + project: t.Optional[str] = None, + batch_args: t.Optional[t.Dict[str, str]] = None, + smts: t.Optional[int] = None, + **kwargs: t.Any, + ) -> None: """Specify ``bsub`` batch parameters for a job :param nodes: number of nodes for batch, defaults to None @@ -421,7 +431,7 @@ def __init__( self.expert_mode = False self.easy_settings = ["ln_slots", "ln_mem", "cn_cu", "nnodes"] - def set_walltime(self, walltime): + def set_walltime(self, walltime: str) -> None: """Set the walltime This sets ``-W``. @@ -437,7 +447,7 @@ def set_walltime(self, walltime): walltime = ":".join(walltime.split(":")[:2]) self.walltime = walltime - def set_smts(self, smts): + def set_smts(self, smts: int) -> None: """Set SMTs This sets ``-alloc_flags``. If the user sets @@ -449,7 +459,7 @@ def set_smts(self, smts): """ self.smts = int(smts) - def set_project(self, project): + def set_project(self, project: str) -> None: """Set the project This sets ``-P``. @@ -460,7 +470,7 @@ def set_project(self, project): if project: self.project = project - def set_account(self, account): + def set_account(self, account: str) -> None: """Set the project this function is an alias for `set_project`. @@ -470,7 +480,7 @@ def set_account(self, account): """ self.set_project(account) - def set_nodes(self, num_nodes): + def set_nodes(self, num_nodes: int) -> None: """Set the number of nodes for this batch job This sets ``-nnodes``. @@ -481,7 +491,7 @@ def set_nodes(self, num_nodes): if num_nodes: self.batch_args["nnodes"] = int(num_nodes) - def set_expert_mode_req(self, res_req, slots): + def set_expert_mode_req(self, res_req: str, slots: int) -> None: """Set allocation for expert mode. This will activate expert mode (``-csm``) and disregard all other allocation options. @@ -493,7 +503,7 @@ def set_expert_mode_req(self, res_req, slots): self.batch_args["R"] = res_req self.batch_args["n"] = slots - def set_hostlist(self, host_list): + def set_hostlist(self, host_list: t.Union[str, t.List[str]]) -> None: """Specify the hostlist for this job :param host_list: hosts to launch on @@ -508,7 +518,7 @@ def set_hostlist(self, host_list): raise TypeError("host_list argument must be list of strings") self.batch_args["m"] = '"' + " ".join(host_list) + '"' - def set_tasks(self, tasks): + def set_tasks(self, tasks: int) -> None: """Set the number of tasks for this job This sets ``-n`` @@ -518,7 +528,7 @@ def set_tasks(self, tasks): """ self.batch_args["n"] = int(tasks) - def set_queue(self, queue): + def set_queue(self, queue: str) -> None: """Set the queue for this job :param queue: The queue to submit the job on @@ -527,7 +537,7 @@ def set_queue(self, queue): if queue: self.batch_args["q"] = queue - def _format_alloc_flags(self): + def _format_alloc_flags(self) -> None: """Format ``alloc_flags`` checking if user already set it. Currently only adds SMT flag if missing and ``self.smts`` is set. @@ -549,7 +559,7 @@ def _format_alloc_flags(self): if len(flags) > 1: self.batch_args["alloc_flags"] = '"' + " ".join(flags) + '"' - def format_batch_args(self): + def format_batch_args(self) -> t.List[str]: """Get the formatted batch arguments for a preview :return: list of batch arguments for Qsub diff --git a/smartsim/settings/mpiSettings.py b/smartsim/settings/mpiSettings.py index 41b6b854c..7ee9818b2 100644 --- a/smartsim/settings/mpiSettings.py +++ b/smartsim/settings/mpiSettings.py @@ -24,8 +24,11 @@ # 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 __future__ import annotations + import shutil import subprocess +import typing as t from ..error import LauncherError, SSUnsupportedError from ..log import get_logger @@ -39,14 +42,14 @@ class _BaseMPISettings(RunSettings): def __init__( self, - exe, - exe_args=None, - run_command="mpiexec", - run_args=None, - env_vars=None, - fail_if_missing_exec=True, - **kwargs, - ): + exe: str, + exe_args: t.Optional[t.Union[str, t.List[str]]] = None, + run_command: str = "mpiexec", + run_args: t.Optional[t.Dict[str, str]] = None, + env_vars: t.Optional[t.Dict[str, str]] = None, + fail_if_missing_exec: bool = True, + **kwargs: t.Any, + ) -> None: """Settings to format run job with an MPI-standard binary Note that environment variables can be passed with a None @@ -91,7 +94,7 @@ def __init__( reserved_run_args = {"wd", "wdir"} - def make_mpmd(self, mpirun_settings): + def make_mpmd(self, mpirun_settings: MpirunSettings) -> None: """Make a mpmd workload by combining two ``mpirun`` commands This connects the two settings to be executed with a single @@ -106,7 +109,7 @@ def make_mpmd(self, mpirun_settings): ) self.mpmd.append(mpirun_settings) - def set_task_map(self, task_mapping): + def set_task_map(self, task_mapping: str) -> None: """Set ``mpirun`` task mapping this sets ``--map-by `` @@ -118,7 +121,7 @@ def set_task_map(self, task_mapping): """ self.run_args["map-by"] = str(task_mapping) - def set_cpus_per_task(self, cpus_per_task): + def set_cpus_per_task(self, cpus_per_task: int) -> None: """Set the number of tasks for this job This sets ``--cpus-per-proc`` for MPI compliant implementations @@ -131,7 +134,7 @@ def set_cpus_per_task(self, cpus_per_task): """ self.run_args["cpus-per-proc"] = int(cpus_per_task) - def set_cpu_binding_type(self, bind_type): + def set_cpu_binding_type(self, bind_type: str) -> None: """Specifies the cores to which MPI processes are bound This sets ``--bind-to`` for MPI compliant implementations @@ -141,7 +144,7 @@ def set_cpu_binding_type(self, bind_type): """ self.run_args["bind-to"] = str(bind_type) - def set_tasks_per_node(self, tasks_per_node): + def set_tasks_per_node(self, tasks_per_node: int) -> None: """Set the number of tasks per node :param tasks_per_node: number of tasks to launch per node @@ -149,7 +152,7 @@ def set_tasks_per_node(self, tasks_per_node): """ self.run_args["npernode"] = int(tasks_per_node) - def set_tasks(self, tasks): + def set_tasks(self, tasks: int) -> None: """Set the number of tasks for this job This sets ``-n`` for MPI compliant implementations @@ -159,7 +162,7 @@ def set_tasks(self, tasks): """ self.run_args["n"] = int(tasks) - def set_hostlist(self, host_list): + def set_hostlist(self, host_list: t.Union[str, t.List[str]]) -> None: """Set the hostlist for the ``mpirun`` command This sets ``--host`` @@ -176,7 +179,7 @@ def set_hostlist(self, host_list): raise TypeError("host_list argument must be list of strings") self.run_args["host"] = ",".join(host_list) - def set_hostlist_from_file(self, file_path): + def set_hostlist_from_file(self, file_path: str) -> None: """Use the contents of a file to set the hostlist This sets ``--hostfile`` @@ -186,7 +189,7 @@ def set_hostlist_from_file(self, file_path): """ self.run_args["hostfile"] = str(file_path) - def set_verbose_launch(self, verbose): + def set_verbose_launch(self, verbose: bool) -> None: """Set the job to run in verbose mode This sets ``--verbose`` @@ -199,7 +202,7 @@ def set_verbose_launch(self, verbose): else: self.run_args.pop("verbose", None) - def set_quiet_launch(self, quiet): + def set_quiet_launch(self, quiet: bool) -> None: """Set the job to run in quiet mode This sets ``--quiet`` @@ -212,7 +215,7 @@ def set_quiet_launch(self, quiet): else: self.run_args.pop("quiet", None) - def set_broadcast(self, dest_path=None): + def set_broadcast(self, dest_path: t.Optional[str] = None) -> None: """Copy the specified executable(s) to remote machines This sets ``--preload-binary`` @@ -229,7 +232,7 @@ def set_broadcast(self, dest_path=None): ) self.run_args["preload-binary"] = None - def set_walltime(self, walltime): + def set_walltime(self, walltime: str) -> None: """Set the maximum number of seconds that a job will run This sets ``--timeout`` @@ -239,7 +242,7 @@ def set_walltime(self, walltime): """ self.run_args["timeout"] = str(walltime) - def format_run_args(self): + def format_run_args(self) -> t.List[str]: """Return a list of MPI-standard formatted run arguments :return: list of MPI-standard arguments for these settings @@ -258,7 +261,7 @@ def format_run_args(self): args += [prefix + opt, str(value)] return args - def format_env_vars(self): + def format_env_vars(self) -> t.List[str]: """Format the environment variables for mpirun :return: list of env vars @@ -277,7 +280,14 @@ def format_env_vars(self): class MpirunSettings(_BaseMPISettings): - def __init__(self, exe, exe_args=None, run_args=None, env_vars=None, **kwargs): + def __init__( + self, + exe: str, + exe_args: t.Optional[t.Union[str, t.List[str]]] = None, + run_args: t.Optional[t.Dict[str, str]] = None, + env_vars: t.Optional[t.Dict[str, str]] = None, + **kwargs: t.Any, + ) -> None: """Settings to run job with ``mpirun`` command (MPI-standard) Note that environment variables can be passed with a None @@ -301,7 +311,14 @@ def __init__(self, exe, exe_args=None, run_args=None, env_vars=None, **kwargs): class MpiexecSettings(_BaseMPISettings): - def __init__(self, exe, exe_args=None, run_args=None, env_vars=None, **kwargs): + def __init__( + self, + exe: str, + exe_args: t.Optional[t.Union[str, t.List[str]]] = None, + run_args: t.Optional[t.Dict[str, str]] = None, + env_vars: t.Optional[t.Dict[str, str]] = None, + **kwargs: t.Any, + ) -> None: """Settings to run job with ``mpiexec`` command (MPI-standard) Note that environment variables can be passed with a None @@ -334,7 +351,14 @@ def __init__(self, exe, exe_args=None, run_args=None, env_vars=None, **kwargs): class OrterunSettings(_BaseMPISettings): - def __init__(self, exe, exe_args=None, run_args=None, env_vars=None, **kwargs): + def __init__( + self, + exe: str, + exe_args: t.Optional[t.Union[str, t.List[str]]] = None, + run_args: t.Optional[t.Dict[str, str]] = None, + env_vars: t.Optional[t.Dict[str, str]] = None, + **kwargs: t.Any, + ) -> None: """Settings to run job with ``orterun`` command (MPI-standard) Note that environment variables can be passed with a None diff --git a/smartsim/settings/palsSettings.py b/smartsim/settings/palsSettings.py index c556cec62..12d461180 100644 --- a/smartsim/settings/palsSettings.py +++ b/smartsim/settings/palsSettings.py @@ -24,10 +24,8 @@ # 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. -import shutil -import subprocess +import typing as t -from ..error import LauncherError, SSUnsupportedError from ..log import get_logger from .mpiSettings import _BaseMPISettings @@ -58,14 +56,14 @@ class PalsMpiexecSettings(_BaseMPISettings): def __init__( self, - exe, - exe_args=None, - run_command="mpiexec", - run_args=None, - env_vars=None, - fail_if_missing_exec=True, - **kwargs, - ): + exe: str, + exe_args: t.Optional[t.Union[str, t.List[str]]] = None, + run_command: str = "mpiexec", + run_args: t.Optional[t.Dict[str, str]] = None, + env_vars: t.Optional[t.Dict[str, str]] = None, + fail_if_missing_exec: bool = True, + **kwargs: t.Any, + ) -> None: """Settings to format run job with an MPI-standard binary Note that environment variables can be passed with a None @@ -98,7 +96,7 @@ def __init__( **kwargs, ) - def set_task_map(self, task_mapping): + def set_task_map(self, task_mapping: str) -> None: """Set ``mpirun`` task mapping this sets ``--map-by `` @@ -110,7 +108,7 @@ def set_task_map(self, task_mapping): """ logger.warning("set_task_map not supported under PALS") - def set_cpus_per_task(self, cpus_per_task): + def set_cpus_per_task(self, cpus_per_task: int) -> None: """Set the number of tasks for this job This sets ``--cpus-per-proc`` for MPI compliant implementations @@ -123,7 +121,7 @@ def set_cpus_per_task(self, cpus_per_task): """ logger.warning("set_cpus_per_task not supported under PALS") - def set_cpu_binding_type(self, bind_type): + def set_cpu_binding_type(self, bind_type: str) -> None: """Specifies the cores to which MPI processes are bound This sets ``--bind-to`` for MPI compliant implementations @@ -133,7 +131,7 @@ def set_cpu_binding_type(self, bind_type): """ self.run_args["cpu-bind"] = str(bind_type) - def set_tasks(self, tasks): + def set_tasks(self, tasks: int) -> None: """Set the number of tasks :param tasks: number of total tasks to launch @@ -149,7 +147,7 @@ def set_tasks_per_node(self, tasks_per_node): """ self.run_args["ppn"] = int(tasks_per_node) - def set_quiet_launch(self, quiet): + def set_quiet_launch(self, quiet: bool) -> None: """Set the job to run in quiet mode This sets ``--quiet`` @@ -160,7 +158,7 @@ def set_quiet_launch(self, quiet): logger.warning("set_quiet_launch not supported under PALS") - def set_broadcast(self, dest_path=None): + def set_broadcast(self, dest_path: t.Optional[str] = None) -> None: """Copy the specified executable(s) to remote machines This sets ``--preload-binary`` @@ -177,7 +175,7 @@ def set_broadcast(self, dest_path=None): ) self.run_args["transfer"] = None - def set_walltime(self, walltime): + def set_walltime(self, walltime: str) -> None: """Set the maximum number of seconds that a job will run :param walltime: number like string of seconds that a job will run in secs @@ -185,7 +183,7 @@ def set_walltime(self, walltime): """ logger.warning("set_walltime not supported under PALS") - def format_run_args(self): + def format_run_args(self) -> t.List[str]: """Return a list of MPI-standard formatted run arguments :return: list of MPI-standard arguments for these settings @@ -204,7 +202,7 @@ def format_run_args(self): args += [prefix + opt, str(value)] return args - def format_env_vars(self): + def format_env_vars(self) -> t.List[str]: """Format the environment variables for mpirun :return: list of env vars diff --git a/smartsim/settings/pbsSettings.py b/smartsim/settings/pbsSettings.py index a7f6a1853..fe7fc23f1 100644 --- a/smartsim/settings/pbsSettings.py +++ b/smartsim/settings/pbsSettings.py @@ -24,6 +24,8 @@ # 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. +import typing as t + from .._core.utils import init_default from ..error import SmartSimError from .base import BatchSettings @@ -32,14 +34,14 @@ class QsubBatchSettings(BatchSettings): def __init__( self, - nodes=None, - ncpus=None, - time=None, - queue=None, - account=None, - resources=None, - batch_args=None, - **kwargs, + nodes: t.Optional[int] = None, + ncpus: t.Optional[int] = None, + time: t.Optional[str] = None, + queue: t.Optional[str] = None, + account: t.Optional[str] = None, + resources: t.Optional[t.Dict[str, str]] = None, + batch_args: t.Optional[t.Dict[str, str]] = None, + **kwargs: t.Any, ): """Specify ``qsub`` batch parameters for a job @@ -64,9 +66,9 @@ def __init__( :param batch_args: overrides for PBS batch arguments, defaults to None :type batch_args: dict[str, str], optional """ - self._time = None - self._nodes = None - self._ncpus = ncpus + self._time: t.Optional[str] = None + self._nodes: t.Optional[int] = None + self._ncpus: int = ncpus # time, queue, nodes, and account set in parent class init super().__init__( @@ -81,7 +83,7 @@ def __init__( self.resources = init_default({}, resources, dict) self._hosts = None - def set_nodes(self, num_nodes): + def set_nodes(self, num_nodes: int) -> None: """Set the number of nodes for this batch job If a select argument is provided in ``QsubBatchSettings.resources`` @@ -93,7 +95,7 @@ def set_nodes(self, num_nodes): if num_nodes: self._nodes = int(num_nodes) - def set_hostlist(self, host_list): + def set_hostlist(self, host_list: t.Union[str, t.List[str]]) -> None: """Specify the hostlist for this job :param host_list: hosts to launch on @@ -108,7 +110,7 @@ def set_hostlist(self, host_list): raise TypeError("host_list argument must be a list of strings") self._hosts = host_list - def set_walltime(self, walltime): + def set_walltime(self, walltime: str) -> None: """Set the walltime of the job format = "HH:MM:SS" @@ -123,7 +125,7 @@ def set_walltime(self, walltime): if walltime: self._time = walltime - def set_queue(self, queue): + def set_queue(self, queue: str) -> None: """Set the queue for the batch job :param queue: queue name @@ -132,7 +134,7 @@ def set_queue(self, queue): if queue: self.batch_args["q"] = str(queue) - def set_ncpus(self, num_cpus): + def set_ncpus(self, num_cpus: t.Union[int, str]) -> None: """Set the number of cpus obtained in each node. If a select argument is provided in @@ -144,7 +146,7 @@ def set_ncpus(self, num_cpus): """ self._ncpus = int(num_cpus) - def set_account(self, account): + def set_account(self, account: str) -> None: """Set the account for this batch job :param acct: account id @@ -153,7 +155,7 @@ def set_account(self, account): if account: self.batch_args["A"] = str(account) - def set_resource(self, resource_name, value): + def set_resource(self, resource_name: str, value: str) -> None: """Set a resource value for the Qsub batch If a select statement is provided, the nodes and ncpus @@ -168,7 +170,7 @@ def set_resource(self, resource_name, value): # TODO include option to overwrite place (warning for orchestrator?) self.resources[resource_name] = value - def format_batch_args(self): + def format_batch_args(self) -> t.List[str]: """Get the formatted batch arguments for a preview :return: batch arguments for Qsub @@ -183,7 +185,7 @@ def format_batch_args(self): opts += [" ".join((prefix + opt, str(value)))] return opts - def _create_resource_list(self): + def _create_resource_list(self) -> t.List[str]: res = [] # get select statement from resources or kwargs diff --git a/smartsim/settings/settings.py b/smartsim/settings/settings.py index f0e87db1a..0576aee17 100644 --- a/smartsim/settings/settings.py +++ b/smartsim/settings/settings.py @@ -24,15 +24,24 @@ # 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. +import typing as t + from .._core.utils.helpers import is_valid_cmd from ..error import SmartSimError from ..wlm import detect_launcher from . import * +from ..settings import base def create_batch_settings( - launcher, nodes=None, time="", queue=None, account=None, batch_args=None, **kwargs -): + launcher: str, + nodes: t.Optional[int] = None, + time: str = "", + queue: t.Optional[str] = None, + account: t.Optional[str] = None, + batch_args: t.Optional[t.Dict[str, str]] = None, + **kwargs: t.Any, +) -> base.BatchSettings: """Create a ``BatchSettings`` instance See Experiment.create_batch_settings for details @@ -89,15 +98,15 @@ def create_batch_settings( def create_run_settings( - launcher, - exe, - exe_args=None, - run_command="auto", - run_args=None, - env_vars=None, - container=None, - **kwargs, -): + launcher: str, + exe: str, + exe_args: t.Optional[t.List[str]] = None, + run_command: str = "auto", + run_args: t.Optional[t.Dict[str, str]] = None, + env_vars: t.Optional[t.Dict[str, str]] = None, + container: bool = None, + **kwargs: t.Any, +) -> RunSettings: """Create a ``RunSettings`` instance. See Experiment.create_run_settings docstring for more details @@ -141,7 +150,7 @@ def create_run_settings( if launcher == "auto": launcher = detect_launcher() - def _detect_command(launcher): + def _detect_command(launcher: str): if launcher in by_launcher: for cmd in by_launcher[launcher]: if is_valid_cmd(cmd): diff --git a/smartsim/settings/slurmSettings.py b/smartsim/settings/slurmSettings.py index 5ecb961b7..3b00c89e0 100644 --- a/smartsim/settings/slurmSettings.py +++ b/smartsim/settings/slurmSettings.py @@ -24,9 +24,11 @@ # 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 __future__ import annotations + import datetime import os -from typing import List, Tuple +import typing as t from ..error import SSUnsupportedError from ..log import get_logger @@ -37,8 +39,14 @@ class SrunSettings(RunSettings): def __init__( - self, exe, exe_args=None, run_args=None, env_vars=None, alloc=None, **kwargs - ): + self, + exe: str, + exe_args: t.Optional[t.Union[str, t.List[str]]] = None, + run_args: t.Optional[t.Dict[str, str]] = None, + env_vars: t.Optional[t.Dict[str, str]] = None, + alloc: t.Optional[str] = None, + **kwargs: t.Any, + ) -> None: """Initialize run parameters for a slurm job with ``srun`` ``SrunSettings`` should only be used on Slurm based systems. @@ -70,7 +78,7 @@ def __init__( reserved_run_args = {"chdir", "D"} - def set_nodes(self, nodes): + def set_nodes(self, nodes: int) -> None: """Set the number of nodes Effectively this is setting: ``srun --nodes `` @@ -80,7 +88,7 @@ def set_nodes(self, nodes): """ self.run_args["nodes"] = int(nodes) - def make_mpmd(self, srun_settings): + def make_mpmd(self, srun_settings: SrunSettings) -> None: """Make a mpmd workload by combining two ``srun`` commands This connects the two settings to be executed with a single @@ -99,7 +107,7 @@ def make_mpmd(self, srun_settings): ) self.mpmd.append(srun_settings) - def set_hostlist(self, host_list): + def set_hostlist(self, host_list: t.Union[str, t.List[str]]) -> None: """Specify the hostlist for this job This sets ``--nodelist`` @@ -116,7 +124,7 @@ def set_hostlist(self, host_list): raise TypeError("host_list argument must be list of strings") self.run_args["nodelist"] = ",".join(host_list) - def set_hostlist_from_file(self, file_path): + def set_hostlist_from_file(self, file_path: str) -> str: """Use the contents of a file to set the node list This sets ``--nodefile`` @@ -126,7 +134,7 @@ def set_hostlist_from_file(self, file_path): """ self.run_args["nodefile"] = str(file_path) - def set_excluded_hosts(self, host_list): + def set_excluded_hosts(self, host_list: t.List[str]) -> None: """Specify a list of hosts to exclude for launching this job :param host_list: hosts to exclude @@ -141,7 +149,7 @@ def set_excluded_hosts(self, host_list): raise TypeError("host_list argument must be list of strings") self.run_args["exclude"] = ",".join(host_list) - def set_cpus_per_task(self, cpus_per_task): + def set_cpus_per_task(self, cpus_per_task: int) -> None: """Set the number of cpus to use per task This sets ``--cpus-per-task`` @@ -151,7 +159,7 @@ def set_cpus_per_task(self, cpus_per_task): """ self.run_args["cpus-per-task"] = int(cpus_per_task) - def set_tasks(self, tasks): + def set_tasks(self, tasks: int) -> None: """Set the number of tasks for this job This sets ``--ntasks`` @@ -161,7 +169,7 @@ def set_tasks(self, tasks): """ self.run_args["ntasks"] = int(tasks) - def set_tasks_per_node(self, tasks_per_node): + def set_tasks_per_node(self, tasks_per_node: int) -> None: """Set the number of tasks for this job This sets ``--ntasks-per-node`` @@ -171,7 +179,7 @@ def set_tasks_per_node(self, tasks_per_node): """ self.run_args["ntasks-per-node"] = int(tasks_per_node) - def set_cpu_bindings(self, bindings): + def set_cpu_bindings(self, bindings: t.Union[int, t.List[int]]) -> None: """Bind by setting CPU masks on tasks This sets ``--cpu-bind`` using the ``map_cpu:`` option @@ -185,7 +193,7 @@ def set_cpu_bindings(self, bindings): str(int(num)) for num in bindings ) - def set_memory_per_node(self, memory_per_node): + def set_memory_per_node(self, memory_per_node: int) -> None: """Specify the real memory required per node This sets ``--mem`` in megabytes @@ -195,7 +203,7 @@ def set_memory_per_node(self, memory_per_node): """ self.run_args["mem"] = f"{int(memory_per_node)}M" - def set_verbose_launch(self, verbose): + def set_verbose_launch(self, verbose: bool) -> None: """Set the job to run in verbose mode This sets ``--verbose`` @@ -208,7 +216,7 @@ def set_verbose_launch(self, verbose): else: self.run_args.pop("verbose", None) - def set_quiet_launch(self, quiet): + def set_quiet_launch(self, quiet: bool) -> None: """Set the job to run in quiet mode This sets ``--quiet`` @@ -221,7 +229,7 @@ def set_quiet_launch(self, quiet): else: self.run_args.pop("quiet", None) - def set_broadcast(self, dest_path=None): + def set_broadcast(self, dest_path: t.Optional[str] = None) -> None: """Copy executable file to allocated compute nodes This sets ``--bcast`` @@ -231,7 +239,7 @@ def set_broadcast(self, dest_path=None): """ self.run_args["bcast"] = dest_path - def _fmt_walltime(self, hours, minutes, seconds): + def _fmt_walltime(self, hours: int, minutes: int, seconds: int) -> str: """Convert hours, minutes, and seconds into valid walltime format Converts time to format HH:MM:SS @@ -251,7 +259,7 @@ def _fmt_walltime(self, hours, minutes, seconds): fmt_str = "0" + fmt_str return fmt_str - def set_walltime(self, walltime): + def set_walltime(self, walltime: str) -> str: """Set the walltime of the job format = "HH:MM:SS" @@ -261,7 +269,7 @@ def set_walltime(self, walltime): """ self.run_args["time"] = str(walltime) - def format_run_args(self): + def format_run_args(self) -> t.List[str]: """Return a list of slurm formatted run arguments :return: list of slurm arguments for these settings @@ -281,7 +289,7 @@ def format_run_args(self): opts += ["=".join((prefix + opt, str(value)))] return opts - def check_env_vars(self): + def check_env_vars(self) -> None: """Warn a user trying to set a variable which is set in the environment Given Slurm's env var precedence, trying to export a variable which is already @@ -298,7 +306,7 @@ def check_env_vars(self): msg += "Please consider removing the variable from the environment and re-run the experiment." logger.warning(msg) - def format_env_vars(self): + def format_env_vars(self) -> t.List[str]: """Build bash compatible environment variable string for Slurm :returns: the formatted string of environment variables @@ -307,7 +315,7 @@ def format_env_vars(self): self.check_env_vars() return [f"{k}={v}" for k, v in self.env_vars.items() if "," not in str(v)] - def format_comma_sep_env_vars(self) -> Tuple[str, List[str]]: + def format_comma_sep_env_vars(self) -> t.Tuple[str, t.List[str]]: """Build environment variable string for Slurm Slurm takes exports in comma separated lists @@ -341,7 +349,14 @@ def format_comma_sep_env_vars(self) -> Tuple[str, List[str]]: class SbatchSettings(BatchSettings): - def __init__(self, nodes=None, time="", account=None, batch_args=None, **kwargs): + def __init__( + self, + nodes: t.Optional[int] = None, + time: str = "", + account: t.Optional[str] = None, + batch_args: t.Optional[t.Dict[str, str]] = None, + **kwargs: t.Any, + ) -> None: """Specify run parameters for a Slurm batch job Slurm `sbatch` arguments can be written into ``batch_args`` @@ -371,7 +386,7 @@ def __init__(self, nodes=None, time="", account=None, batch_args=None, **kwargs) **kwargs, ) - def set_walltime(self, walltime): + def set_walltime(self, walltime: str) -> None: """Set the walltime of the job format = "HH:MM:SS" @@ -383,7 +398,7 @@ def set_walltime(self, walltime): if walltime: self.batch_args["time"] = walltime - def set_nodes(self, num_nodes): + def set_nodes(self, num_nodes: int) -> None: """Set the number of nodes for this batch job :param num_nodes: number of nodes @@ -392,7 +407,7 @@ def set_nodes(self, num_nodes): if num_nodes: self.batch_args["nodes"] = int(num_nodes) - def set_account(self, account): + def set_account(self, account: str) -> None: """Set the account for this batch job :param account: account id @@ -401,7 +416,7 @@ def set_account(self, account): if account: self.batch_args["account"] = account - def set_partition(self, partition): + def set_partition(self, partition: str) -> None: """Set the partition for the batch job :param partition: partition name @@ -409,7 +424,7 @@ def set_partition(self, partition): """ self.batch_args["partition"] = str(partition) - def set_queue(self, queue): + def set_queue(self, queue: str) -> None: """alias for set_partition Sets the partition for the slurm batch job @@ -420,7 +435,7 @@ def set_queue(self, queue): if queue: self.set_partition(queue) - def set_cpus_per_task(self, cpus_per_task): + def set_cpus_per_task(self, cpus_per_task: int) -> None: """Set the number of cpus to use per task This sets ``--cpus-per-task`` @@ -430,7 +445,7 @@ def set_cpus_per_task(self, cpus_per_task): """ self.batch_args["cpus-per-task"] = int(cpus_per_task) - def set_hostlist(self, host_list): + def set_hostlist(self, host_list: t.Union[str, t.List[str]]) -> None: """Specify the hostlist for this job :param host_list: hosts to launch on @@ -445,7 +460,7 @@ def set_hostlist(self, host_list): raise TypeError("host_list argument must be list of strings") self.batch_args["nodelist"] = ",".join(host_list) - def format_batch_args(self): + def format_batch_args(self) -> t.List[str]: """Get the formatted batch arguments for a preview :return: batch arguments for Sbatch From d18b8fc909b6458dc202eee2224e19d97c2ba1be Mon Sep 17 00:00:00 2001 From: Chris McBride Date: Thu, 1 Jun 2023 16:32:30 -0400 Subject: [PATCH 14/44] generator.py typehints --- smartsim/_core/generation/generator.py | 28 +++++++++++++++++--------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/smartsim/_core/generation/generator.py b/smartsim/_core/generation/generator.py index d1628cbb8..afbcfc125 100644 --- a/smartsim/_core/generation/generator.py +++ b/smartsim/_core/generation/generator.py @@ -26,6 +26,8 @@ import pathlib import shutil +import typing as t + from distutils import dir_util from os import mkdir, path, symlink @@ -33,6 +35,9 @@ from ...log import get_logger from ..control import Manifest from .modelwriter import ModelWriter +from ...database import Orchestrator +from ...entity import SmartSimEntity, EntityList + logger = get_logger(__name__) logger.propagate = False @@ -44,7 +49,7 @@ class Generator: and writing into configuration files as well. """ - def __init__(self, gen_path, overwrite=False): + def __init__(self, gen_path: str, overwrite: bool = False) -> None: """Initialize a generator object if overwrite is true, replace any existing @@ -61,7 +66,7 @@ def __init__(self, gen_path, overwrite=False): self.gen_path = gen_path self.overwrite = overwrite - def generate_experiment(self, *args): + def generate_experiment(self, *args: t.Any) -> None: """Run ensemble and experiment file structure generation Generate the file structure for a SmartSim experiment. This @@ -87,7 +92,7 @@ def generate_experiment(self, *args): self._gen_entity_list_dir(generator_manifest.ensembles) self._gen_entity_dirs(generator_manifest.models) - def set_tag(self, tag, regex=None): + def set_tag(self, tag: str, regex: t.Optional[str] = None) -> None: """Set the tag used for tagging input files Set a tag or a regular expression for the @@ -122,7 +127,7 @@ def _gen_exp_dir(self): else: logger.info("Working in previously created experiment") - def _gen_orc_dir(self, orchestrator): + def _gen_orc_dir(self, orchestrator: Orchestrator) -> None: """Create the directory that will hold the error, output and configuration files for the orchestrator. @@ -141,7 +146,7 @@ def _gen_orc_dir(self, orchestrator): shutil.rmtree(orc_path, ignore_errors=True) pathlib.Path(orc_path).mkdir(exist_ok=True) - def _gen_entity_list_dir(self, entity_lists): + def _gen_entity_list_dir(self, entity_lists: t.List[EntityList]) -> None: """Generate directories for EntityList instances :param entity_lists: list of EntityList instances @@ -152,7 +157,6 @@ def _gen_entity_list_dir(self, entity_lists): return for elist in entity_lists: - elist_dir = path.join(self.gen_path, elist.name) if path.isdir(elist_dir): if self.overwrite: @@ -164,7 +168,11 @@ def _gen_entity_list_dir(self, entity_lists): self._gen_entity_dirs(elist.entities, entity_list=elist) - def _gen_entity_dirs(self, entities, entity_list=None): + def _gen_entity_dirs( + self, + entities: t.List[SmartSimEntity], + entity_list: t.Optional[EntityList] = None, + ) -> None: """Generate directories for Entity instances :param entities: list of Entity instances @@ -198,7 +206,7 @@ def _gen_entity_dirs(self, entities, entity_list=None): self._link_entity_files(entity) self._write_tagged_entity_files(entity) - def _write_tagged_entity_files(self, entity): + def _write_tagged_entity_files(self, entity: SmartSimEntity) -> None: """Read, configure and write the tagged input files for a Model instance within an ensemble. This function specifically deals with the tagged files attached to @@ -236,7 +244,7 @@ def _build_tagged_files(tagged): ) self._writer.configure_tagged_model_files(to_write, entity.params) - def _copy_entity_files(self, entity): + def _copy_entity_files(self, entity: SmartSimEntity) -> None: """Copy the entity files and directories attached to this entity. :param entity: SmartSimEntity @@ -250,7 +258,7 @@ def _copy_entity_files(self, entity): else: shutil.copyfile(to_copy, dst_path) - def _link_entity_files(self, entity): + def _link_entity_files(self, entity: SmartSimEntity) -> None: """Symlink the entity files attached to this entity. :param entity: SmartSimEntity From 7fb8507b51cee5c791688ab8d780c993d304bc13 Mon Sep 17 00:00:00 2001 From: Chris McBride Date: Thu, 1 Jun 2023 16:48:02 -0400 Subject: [PATCH 15/44] wlm init typehints --- smartsim/wlm/__init__.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/smartsim/wlm/__init__.py b/smartsim/wlm/__init__.py index 03899297e..8d2665015 100644 --- a/smartsim/wlm/__init__.py +++ b/smartsim/wlm/__init__.py @@ -27,13 +27,14 @@ import os from shutil import which from subprocess import run +import typing as t from ..error import SSUnsupportedError from . import pbs as _pbs from . import slurm as _slurm -def detect_launcher(): +def detect_launcher() -> str: """Detect available launcher.""" # Precedence: PBS, Cobalt, LSF, Slurm, local if which("qsub") and which("qstat") and which("qdel"): @@ -71,7 +72,7 @@ def detect_launcher(): return "local" -def get_hosts(launcher=None): +def get_hosts(launcher: t.Optional[str] = None) -> t.List[str]: """Get the name of the hosts used in an allocation. :param launcher: Name of the WLM to use to collect allocation info. If no launcher @@ -90,7 +91,7 @@ def get_hosts(launcher=None): raise SSUnsupportedError(f"SmartSim cannot get hosts for launcher `{launcher}`") -def get_queue(launcher=None): +def get_queue(launcher: t.Optional[str] = None) -> str: """Get the name of the queue used in an allocation. :param launcher: Name of the WLM to use to collect allocation info. If no launcher @@ -109,7 +110,7 @@ def get_queue(launcher=None): raise SSUnsupportedError(f"SmartSim cannot get queue for launcher `{launcher}`") -def get_tasks(launcher=None): +def get_tasks(launcher: t.Optional[str] = None) -> int: """Get the number of tasks in an allocation. :param launcher: Name of the WLM to use to collect allocation info. If no launcher @@ -128,7 +129,7 @@ def get_tasks(launcher=None): raise SSUnsupportedError(f"SmartSim cannot get tasks for launcher `{launcher}`") -def get_tasks_per_node(launcher=None): +def get_tasks_per_node(launcher: t.Optional[str] = None) -> t.Dict[str, int]: """Get a map of nodes in an allocation to the number of tasks on each node. :param launcher: Name of the WLM to use to collect allocation info. If no launcher From 0c96faf49a9c8814d733e3efb7153c11f5999390 Mon Sep 17 00:00:00 2001 From: Chris McBride Date: Thu, 1 Jun 2023 17:11:54 -0400 Subject: [PATCH 16/44] _core.launcher utils typehints --- smartsim/_core/launcher/colocated.py | 31 ++++++----- smartsim/_core/launcher/stepInfo.py | 74 ++++++++++++++++++++------ smartsim/_core/launcher/stepMapping.py | 24 ++++++--- smartsim/_core/launcher/taskManager.py | 72 ++++++++++++++++--------- 4 files changed, 139 insertions(+), 62 deletions(-) diff --git a/smartsim/_core/launcher/colocated.py b/smartsim/_core/launcher/colocated.py index 27f3caf8f..d9f5e6af7 100644 --- a/smartsim/_core/launcher/colocated.py +++ b/smartsim/_core/launcher/colocated.py @@ -25,13 +25,16 @@ # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. import sys +import typing as t -from ...error import SSInternalError, SSUnsupportedError + + +from ...error import SSInternalError from ..config import CONFIG from ..utils.helpers import create_lockfile_name +from ...entity.dbobject import DBModel, DBScript - -def write_colocated_launch_script(file_name, db_log, colocated_settings): +def write_colocated_launch_script(file_name: str, db_log: str, colocated_settings: t.Dict[str, t.Any]) -> None: """Write the colocated launch script This file will be written into the cwd of the step that @@ -75,16 +78,18 @@ def write_colocated_launch_script(file_name, db_log, colocated_settings): def _build_colocated_wrapper_cmd( - db_log, - cpus=1, - rai_args=None, - extra_db_args=None, - port=6780, - ifname=None, - **kwargs, -): + db_log: str, + cpus: int = 1, + rai_args: t.Optional[t.Dict[str, str]] = None, + extra_db_args: t.Optional[t.Dict[str, str]] = None, + port: int = 6780, + ifname: t.Union[str, t.List[str]] = None, + **kwargs: t.Any, +) -> None: """Build the command use to run a colocated DB application + :param db_log: log file for the db + :type db_log: str :param cpus: db cpus, defaults to 1 :type cpus: int, optional :param rai_args: redisai args, defaults to None @@ -182,7 +187,7 @@ def _build_colocated_wrapper_cmd( return " ".join(cmd) -def _build_db_model_cmd(db_models): +def _build_db_model_cmd(db_models: t.List[DBModel]) -> t.List[str]: cmd = [] for db_model in db_models: cmd.append("+db_model") @@ -211,7 +216,7 @@ def _build_db_model_cmd(db_models): return cmd -def _build_db_script_cmd(db_scripts): +def _build_db_script_cmd(db_scripts: t.List[DBScript]) -> t.List[str]: cmd = [] for db_script in db_scripts: cmd.append("+db_script") diff --git a/smartsim/_core/launcher/stepInfo.py b/smartsim/_core/launcher/stepInfo.py index b4d4662b2..c1e6285ba 100644 --- a/smartsim/_core/launcher/stepInfo.py +++ b/smartsim/_core/launcher/stepInfo.py @@ -25,6 +25,7 @@ # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. import psutil +import typing as t from ...status import ( SMARTSIM_STATUS, @@ -38,15 +39,20 @@ class StepInfo: def __init__( - self, status="", launcher_status="", returncode=None, output=None, error=None - ): + self, + status: str = "", + launcher_status: str = "", + returncode: t.Optional[str] = None, + output: t.Optional[str] = None, + error: t.Optional[str] = None, + ) -> None: self.status = status self.launcher_status = launcher_status self.returncode = returncode self.output = output self.error = error - def __str__(self): + def __str__(self) -> str: info_str = f"Status: {self.status}" info_str += f" | Launcher Status {self.launcher_status}" info_str += f" | Returncode {str(self.returncode)}" @@ -54,7 +60,6 @@ def __str__(self): class UnmanagedStepInfo(StepInfo): - # see https://github.com/giampaolo/psutil/blob/master/psutil/_pslinux.py # see https://github.com/giampaolo/psutil/blob/master/psutil/_common.py mapping = { @@ -72,13 +77,19 @@ class UnmanagedStepInfo(StepInfo): psutil.STATUS_ZOMBIE: STATUS_COMPLETED, } - def __init__(self, status="", returncode=None, output=None, error=None): + def __init__( + self, + status: str = "", + returncode: t.Optional[str] = None, + output: t.Optional[str] = None, + error: t.Optional[str] = None, + ) -> None: smartsim_status = self._get_smartsim_status(status) super().__init__( smartsim_status, status, returncode, output=output, error=error ) - def _get_smartsim_status(self, status): + def _get_smartsim_status(self, status: str) -> str: if status in SMARTSIM_STATUS: return SMARTSIM_STATUS[status] if status in self.mapping: @@ -117,13 +128,19 @@ class SlurmStepInfo(StepInfo): # cov-slurm "SUSPENDED": STATUS_PAUSED, } - def __init__(self, status="", returncode=None, output=None, error=None): + def __init__( + self, + status: str = "", + returncode: t.Optional[str] = None, + output: t.Optional[str] = None, + error: t.Optional[str] = None, + ) -> None: smartsim_status = self._get_smartsim_status(status) super().__init__( smartsim_status, status, returncode, output=output, error=error ) - def _get_smartsim_status(self, status): + def _get_smartsim_status(self, status: str) -> str: if status in SMARTSIM_STATUS: return SMARTSIM_STATUS[status] if status in self.mapping: @@ -150,7 +167,13 @@ class PBSStepInfo(StepInfo): # cov-pbs "X": STATUS_COMPLETED, } - def __init__(self, status="", returncode=None, output=None, error=None): + def __init__( + self, + status: str = "", + returncode: t.Optional[str] = None, + output: t.Optional[str] = None, + error: t.Optional[str] = None, + ) -> None: if status == "NOTFOUND": if returncode is not None: smartsim_status = "Completed" if returncode == 0 else "Failed" @@ -164,7 +187,7 @@ def __init__(self, status="", returncode=None, output=None, error=None): smartsim_status, status, returncode, output=output, error=error ) - def _get_smartsim_status(self, status): + def _get_smartsim_status(self, status: str) -> str: if status in SMARTSIM_STATUS: return SMARTSIM_STATUS[status] elif status in self.mapping: @@ -173,7 +196,6 @@ def _get_smartsim_status(self, status): class CobaltStepInfo(StepInfo): # cov-cobalt - mapping = { "running": STATUS_RUNNING, "queued": STATUS_PAUSED, @@ -187,7 +209,13 @@ class CobaltStepInfo(StepInfo): # cov-cobalt "exiting": STATUS_COMPLETED, } - def __init__(self, status="", returncode=None, output=None, error=None): + def __init__( + self, + status: str = "", + returncode: t.Optional[str] = None, + output: t.Optional[str] = None, + error: t.Optional[str] = None, + ) -> None: if status == "NOTFOUND": # returncode not logged by Cobalt # if job has exited the queue then we consider it "completed" @@ -200,7 +228,7 @@ def __init__(self, status="", returncode=None, output=None, error=None): smartsim_status, status, returncode, output=output, error=error ) - def _get_smartsim_status(self, status): + def _get_smartsim_status(self, status: str): if status in SMARTSIM_STATUS: return SMARTSIM_STATUS[status] elif status in self.mapping: @@ -220,7 +248,13 @@ class LSFBatchStepInfo(StepInfo): # cov-lsf "DONE": STATUS_COMPLETED, } - def __init__(self, status="", returncode=None, output=None, error=None): + def __init__( + self, + status: str = "", + returncode: t.Optional[str] = None, + output: t.Optional[str] = None, + error: t.Optional[str] = None, + ) -> None: if status == "NOTFOUND": if returncode is not None: smartsim_status = "Completed" if returncode == 0 else "Failed" @@ -233,7 +267,7 @@ def __init__(self, status="", returncode=None, output=None, error=None): smartsim_status, status, returncode, output=output, error=error ) - def _get_smartsim_status(self, status, returncode): + def _get_smartsim_status(self, status: str, returncode: str) -> str: if status in SMARTSIM_STATUS: return SMARTSIM_STATUS[status] elif status in self.mapping: @@ -251,7 +285,13 @@ class LSFJsrunStepInfo(StepInfo): # cov-lsf "Complete": STATUS_COMPLETED, } - def __init__(self, status="", returncode=None, output=None, error=None): + def __init__( + self, + status: str = "", + returncode: t.Optional[str] = None, + output: t.Optional[str] = None, + error: t.Optional[str] = None, + ) -> None: if status == "NOTFOUND": if returncode is not None: smartsim_status = "Completed" if returncode == 0 else "Failed" @@ -264,7 +304,7 @@ def __init__(self, status="", returncode=None, output=None, error=None): smartsim_status, status, returncode, output=output, error=error ) - def _get_smartsim_status(self, status, returncode): + def _get_smartsim_status(self, status: str, returncode: str) -> str: if status in SMARTSIM_STATUS: return SMARTSIM_STATUS[status] elif status in self.mapping: diff --git a/smartsim/_core/launcher/stepMapping.py b/smartsim/_core/launcher/stepMapping.py index 4e40874d0..fac1ced1f 100644 --- a/smartsim/_core/launcher/stepMapping.py +++ b/smartsim/_core/launcher/stepMapping.py @@ -24,26 +24,34 @@ # 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. +import typing as t + from collections import namedtuple StepMap = namedtuple("StepMap", ["step_id", "task_id", "managed"]) class StepMapping: - def __init__(self): + def __init__(self) -> None: # step_name : wlm_id, pid, wlm_managed? - self.mapping = {} + self.mapping: t.Dict[str, StepMap] = {} - def __getitem__(self, step_name): + def __getitem__(self, step_name: str) -> StepMap: return self.mapping[step_name] - def __setitem__(self, step_name, step_map): + def __setitem__(self, step_name: str, step_map: StepMap) -> None: self.mapping[step_name] = step_map - def add(self, step_name, step_id=None, task_id=None, managed=True): + def add( + self, + step_name: str, + step_id: t.Optional[int] = None, + task_id: t.Optional[int] = None, + managed: bool = True, + ) -> None: self.mapping[step_name] = StepMap(step_id, task_id, managed) - def get_task_id(self, step_id): + def get_task_id(self, step_id: int) -> t.Optional[int]: """Get the task id from the step id""" task_id = None for stepmap in self.mapping.values(): @@ -52,7 +60,9 @@ def get_task_id(self, step_id): break return task_id - def get_ids(self, step_names, managed=True): + def get_ids( + self, step_names: t.List[str], managed: bool = True + ) -> t.Tuple[t.List[str], t.List[int]]: ids = [] names = [] for name in step_names: diff --git a/smartsim/_core/launcher/taskManager.py b/smartsim/_core/launcher/taskManager.py index 00ad78a2c..6639d17d3 100644 --- a/smartsim/_core/launcher/taskManager.py +++ b/smartsim/_core/launcher/taskManager.py @@ -24,12 +24,15 @@ # 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 __future__ import annotations + +import psutil import time +import typing as t + from subprocess import PIPE from threading import RLock, Thread -import psutil - from ...error import LauncherError from ...log import get_logger from ..utils.helpers import check_dev_log_level @@ -57,14 +60,14 @@ class TaskManager: lifecycle of the process. """ - def __init__(self): + def __init__(self) -> None: """Initialize a task manager thread.""" self.actively_monitoring = False self.task_history = dict() - self.tasks = [] + self.tasks: t.List[Task] = [] self._lock = RLock() - def start(self): + def start(self) -> None: """Start the task manager thread The TaskManager is run as a daemon thread meaning @@ -73,7 +76,7 @@ def start(self): monitor = Thread(name="TaskManager", daemon=True, target=self.run) monitor.start() - def run(self): + def run(self) -> None: """Start monitoring Tasks""" global verbose_tm @@ -97,7 +100,14 @@ def run(self): if verbose_tm: logger.debug("Sleeping, no tasks to monitor") - def start_task(self, cmd_list, cwd, env=None, out=PIPE, err=PIPE): + def start_task( + self, + cmd_list: t.List[str], + cwd: str, + env: t.Dict[str, str] = None, + out=PIPE, + err=PIPE, + ) -> None: """Start a task managed by the TaskManager This is an "unmanaged" task, meaning it is NOT managed @@ -129,7 +139,13 @@ def start_task(self, cmd_list, cwd, env=None, out=PIPE, err=PIPE): finally: self._lock.release() - def start_and_wait(self, cmd_list, cwd, env=None, timeout=None): + def start_and_wait( + self, + cmd_list: t.List[str], + cwd: str, + env: t.Dict[str, str] = None, + timeout: int = None, + ) -> t.Tuple[int, str, str]: """Start a task not managed by the TaskManager This method is used by launchers to launch managed tasks @@ -153,11 +169,11 @@ def start_and_wait(self, cmd_list, cwd, env=None, timeout=None): logger.debug("Ran and waited on task") return returncode, out, err - def add_existing(self, task_id): + def add_existing(self, task_id: str) -> None: """Add existing task to be managed by the TaskManager :param task_id: task id of existing task - :type task_id: int + :type task_id: str :raises LauncherError: If task cannot be found """ self._lock.acquire() @@ -171,7 +187,7 @@ def add_existing(self, task_id): finally: self._lock.release() - def remove_task(self, task_id): + def remove_task(self, task_id: str) -> None: """Remove a task from the TaskManager :param task_id: id of the task to remove @@ -195,7 +211,7 @@ def remove_task(self, task_id): finally: self._lock.release() - def get_task_update(self, task_id): + def get_task_update(self, task_id: str) -> str: """Get the update of a task :param task_id: task id @@ -226,7 +242,13 @@ def get_task_update(self, task_id): finally: self._lock.release() - def add_task_history(self, task_id, returncode, out=None, err=None): + def add_task_history( + self, + task_id: str, + returncode: int, + out: t.Optional[str] = None, + err: t.Optional[str] = None, + ): """Add a task to the task history Add a task to record its future returncode, output and error @@ -242,7 +264,7 @@ def add_task_history(self, task_id, returncode, out=None, err=None): """ self.task_history[task_id] = (returncode, out, err) - def __getitem__(self, task_id): + def __getitem__(self, task_id: str) -> Task: self._lock.acquire() try: for task in self.tasks: @@ -252,7 +274,7 @@ def __getitem__(self, task_id): finally: self._lock.release() - def __len__(self): + def __len__(self) -> int: self._lock.acquire() try: return len(self.tasks) @@ -261,7 +283,7 @@ def __len__(self): class Task: - def __init__(self, process): + def __init__(self, process: psutil.Popen) -> None: """Initialize a task :param process: Popen object @@ -270,7 +292,7 @@ def __init__(self, process): self.process = process self.pid = str(self.process.pid) - def check_status(self): + def check_status(self) -> int: """Ping the job and return the returncode if finished :return: returncode if finished otherwise None @@ -282,7 +304,7 @@ def check_status(self): # have to rely on .kill() to stop. return self.returncode - def get_io(self): + def get_io(self) -> t.Tuple[str, str]: """Get the IO from the subprocess :return: output and error from the Popen @@ -298,7 +320,7 @@ def get_io(self): error = error.decode("utf-8") return output, error - def kill(self, timeout=10): + def kill(self, timeout: int = 10) -> None: """Kill the subprocess and all children""" def kill_callback(proc): @@ -315,7 +337,7 @@ def kill_callback(proc): for proc in alive: logger.warning(f"Unable to kill emitted process {proc.pid}") - def terminate(self, timeout=10): + def terminate(self, timeout: int = 10) -> None: """Terminate a this process and all children. :param timeout: time to wait for task death, defaults to 10 @@ -343,11 +365,11 @@ def terminate_callback(proc): logger.debug(f"SIGTERM failed, using SIGKILL") self.process.kill() - def wait(self): + def wait(self) -> None: self.process.wait() @property - def returncode(self): + def returncode(self) -> t.Optional[int]: if self.owned: return self.process.returncode if self.is_alive: @@ -355,15 +377,15 @@ def returncode(self): return 0 @property - def is_alive(self): + def is_alive(self) -> bool: return self.process.is_running() @property - def status(self): + def status(self) -> str: return self.process.status() @property - def owned(self): + def owned(self) -> bool: if isinstance(self.process, psutil.Popen): return True return False From 51a0267ed3bd9a6f1901673a4e7faa31b14dd0c5 Mon Sep 17 00:00:00 2001 From: Chris McBride Date: Thu, 1 Jun 2023 17:29:17 -0400 Subject: [PATCH 17/44] core.utils helpers.py typehints --- smartsim/_core/utils/helpers.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/smartsim/_core/utils/helpers.py b/smartsim/_core/utils/helpers.py index 51dffc844..fb22794d9 100644 --- a/smartsim/_core/utils/helpers.py +++ b/smartsim/_core/utils/helpers.py @@ -29,19 +29,20 @@ """ import os import uuid +import typing as t from functools import lru_cache from pathlib import Path from shutil import which -def create_lockfile_name(): +def create_lockfile_name() -> str: """Generate a unique lock filename using UUID""" lock_suffix = str(uuid.uuid4())[:7] return f"smartsim-{lock_suffix}.lock" @lru_cache(maxsize=20, typed=False) -def check_dev_log_level(): +def check_dev_log_level() -> bool: try: lvl = os.environ["SMARTSIM_LOG_LEVEL"] if lvl == "developer": @@ -51,7 +52,7 @@ def check_dev_log_level(): return False -def fmt_dict(d): +def fmt_dict(d: t.Dict[str, t.Any]) -> str: fmt_str = "" for k, v in d.items(): fmt_str += "\t" + str(k) + " = " + str(v) @@ -59,7 +60,7 @@ def fmt_dict(d): return fmt_str -def get_base_36_repr(positive_int): +def get_base_36_repr(positive_int: int) -> str: """Converts a positive integer to its base 36 representation :param positive_int: the positive integer to convert :type positive_int: int @@ -76,8 +77,7 @@ def get_base_36_repr(positive_int): return "".join(reversed(result)) - -def init_default(default, init_value, expected_type=None): +def init_default(default: t.Any, init_value: t.Any, expected_type: t.Optional[t.Type] = None) -> t.Any: if init_value is None: return default if expected_type is not None and not isinstance(init_value, expected_type): @@ -85,7 +85,7 @@ def init_default(default, init_value, expected_type=None): return init_value -def expand_exe_path(exe): +def expand_exe_path(exe: str) -> str: """Takes an executable and returns the full path to that executable :param exe: executable or file @@ -105,7 +105,7 @@ def expand_exe_path(exe): return os.path.abspath(in_path) -def is_valid_cmd(command): +def is_valid_cmd(command: str) -> bool: try: expand_exe_path(command) return True @@ -126,7 +126,7 @@ def is_valid_cmd(command): ) -def colorize(string, color, bold=False, highlight=False): +def colorize(string: str, color: str, bold: bool = False, highlight: bool = False) -> None: """ Colorize a string. This function was originally written by John Schulman. @@ -143,7 +143,7 @@ def colorize(string, color, bold=False, highlight=False): return "\x1b[%sm%s\x1b[0m" % (";".join(attr), string) -def delete_elements(dictionary, key_list): +def delete_elements(dictionary: t.Dict[str, t.Any], key_list: t.List[str]) -> None: """Delete elements from a dictionary. :param dictionary: the dictionary from which the elements must be deleted. :type dictionary: dict @@ -155,7 +155,7 @@ def delete_elements(dictionary, key_list): del dictionary[key] -def cat_arg_and_value(arg_name, value): +def cat_arg_and_value(arg_name: str, value: str) -> str: """Concatenate a command line argument and its value This function returns ``arg_name`` and ``value @@ -188,7 +188,7 @@ def cat_arg_and_value(arg_name, value): return "=".join(("--" + arg_name, str(value))) -def installed_redisai_backends(backends_path=None): +def installed_redisai_backends(backends_path: t.Optional[str] = None) -> t.List[str]: """Check which ML backends are available for the RedisAI module. The optional argument ``backends_path`` is needed if the backends From f207b2720d2e7a2d32302a9ec5245388a9e24f16 Mon Sep 17 00:00:00 2001 From: Chris McBride Date: Thu, 1 Jun 2023 18:58:00 -0400 Subject: [PATCH 18/44] entitylist typehints --- smartsim/entity/entityList.py | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/smartsim/entity/entityList.py b/smartsim/entity/entityList.py index b892e87d3..1bdcf4da3 100644 --- a/smartsim/entity/entityList.py +++ b/smartsim/entity/entityList.py @@ -24,22 +24,27 @@ # 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. +import typing as t + +if t.TYPE_CHECKING: + import smartsim + class EntityList: """Abstract class for containers for SmartSimEntities""" - def __init__(self, name, path, **kwargs): - self.name = name - self.path = path - self.entities = [] + def __init__(self, name: str, path: str, **kwargs: t.Any) -> None: + self.name: str = name + self.path: str = path + self.entities: t.List["smartsim.entity.SmartSimEntity"] = [] self._initialize_entities(**kwargs) - def _initialize_entities(self, **kwargs): + def _initialize_entities(self, **kwargs: t.Any) -> None: """Initialize the SmartSimEntity objects in the container""" raise NotImplementedError @property - def batch(self): + def batch(self) -> bool: try: if self.batch_settings: return True @@ -49,23 +54,24 @@ def batch(self): return False @property - def type(self): + def type(self) -> str: """Return the name of the class""" return type(self).__name__ - def set_path(self, new_path): + def set_path(self, new_path: str) -> None: self.path = new_path for entity in self.entities: entity.path = new_path - def __getitem__(self, name): + def __getitem__(self, name: str) -> t.Optional["smartsim.entity.SmartSimEntity"]: for entity in self.entities: if entity.name == name: return entity + return None - def __iter__(self): + def __iter__(self) -> t.Iterator["smartsim.entity.SmartSimEntity"]: for entity in self.entities: yield entity - def __len__(self): + def __len__(self) -> int: return len(self.entities) From 1d7d0f29564c9dcb7063414c86c197bd408c5aa9 Mon Sep 17 00:00:00 2001 From: Chris McBride Date: Thu, 1 Jun 2023 19:18:10 -0400 Subject: [PATCH 19/44] experiment.py typehints --- smartsim/experiment.py | 123 ++++++++++++++++++++++++----------------- 1 file changed, 72 insertions(+), 51 deletions(-) diff --git a/smartsim/experiment.py b/smartsim/experiment.py index dda2e8ba5..7bba8f21c 100644 --- a/smartsim/experiment.py +++ b/smartsim/experiment.py @@ -26,6 +26,7 @@ import os.path as osp import time +import typing as t from os import getcwd from tabulate import tabulate @@ -34,10 +35,10 @@ from ._core import Controller, Generator, Manifest from ._core.utils import init_default from .database import Orchestrator -from .entity import Ensemble, Model +from .entity import Ensemble, Model, SmartSimEntity from .error import SmartSimError from .log import get_logger -from .settings import settings +from .settings import settings, base from .wlm import detect_launcher logger = get_logger(__name__) @@ -62,7 +63,9 @@ class Experiment: and utilized throughout runtime. """ - def __init__(self, name, exp_path=None, launcher="local"): + def __init__( + self, name: str, exp_path: t.Optional[str] = None, launcher: str = "local" + ): """Initialize an Experiment instance With the default settings, the Experiment will use the @@ -125,7 +128,13 @@ def __init__(self, name, exp_path=None, launcher="local"): self._control = Controller(launcher=launcher) self._launcher = launcher.lower() - def start(self, *args, block=True, summary=False, kill_on_interrupt=True): + def start( + self, + *args: t.Any, + block: bool = True, + summary: bool = False, + kill_on_interrupt: bool = True, + ) -> None: """Start passed instances using Experiment launcher Any instance ``Model``, ``Ensemble`` or ``Orchestrator`` @@ -189,7 +198,7 @@ def start(self, *args, block=True, summary=False, kill_on_interrupt=True): logger.error(e) raise - def stop(self, *args): + def stop(self, *args: t.Any) -> None: """Stop specific instances launched by this ``Experiment`` Instances of ``Model``, ``Ensemble`` and ``Orchestrator`` @@ -222,7 +231,9 @@ def stop(self, *args): logger.error(e) raise - def generate(self, *args, tag=None, overwrite=False): + def generate( + self, *args: t.Any, tag: t.Optional[str] = None, overwrite: bool = False + ) -> None: """Generate the file structure for an ``Experiment`` ``Experiment.generate`` creates directories for each instance @@ -251,7 +262,9 @@ def generate(self, *args, tag=None, overwrite=False): logger.error(e) raise - def poll(self, interval=10, verbose=True, kill_on_interrupt=True): + def poll( + self, interval: int = 10, verbose: bool = True, kill_on_interrupt: bool = True + ) -> None: """Monitor jobs through logging to stdout. This method should only be used if jobs were launched @@ -292,7 +305,7 @@ def poll(self, interval=10, verbose=True, kill_on_interrupt=True): logger.error(e) raise - def finished(self, entity): + def finished(self, entity: SmartSimEntity) -> bool: """Query if a job has completed. An instance of ``Model`` or ``Ensemble`` can be passed @@ -315,7 +328,7 @@ def finished(self, entity): logger.error(e) raise - def get_status(self, *args): + def get_status(self, *args: t.Any) -> t.List[str]: """Query the status of launched instances Return a smartsim.status string representing @@ -342,7 +355,7 @@ def get_status(self, *args): """ try: manifest = Manifest(*args) - statuses = [] + statuses: t.List[str] = [] for entity in manifest.models: statuses.append(self._control.get_entity_status(entity)) for entity_list in manifest.all_entity_lists: @@ -354,14 +367,14 @@ def get_status(self, *args): def create_ensemble( self, - name, - params=None, - batch_settings=None, - run_settings=None, - replicas=None, - perm_strategy="all_perm", - **kwargs, - ): + name: str, + params: t.Optional[t.Dict[str, t.Any]] = None, + batch_settings: t.Optional[base.BatchSettings] = None, + run_settings: t.Optional[base.RunSettings] = None, + replicas: t.Optional[int] = None, + perm_strategy: str = "all_perm", + **kwargs: t.Any, + ) -> Ensemble: """Create an ``Ensemble`` of ``Model`` instances Ensembles can be launched sequentially or as a batch @@ -428,13 +441,13 @@ def create_ensemble( def create_model( self, - name, - run_settings, - params=None, - path=None, - enable_key_prefixing=False, - batch_settings=None, - ): + name: str, + run_settings: base.RunSettings, + params: t.Optional[t.Dict[str, t.Any]] = None, + path: t.Optional[str] = None, + enable_key_prefixing: bool = False, + batch_settings: t.Optional[base.BatchSettings] = None, + ) -> Model: """Create a general purpose ``Model`` The ``Model`` class is the most general encapsulation of @@ -536,14 +549,14 @@ def create_model( def create_run_settings( self, - exe, - exe_args=None, - run_command="auto", - run_args=None, - env_vars=None, - container=None, - **kwargs, - ): + exe: str, + exe_args: t.Optional[t.List[str]] = None, + run_command: str = "auto", + run_args: t.Optional[t.Dict[str, str]] = None, + env_vars: t.Optional[t.Dict[str, str]] = None, + container: bool = None, + **kwargs: t.Any, + ) -> settings.RunSettings: """Create a ``RunSettings`` instance. run_command="auto" will attempt to automatically @@ -577,6 +590,8 @@ class in SmartSim. If found, the class corresponding :type run_args: dict[str, str], optional :param env_vars: environment variables to pass to the executable :type env_vars: dict[str, str], optional + :param container: if execution environment is containerized + :type container: bool, optional :return: the created ``RunSettings`` :rtype: RunSettings """ @@ -596,8 +611,14 @@ class in SmartSim. If found, the class corresponding raise def create_batch_settings( - self, nodes=1, time="", queue="", account="", batch_args=None, **kwargs - ): + self, + nodes: int = 1, + time: str = "", + queue: str = "", + account: str = "", + batch_args: t.Optional[t.Dict[str, str]] = None, + **kwargs: t.Any, + ) -> base.BatchSettings: """Create a ``BatchSettings`` instance Batch settings parameterize batch workloads. The result of this @@ -651,18 +672,18 @@ def create_batch_settings( def create_database( self, - port=6379, - db_nodes=1, - batch=False, - hosts=None, - run_command="auto", - interface="ipogif0", - account=None, - time=None, - queue=None, - single_cmd=True, - **kwargs, - ): + port: int = 6379, + db_nodes: int = 1, + batch: bool = False, + hosts: t.Optional[t.List[str]] = None, + run_command: str = "auto", + interface: str = "ipogif0", + account: str = None, + time: str = None, + queue: str = None, + single_cmd: bool = True, + **kwargs: t.Any, + ) -> Orchestrator: """Initialize an Orchestrator database The ``Orchestrator`` database is a key-value store based @@ -722,7 +743,7 @@ def create_database( **kwargs, ) - def reconnect_orchestrator(self, checkpoint): + def reconnect_orchestrator(self, checkpoint: str) -> Orchestrator: """Reconnect to a running ``Orchestrator`` This method can be used to connect to a ``Orchestrator`` deployment @@ -742,7 +763,7 @@ def reconnect_orchestrator(self, checkpoint): logger.error(e) raise - def summary(self, format="github"): + def summary(self, format: str = "github") -> str: """Return a summary of the ``Experiment`` The summary will show each instance that has been @@ -789,7 +810,7 @@ def summary(self, format="github"): disable_numparse=True, ) - def _launch_summary(self, manifest): + def _launch_summary(self, manifest: Manifest) -> None: """Experiment pre-launch summary of entities that will be launched :param manifest: Manifest of deployables. @@ -826,5 +847,5 @@ def _launch_summary(self, manifest): for _ in prog_bar: time.sleep(wait / steps) - def __str__(self): + def __str__(self) -> str: return self.name From 468fc73e21f05a52d28563c3c68cc38bb83ff8fe Mon Sep 17 00:00:00 2001 From: Chris McBride Date: Thu, 1 Jun 2023 19:53:09 -0400 Subject: [PATCH 20/44] dbnode typehints --- smartsim/entity/dbnode.py | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/smartsim/entity/dbnode.py b/smartsim/entity/dbnode.py index a3f65a86b..d3280a04c 100644 --- a/smartsim/entity/dbnode.py +++ b/smartsim/entity/dbnode.py @@ -27,10 +27,13 @@ import os import os.path as osp import time +import typing as t from ..error import SmartSimError from ..log import get_logger from .entity import SmartSimEntity +from ..settings.base import RunSettings + logger = get_logger(__name__) @@ -44,14 +47,14 @@ class DBNode(SmartSimEntity): into the smartsimdb.conf. """ - def __init__(self, name, path, run_settings, ports, output_files): + def __init__(self, name: str, path: str, run_settings: RunSettings, ports: t.List[int], output_files: t.List[str]) -> None: """Initialize a database node within an orchestrator.""" self.ports = ports self._host = None super().__init__(name, path, run_settings) self._mpmd = False self._num_shards = None - self._hosts = None + self._hosts: t.List[str] = None if not output_files: raise ValueError("output_files cannot be empty") if not isinstance(output_files, list) or not all( @@ -61,24 +64,24 @@ def __init__(self, name, path, run_settings, ports, output_files): self._output_files = output_files @property - def host(self): + def host(self) -> str: if not self._host: self._host = self._parse_db_host() return self._host @property - def hosts(self): + def hosts(self) -> t.List[str]: if not self._hosts: self._hosts = self._parse_db_hosts() return self._hosts - def set_host(self, host): + def set_host(self, host: str) -> None: self._host = str(host) - def set_hosts(self, hosts): + def set_hosts(self, hosts: t.List[str]) -> None: self._hosts = [str(host) for host in hosts] - def remove_stale_dbnode_files(self): + def remove_stale_dbnode_files(self) -> None: """This function removes the .conf, .err, and .out files that have the same names used by this dbnode that may have been created from a previous experiment execution. @@ -111,7 +114,7 @@ def remove_stale_dbnode_files(self): if osp.exists(file_name): os.remove(file_name) - def _get_cluster_conf_filename(self, port): + def _get_cluster_conf_filename(self, port: int) -> str: """Returns the .conf file name for the given port number :param port: port number @@ -121,7 +124,7 @@ def _get_cluster_conf_filename(self, port): """ return "".join(("nodes-", self.name, "-", str(port), ".conf")) - def _get_cluster_conf_filenames(self, port): # cov-lsf + def _get_cluster_conf_filenames(self, port: int) -> t.List[str]: # cov-lsf """Returns the .conf file name for the given port number This function should bu used if and only if ``_mpmd==True`` @@ -136,7 +139,7 @@ def _get_cluster_conf_filenames(self, port): # cov-lsf for shard_id in range(self._num_shards) ] - def _parse_ips(self, filepath, num_ips=None): + def _parse_ips(self, filepath: str, num_ips: int = None) -> t.List[str]: ips = [] with open(filepath, "r") as f: lines = f.readlines() @@ -149,7 +152,7 @@ def _parse_ips(self, filepath, num_ips=None): return ips - def _parse_db_host(self, filepath=None): + def _parse_db_host(self, filepath: t.Optional[str] = None) -> str: """Parse the database host/IP from the output file If no file is passed as argument, then the first @@ -186,7 +189,7 @@ def _parse_db_host(self, filepath=None): return ip - def _parse_db_hosts(self): + def _parse_db_hosts(self) -> t.List[str]: """Parse the database hosts/IPs from the output files this uses the RedisIP module that is built as a dependency @@ -198,13 +201,13 @@ def _parse_db_hosts(self): :return: ip addresses | hostnames :rtype: list[str] """ - ips = [] + ips: t.List[str] = [] # Find out if all shards' output streams are piped to separate files if len(self._output_files) > 1: for output_file in self._output_files: filepath = osp.join(self.path, output_file) - ip = self._parse_db_host(filepath) + _ = self._parse_db_host(filepath) else: filepath = osp.join(self.path, self._output_files[0]) trials = 10 From a92a2841fc31b18309541217b8a9aed428c7d5ab Mon Sep 17 00:00:00 2001 From: Chris McBride Date: Thu, 1 Jun 2023 21:09:29 -0400 Subject: [PATCH 21/44] dbobject typehints --- smartsim/entity/dbobject.py | 67 +++++++++++++++++++++++-------------- 1 file changed, 41 insertions(+), 26 deletions(-) diff --git a/smartsim/entity/dbobject.py b/smartsim/entity/dbobject.py index 3730ac403..f1a58410a 100644 --- a/smartsim/entity/dbobject.py +++ b/smartsim/entity/dbobject.py @@ -24,10 +24,11 @@ # 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. +import typing as t from pathlib import Path +from .._core.utils import init_default -from .._core.utils.helpers import init_default __all__ = ["DBObject", "DBModel", "DBScript"] @@ -37,7 +38,14 @@ class DBObject: be instantiated. """ - def __init__(self, name, func, file_path, device, devices_per_node): + def __init__( + self, + name: str, + func: t.Optional[str], + file_path: t.Optional[str], + device: str, + devices_per_node: int, + ) -> None: self.name = name self.func = func if file_path: @@ -49,13 +57,15 @@ def __init__(self, name, func, file_path, device, devices_per_node): self.devices_per_node = devices_per_node @property - def is_file(self): + def is_file(self) -> bool: if self.func: return False return True @staticmethod - def _check_tensor_args(inputs, outputs): + def _check_tensor_args( + inputs: t.Union[str, t.List[str]], outputs: t.Union[str, t.List[str]] + ) -> t.Tuple[t.List[str], t.List[str]]: inputs = init_default([], inputs, (list, str)) outputs = init_default([], outputs, (list, str)) if isinstance(inputs, str): @@ -65,7 +75,7 @@ def _check_tensor_args(inputs, outputs): return inputs, outputs @staticmethod - def _check_backend(backend): + def _check_backend(backend: str) -> str: backend = backend.upper() all_backends = ["TF", "TORCH", "ONNX"] if backend in all_backends: @@ -76,20 +86,20 @@ def _check_backend(backend): ) @staticmethod - def _check_filepath(file): + def _check_filepath(file: str) -> Path: file_path = Path(file).resolve() if not file_path.is_file(): raise FileNotFoundError(file_path) return file_path @staticmethod - def _check_device(device): + def _check_device(device: str) -> str: device = device.upper() if not device.startswith("CPU") and not device.startswith("GPU"): raise ValueError("Device argument must start with either CPU or GPU") return device - def _enumerate_devices(self): + def _enumerate_devices(self) -> t.List[str]: """Enumerate devices for a DBObject :param dbobject: DBObject to enumerate @@ -113,7 +123,12 @@ def _enumerate_devices(self): class DBScript(DBObject): def __init__( - self, name, script=None, script_path=None, device="CPU", devices_per_node=1 + self, + name: str, + script: t.Optional[str] = None, + script_path: t.Optional[str] = None, + device: str = "CPU", + devices_per_node: int = 1, ): """TorchScript code represenation @@ -142,10 +157,10 @@ def __init__( raise ValueError("Either script or script_path must be provided") @property - def script(self): + def script(self) -> t.Optional[str]: return self.func - def __str__(self): + def __str__(self) -> str: desc_str = "Name: " + self.name + "\n" if self.func: desc_str += "Func: " + self.func + "\n" @@ -161,19 +176,19 @@ def __str__(self): class DBModel(DBObject): def __init__( self, - name, - backend, - model=None, - model_file=None, - device="CPU", - devices_per_node=1, - batch_size=0, - min_batch_size=0, - min_batch_timeout=0, - tag="", - inputs=None, - outputs=None, - ): + name: str, + backend: str, + model: t.Optional[str] = None, + model_file: t.Optional[str] = None, + device: str = "CPU", + devices_per_node: int = 1, + 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 DB at runtime One of either model (in memory representation) or model_path (file) @@ -215,10 +230,10 @@ def __init__( self.inputs, self.outputs = self._check_tensor_args(inputs, outputs) @property - def model(self): + def model(self) -> t.Union[str, None]: return self.func - def __str__(self): + def __str__(self) -> str: desc_str = "Name: " + self.name + "\n" if self.model: desc_str += "Model stored in memory\n" From 95d24d34a458a3a9ef785143cbf73c205dbadc04 Mon Sep 17 00:00:00 2001 From: Chris McBride Date: Thu, 1 Jun 2023 21:31:19 -0400 Subject: [PATCH 22/44] model typehints --- smartsim/entity/model.py | 127 ++++++++++++++++++++++++--------------- 1 file changed, 78 insertions(+), 49 deletions(-) diff --git a/smartsim/entity/model.py b/smartsim/entity/model.py index c4b5c9a13..486ae72ca 100644 --- a/smartsim/entity/model.py +++ b/smartsim/entity/model.py @@ -24,7 +24,9 @@ # 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 __future__ import annotations +import typing as t import warnings from .._core.utils.helpers import cat_arg_and_value, init_default @@ -32,11 +34,18 @@ from .dbobject import DBModel, DBScript from .entity import SmartSimEntity from .files import EntityFiles +from ..settings.base import BatchSettings, RunSettings class Model(SmartSimEntity): def __init__( - self, name, params, path, run_settings, params_as_args=None, batch_settings=None + self, + name: str, + params: t.Dict[str, str], + path: str, + run_settings: RunSettings, + params_as_args: t.Optional[t.List[str]] = None, + batch_settings: t.Optional[BatchSettings] = None, ): """Initialize a ``Model`` @@ -60,19 +69,19 @@ def __init__( super().__init__(name, path, run_settings) self.params = params self.params_as_args = params_as_args - self.incoming_entities = [] + self.incoming_entities: t.List[SmartSimEntity] = [] self._key_prefixing_enabled = False self.batch_settings = batch_settings - self._db_models = [] - self._db_scripts = [] - self.files = None - + self._db_models: t.List[DBModel] = [] + self._db_scripts: t.List[DBScript] = [] + self.files: t.Optional[EntityFiles] = None + @property - def colocated(self): + def colocated(self) -> bool: """Return True if this Model will run with a colocated Orchestrator""" return bool(self.run_settings.colocated_db_settings) - def register_incoming_entity(self, incoming_entity): + def register_incoming_entity(self, incoming_entity: SmartSimEntity) -> None: """Register future communication between entities. Registers the named data sources that this entity @@ -93,19 +102,24 @@ def register_incoming_entity(self, incoming_entity): self.incoming_entities.append(incoming_entity) - def enable_key_prefixing(self): + def enable_key_prefixing(self) -> None: """If called, the entity will prefix its keys with its own model name""" self._key_prefixing_enabled = True - def disable_key_prefixing(self): + def disable_key_prefixing(self) -> None: """If called, the entity will not prefix its keys with its own model name""" self._key_prefixing_enabled = False - def query_key_prefixing(self): + def query_key_prefixing(self) -> bool: """Inquire as to whether this entity will prefix its keys with its name""" return self._key_prefixing_enabled - def attach_generator_files(self, to_copy=None, to_symlink=None, to_configure=None): + def attach_generator_files( + self, + to_copy: t.Optional[t.List[str]] = None, + to_symlink: t.Optional[t.List[str]] = None, + to_configure: t.Optional[t.List[str]] = None, + ) -> None: """Attach files to an entity for generation Attach files needed for the entity that, upon generation, @@ -136,7 +150,7 @@ def attach_generator_files(self, to_copy=None, to_symlink=None, to_configure=Non to_configure = init_default([], to_configure, (list, str)) self.files = EntityFiles(to_configure, to_copy, to_symlink) - def colocate_db(self, *args, **kwargs): + def colocate_db(self, *args: t.Any, **kwargs: t.Any) -> None: """An alias for ``Model.colocate_db_tcp``""" warnings.warn( ( @@ -149,13 +163,13 @@ def colocate_db(self, *args, **kwargs): def colocate_db_uds( self, - unix_socket="/tmp/redis.socket", - socket_permissions=755, - db_cpus=1, - limit_app_cpus=True, - debug=False, - **kwargs, - ): + unix_socket: str = "/tmp/redis.socket", + socket_permissions: int = 755, + db_cpus: int = 1, + limit_app_cpus: bool = True, + debug: bool = False, + **kwargs: t.Any, + ) -> None: """Colocate an Orchestrator instance with this Model over UDS. This method will initialize settings which add an unsharded @@ -206,13 +220,13 @@ def colocate_db_uds( def colocate_db_tcp( self, - port=6379, - ifname="lo", - db_cpus=1, - limit_app_cpus=True, - debug=False, - **kwargs, - ): + port: int = 6379, + ifname: str = "lo", + db_cpus: int = 1, + limit_app_cpus: bool = True, + debug: bool = False, + **kwargs: t.Any, + ) -> None: """Colocate an Orchestrator instance with this Model over TCP/IP. This method will initialize settings which add an unsharded @@ -258,7 +272,12 @@ def colocate_db_tcp( } self._set_colocated_db_settings(tcp_options, common_options, **kwargs) - def _set_colocated_db_settings(self, connection_options, common_options, **kwargs): + def _set_colocated_db_settings( + self, + connection_options: t.Dict[str, t.Any], + common_options: t.Dict[str, t.Any], + **kwargs: t.Any, + ) -> None: """ Ingest the connection-specific options (UDS/TCP) and set the final settings for the colocated database @@ -296,7 +315,7 @@ def _set_colocated_db_settings(self, connection_options, common_options, **kwarg self.run_settings.colocated_db_settings = colo_db_config - def params_to_args(self): + def params_to_args(self) -> None: """Convert parameters to command line arguments and update run settings.""" for param in self.params_as_args: if not param in self.params: @@ -313,18 +332,18 @@ def params_to_args(self): def add_ml_model( self, - name, - backend, - model=None, - model_path=None, - device="CPU", - devices_per_node=1, - batch_size=0, - min_batch_size=0, - tag="", - inputs=None, - outputs=None, - ): + name: str, + backend: str, + model: t.Optional[str] = None, + model_path: t.Optional[str] = None, + device: str = "CPU", + devices_per_node: int = 1, + batch_size: int = 0, + min_batch_size: 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 DB at runtime Each ML Model added will be loaded into an @@ -371,8 +390,13 @@ def add_ml_model( self._append_db_model(db_model) def add_script( - self, name, script=None, script_path=None, device="CPU", devices_per_node=1 - ): + self, + name: str, + script: t.Optional[str] = None, + script_path: t.Optional[str] = None, + device: str = "CPU", + devices_per_node: int = 1, + ) -> None: """TorchScript to launch with this Model instance Each script added to the model will be loaded into an @@ -408,7 +432,13 @@ def add_script( ) self._append_db_script(db_script) - def add_function(self, name, function=None, device="CPU", devices_per_node=1): + def add_function( + self, + name: str, + function: t.Optional[str] = None, + device: str = "CPU", + devices_per_node: int = 1, + ) -> None: """TorchScript function to launch with this Model instance Each script function to the model will be loaded into a @@ -439,7 +469,7 @@ def add_function(self, name, function=None, device="CPU", devices_per_node=1): ) self._append_db_script(db_script) - def __eq__(self, other): + def __eq__(self, other: Model) -> bool: if self.name == other.name: return True return False @@ -454,7 +484,7 @@ def __str__(self): # pragma: no cover entity_str += "DB Scripts: \n" + str(len(self._db_scripts)) + "\n" return entity_str - def _append_db_model(self, db_model): + def _append_db_model(self, db_model: DBModel) -> None: if not db_model.is_file and self.colocated: err_msg = "ML model can not be set from memory for colocated databases.\n" err_msg += ( @@ -465,7 +495,7 @@ def _append_db_model(self, db_model): self._db_models.append(db_model) - def _append_db_script(self, db_script): + def _append_db_script(self, db_script: DBScript) -> None: if db_script.func and self.colocated: if not isinstance(db_script.func, str): err_msg = ( @@ -476,8 +506,7 @@ def _append_db_script(self, db_script): raise SSUnsupportedError(err_msg) self._db_scripts.append(db_script) - def _check_db_objects_colo(self): - + def _check_db_objects_colo(self) -> None: for db_model in self._db_models: if not db_model.is_file: err_msg = ( From e68d0273641975b794f87c0eeacad2a76d2e73f2 Mon Sep 17 00:00:00 2001 From: Chris McBride Date: Thu, 1 Jun 2023 22:09:25 -0400 Subject: [PATCH 23/44] ensemble type hints --- smartsim/entity/ensemble.py | 111 ++++++++++++++++++++++-------------- tests/test_ensemble.py | 6 +- 2 files changed, 71 insertions(+), 46 deletions(-) diff --git a/smartsim/entity/ensemble.py b/smartsim/entity/ensemble.py index 771fc9534..b01d59412 100644 --- a/smartsim/entity/ensemble.py +++ b/smartsim/entity/ensemble.py @@ -24,6 +24,8 @@ # 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. +import typing as t + from copy import deepcopy from os import getcwd @@ -39,6 +41,7 @@ from .dbobject import DBModel, DBScript from .entityList import EntityList from .model import Model +from .entity import SmartSimEntity from .strategies import create_all_permutations, random_permutations, step_values logger = get_logger(__name__) @@ -51,14 +54,14 @@ class Ensemble(EntityList): def __init__( self, - name, - params, - params_as_args=None, - batch_settings=None, - run_settings=None, - perm_strat="all_perm", - **kwargs, - ): + name: str, + params: t.Dict[str, t.Any], + params_as_args: t.Optional[t.List[str]] = None, + batch_settings: t.Optional[BatchSettings] = None, + run_settings: t.Optional[RunSettings] = None, + perm_strat: str = "all_perm", + **kwargs: t.Any, + ) -> None: """Initialize an Ensemble of Model instances. The kwargs argument can be used to pass custom input @@ -91,15 +94,16 @@ def __init__( self._key_prefixing_enabled = True self.batch_settings = init_default({}, batch_settings, BatchSettings) self.run_settings = init_default({}, run_settings, RunSettings) - self._db_models = [] - self._db_scripts = [] + + self._db_models: t.List[DBModel] = [] + self._db_scripts: t.List[DBScript] = [] super().__init__(name, getcwd(), perm_strat=perm_strat, **kwargs) @property - def models(self): + def models(self) -> t.List[Model]: return self.entities - def _initialize_entities(self, **kwargs): + def _initialize_entities(self, **kwargs: t.Any) -> None: """Initialize all the models within the ensemble based on the parameters passed to the ensemble and the permutation strategy given at init. @@ -116,7 +120,8 @@ def _initialize_entities(self, **kwargs): param_names, params = self._read_model_parameters() # Compute all combinations of model parameters and arguments - all_model_params = strategy(param_names, params, **kwargs) + n_models = kwargs.get("n_models", 0) + all_model_params = strategy(param_names, params, n_models) if not isinstance(all_model_params, list): raise UserStrategyError(strategy) @@ -172,7 +177,7 @@ def _initialize_entities(self, **kwargs): else: logger.info("Empty ensemble created for batch launch") - def add_model(self, model): + def add_model(self, model: Model) -> None: """Add a model to this ensemble :param model: model instance to be added @@ -197,7 +202,7 @@ def add_model(self, model): self.entities.append(model) - def register_incoming_entity(self, incoming_entity): + def register_incoming_entity(self, incoming_entity: SmartSimEntity) -> None: """Register future communication between entities. Registers the named data sources that this entity @@ -212,14 +217,14 @@ def register_incoming_entity(self, incoming_entity): for model in self.entities: model.register_incoming_entity(incoming_entity) - def enable_key_prefixing(self): + def enable_key_prefixing(self) -> None: """If called, all models within this ensemble will prefix their keys with its own model name. """ for model in self.entities: model.enable_key_prefixing() - def query_key_prefixing(self): + def query_key_prefixing(self) -> bool: """Inquire as to whether each model within the ensemble will prefix its keys :returns: True if all models have key prefixing enabled, False otherwise @@ -227,7 +232,12 @@ def query_key_prefixing(self): """ return all([model.query_key_prefixing() for model in self.entities]) - def attach_generator_files(self, to_copy=None, to_symlink=None, to_configure=None): + def attach_generator_files( + self, + to_copy: t.Optional[t.List[str]] = None, + to_symlink: t.Optional[t.List[str]] = None, + to_configure: t.Optional[t.List[str]] = None, + ) -> None: """Attach files to each model within the ensemble for generation Attach files needed for the entity that, upon generation, @@ -256,7 +266,9 @@ def attach_generator_files(self, to_copy=None, to_symlink=None, to_configure=Non to_copy=to_copy, to_symlink=to_symlink, to_configure=to_configure ) - def _set_strategy(self, strategy): + def _set_strategy( + self, strategy: str + ) -> t.Callable[[t.List[str], t.List[t.List[str]], int], t.List[t.Dict[str, str]]]: """Set the permutation strategy for generating models within the ensemble @@ -278,7 +290,7 @@ def _set_strategy(self, strategy): f"Permutation strategy given is not supported: {strategy}" ) - def _read_model_parameters(self): + def _read_model_parameters(self) -> t.Tuple[t.List[str], t.List[t.List[str]]]: """Take in the parameters given to the ensemble and prepare to create models for the ensemble @@ -292,8 +304,8 @@ def _read_model_parameters(self): "Ensemble initialization argument 'params' must be of type dict" ) - param_names = [] - parameters = [] + param_names: t.List[str] = [] + parameters: t.List[t.List[str]] = [] for name, val in self.params.items(): param_names.append(name) @@ -310,18 +322,18 @@ def _read_model_parameters(self): def add_ml_model( self, - name, - backend, - model=None, - model_path=None, - device="CPU", - devices_per_node=1, - batch_size=0, - min_batch_size=0, - tag="", - inputs=None, - outputs=None, - ): + name: str, + backend: str, + model: t.Optional[str] = None, + model_path: t.Optional[str] = None, + device: str = "CPU", + devices_per_node: int = 1, + batch_size: int = 0, + min_batch_size: 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 DB at runtime Each ML Model added will be loaded into an @@ -370,8 +382,13 @@ def add_ml_model( self._extend_entity_db_models(entity, [db_model]) def add_script( - self, name, script=None, script_path=None, device="CPU", devices_per_node=1 - ): + self, + name: str, + script: t.Optional[str] = None, + script_path: t.Optional[str] = None, + device: str = "CPU", + devices_per_node: int = 1, + ) -> None: """TorchScript to launch with every entity belonging to this ensemble Each script added to the model will be loaded into an @@ -409,7 +426,13 @@ def add_script( for entity in self: self._extend_entity_db_scripts(entity, [db_script]) - def add_function(self, name, function=None, device="CPU", devices_per_node=1): + def add_function( + self, + name: str, + function: t.Optional[str] = None, + device: str = "CPU", + devices_per_node: int = 1, + ) -> None: """TorchScript function to launch with every entity belonging to this ensemble Each script function to the model will be loaded into a @@ -426,10 +449,8 @@ def add_function(self, name, function=None, device="CPU", devices_per_node=1): :param name: key to store function under :type name: str - :param script: TorchScript code - :type script: str, optional - :param script_path: path to TorchScript code - :type script_path: str, optional + :param function: TorchScript code + :type function: str, optional :param device: device for script execution, defaults to "CPU" :type device: str, optional :param devices_per_node: number of devices on each host @@ -442,13 +463,17 @@ def add_function(self, name, function=None, device="CPU", devices_per_node=1): for entity in self: self._extend_entity_db_scripts(entity, [db_script]) - def _extend_entity_db_models(self, model, db_models): + def _extend_entity_db_models( + self, model: Model, db_models: t.List[DBModel] + ) -> None: entity_db_models = [db_model.name for db_model in model._db_models] for db_model in db_models: if not db_model.name in entity_db_models: model._append_db_model(db_model) - def _extend_entity_db_scripts(self, model, db_scripts): + def _extend_entity_db_scripts( + self, model: Model, db_scripts: t.List[DBScript] + ) -> None: entity_db_scripts = [db_script.name for db_script in model._db_scripts] for db_script in db_scripts: if not db_script.name in entity_db_scripts: diff --git a/tests/test_ensemble.py b/tests/test_ensemble.py index a3d35f8dd..321816e21 100644 --- a/tests/test_ensemble.py +++ b/tests/test_ensemble.py @@ -45,7 +45,7 @@ # ---- helpers ------------------------------------------------------ -def step_values(param_names, param_values): +def step_values(param_names, param_values, n_models = 0): permutations = [] for p in zip(*param_values): permutations.append(dict(zip(param_names, p))) @@ -54,13 +54,13 @@ def step_values(param_names, param_values): # bad permuation strategy that doesnt return # a list of dictionaries -def bad_strategy(names, values): +def bad_strategy(names, values, n_models = 0): return -1 # test bad perm strat that returns a list but of lists # not dictionaries -def bad_strategy_2(names, values): +def bad_strategy_2(names, values, n_models = 0): return [values] From c0f1953ed215992b7bdc7ca99918aefc9cc5d8dd Mon Sep 17 00:00:00 2001 From: Chris McBride Date: Fri, 2 Jun 2023 12:22:57 -0400 Subject: [PATCH 24/44] orchestrator typehints --- smartsim/database/orchestrator.py | 129 +++++++++++++++++------------- 1 file changed, 74 insertions(+), 55 deletions(-) diff --git a/smartsim/database/orchestrator.py b/smartsim/database/orchestrator.py index 257284904..c1e8898a8 100644 --- a/smartsim/database/orchestrator.py +++ b/smartsim/database/orchestrator.py @@ -24,12 +24,14 @@ # 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. import itertools +import psutil import sys +import typing as t + from os import getcwd from shlex import split as sh_split from warnings import simplefilter, warn -import psutil from smartredis import Client from smartredis.error import RedisReplyError @@ -40,6 +42,7 @@ from ..entity import DBNode, EntityList from ..error import SmartSimError, SSConfigError, SSUnsupportedError from ..log import get_logger +from ..settings.base import BatchSettings, RunSettings from ..settings import ( AprunSettings, BsubBatchSettings, @@ -68,19 +71,19 @@ class Orchestrator(EntityList): def __init__( self, - port=6379, - interface="lo", - launcher="local", - run_command="auto", - db_nodes=1, - batch=False, - hosts=None, - account=None, - time=None, - alloc=None, - single_cmd=False, - **kwargs, - ): + port: int = 6379, + interface: t.Union[str, t.List[str]] = "lo", + launcher: str = "local", + run_command: str = "auto", + db_nodes: int = 1, + batch: bool = False, + hosts: t.Optional[t.List[str]] = None, + account: str = None, + time: str = None, + alloc: str = None, + single_cmd: bool = False, + **kwargs: t.Any, + ) -> None: """Initialize an Orchestrator reference for local launch :param port: TCP/IP port, defaults to 6379 @@ -103,7 +106,7 @@ def __init__( if launcher == "auto": launcher = detect_launcher() - by_launcher = { + by_launcher: t.Dict[str, t.List[t.Union[str, None]]] = { "slurm": ["srun", "mpirun", "mpiexec"], "pbs": ["aprun", "mpirun", "mpiexec"], "cobalt": ["aprun", "mpirun", "mpiexec"], @@ -133,6 +136,7 @@ def _detect_command(launcher): if launcher == "local" and batch: msg = "Local orchestrator can not be launched with batch=True" raise SmartSimError(msg) + if run_command == "aprun" and batch and single_cmd: msg = "aprun can not launch an orchestrator with batch=True and single_cmd=True. " msg += "Automatically switching to single_cmd=False." @@ -142,9 +146,9 @@ def _detect_command(launcher): self.launcher = launcher self.run_command = run_command - self.ports = [] + self.ports: t.List[int] = [] self.path = getcwd() - self._hosts = [] + self._hosts: t.List[str] = [] if isinstance(interface, str): interface = [interface] self._interfaces = interface @@ -181,9 +185,9 @@ def _detect_command(launcher): try: # try to obtain redis binaries needed to launch Redis # will raise SSConfigError if not found - self._redis_exe - self._redis_conf - CONFIG.database_cli + self._redis_exe # pylint: disable=W0104 + self._redis_conf # pylint: disable=W0104 + CONFIG.database_cli # pylint: disable=W0104 except SSConfigError as e: msg = "SmartSim not installed with pre-built extensions (Redis)\n" msg += "Use the `smart` cli tool to install needed extensions\n" @@ -201,12 +205,12 @@ def _detect_command(launcher): raise SmartSimError( "hosts argument is required when launching Orchestrator with mpirun" ) - self._reserved_run_args = {} - self._reserved_batch_args = {} + self._reserved_run_args: t.Dict[t.Type[RunSettings], t.List[str]] = {} + self._reserved_batch_args: t.Dict[t.Type[BatchSettings], t.List[str]] = {} self._fill_reserved() @property - def num_shards(self): + def num_shards(self) -> int: """Return the number of DB shards contained in the orchestrator. This might differ from the number of ``DBNode`` objects, as each ``DBNode`` may start more than one shard (e.g. with MPMD). @@ -217,7 +221,7 @@ def num_shards(self): return self.db_nodes @property - def hosts(self): + def hosts(self) -> t.List[str]: """Return the hostnames of orchestrator instance hosts Note that this will only be populated after the orchestrator @@ -230,13 +234,13 @@ def hosts(self): self._hosts = self._get_db_hosts() return self._hosts - def remove_stale_files(self): + def remove_stale_files(self) -> None: """Can be used to remove database files of a previous launch""" for dbnode in self.entities: dbnode.remove_stale_dbnode_files() - def get_address(self): + def get_address(self) -> t.List[str]: """Return database addresses :return: addresses @@ -250,13 +254,13 @@ def get_address(self): raise SmartSimError("Database is not active") return self._get_address() - def _get_address(self): - addresses = [] + def _get_address(self) -> t.List[str]: + addresses: t.List[str] = [] for ip, port in itertools.product(self._hosts, self.ports): addresses.append(":".join((ip, str(port)))) return addresses - def is_active(self): + def is_active(self) -> bool: """Check if the database is active :return: True if database is active, False otherwise @@ -268,7 +272,7 @@ def is_active(self): return db_is_active(self._hosts, self.ports, self.num_shards) @property - def _rai_module(self): + def _rai_module(self) -> str: """Get the RedisAI module from third-party installations :return: path to module or "" if not found @@ -284,14 +288,14 @@ def _rai_module(self): return " ".join(module) @property - def _redis_exe(self): + def _redis_exe(self) -> str: return CONFIG.database_exe @property - def _redis_conf(self): + def _redis_conf(self) -> str: return CONFIG.database_conf - def set_cpus(self, num_cpus): + def set_cpus(self, num_cpus: int) -> None: """Set the number of CPUs available to each database shard This effectively will determine how many cpus can be used for @@ -311,7 +315,7 @@ def set_cpus(self, num_cpus): for mpmd in db.run_settings.mpmd: mpmd.set_cpus_per_task(num_cpus) - def set_walltime(self, walltime): + def set_walltime(self, walltime: str) -> None: """Set the batch walltime of the orchestrator Note: This will only effect orchestrators launched as a batch @@ -322,9 +326,10 @@ def set_walltime(self, walltime): """ if not self.batch: raise SmartSimError("Not running as batch, cannot set walltime") + self.batch_settings.set_walltime(walltime) - def set_hosts(self, host_list): + def set_hosts(self, host_list: t.List[str]) -> None: """Specify the hosts for the ``Orchestrator`` to launch on :param host_list: list of host (compute node names) @@ -355,7 +360,7 @@ def set_hosts(self, host_list): for i, mpmd_runsettings in enumerate(db.run_settings.mpmd): mpmd_runsettings.set_hostlist(host_list[i + 1]) - def set_batch_arg(self, arg, value): + def set_batch_arg(self, arg: str, value: t.Optional[str] = None) -> None: """Set a batch argument the orchestrator should launch with Some commonly used arguments such as --job-name are used @@ -376,7 +381,7 @@ def set_batch_arg(self, arg, value): else: self.batch_settings.batch_args[arg] = value - def set_run_arg(self, arg, value): + def set_run_arg(self, arg: str, value: t.Optional[str] = None) -> None: """Set a run argument the orchestrator should launch each node with (it will be passed to `jrun`) @@ -400,7 +405,7 @@ def set_run_arg(self, arg, value): for mpmd in db.run_settings.mpmd: mpmd.run_args[arg] = value - def enable_checkpoints(self, frequency): + def enable_checkpoints(self, frequency: int) -> None: """Sets the database's save configuration to save the DB every 'frequency' seconds given that at least one write operation against the DB occurred in that time. @@ -413,7 +418,7 @@ def enable_checkpoints(self, frequency): """ self.set_db_conf("save", str(frequency) + " 1") - def set_max_memory(self, mem): + def set_max_memory(self, mem: int) -> None: """Sets the max memory configuration. By default there is no memory limit. Setting max memory to zero also results in no memory limit. Once a limit is surpassed, keys will be removed according to the eviction strategy. The @@ -433,7 +438,7 @@ def set_max_memory(self, mem): """ self.set_db_conf("maxmemory", mem) - def set_eviction_strategy(self, strategy): + def set_eviction_strategy(self, strategy: str) -> None: """Sets how the database will select what to remove when 'maxmemory' is reached. The default is noeviction. @@ -445,7 +450,7 @@ def set_eviction_strategy(self, strategy): """ self.set_db_conf("maxmemory-policy", strategy) - def set_max_clients(self, clients=50_000): + def set_max_clients(self, clients: int = 50_000) -> None: """Sets the max number of connected clients at the same time. When the number of DB shards contained in the orchestrator is more than two, then every node will use two connections, one @@ -456,7 +461,7 @@ def set_max_clients(self, clients=50_000): """ self.set_db_conf("maxclients", str(clients)) - def set_max_message_size(self, size=1_073_741_824): + def set_max_message_size(self, size: int = 1_073_741_824) -> None: """Sets the database's memory size limit for bulk requests, which are elements representing single strings. The default is 1 gigabyte. Message size must be greater than or equal to 1mb. @@ -469,7 +474,7 @@ def set_max_message_size(self, size=1_073_741_824): """ self.set_db_conf("proto-max-bulk-len", str(size)) - def set_db_conf(self, key, value): + def set_db_conf(self, key: str, value: t.Union[int, str]) -> None: """Set any valid configuration at runtime without the need to restart the database. All configuration parameters that are set are immediately loaded by the database and @@ -506,7 +511,15 @@ def set_db_conf(self, key, value): "The SmartSim Orchestrator must be active in order to set the database's configurations." ) - def _build_batch_settings(self, db_nodes, alloc, batch, account, time, **kwargs): + def _build_batch_settings( + self, + db_nodes: int, + alloc: str, + batch: bool, + account: str, + time: str, + **kwargs: t.Any, + ) -> t.Optional[BatchSettings]: batch_settings = None launcher = kwargs.pop("launcher") @@ -519,7 +532,9 @@ def _build_batch_settings(self, db_nodes, alloc, batch, account, time, **kwargs) return batch_settings - def _build_run_settings(self, exe, exe_args, **kwargs): + def _build_run_settings( + self, exe: str, exe_args: t.List[t.List[str]], **kwargs: t.Any + ) -> RunSettings: run_args = kwargs.pop("run_args", {}) db_nodes = kwargs.get("db_nodes", 1) single_cmd = kwargs.get("single_cmd", True) @@ -556,11 +571,13 @@ def _build_run_settings(self, exe, exe_args, **kwargs): return run_settings - def _build_run_settings_lsf(self, exe, exe_args, **kwargs): + def _build_run_settings_lsf( + self, exe: str, exe_args: t.List[t.List[str]], **kwargs: t.Any + ) -> RunSettings: run_args = kwargs.pop("run_args", {}) cpus_per_shard = kwargs.get("cpus_per_shard", None) gpus_per_shard = kwargs.get("gpus_per_shard", None) - erf_rs = None + erf_rs: t.Optional[RunSettings] = None # We always run the DB on cpus 0:cpus_per_shard-1 # and gpus 0:gpus_per_shard-1 @@ -596,7 +613,7 @@ def _build_run_settings_lsf(self, exe, exe_args, **kwargs): kwargs["run_args"] = run_args return erf_rs - def _initialize_entities(self, **kwargs): + def _initialize_entities(self, **kwargs: t.Any) -> None: self.db_nodes = kwargs.get("db_nodes", 1) single_cmd = kwargs.get("single_cmd", True) @@ -643,11 +660,11 @@ def _initialize_entities(self, **kwargs): self.ports = [port] - def _initialize_entities_mpmd(self, **kwargs): + def _initialize_entities_mpmd(self, **kwargs: t.Any) -> None: port = kwargs.get("port", 6379) cluster = not bool(self.db_nodes < 3) - exe_args_mpmd = [] + exe_args_mpmd: t.List[t.List[str]] = [] for db_id in range(self.db_nodes): db_shard_name = "_".join((self.name, str(db_id))) @@ -680,13 +697,15 @@ def _initialize_entities_mpmd(self, **kwargs): self.ports = [port] @staticmethod - def _get_cluster_args(name, port): + def _get_cluster_args(name: str, port: int) -> t.List[str]: """Create the arguments necessary for cluster creation""" cluster_conf = "".join(("nodes-", name, "-", str(port), ".conf")) db_args = ["--cluster-enabled yes", "--cluster-config-file", cluster_conf] return db_args - def _get_start_script_args(self, name, port, cluster): + def _get_start_script_args( + self, name: str, port: int, cluster: bool + ) -> t.List[str]: start_script_args = [ "-m", "smartsim._core.entrypoints.redis", # entrypoint @@ -703,7 +722,7 @@ def _get_start_script_args(self, name, port, cluster): return start_script_args - def _get_db_hosts(self): + def _get_db_hosts(self) -> t.List[str]: hosts = [] for dbnode in self.entities: if not dbnode._mpmd: @@ -712,7 +731,7 @@ def _get_db_hosts(self): hosts.extend(dbnode.hosts) return hosts - def _check_network_interface(self): + def _check_network_interface(self) -> None: net_if_addrs = psutil.net_if_addrs() for interface in self._interfaces: if interface not in net_if_addrs and interface != "lo": @@ -723,7 +742,7 @@ def _check_network_interface(self): ) logger.warning(f"Found network interfaces are: {available}") - def _fill_reserved(self): + def _fill_reserved(self) -> None: """Fill the reserved batch and run arguments dictionaries""" mpi_like_settings = [ From a9fc067b09522bc5e922b0801194ca063f35f98b Mon Sep 17 00:00:00 2001 From: Chris McBride Date: Fri, 2 Jun 2023 20:55:09 -0400 Subject: [PATCH 25/44] typehint fixes --- smartsim/database/orchestrator.py | 36 ++++++++++++++++++++----------- smartsim/entity/dbnode.py | 4 ++-- smartsim/entity/ensemble.py | 20 ++++++++++------- smartsim/experiment.py | 8 +++---- 4 files changed, 41 insertions(+), 27 deletions(-) diff --git a/smartsim/database/orchestrator.py b/smartsim/database/orchestrator.py index c1e8898a8..b5e9045a2 100644 --- a/smartsim/database/orchestrator.py +++ b/smartsim/database/orchestrator.py @@ -78,9 +78,9 @@ def __init__( db_nodes: int = 1, batch: bool = False, hosts: t.Optional[t.List[str]] = None, - account: str = None, - time: str = None, - alloc: str = None, + account: t.Optional[str] = None, + time: t.Optional[str] = None, + alloc: t.Optional[str] = None, single_cmd: bool = False, **kwargs: t.Any, ) -> None: @@ -114,7 +114,7 @@ def __init__( "local": [None], } - def _detect_command(launcher): + def _detect_command(launcher: str) -> str: if launcher in by_launcher: for cmd in by_launcher[launcher]: if launcher == "local": @@ -237,7 +237,7 @@ def hosts(self) -> t.List[str]: def remove_stale_files(self) -> None: """Can be used to remove database files of a previous launch""" - for dbnode in self.entities: + for dbnode in self.dbnodes: dbnode.remove_stale_dbnode_files() def get_address(self) -> t.List[str]: @@ -309,7 +309,8 @@ def set_cpus(self, num_cpus: int) -> None: self.batch_settings.set_ncpus(num_cpus) if self.launcher == "slurm": self.batch_settings.set_cpus_per_task(num_cpus) - for db in self: + + for db in self.dbnodes: db.run_settings.set_cpus_per_task(num_cpus) if db._mpmd: for mpmd in db.run_settings.mpmd: @@ -347,15 +348,16 @@ def set_hosts(self, host_list: t.List[str]) -> None: self.batch_settings.set_hostlist(host_list) if self.launcher == "lsf": - for db in self: + for db in self.dbnodes: db.set_hosts(host_list) else: - for host, db in zip(host_list, self.entities): + for host, db in zip(host_list, self.dbnodes): if isinstance(db.run_settings, AprunSettings): if not self.batch: db.run_settings.set_hostlist([host]) else: db.run_settings.set_hostlist([host]) + if db._mpmd: for i, mpmd_runsettings in enumerate(db.run_settings.mpmd): mpmd_runsettings.set_hostlist(host_list[i + 1]) @@ -399,7 +401,7 @@ def set_run_arg(self, arg: str, value: t.Optional[str] = None) -> None: f"Can not set run argument {arg}: it is a reserved keyword in Orchestrator" ) else: - for db in self.entities: + for db in self.dbnodes: db.run_settings.run_args[arg] = value if db._mpmd: for mpmd in db.run_settings.mpmd: @@ -721,14 +723,22 @@ def _get_start_script_args( start_script_args += self._get_cluster_args(name, port) return start_script_args + + @property + def dbnodes(self) -> t.List[DBNode]: + """ + Helper property to cast self.entities to DBNode type for type correctness + """ + dbnodes = [node for node in self.entities if isinstance(node, DBNode)] + return dbnodes def _get_db_hosts(self) -> t.List[str]: hosts = [] - for dbnode in self.entities: - if not dbnode._mpmd: - hosts.append(dbnode.host) + for db in self.dbnodes: + if not db._mpmd: + hosts.append(db.host) else: - hosts.extend(dbnode.hosts) + hosts.extend(db.hosts) return hosts def _check_network_interface(self) -> None: diff --git a/smartsim/entity/dbnode.py b/smartsim/entity/dbnode.py index d3280a04c..1077eb405 100644 --- a/smartsim/entity/dbnode.py +++ b/smartsim/entity/dbnode.py @@ -53,7 +53,7 @@ def __init__(self, name: str, path: str, run_settings: RunSettings, ports: t.Lis self._host = None super().__init__(name, path, run_settings) self._mpmd = False - self._num_shards = None + self._num_shards: t.Optional[int] = None self._hosts: t.List[str] = None if not output_files: raise ValueError("output_files cannot be empty") @@ -139,7 +139,7 @@ def _get_cluster_conf_filenames(self, port: int) -> t.List[str]: # cov-lsf for shard_id in range(self._num_shards) ] - def _parse_ips(self, filepath: str, num_ips: int = None) -> t.List[str]: + def _parse_ips(self, filepath: str, num_ips: t.Optional[int] = None) -> t.List[str]: ips = [] with open(filepath, "r") as f: lines = f.readlines() diff --git a/smartsim/entity/ensemble.py b/smartsim/entity/ensemble.py index b01d59412..532688347 100644 --- a/smartsim/entity/ensemble.py +++ b/smartsim/entity/ensemble.py @@ -74,7 +74,7 @@ def __init__( :param params_as_args: list of params which should be used as command line arguments to the ``Model`` member executables and not written to generator files - :type arg_params: list[str] + :type params_as_args: list[str] :param batch_settings: describes settings for ``Ensemble`` as batch workload :type batch_settings: BatchSettings, optional :param run_settings: describes how each ``Model`` should be executed @@ -101,7 +101,11 @@ def __init__( @property def models(self) -> t.List[Model]: - return self.entities + """ + Helper property to cast self.entities to Model type for type correctness + """ + model_entities = [node for node in self.entities if isinstance(node, Model)] + return model_entities def _initialize_entities(self, **kwargs: t.Any) -> None: """Initialize all the models within the ensemble based @@ -214,14 +218,14 @@ def register_incoming_entity(self, incoming_entity: SmartSimEntity) -> None: :param incoming_entity: The entity that data will be received from :type incoming_entity: SmartSimEntity """ - for model in self.entities: + for model in self.models: model.register_incoming_entity(incoming_entity) def enable_key_prefixing(self) -> None: """If called, all models within this ensemble will prefix their keys with its own model name. """ - for model in self.entities: + for model in self.models: model.enable_key_prefixing() def query_key_prefixing(self) -> bool: @@ -230,7 +234,7 @@ def query_key_prefixing(self) -> bool: :returns: True if all models have key prefixing enabled, False otherwise :rtype: bool """ - return all([model.query_key_prefixing() for model in self.entities]) + return all([model.query_key_prefixing() for model in self.models]) def attach_generator_files( self, @@ -261,7 +265,7 @@ def attach_generator_files( :param to_configure: input files with tagged parameters, defaults to [] :type to_configure: list, optional """ - for model in self.entities: + for model in self.models: model.attach_generator_files( to_copy=to_copy, to_symlink=to_symlink, to_configure=to_configure ) @@ -378,7 +382,7 @@ def add_ml_model( outputs=outputs, ) self._db_models.append(db_model) - for entity in self: + for entity in self.models: self._extend_entity_db_models(entity, [db_model]) def add_script( @@ -460,7 +464,7 @@ def add_function( name=name, script=function, device=device, devices_per_node=devices_per_node ) self._db_scripts.append(db_script) - for entity in self: + for entity in self.models: self._extend_entity_db_scripts(entity, [db_script]) def _extend_entity_db_models( diff --git a/smartsim/experiment.py b/smartsim/experiment.py index 7bba8f21c..a6cabe867 100644 --- a/smartsim/experiment.py +++ b/smartsim/experiment.py @@ -554,7 +554,7 @@ def create_run_settings( run_command: str = "auto", run_args: t.Optional[t.Dict[str, str]] = None, env_vars: t.Optional[t.Dict[str, str]] = None, - container: bool = None, + container: t.Optional[bool] = None, **kwargs: t.Any, ) -> settings.RunSettings: """Create a ``RunSettings`` instance. @@ -678,9 +678,9 @@ def create_database( hosts: t.Optional[t.List[str]] = None, run_command: str = "auto", interface: str = "ipogif0", - account: str = None, - time: str = None, - queue: str = None, + account: t.Optional[str] = None, + time: t.Optional[str] = None, + queue: t.Optional[str] = None, single_cmd: bool = True, **kwargs: t.Any, ) -> Orchestrator: From 48c6a8aaabcc399e2df766f0e0ad207cd931bfb9 Mon Sep 17 00:00:00 2001 From: Chris McBride Date: Fri, 2 Jun 2023 22:39:18 -0400 Subject: [PATCH 26/44] more typehint fixes --- smartsim/_core/utils/helpers.py | 4 +-- smartsim/database/orchestrator.py | 45 ++++++++++++++++++++----------- smartsim/entity/dbnode.py | 7 ++--- smartsim/entity/dbobject.py | 9 +++---- smartsim/entity/ensemble.py | 4 +-- smartsim/entity/entityList.py | 3 +++ smartsim/entity/files.py | 6 ++--- smartsim/entity/model.py | 32 ++++++++++++---------- smartsim/experiment.py | 12 +++++++-- smartsim/settings/base.py | 2 +- 10 files changed, 76 insertions(+), 48 deletions(-) diff --git a/smartsim/_core/utils/helpers.py b/smartsim/_core/utils/helpers.py index fb22794d9..49d0a7d9e 100644 --- a/smartsim/_core/utils/helpers.py +++ b/smartsim/_core/utils/helpers.py @@ -77,7 +77,7 @@ def get_base_36_repr(positive_int: int) -> str: return "".join(reversed(result)) -def init_default(default: t.Any, init_value: t.Any, expected_type: t.Optional[t.Type] = None) -> t.Any: +def init_default(default: t.Any, init_value: t.Any, expected_type: t.Optional[t.Union[t.Type, t.Tuple]] = None) -> t.Any: if init_value is None: return default if expected_type is not None and not isinstance(init_value, expected_type): @@ -105,7 +105,7 @@ def expand_exe_path(exe: str) -> str: return os.path.abspath(in_path) -def is_valid_cmd(command: str) -> bool: +def is_valid_cmd(command: t.Union[str, None]) -> bool: try: expand_exe_path(command) return True diff --git a/smartsim/database/orchestrator.py b/smartsim/database/orchestrator.py index b5e9045a2..1da153427 100644 --- a/smartsim/database/orchestrator.py +++ b/smartsim/database/orchestrator.py @@ -114,7 +114,7 @@ def __init__( "local": [None], } - def _detect_command(launcher: str) -> str: + def _detect_command(launcher: str) -> t.Optional[str]: if launcher in by_launcher: for cmd in by_launcher[launcher]: if launcher == "local": @@ -237,8 +237,8 @@ def hosts(self) -> t.List[str]: def remove_stale_files(self) -> None: """Can be used to remove database files of a previous launch""" - for dbnode in self.dbnodes: - dbnode.remove_stale_dbnode_files() + for db in self.dbnodes: + db.remove_stale_dbnode_files() def get_address(self) -> t.List[str]: """Return database addresses @@ -306,15 +306,19 @@ def set_cpus(self, num_cpus: int) -> None: """ if self.batch: if self.launcher == "pbs" or self.launcher == "cobalt": - self.batch_settings.set_ncpus(num_cpus) + if hasattr(self, 'batch_settings') and self.batch_settings: + if hasattr(self.batch_settings, 'set_ncpus'): + self.batch_settings.set_ncpus(num_cpus) if self.launcher == "slurm": - self.batch_settings.set_cpus_per_task(num_cpus) + if hasattr(self, 'batch_settings') and self.batch_settings: + if hasattr(self.batch_settings, 'set_cpus_per_task'): + self.batch_settings.set_cpus_per_task(num_cpus) for db in self.dbnodes: db.run_settings.set_cpus_per_task(num_cpus) - if db._mpmd: - for mpmd in db.run_settings.mpmd: - mpmd.set_cpus_per_task(num_cpus) + if db._mpmd and hasattr(db.run_settings, 'mpmd'): + for mpmd in db.run_settings.mpmd: + mpmd.set_cpus_per_task(num_cpus) def set_walltime(self, walltime: str) -> None: """Set the batch walltime of the orchestrator @@ -328,7 +332,8 @@ def set_walltime(self, walltime: str) -> None: if not self.batch: raise SmartSimError("Not running as batch, cannot set walltime") - self.batch_settings.set_walltime(walltime) + if hasattr(self, 'batch_settings') and self.batch_settings: + self.batch_settings.set_walltime(walltime) def set_hosts(self, host_list: t.List[str]) -> None: """Specify the hosts for the ``Orchestrator`` to launch on @@ -345,7 +350,8 @@ def set_hosts(self, host_list: t.List[str]) -> None: raise TypeError("host_list argument must be list of strings") # TODO check length if self.batch: - self.batch_settings.set_hostlist(host_list) + if hasattr(self, 'batch_settings') and self.batch_settings: + self.batch_settings.set_hostlist(host_list) if self.launcher == "lsf": for db in self.dbnodes: @@ -358,7 +364,7 @@ def set_hosts(self, host_list: t.List[str]) -> None: else: db.run_settings.set_hostlist([host]) - if db._mpmd: + if db._mpmd and hasattr(db.run_settings, 'mpmd'): for i, mpmd_runsettings in enumerate(db.run_settings.mpmd): mpmd_runsettings.set_hostlist(host_list[i + 1]) @@ -373,15 +379,17 @@ def set_batch_arg(self, arg: str, value: t.Optional[str] = None) -> None: :param value: batch param - set to None if no param value :type value: str | None :raises SmartSimError: if orchestrator not launching as batch - """ - if not self.batch: + """ + if not hasattr(self, 'batch_settings') or not self.batch_settings: raise SmartSimError("Not running as batch, cannot set batch_arg") + if arg in self._reserved_batch_args[type(self.batch_settings)]: logger.warning( f"Can not set batch argument {arg}: it is a reserved keyword in Orchestrator" ) else: - self.batch_settings.batch_args[arg] = value + if hasattr(self, 'batch_settings') and self.batch_settings: + self.batch_settings.batch_args[arg] = value def set_run_arg(self, arg: str, value: t.Optional[str] = None) -> None: """Set a run argument the orchestrator should launch @@ -403,7 +411,7 @@ def set_run_arg(self, arg: str, value: t.Optional[str] = None) -> None: else: for db in self.dbnodes: db.run_settings.run_args[arg] = value - if db._mpmd: + if db._mpmd and hasattr(db.run_settings, 'mpmd'): for mpmd in db.run_settings.mpmd: mpmd.run_args[arg] = value @@ -575,7 +583,7 @@ def _build_run_settings( def _build_run_settings_lsf( self, exe: str, exe_args: t.List[t.List[str]], **kwargs: t.Any - ) -> RunSettings: + ) -> t.Optional[RunSettings]: run_args = kwargs.pop("run_args", {}) cpus_per_shard = kwargs.get("cpus_per_shard", None) gpus_per_shard = kwargs.get("gpus_per_shard", None) @@ -613,6 +621,7 @@ def _build_run_settings_lsf( erf_rs = run_settings kwargs["run_args"] = run_args + return erf_rs def _initialize_entities(self, **kwargs: t.Any) -> None: @@ -691,6 +700,10 @@ def _initialize_entities_mpmd(self, **kwargs: t.Any) -> None: sys.executable, exe_args_mpmd, **kwargs ) output_files = [self.name + ".out"] + + if not run_settings: + raise ValueError(f"Could not build run settings for {self.launcher}") + node = DBNode(self.name, self.path, run_settings, [port], output_files) node._mpmd = True node._num_shards = self.db_nodes diff --git a/smartsim/entity/dbnode.py b/smartsim/entity/dbnode.py index 1077eb405..9367adffd 100644 --- a/smartsim/entity/dbnode.py +++ b/smartsim/entity/dbnode.py @@ -50,11 +50,12 @@ class DBNode(SmartSimEntity): def __init__(self, name: str, path: str, run_settings: RunSettings, ports: t.List[int], output_files: t.List[str]) -> None: """Initialize a database node within an orchestrator.""" self.ports = ports - self._host = None + self._host: t.Optional[str] = None super().__init__(name, path, run_settings) self._mpmd = False - self._num_shards: t.Optional[int] = None - self._hosts: t.List[str] = None + self._num_shards: int = 0 + self._hosts: t.Optional[t.List[str]] = None + if not output_files: raise ValueError("output_files cannot be empty") if not isinstance(output_files, list) or not all( diff --git a/smartsim/entity/dbobject.py b/smartsim/entity/dbobject.py index f1a58410a..7eabc344b 100644 --- a/smartsim/entity/dbobject.py +++ b/smartsim/entity/dbobject.py @@ -48,11 +48,9 @@ def __init__( ) -> None: self.name = name self.func = func + self.file: t.Optional[Path] = None # Need to have this explicitly to check on it if file_path: self.file = self._check_filepath(file_path) - else: - # Need to have this explicitly to check on it - self.file = None self.device = self._check_device(device) self.devices_per_node = devices_per_node @@ -64,15 +62,16 @@ def is_file(self) -> bool: @staticmethod def _check_tensor_args( - inputs: t.Union[str, t.List[str]], outputs: t.Union[str, t.List[str]] + inputs: t.Union[str, t.Optional[t.List[str]]], outputs: t.Union[str, t.Optional[t.List[str]]] ) -> t.Tuple[t.List[str], t.List[str]]: inputs = init_default([], inputs, (list, str)) outputs = init_default([], outputs, (list, str)) + if isinstance(inputs, str): inputs = [inputs] if isinstance(outputs, str): outputs = [outputs] - return inputs, outputs + return inputs or [], outputs or [] @staticmethod def _check_backend(backend: str) -> str: diff --git a/smartsim/entity/ensemble.py b/smartsim/entity/ensemble.py index 532688347..74d8b5730 100644 --- a/smartsim/entity/ensemble.py +++ b/smartsim/entity/ensemble.py @@ -309,7 +309,7 @@ def _read_model_parameters(self) -> t.Tuple[t.List[str], t.List[t.List[str]]]: ) param_names: t.List[str] = [] - parameters: t.List[t.List[str]] = [] + parameters: t.List[t.List[t.Union[int, str]]] = [] for name, val in self.params.items(): param_names.append(name) @@ -427,7 +427,7 @@ def add_script( devices_per_node=devices_per_node, ) self._db_scripts.append(db_script) - for entity in self: + for entity in self.models: self._extend_entity_db_scripts(entity, [db_script]) def add_function( diff --git a/smartsim/entity/entityList.py b/smartsim/entity/entityList.py index 1bdcf4da3..cb0183b96 100644 --- a/smartsim/entity/entityList.py +++ b/smartsim/entity/entityList.py @@ -46,6 +46,9 @@ def _initialize_entities(self, **kwargs: t.Any) -> None: @property def batch(self) -> bool: try: + if not hasattr(self, "batch_settings"): + return False + if self.batch_settings: return True return False diff --git a/smartsim/entity/files.py b/smartsim/entity/files.py index cb36e7829..9fc0798b9 100644 --- a/smartsim/entity/files.py +++ b/smartsim/entity/files.py @@ -49,7 +49,7 @@ class EntityFiles: """ def __init__( - self, tagged: t.List[str], copy: t.List[str], symlink: t.List[str] + self, tagged: t.Optional[t.List[str]] = None, copy: t.Optional[t.List[str]] = None, symlink: t.Optional[t.List[str]] = None ) -> None: """Initialize an EntityFiles instance @@ -90,7 +90,7 @@ def _check_files(self) -> None: for i in range(len(self.link)): self.link[i] = self._check_path(self.link[i]) - def _type_check_files(self, file_list: t.List[str], file_type: str) -> t.List[str]: + def _type_check_files(self, file_list: t.Union[t.List[str], None], file_type: str) -> t.List[str]: """Check the type of the files provided by the user. :param file_list: either tagged, copy, or symlink files @@ -112,7 +112,7 @@ def _type_check_files(self, file_list: t.List[str], file_type: str) -> t.List[st else: if not all([isinstance(f, str) for f in file_list]): raise TypeError(f"Not all {file_type} files were of type str") - return file_list + return file_list or [] @staticmethod def _check_path(file_path: str) -> str: diff --git a/smartsim/entity/model.py b/smartsim/entity/model.py index 486ae72ca..3729aa720 100644 --- a/smartsim/entity/model.py +++ b/smartsim/entity/model.py @@ -317,18 +317,19 @@ def _set_colocated_db_settings( def params_to_args(self) -> None: """Convert parameters to command line arguments and update run settings.""" - for param in self.params_as_args: - if not param in self.params: - raise ValueError( - f"Tried to convert {param} to command line argument " - + f"for Model {self.name}, but its value was not found in model params" - ) - if self.run_settings is None: - raise ValueError( - f"Tried to configure command line parameter for Model {self.name}, " - + "but no RunSettings are set." - ) - self.run_settings.add_exe_args(cat_arg_and_value(param, self.params[param])) + 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 " + + f"for Model {self.name}, but its value was not found in model params" + ) + if self.run_settings is None: + raise ValueError( + f"Tried to configure command line parameter for Model {self.name}, " + + "but no RunSettings are set." + ) + self.run_settings.add_exe_args(cat_arg_and_value(param, self.params[param])) def add_ml_model( self, @@ -469,12 +470,15 @@ def add_function( ) self._append_db_script(db_script) - def __eq__(self, other: Model) -> bool: + def __eq__(self, other: object) -> bool: + if not isinstance(other, Model): + return False + if self.name == other.name: return True return False - def __str__(self): # pragma: no cover + 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" diff --git a/smartsim/experiment.py b/smartsim/experiment.py index a6cabe867..4a4e48994 100644 --- a/smartsim/experiment.py +++ b/smartsim/experiment.py @@ -427,7 +427,7 @@ def create_ensemble( try: new_ensemble = Ensemble( name, - params, + params or {}, batch_settings=batch_settings, run_settings=run_settings, perm_strat=perm_strategy, @@ -535,7 +535,13 @@ def create_model( :rtype: Model """ path = init_default(getcwd(), path, str) - params = init_default({}, params, dict) + + # mcb + if path is None: + path = getcwd() + if params is None: + params = {} + try: new_model = Model( name, params, path, run_settings, batch_settings=batch_settings @@ -595,6 +601,8 @@ class in SmartSim. If found, the class corresponding :return: the created ``RunSettings`` :rtype: RunSettings """ + container = container or False + try: return settings.create_run_settings( self._launcher, diff --git a/smartsim/settings/base.py b/smartsim/settings/base.py index 90e086912..8fcd82fe2 100644 --- a/smartsim/settings/base.py +++ b/smartsim/settings/base.py @@ -89,7 +89,7 @@ def __init__( self.container = container self._run_command = run_command self.in_batch = False - self.colocated_db_settings = None + self.colocated_db_settings: t.Optional[t.Dict[str, str]] = None # To be overwritten by subclasses. Set of reserved args a user cannot change reserved_run_args = set() # type: set[str] From eb9a3f96f83a2066e90a6b6896354069cf118f01 Mon Sep 17 00:00:00 2001 From: Chris McBride Date: Fri, 2 Jun 2023 23:30:35 -0400 Subject: [PATCH 27/44] ran black formatter, remove unused import --- smartsim/_core/control/controller.py | 25 ++++++++------------ smartsim/_core/control/job.py | 1 - smartsim/_core/launcher/util/launcherUtil.py | 4 +++- smartsim/entity/strategies.py | 16 +++++++++---- 4 files changed, 24 insertions(+), 22 deletions(-) diff --git a/smartsim/_core/control/controller.py b/smartsim/_core/control/controller.py index b339c4926..320bcc8c4 100644 --- a/smartsim/_core/control/controller.py +++ b/smartsim/_core/control/controller.py @@ -60,7 +60,7 @@ class Controller: underlying workload manager or run framework. """ - def __init__(self, launcher: str="local") -> None: + def __init__(self, launcher: str = "local") -> None: """Initialize a Controller :param launcher: the type of launcher being used @@ -69,10 +69,9 @@ def __init__(self, launcher: str="local") -> None: self._jobs = JobManager(JM_LOCK) self.init_launcher(launcher) - def start(self, - manifest: Manifest, - block: bool = True, - kill_on_interrupt: bool = True) -> None: + def start( + self, manifest: Manifest, block: bool = True, kill_on_interrupt: bool = True + ) -> None: """Start the passed SmartSim entities This function should not be called directly, but rather @@ -106,10 +105,9 @@ def orchestrator_active(self) -> bool: finally: JM_LOCK.release() - def poll(self, - interval: int, - verbose: bool, - kill_on_interrupt: bool = True) -> None: + def poll( + self, interval: int, verbose: bool, kill_on_interrupt: bool = True + ) -> None: """Poll running jobs and receive logging output of job status :param interval: number of seconds to wait before polling again @@ -552,7 +550,6 @@ def _orchestrator_launch_wait(self, orchestrator: Orchestrator) -> None: else: logger.debug("Waiting for orchestrator instances to spin up...") except KeyboardInterrupt: - logger.info("Orchestrator launch cancelled - requesting to stop") self.stop_entity_list(orchestrator) @@ -659,11 +656,9 @@ def _set_dbobjects(self, manifest: Manifest) -> None: class _AnonymousBatchJob(EntityList): - def __init__(self, - name: str, - path: str, - batch_settings: BatchSettings, - **kwargs: t.Any) -> None: + def __init__( + self, name: str, path: str, batch_settings: BatchSettings, **kwargs: t.Any + ) -> None: super().__init__(name, path) self.batch_settings = batch_settings diff --git a/smartsim/_core/control/job.py b/smartsim/_core/control/job.py index c47dfefa0..2bb08e310 100644 --- a/smartsim/_core/control/job.py +++ b/smartsim/_core/control/job.py @@ -25,7 +25,6 @@ # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. import time -import typing as t from ...status import STATUS_NEW from ...entity import SmartSimEntity diff --git a/smartsim/_core/launcher/util/launcherUtil.py b/smartsim/_core/launcher/util/launcherUtil.py index 7b1961daa..c455d6d9b 100644 --- a/smartsim/_core/launcher/util/launcherUtil.py +++ b/smartsim/_core/launcher/util/launcherUtil.py @@ -32,7 +32,9 @@ class ComputeNode: # cov-slurm about a physical compute node """ - def __init__(self, node_name: t.Optional[str] = None, node_ppn: t.Optional[int] =None) -> None: + def __init__( + self, node_name: t.Optional[str] = None, node_ppn: t.Optional[int] = None + ) -> None: """Initialize a ComputeNode :param node_name: the name of the node diff --git a/smartsim/entity/strategies.py b/smartsim/entity/strategies.py index a9c8ca01b..7923348e5 100644 --- a/smartsim/entity/strategies.py +++ b/smartsim/entity/strategies.py @@ -33,7 +33,9 @@ # create permutations of all parameters # single model if parameters only have one value -def create_all_permutations(param_names: t.List[str], param_values: t.List[t.List[str]], _n_models: int = 0) -> t.List[t.Dict[str, str]]: +def create_all_permutations( + param_names: t.List[str], param_values: t.List[t.List[str]], _n_models: int = 0 +) -> t.List[t.Dict[str, str]]: perms = list(product(*param_values)) all_permutations = [] for p in perms: @@ -42,18 +44,22 @@ def create_all_permutations(param_names: t.List[str], param_values: t.List[t.Lis return all_permutations -def step_values(param_names: t.List[str], param_values: t.List[t.List[str]], _n_models: int = 0) -> t.List[t.Dict[str, str]]: +def step_values( + param_names: t.List[str], param_values: t.List[t.List[str]], _n_models: int = 0 +) -> t.List[t.Dict[str, str]]: permutations = [] for p in zip(*param_values): permutations.append(dict(zip(param_names, p))) return permutations -def random_permutations(param_names: t.List[str], param_values: t.List[t.List[str]], n_models: int = 0) -> t.List[t.Dict[str, str]]: +def random_permutations( + param_names: t.List[str], param_values: t.List[t.List[str]], n_models: int = 0 +) -> t.List[t.Dict[str, str]]: # first, check if we've requested more values than possible. permutations = create_all_permutations(param_names, param_values) - + if n_models and n_models < len(permutations): permutations = random.sample(permutations, n_models) - + return permutations From 822c7baad06dcb3679014b62e27d29067b5ec2f2 Mon Sep 17 00:00:00 2001 From: Chris McBride Date: Mon, 5 Jun 2023 11:33:01 -0400 Subject: [PATCH 28/44] launcher cmd typehint fix --- smartsim/database/orchestrator.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/smartsim/database/orchestrator.py b/smartsim/database/orchestrator.py index 1da153427..d7f5a0731 100644 --- a/smartsim/database/orchestrator.py +++ b/smartsim/database/orchestrator.py @@ -106,15 +106,15 @@ def __init__( if launcher == "auto": launcher = detect_launcher() - by_launcher: t.Dict[str, t.List[t.Union[str, None]]] = { + by_launcher: t.Dict[str, t.List[str]] = { "slurm": ["srun", "mpirun", "mpiexec"], "pbs": ["aprun", "mpirun", "mpiexec"], "cobalt": ["aprun", "mpirun", "mpiexec"], "lsf": ["jsrun"], - "local": [None], + "local": [""], } - def _detect_command(launcher: str) -> t.Optional[str]: + def _detect_command(launcher: str) -> str: if launcher in by_launcher: for cmd in by_launcher[launcher]: if launcher == "local": From e7ac4f41a330cb9e9b8e41eb53e3d8dcb3d261a6 Mon Sep 17 00:00:00 2001 From: Chris McBride Date: Mon, 5 Jun 2023 11:48:02 -0400 Subject: [PATCH 29/44] exe_args list of lists typehint fix --- smartsim/database/orchestrator.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/smartsim/database/orchestrator.py b/smartsim/database/orchestrator.py index d7f5a0731..e4a3ca0c6 100644 --- a/smartsim/database/orchestrator.py +++ b/smartsim/database/orchestrator.py @@ -567,7 +567,7 @@ def _build_run_settings( run_settings.make_mpmd(mpmd_run_settings) else: run_settings = create_run_settings( - exe=exe, exe_args=exe_args, run_args=run_args.copy(), **kwargs + exe=exe, exe_args=exe_args[0], run_args=run_args.copy(), **kwargs ) if self.launcher != "local": @@ -653,11 +653,11 @@ def _initialize_entities(self, **kwargs: t.Any) -> None: db_node_name, port, cluster ) - exe_args = " ".join(start_script_args) + # exe_args = " ".join(start_script_args) # if only launching 1 db per command, we don't need a list of exe args lists run_settings = self._build_run_settings( - sys.executable, exe_args, **kwargs + sys.executable, [start_script_args], **kwargs ) node = DBNode( From fde32918e1abc2d4c52ce5e891ce909a459b5501 Mon Sep 17 00:00:00 2001 From: Chris McBride Date: Mon, 5 Jun 2023 12:51:06 -0400 Subject: [PATCH 30/44] run settings input to string conversion typehint fixes --- smartsim/database/orchestrator.py | 2 -- smartsim/entity/ensemble.py | 5 +++-- tests/test_ensemble.py | 28 ++++++++++++++-------------- 3 files changed, 17 insertions(+), 18 deletions(-) diff --git a/smartsim/database/orchestrator.py b/smartsim/database/orchestrator.py index e4a3ca0c6..4ea6bec1b 100644 --- a/smartsim/database/orchestrator.py +++ b/smartsim/database/orchestrator.py @@ -653,8 +653,6 @@ def _initialize_entities(self, **kwargs: t.Any) -> None: db_node_name, port, cluster ) - # exe_args = " ".join(start_script_args) - # if only launching 1 db per command, we don't need a list of exe args lists run_settings = self._build_run_settings( sys.executable, [start_script_args], **kwargs diff --git a/smartsim/entity/ensemble.py b/smartsim/entity/ensemble.py index 74d8b5730..7c3127c56 100644 --- a/smartsim/entity/ensemble.py +++ b/smartsim/entity/ensemble.py @@ -309,14 +309,15 @@ def _read_model_parameters(self) -> t.Tuple[t.List[str], t.List[t.List[str]]]: ) param_names: t.List[str] = [] - parameters: t.List[t.List[t.Union[int, str]]] = [] + parameters: t.List[t.List[str]] = [] for name, val in self.params.items(): param_names.append(name) if isinstance(val, list): + val = [str(v) for v in val] parameters.append(val) elif isinstance(val, (int, str)): - parameters.append([val]) + parameters.append([str(val)]) else: raise TypeError( "Incorrect type for ensemble parameters\n" diff --git a/tests/test_ensemble.py b/tests/test_ensemble.py index 321816e21..9258e1c45 100644 --- a/tests/test_ensemble.py +++ b/tests/test_ensemble.py @@ -74,8 +74,8 @@ def test_all_perm(): params = {"h": [5, 6]} ensemble = Ensemble("all_perm", params, run_settings=rs, perm_strat="all_perm") assert len(ensemble) == 2 - assert ensemble.entities[0].params["h"] == 5 - assert ensemble.entities[1].params["h"] == 6 + assert ensemble.entities[0].params["h"] == "5" + assert ensemble.entities[1].params["h"] == "6" def test_step(): @@ -84,10 +84,10 @@ def test_step(): ensemble = Ensemble("step", params, run_settings=rs, perm_strat="step") assert len(ensemble) == 2 - model_1_params = {"h": 5, "g": 7} + model_1_params = {"h": "5", "g": "7"} assert ensemble.entities[0].params == model_1_params - model_2_params = {"h": 6, "g": 8} + model_2_params = {"h": "6", "g": "8"} assert ensemble.entities[1].params == model_2_params @@ -104,7 +104,7 @@ def test_random(): ) assert len(ensemble) == len(random_ints) assigned_params = [m.params["h"] for m in ensemble.entities] - assert all([x in random_ints for x in assigned_params]) + assert all([int(x) in random_ints for x in assigned_params]) ensemble = Ensemble( "random_test", @@ -115,7 +115,7 @@ def test_random(): ) assert len(ensemble) == len(random_ints) - 1 assigned_params = [m.params["h"] for m in ensemble.entities] - assert all([x in random_ints for x in assigned_params]) + assert all([int(x) in random_ints for x in assigned_params]) def test_user_strategy(): @@ -124,10 +124,10 @@ def test_user_strategy(): ensemble = Ensemble("step", params, run_settings=rs, perm_strat=step_values) assert len(ensemble) == 2 - model_1_params = {"h": 5, "g": 7} + model_1_params = {"h": "5", "g": "7"} assert ensemble.entities[0].params == model_1_params - model_2_params = {"h": 6, "g": 8} + model_2_params = {"h": "6", "g": "8"} assert ensemble.entities[1].params == model_2_params @@ -181,10 +181,10 @@ def test_arg_and_model_params_step(): exe_args_1 = rs_orig_args + ["-H", "6", "--g_param=b"] assert ensemble.entities[1].run_settings.exe_args == exe_args_1 - model_1_params = {"H": 5, "g_param": "a", "h": 5, "g": 7} + model_1_params = {"H": "5", "g_param": "a", "h": "5", "g": "7"} assert ensemble.entities[0].params == model_1_params - model_2_params = {"H": 6, "g_param": "b", "h": 6, "g": 8} + model_2_params = {"H": "6", "g_param": "b", "h": "6", "g": "8"} assert ensemble.entities[1].params == model_2_params @@ -214,13 +214,13 @@ def test_arg_and_model_params_all_perms(): assert ensemble.entities[1].run_settings.exe_args == exe_args_1 assert ensemble.entities[3].run_settings.exe_args == exe_args_1 - model_0_params = {"g_param": "a", "h": 5} + model_0_params = {"g_param": "a", "h": "5"} assert ensemble.entities[0].params == model_0_params - model_1_params = {"g_param": "b", "h": 5} + model_1_params = {"g_param": "b", "h": "5"} assert ensemble.entities[1].params == model_1_params - model_2_params = {"g_param": "a", "h": 6} + model_2_params = {"g_param": "a", "h": "6"} assert ensemble.entities[2].params == model_2_params - model_3_params = {"g_param": "b", "h": 6} + model_3_params = {"g_param": "b", "h": "6"} assert ensemble.entities[3].params == model_3_params From f98e92b151f52de961b2660faa844d4cc16c0fe8 Mon Sep 17 00:00:00 2001 From: Chris McBride Date: Mon, 5 Jun 2023 13:13:10 -0400 Subject: [PATCH 31/44] add check-mypy CI action --- .github/workflows/check_mypy.yml | 27 +++++++++++++++++++++++++++ requirements-dev.txt | 2 -- requirements-mypy.txt | 2 ++ 3 files changed, 29 insertions(+), 2 deletions(-) create mode 100644 .github/workflows/check_mypy.yml diff --git a/.github/workflows/check_mypy.yml b/.github/workflows/check_mypy.yml new file mode 100644 index 000000000..fc0576471 --- /dev/null +++ b/.github/workflows/check_mypy.yml @@ -0,0 +1,27 @@ +name: check-mypy +on: + pull_request: + push: + branches: + - master + - develop + - /feature\/.*/ + +jobs: + check_mypy: + name: Static Type Check + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - uses: actions/setup-python@v2 + with: + python-version: "3.9" + + - name: Install SmartSim + run: python -m pip install .[dev] + + - name: Install mypy and type stubs + run: python -m pip install -r./requirements-mypy.txt + + - name: Run mypy + run: make check-mypy diff --git a/requirements-dev.txt b/requirements-dev.txt index 044ebfb10..a1f7d4486 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -4,5 +4,3 @@ pylint>=2.6.0 pytest>=6.0.0 pytest-cov>=2.10.1 click==8.0.2 -mypy>=1.3.0 - diff --git a/requirements-mypy.txt b/requirements-mypy.txt index 238ef3e7c..15d399327 100644 --- a/requirements-mypy.txt +++ b/requirements-mypy.txt @@ -1,3 +1,5 @@ +mypy>=1.3.0 + # From typeshed types-psutil types-redis From 5e640179215bb91f0d6c3c3385d138e62823d752 Mon Sep 17 00:00:00 2001 From: Chris McBride Date: Mon, 5 Jun 2023 13:14:26 -0400 Subject: [PATCH 32/44] build_batch_settings argument fixes for passing typecheck --- smartsim/database/orchestrator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/smartsim/database/orchestrator.py b/smartsim/database/orchestrator.py index 4ea6bec1b..1a83d6aca 100644 --- a/smartsim/database/orchestrator.py +++ b/smartsim/database/orchestrator.py @@ -197,7 +197,7 @@ def _detect_command(launcher: str) -> str: if launcher != "local": self.batch_settings = self._build_batch_settings( - db_nodes, alloc, batch, account, time, launcher=launcher, **kwargs + db_nodes, alloc or "", batch, account or "", time or "", launcher=launcher, **kwargs ) if hosts: self.set_hosts(hosts) From dd4960410a881c58062013571ee0170a5a68d9f8 Mon Sep 17 00:00:00 2001 From: Chris McBride Date: Mon, 5 Jun 2023 16:10:43 -0400 Subject: [PATCH 33/44] use gh action matrix to parameterize python in type checking --- .github/workflows/check_mypy.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/check_mypy.yml b/.github/workflows/check_mypy.yml index fc0576471..f1dd67d71 100644 --- a/.github/workflows/check_mypy.yml +++ b/.github/workflows/check_mypy.yml @@ -11,11 +11,14 @@ jobs: check_mypy: name: Static Type Check runs-on: ubuntu-latest + strategy: + matrix: + python-version: [3.8, 3.9, "3.10"] steps: - uses: actions/checkout@v2 - uses: actions/setup-python@v2 with: - python-version: "3.9" + python-version: ${{ matrix.python-version }} - name: Install SmartSim run: python -m pip install .[dev] From 3d8fd8039467f361a3df6f7f01deb906d862a001 Mon Sep 17 00:00:00 2001 From: Chris McBride Date: Mon, 5 Jun 2023 16:22:25 -0400 Subject: [PATCH 34/44] combine mypy jobs with existing job to avoid matrix sync issues --- .github/workflows/check_mypy.yml | 30 ------------------------------ .github/workflows/run_tests.yml | 6 ++++++ 2 files changed, 6 insertions(+), 30 deletions(-) delete mode 100644 .github/workflows/check_mypy.yml diff --git a/.github/workflows/check_mypy.yml b/.github/workflows/check_mypy.yml deleted file mode 100644 index f1dd67d71..000000000 --- a/.github/workflows/check_mypy.yml +++ /dev/null @@ -1,30 +0,0 @@ -name: check-mypy -on: - pull_request: - push: - branches: - - master - - develop - - /feature\/.*/ - -jobs: - check_mypy: - name: Static Type Check - runs-on: ubuntu-latest - strategy: - matrix: - python-version: [3.8, 3.9, "3.10"] - steps: - - uses: actions/checkout@v2 - - uses: actions/setup-python@v2 - with: - python-version: ${{ matrix.python-version }} - - - name: Install SmartSim - run: python -m pip install .[dev] - - - name: Install mypy and type stubs - run: python -m pip install -r./requirements-mypy.txt - - - name: Run mypy - run: make check-mypy diff --git a/.github/workflows/run_tests.yml b/.github/workflows/run_tests.yml index d45ab509b..338444dba 100644 --- a/.github/workflows/run_tests.yml +++ b/.github/workflows/run_tests.yml @@ -128,3 +128,9 @@ jobs: with: fail_ci_if_error: true files: ./coverage.xml + + - name: Install mypy and type stubs + run: python -m pip install -r./requirements-mypy.txt + + - name: Run mypy + run: make check-mypy From da83abfc05ecdb3c9cb6742e9ee9e5663a00c999 Mon Sep 17 00:00:00 2001 From: Chris McBride Date: Tue, 6 Jun 2023 13:26:45 -0400 Subject: [PATCH 35/44] docstring fix --- smartsim/entity/ensemble.py | 2 +- smartsim/experiment.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/smartsim/entity/ensemble.py b/smartsim/entity/ensemble.py index 7c3127c56..2b56f7786 100644 --- a/smartsim/entity/ensemble.py +++ b/smartsim/entity/ensemble.py @@ -83,7 +83,7 @@ def __init__( :type replicas: int, optional :param perm_strategy: strategy for expanding ``params`` into ``Model`` instances from params argument - options are "all_perm", "stepped", "random" + options are "all_perm", "step", "random" or a callable function. Defaults to "all_perm". :type perm_strategy: str :return: ``Ensemble`` instance diff --git a/smartsim/experiment.py b/smartsim/experiment.py index 4a4e48994..21cacf8eb 100644 --- a/smartsim/experiment.py +++ b/smartsim/experiment.py @@ -417,7 +417,7 @@ def create_ensemble( :type replicas: int :param perm_strategy: strategy for expanding ``params`` into ``Model`` instances from params argument - options are "all_perm", "stepped", "random" + options are "all_perm", "step", "random" or a callable function. Default is "all_perm". :type perm_strategy: str, optional :raises SmartSimError: if initialization fails From 6889a1817a49195c42a18979ba11ae906906ecba Mon Sep 17 00:00:00 2001 From: Chris McBride Date: Tue, 6 Jun 2023 17:04:45 -0400 Subject: [PATCH 36/44] combine mypy deps into regular dev reqs.txt --- requirements-dev.txt | 7 +++++++ requirements-mypy.txt | 9 --------- 2 files changed, 7 insertions(+), 9 deletions(-) delete mode 100644 requirements-mypy.txt diff --git a/requirements-dev.txt b/requirements-dev.txt index a1f7d4486..eb6568d20 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -4,3 +4,10 @@ pylint>=2.6.0 pytest>=6.0.0 pytest-cov>=2.10.1 click==8.0.2 +mypy>=1.3.0 + +# mypy: From typeshed +types-psutil +types-redis +types-tabulate +types-tqdm diff --git a/requirements-mypy.txt b/requirements-mypy.txt deleted file mode 100644 index 15d399327..000000000 --- a/requirements-mypy.txt +++ /dev/null @@ -1,9 +0,0 @@ -mypy>=1.3.0 - -# From typeshed -types-psutil -types-redis -types-tabulate -types-tqdm - -# Not from typeshed From 4463331e05b29554d68a5c08a1a4716c8b09990d Mon Sep 17 00:00:00 2001 From: Chris McBride Date: Tue, 6 Jun 2023 17:05:54 -0400 Subject: [PATCH 37/44] remove python version from .toml, rely on default/installed python ver --- pyproject.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 8cec5ded3..7a7ce174b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -67,7 +67,6 @@ ignore_errors = true directory = "htmlcov" [tool.mypy] -python_version = "3.9" namespace_packages = true files = [ "smartsim" From 21ba60a6bfa38ba654dd779c17d4fa924b730b85 Mon Sep 17 00:00:00 2001 From: Chris McBride Date: Tue, 6 Jun 2023 17:10:33 -0400 Subject: [PATCH 38/44] Alias StrategyFunction typehint --- smartsim/entity/ensemble.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/smartsim/entity/ensemble.py b/smartsim/entity/ensemble.py index 2b56f7786..d2d41bfda 100644 --- a/smartsim/entity/ensemble.py +++ b/smartsim/entity/ensemble.py @@ -46,6 +46,7 @@ logger = get_logger(__name__) +StrategyFunction = t.Callable[[t.List[str], t.List[t.List[str]], int], t.List[t.Dict[str, str]]] class Ensemble(EntityList): """``Ensemble`` is a group of ``Model`` instances that can @@ -272,7 +273,7 @@ def attach_generator_files( def _set_strategy( self, strategy: str - ) -> t.Callable[[t.List[str], t.List[t.List[str]], int], t.List[t.Dict[str, str]]]: + ) -> StrategyFunction: """Set the permutation strategy for generating models within the ensemble From deaa2709327b14355919118672eb2aed24afd999 Mon Sep 17 00:00:00 2001 From: Chris McBride Date: Tue, 6 Jun 2023 17:13:49 -0400 Subject: [PATCH 39/44] revert permutations refactor --- smartsim/entity/strategies.py | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/smartsim/entity/strategies.py b/smartsim/entity/strategies.py index 7923348e5..0ef47e13d 100644 --- a/smartsim/entity/strategies.py +++ b/smartsim/entity/strategies.py @@ -57,9 +57,20 @@ def random_permutations( param_names: t.List[str], param_values: t.List[t.List[str]], n_models: int = 0 ) -> t.List[t.Dict[str, str]]: # first, check if we've requested more values than possible. - permutations = create_all_permutations(param_names, param_values) - - if n_models and n_models < len(permutations): - permutations = random.sample(permutations, n_models) - - return permutations + perms = list(product(*param_values)) + if n_models >= len(perms): + return create_all_permutations(param_names, param_values) + else: + permutations: t.List[t.Dict[str, str]] = [] + permutation_strings = set() + while len(permutations) < n_models: + model_dict = dict( + zip( + param_names, + map(lambda x: x[random.randint(0, len(x) - 1)], param_values), + ) + ) + if str(model_dict) not in permutation_strings: + permutation_strings.add(str(model_dict)) + permutations.append(model_dict) + return permutations From 099aadb8140a143238c16b03db08316a35e761ee Mon Sep 17 00:00:00 2001 From: Chris McBride <3595025+ankona@users.noreply.github.com> Date: Tue, 6 Jun 2023 18:12:36 -0400 Subject: [PATCH 40/44] Update action after combining reqs-mypy and reqs-dev --- .github/workflows/run_tests.yml | 3 --- 1 file changed, 3 deletions(-) diff --git a/.github/workflows/run_tests.yml b/.github/workflows/run_tests.yml index 338444dba..ea3b6d18c 100644 --- a/.github/workflows/run_tests.yml +++ b/.github/workflows/run_tests.yml @@ -129,8 +129,5 @@ jobs: fail_ci_if_error: true files: ./coverage.xml - - name: Install mypy and type stubs - run: python -m pip install -r./requirements-mypy.txt - - name: Run mypy run: make check-mypy From 6771fc63d8523effa3b66509e0d77fb098c5579e Mon Sep 17 00:00:00 2001 From: Chris McBride Date: Tue, 6 Jun 2023 19:01:59 -0400 Subject: [PATCH 41/44] update deps in setup.py, update changelog --- doc/changelog.rst | 3 +++ setup.py | 5 +++++ 2 files changed, 8 insertions(+) diff --git a/doc/changelog.rst b/doc/changelog.rst index 6e9fb8d27..20f62bdab 100644 --- a/doc/changelog.rst +++ b/doc/changelog.rst @@ -48,6 +48,8 @@ former began deprecation in May 2022 and was finally removed in May 2023. (PR285 codes. These have now all been updated. (PR284_) - Orchestrator and Colocated DB now accept a list of interfaces to bind to. The argument name is still `interface` for backward compatibility reasons. (PR281_) +- Typehints have been added to public APIs. A makefile target to execute static +analysis with mypy is available `make check-mypy`. (PR295_) .. _PR292: https://github.com/CrayLabs/SmartSim/pull/292 .. _PR291: https://github.com/CrayLabs/SmartSim/pull/291 @@ -57,6 +59,7 @@ argument name is still `interface` for backward compatibility reasons. (PR281_) .. _PR285: https://github.com/CrayLabs/SmartSim/pull/285 .. _PR284: https://github.com/CrayLabs/SmartSim/pull/284 .. _PR281: https://github.com/CrayLabs/SmartSim/pull/281 +.. _PR295: https://github.com/CrayLabs/SmartSim/pull/295 0.4.2 ----- diff --git a/setup.py b/setup.py index 9a4499ae8..53c46773d 100644 --- a/setup.py +++ b/setup.py @@ -180,6 +180,11 @@ def has_ext_modules(_placeholder): "pytest>=6.0.0", "pytest-cov>=2.10.1", "click==8.0.2", + "mypy>=1.3.0", + "types-psutil", + "types-redis", + "types-tabulate", + "types-tqdm", ], # see smartsim/_core/_install/buildenv.py for more details "ml": versions.ml_extras_required(), From cb6e47272349fa4b9a0efec496cb27dcbf73e9d5 Mon Sep 17 00:00:00 2001 From: Chris McBride Date: Tue, 6 Jun 2023 19:12:49 -0400 Subject: [PATCH 42/44] adjust mypy version to avoid dependency conflict --- requirements-dev.txt | 2 +- setup.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/requirements-dev.txt b/requirements-dev.txt index eb6568d20..044e79d63 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -4,7 +4,7 @@ pylint>=2.6.0 pytest>=6.0.0 pytest-cov>=2.10.1 click==8.0.2 -mypy>=1.3.0 +mypy>=1.2.0 # mypy: From typeshed types-psutil diff --git a/setup.py b/setup.py index 53c46773d..08592b6ec 100644 --- a/setup.py +++ b/setup.py @@ -180,7 +180,7 @@ def has_ext_modules(_placeholder): "pytest>=6.0.0", "pytest-cov>=2.10.1", "click==8.0.2", - "mypy>=1.3.0", + "mypy>=1.2.0", "types-psutil", "types-redis", "types-tabulate", From 049853a6fcc18b8b86d0cbad027b6c5545eda5ef Mon Sep 17 00:00:00 2001 From: Chris McBride Date: Wed, 7 Jun 2023 10:42:49 -0400 Subject: [PATCH 43/44] avoid dep conflict by skipping mypy w/tf 2.6.2 --- .github/workflows/run_tests.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/run_tests.yml b/.github/workflows/run_tests.yml index ea3b6d18c..14f8547d5 100644 --- a/.github/workflows/run_tests.yml +++ b/.github/workflows/run_tests.yml @@ -130,4 +130,5 @@ jobs: files: ./coverage.xml - name: Run mypy + if: (matrix.rai != '1.2.5') run: make check-mypy From d857bdd76e2d4ed733b70ed61f17738d237f2987 Mon Sep 17 00:00:00 2001 From: Chris McBride Date: Wed, 7 Jun 2023 11:11:19 -0400 Subject: [PATCH 44/44] separate mypy deps & conditionally run check --- .github/workflows/run_tests.yml | 5 ++++- requirements-dev.txt | 7 ------- requirements-mypy.txt | 11 +++++++++++ setup.py | 6 +++++- 4 files changed, 20 insertions(+), 9 deletions(-) create mode 100644 requirements-mypy.txt diff --git a/.github/workflows/run_tests.yml b/.github/workflows/run_tests.yml index 14f8547d5..91e717c09 100644 --- a/.github/workflows/run_tests.yml +++ b/.github/workflows/run_tests.yml @@ -130,5 +130,8 @@ jobs: files: ./coverage.xml - name: Run mypy + # TF 2.6.2 has a dep conflict with new mypy versions if: (matrix.rai != '1.2.5') - run: make check-mypy + run: | + python -m pip install .[mypy] + make check-mypy diff --git a/requirements-dev.txt b/requirements-dev.txt index 044e79d63..a1f7d4486 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -4,10 +4,3 @@ pylint>=2.6.0 pytest>=6.0.0 pytest-cov>=2.10.1 click==8.0.2 -mypy>=1.2.0 - -# mypy: From typeshed -types-psutil -types-redis -types-tabulate -types-tqdm diff --git a/requirements-mypy.txt b/requirements-mypy.txt new file mode 100644 index 000000000..e2100944c --- /dev/null +++ b/requirements-mypy.txt @@ -0,0 +1,11 @@ +mypy>=1.3.0 + +# From typeshed +types-psutil +types-redis +types-tabulate +types-tqdm +types-tensorflow +types-setuptools + +# Not from typeshed diff --git a/setup.py b/setup.py index 08592b6ec..8c37ba0f8 100644 --- a/setup.py +++ b/setup.py @@ -180,11 +180,15 @@ def has_ext_modules(_placeholder): "pytest>=6.0.0", "pytest-cov>=2.10.1", "click==8.0.2", - "mypy>=1.2.0", + ], + "mypy": [ + "mypy>=1.3.0", "types-psutil", "types-redis", "types-tabulate", "types-tqdm", + "types-tensorflow", + "types-setuptools", ], # see smartsim/_core/_install/buildenv.py for more details "ml": versions.ml_extras_required(),