Merge remote-tracking branch 'upstream/smartsim-refactor' into kill-job

.github/workflows/run_tests.yml

@@ -157,7 +157,7 @@ jobs:
         run: |
           echo "SMARTSIM_LOG_LEVEL=debug" >> $GITHUB_ENV
           py.test -s --import-mode=importlib -o log_cli=true --cov=$(smart site) --cov-report=xml --cov-config=./tests/test_configs/cov/local_cov.cfg --ignore=tests/full_wlm/ -m ${{ matrix.subset }} ./tests
-
+    
       # Upload artifacts on failure, ignoring binary files
       - name: Upload Artifact
         if: failure()

pyproject.toml

@@ -147,7 +147,6 @@ module = [
   # FIXME: DO NOT MERGE THIS INTO DEVELOP BRANCH UNLESS THESE ARE PASSING OR
   #        REMOVED!!
   "smartsim._core._cli.*",
-  "smartsim._core.commands.*",
   "smartsim._core.control.controller",
   "smartsim._core.control.manifest",
   "smartsim._core.entrypoints.dragon_client",

smartsim/_core/commands/command.py

@@ -26,41 +26,60 @@
 
 import typing as t
 from collections.abc import MutableSequence
+from copy import deepcopy
 
-from ...settings.launchCommand import LauncherType
+from typing_extensions import Self
 
 
 class Command(MutableSequence[str]):
     """Basic container for command information"""
 
-    def __init__(self, launcher: LauncherType, command: t.List[str]) -> None:
+    def __init__(self, command: t.List[str]) -> None:
         """Command constructor"""
-        self._launcher = launcher
         self._command = command
 
-    @property
-    def launcher(self) -> LauncherType:
-        """Get the launcher type.
-        Return a reference to the LauncherType.
-        """
-        return self._launcher
-
     @property
     def command(self) -> t.List[str]:
         """Get the command list.
         Return a reference to the command list.
         """
         return self._command
 
-    def __getitem__(self, idx: int) -> str:
+    @t.overload
+    def __getitem__(self, idx: int) -> str: ...
+    @t.overload
+    def __getitem__(self, idx: slice) -> Self: ...
+    def __getitem__(self, idx: t.Union[int, slice]) -> t.Union[str, Self]:
         """Get the command at the specified index."""
-        return self._command[idx]
+        cmd = self._command[idx]
+        if isinstance(cmd, str):
+            return cmd
+        return type(self)(cmd)
 
-    def __setitem__(self, idx: int, value: str) -> None:
+    @t.overload
+    def __setitem__(self, idx: int, value: str) -> None: ...
+    @t.overload
+    def __setitem__(self, idx: slice, value: t.Iterable[str]) -> None: ...
+    def __setitem__(
+        self, idx: t.Union[int, slice], value: t.Union[str, t.Iterable[str]]
+    ) -> None:
         """Set the command at the specified index."""
-        self._command[idx] = value
+        if isinstance(idx, int):
+            if not isinstance(value, str):
+                raise ValueError(
+                    "Value must be of type `str` when assigning to an index"
+                )
+            self._command[idx] = deepcopy(value)
+            return
+        if not isinstance(value, list) or not all(
+            isinstance(item, str) for item in value
+        ):
+            raise ValueError(
+                "Value must be a list of strings when assigning to a slice"
+            )
+        self._command[idx] = (deepcopy(val) for val in value)
 
-    def __delitem__(self, idx: int) -> None:
+    def __delitem__(self, idx: t.Union[int, slice]) -> None:
         """Delete the command at the specified index."""
         del self._command[idx]
 
@@ -73,6 +92,5 @@ def insert(self, idx: int, value: str) -> None:
         self._command.insert(idx, value)
 
     def __str__(self) -> str:  # pragma: no cover
-        string = f"\nLauncher: {self.launcher.value}\n"
-        string += f"Command: {' '.join(str(cmd) for cmd in self.command)}"
+        string = f"\nCommand: {' '.join(str(cmd) for cmd in self.command)}"
         return string

smartsim/_core/commands/commandList.py

@@ -26,6 +26,7 @@
 
 import typing as t
 from collections.abc import MutableSequence
+from copy import deepcopy
 
 from .command import Command
 
@@ -46,15 +47,45 @@ def commands(self) -> t.List[Command]:
         """
         return self._commands
 
-    def __getitem__(self, idx: int) -> Command:
+    @t.overload
+    def __getitem__(self, idx: int) -> Command: ...
+    @t.overload
+    def __getitem__(self, idx: slice) -> t.List[Command]: ...
+    def __getitem__(
+        self, idx: t.Union[slice, int]
+    ) -> t.Union[Command, t.List[Command]]:
         """Get the Command at the specified index."""
         return self._commands[idx]
 
-    def __setitem__(self, idx: int, value: Command) -> None:
-        """Set the Command at the specified index."""
-        self._commands[idx] = value
+    @t.overload
+    def __setitem__(self, idx: int, value: Command) -> None: ...
+    @t.overload
+    def __setitem__(self, idx: slice, value: t.Iterable[Command]) -> None: ...
+    def __setitem__(
+        self, idx: t.Union[int, slice], value: t.Union[Command, t.Iterable[Command]]
+    ) -> None:
+        """Set the Commands at the specified index."""
+        if isinstance(idx, int):
+            if not isinstance(value, Command):
+                raise ValueError(
+                    "Value must be of type `Command` when assigning to an index"
+                )
+            self._commands[idx] = deepcopy(value)
+            return
+        if not isinstance(value, list):
+            raise ValueError(
+                "Value must be a list of Commands when assigning to a slice"
+            )
+        for sublist in value:
+            if not isinstance(sublist.command, list) or not all(
+                isinstance(item, str) for item in sublist.command
+            ):
+                raise ValueError(
+                    "Value sublists must be a list of Commands when assigning to a slice"
+                )
+        self._commands[idx] = (deepcopy(val) for val in value)
 
-    def __delitem__(self, idx: int) -> None:
+    def __delitem__(self, idx: t.Union[int, slice]) -> None:
         """Delete the Command at the specified index."""
         del self._commands[idx]
 

smartsim/_core/dispatch.py

@@ -28,6 +28,7 @@
 
 import dataclasses
 import os
+import pathlib
 import typing as t
 
 from typing_extensions import Self, TypeAlias, TypeVarTuple, Unpack
@@ -42,10 +43,11 @@
     from smartsim.experiment import Experiment
     from smartsim.settings.arguments import LaunchArguments
 
+
 _Ts = TypeVarTuple("_Ts")
 
 
-_WorkingDirectory: TypeAlias = t.Union[str, os.PathLike[str]]
+WorkingDirectory: TypeAlias = pathlib.Path
 """A working directory represented as a string or PathLike object"""
 
 _DispatchableT = t.TypeVar("_DispatchableT", bound="LaunchArguments")
@@ -57,20 +59,30 @@
 to the to the `LauncherProtocol.start` method
 """
 
-_EnvironMappingType: TypeAlias = t.Mapping[str, "str | None"]
+EnvironMappingType: TypeAlias = t.Mapping[str, "str | None"]
 """A mapping of user provided mapping of environment variables in which to run
 a job
 """
-_FormatterType: TypeAlias = t.Callable[
-    [_DispatchableT, "ExecutableProtocol", _WorkingDirectory, _EnvironMappingType],
+FormatterType: TypeAlias = t.Callable[
+    [
+        _DispatchableT,
+        "ExecutableProtocol",
+        WorkingDirectory,
+        EnvironMappingType,
+        pathlib.Path,
+        pathlib.Path,
+    ],
     _LaunchableT,
 ]
 """A callable that is capable of formatting the components of a job into a type
 capable of being launched by a launcher.
 """
-_LaunchConfigType: TypeAlias = (
-    "_LauncherAdapter[ExecutableProtocol, _WorkingDirectory, _EnvironMappingType]"
-)
+_LaunchConfigType: TypeAlias = """_LauncherAdapter[
+        ExecutableProtocol,
+        WorkingDirectory,
+        EnvironMappingType,
+        pathlib.Path,
+        pathlib.Path]"""
 
 """A launcher adapater that has configured a launcher to launch the components
 of a job with some pre-determined launch settings
@@ -133,7 +145,7 @@ def dispatch(  # Signature when used as a decorator
         self,
         args: None = ...,
         *,
-        with_format: _FormatterType[_DispatchableT, _LaunchableT],
+        with_format: FormatterType[_DispatchableT, _LaunchableT],
         to_launcher: type[LauncherProtocol[_LaunchableT]],
         allow_overwrite: bool = ...,
     ) -> t.Callable[[type[_DispatchableT]], type[_DispatchableT]]: ...
@@ -142,15 +154,15 @@ def dispatch(  # Signature when used as a method
         self,
         args: type[_DispatchableT],
         *,
-        with_format: _FormatterType[_DispatchableT, _LaunchableT],
+        with_format: FormatterType[_DispatchableT, _LaunchableT],
         to_launcher: type[LauncherProtocol[_LaunchableT]],
         allow_overwrite: bool = ...,
     ) -> None: ...
     def dispatch(  # Actual implementation
         self,
         args: type[_DispatchableT] | None = None,
         *,
-        with_format: _FormatterType[_DispatchableT, _LaunchableT],
+        with_format: FormatterType[_DispatchableT, _LaunchableT],
         to_launcher: type[LauncherProtocol[_LaunchableT]],
         allow_overwrite: bool = False,
     ) -> t.Callable[[type[_DispatchableT]], type[_DispatchableT]] | None:
@@ -216,7 +228,7 @@ class _DispatchRegistration(t.Generic[_DispatchableT, _LaunchableT]):
     to be launched by the afore mentioned launcher.
     """
 
-    formatter: _FormatterType[_DispatchableT, _LaunchableT]
+    formatter: FormatterType[_DispatchableT, _LaunchableT]
     launcher_type: type[LauncherProtocol[_LaunchableT]]
 
     def _is_compatible_launcher(self, launcher: LauncherProtocol[t.Any]) -> bool:
@@ -260,10 +272,12 @@ def create_adapter_from_launcher(
 
         def format_(
             exe: ExecutableProtocol,
-            path: str | os.PathLike[str],
-            env: _EnvironMappingType,
+            path: pathlib.Path,
+            env: EnvironMappingType,
+            out: pathlib.Path,
+            err: pathlib.Path,
         ) -> _LaunchableT:
-            return self.formatter(arguments, exe, path, env)
+            return self.formatter(arguments, exe, path, env, out, err)
 
         return _LauncherAdapter(launcher, format_)
 

smartsim/_core/generation/generator.py

@@ -108,7 +108,17 @@ def _log_file(log_path: pathlib.Path) -> pathlib.Path:
         """
         return pathlib.Path(log_path) / "smartsim_params.txt"
 
-    def generate_job(self, job: Job, job_index: int) -> pathlib.Path:
+    @staticmethod
+    def _output_files(
+        log_path: pathlib.Path, job_name: str
+    ) -> t.Tuple[pathlib.Path, pathlib.Path]:
+        out_file_path = log_path / f"{job_name}.out"
+        err_file_path = log_path / f"{job_name}.err"
+        return out_file_path, err_file_path
+
+    def generate_job(
+        self, job: Job, job_index: int
+    ) -> t.Tuple[pathlib.Path, pathlib.Path, pathlib.Path]:
         """Write and configure input files for a Job.
 
         To have files or directories present in the created Job
@@ -136,10 +146,13 @@ def generate_job(self, job: Job, job_index: int) -> pathlib.Path:
             dt_string = datetime.now().strftime("%d/%m/%Y %H:%M:%S")
             log_file.write(f"Generation start date and time: {dt_string}\n")
 
+        # Create output files
+        out_file, err_file = self._output_files(log_path, job.entity.name)
+
         # Perform file system operations on attached files
         self._build_operations(job, job_path)
 
-        return job_path
+        return job_path, out_file, err_file
 
     @classmethod
     def _build_operations(cls, job: Job, job_path: pathlib.Path) -> None:

smartsim/_core/launcher/dragon/dragonLauncher.py

@@ -27,6 +27,7 @@
 from __future__ import annotations
 
 import os
+import pathlib
 import typing as t
 
 from smartsim._core.schemas.dragonRequests import DragonRunPolicy
@@ -376,6 +377,8 @@ def _as_run_request_args_and_policy(
     exe: ExecutableProtocol,
     path: str | os.PathLike[str],
     env: t.Mapping[str, str | None],
+    stdout_path: pathlib.Path,
+    stderr_path: pathlib.Path,
 ) -> tuple[DragonRunRequestView, DragonRunPolicy]:
     # ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     # FIXME: This type is 100% unacceptable, but I don't want to spend too much
@@ -397,8 +400,8 @@ def _as_run_request_args_and_policy(
             env=env,
             # TODO: Not sure how this info is injected
             name=None,
-            output_file=None,
-            error_file=None,
+            output_file=stdout_path,
+            error_file=stderr_path,
             **run_args,
         ),
         policy,