Refine SmartSimEntity Interface (#688)

Refactor SmartSimEntity class, remove ExecutableProtocol protocol,
Application becomes subclass of ABC SmartSImEntity.

[ reviewed by @MattToast @mellis13 ]
[ committed by @amandarichardsonn ]

Author: amandarichardsonn

smartsim/_core/control/job.py

@@ -299,7 +299,7 @@ def error_report(self) -> str:
         warning += f"Job status at failure: {self.status} \n"
         warning += f"Launcher status at failure: {self.raw_status} \n"
         warning += f"Job returncode: {self.returncode} \n"
-        warning += f"Error and output file located at: {self.entity.path}"
+        # warning += f"Error and output file located at: {self.entity.path}"
         return warning
 
     def __str__(self) -> str:

smartsim/_core/dispatch.py

@@ -39,7 +39,7 @@
 
 if t.TYPE_CHECKING:
     from smartsim._core.arguments.shell import ShellLaunchArguments
-    from smartsim._core.utils.launcher import ExecutableProtocol, LauncherProtocol
+    from smartsim._core.utils.launcher import LauncherProtocol
     from smartsim.experiment import Experiment
     from smartsim.settings.arguments import LaunchArguments
 
@@ -66,7 +66,7 @@
 FormatterType: TypeAlias = t.Callable[
     [
         _DispatchableT,
-        "ExecutableProtocol",
+        t.Sequence[str],
         WorkingDirectory,
         EnvironMappingType,
         pathlib.Path,
@@ -78,7 +78,7 @@
 capable of being launched by a launcher.
 """
 _LaunchConfigType: TypeAlias = """_LauncherAdapter[
-        ExecutableProtocol,
+        t.Sequence[str],
         WorkingDirectory,
         EnvironMappingType,
         pathlib.Path,
@@ -271,7 +271,7 @@ def create_adapter_from_launcher(
             )
 
         def format_(
-            exe: ExecutableProtocol,
+            exe: t.Sequence[str],
             path: pathlib.Path,
             env: EnvironMappingType,
             out: pathlib.Path,

smartsim/_core/launcher/dragon/dragonLauncher.py

@@ -64,7 +64,6 @@
 if t.TYPE_CHECKING:
     from typing_extensions import Self
 
-    from smartsim._core.utils.launcher import ExecutableProtocol
     from smartsim.experiment import Experiment
 
 
@@ -369,7 +368,7 @@ def _assert_schema_type(obj: object, typ: t.Type[_SchemaT], /) -> _SchemaT:
 
 def _as_run_request_args_and_policy(
     run_req_args: DragonLaunchArguments,
-    exe: ExecutableProtocol,
+    exe: t.Sequence[str],
     path: str | os.PathLike[str],
     env: t.Mapping[str, str | None],
     stdout_path: pathlib.Path,
@@ -379,7 +378,7 @@ def _as_run_request_args_and_policy(
     # FIXME: This type is 100% unacceptable, but I don't want to spend too much
     #        time on fixing the dragon launcher API. Something that we need to
     #        revisit in the future though.
-    exe_, *args = exe.as_program_arguments()
+    exe_, *args = exe
     run_args = dict[str, "int | str | float | None"](run_req_args._launch_args)
     policy = DragonRunPolicy.from_run_args(run_args)
     return (

smartsim/_core/shell/shellLauncher.py

@@ -37,7 +37,7 @@
 from smartsim._core.arguments.shell import ShellLaunchArguments
 from smartsim._core.dispatch import EnvironMappingType, FormatterType, WorkingDirectory
 from smartsim._core.utils import helpers
-from smartsim._core.utils.launcher import ExecutableProtocol, create_job_id
+from smartsim._core.utils.launcher import create_job_id
 from smartsim.error import errors
 from smartsim.log import get_logger
 from smartsim.settings.arguments.launchArguments import LaunchArguments
@@ -94,7 +94,7 @@ def make_shell_format_fn(
 
     def impl(
         args: ShellLaunchArguments,
-        exe: ExecutableProtocol,
+        exe: t.Sequence[str],
         path: WorkingDirectory,
         env: EnvironMappingType,
         stdout_path: pathlib.Path,
@@ -105,10 +105,10 @@ def impl(
                 run_command,
                 *(args.format_launch_args() or ()),
                 "--",
-                *exe.as_program_arguments(),
+                *exe,
             )
             if run_command is not None
-            else exe.as_program_arguments()
+            else exe
         )
         # pylint: disable-next=consider-using-with
         return ShellLauncherCommand(

smartsim/_core/utils/launcher.py

@@ -46,10 +46,6 @@ def create_job_id() -> LaunchedJobID:
     return LaunchedJobID(str(uuid.uuid4()))
 
 
-class ExecutableProtocol(t.Protocol):
-    def as_program_arguments(self) -> t.Sequence[str]: ...
-
-
 class LauncherProtocol(collections.abc.Hashable, t.Protocol[_T_contra]):
     """The protocol defining a launcher that can be used by a SmartSim
     experiment

smartsim/entity/application.py

@@ -112,6 +112,14 @@ def exe_args(self, value: t.Union[str, t.Sequence[str], None]) -> None:
         """
         self._exe_args = self._build_exe_args(value)
 
+    def add_exe_args(self, args: t.Union[str, t.List[str], None]) -> None:
+        """Add executable arguments to executable
+
+        :param args: executable arguments
+        """
+        args = self._build_exe_args(args)
+        self._exe_args.extend(args)
+
     @property
     def files(self) -> t.Optional[EntityFiles]:
         """Return files to be copied, symlinked, and/or configured prior to
@@ -178,13 +186,12 @@ def key_prefixing_enabled(self, value: bool) -> None:
         """
         self.key_prefixing_enabled = copy.deepcopy(value)
 
-    def add_exe_args(self, args: t.Union[str, t.List[str], None]) -> None:
-        """Add executable arguments to executable
+    def as_executable_sequence(self) -> t.Sequence[str]:
+        """Converts the executable and its arguments into a sequence of program arguments.
 
-        :param args: executable arguments
+        :return: a sequence of strings representing the executable and its arguments
         """
-        args = self._build_exe_args(args)
-        self._exe_args.extend(args)
+        return [self.exe, *self.exe_args]
 
     def attach_generator_files(
         self,
@@ -242,27 +249,6 @@ def attached_files_table(self) -> str:
             return "No file attached to this application."
         return str(self.files)
 
-    def print_attached_files(self) -> None:
-        """Print a table of the attached files on std out"""
-        print(self.attached_files_table)
-
-    def __str__(self) -> str:  # pragma: no cover
-        exe_args_str = "\n".join(self.exe_args)
-        entities_str = "\n".join(str(entity) for entity in self.incoming_entities)
-        return textwrap.dedent(f"""\
-            Name: {self.name}
-            Type: {self.type}
-            Executable:
-            {self.exe}
-            Executable Arguments:
-            {exe_args_str}
-            Entity Files: {self.files}
-            File Parameters: {self.file_parameters}
-            Incoming Entities:
-            {entities_str}
-            Key Prefixing Enabled: {self.key_prefixing_enabled}
-            """)
-
     @staticmethod
     def _build_exe_args(exe_args: t.Union[str, t.Sequence[str], None]) -> t.List[str]:
         """Check and convert exe_args input to a desired collection format
@@ -286,3 +272,24 @@ def _build_exe_args(exe_args: t.Union[str, t.Sequence[str], None]) -> t.List[str
             return exe_args.split()
 
         return exe_args
+
+    def print_attached_files(self) -> None:
+        """Print a table of the attached files on std out"""
+        print(self.attached_files_table)
+
+    def __str__(self) -> str:  # pragma: no cover
+        exe_args_str = "\n".join(self.exe_args)
+        entities_str = "\n".join(str(entity) for entity in self.incoming_entities)
+        return textwrap.dedent(f"""\
+            Name: {self.name}
+            Type: {self.type}
+            Executable:
+            {self.exe}
+            Executable Arguments:
+            {exe_args_str}
+            Entity Files: {self.files}
+            File Parameters: {self.file_parameters}
+            Incoming Entities:
+            {entities_str}
+            Key Prefixing Enabled: {self.key_prefixing_enabled}
+            """)

smartsim/entity/entity.py

@@ -34,9 +34,6 @@
 if t.TYPE_CHECKING:
     from smartsim.launchable.job import Job
     from smartsim.settings.launchSettings import LaunchSettings
-    from smartsim.types import TODO
-
-    RunSettings = TODO
 
 
 class TelemetryConfiguration:
@@ -97,8 +94,11 @@ def _on_disable(self) -> None:
         to perform actions when attempts to change configuration are made"""
 
 
-class SmartSimEntity:
-    def __init__(self, name: str) -> None:
+class SmartSimEntity(abc.ABC):
+    def __init__(
+        self,
+        name: str,
+    ) -> None:
         """Initialize a SmartSim entity.
 
         Each entity must have a name and path. All entities within SmartSim
@@ -107,17 +107,20 @@ def __init__(self, name: str) -> None:
         :param name: Name of the entity
         """
         self.name = name
+        """The name of the application"""
+
+    @abc.abstractmethod
+    def as_executable_sequence(self) -> t.Sequence[str]:
+        """Converts the executable and its arguments into a sequence of program arguments.
+
+        :return: a sequence of strings representing the executable and its arguments
+        """
 
     @property
     def type(self) -> str:
         """Return the name of the class"""
         return type(self).__name__
 
-    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) -> str:
         return self.name
 

smartsim/entity/entityList.py

@@ -41,9 +41,8 @@
 class EntitySequence(t.Generic[_T_co]):
     """Abstract class for containers for SmartSimEntities"""
 
-    def __init__(self, name: str, path: str, **kwargs: t.Any) -> None:
+    def __init__(self, name: str, **kwargs: t.Any) -> None:
         self.name: str = name
-        self.path: str = path
 
         # >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
         # WARNING: This class cannot be made truly covariant until the
@@ -105,11 +104,6 @@ def type(self) -> str:
         """Return the name of the class"""
         return type(self).__name__
 
-    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: str) -> t.Optional[_T_co]:
         for entity in self.entities:
             if entity.name == name:
@@ -127,8 +121,8 @@ def __len__(self) -> int:
 class EntityList(EntitySequence[_T]):
     """An invariant subclass of an ``EntitySequence`` with mutable containers"""
 
-    def __init__(self, name: str, path: str, **kwargs: t.Any) -> None:
-        super().__init__(name=name, path=path, **kwargs)
+    def __init__(self, name: str, **kwargs: t.Any) -> None:
+        super().__init__(name=name, **kwargs)
         # Change container types to be invariant ``list``s
         self.entities: t.List[_T] = list(self.entities)
         self._fs_models: t.List["smartsim.entity.FSModel"] = list(self._fs_models)

smartsim/experiment.py

@@ -49,7 +49,6 @@
 from .log import ctx_exp_path, get_logger, method_contextualizer
 
 if t.TYPE_CHECKING:
-    from smartsim._core.utils.launcher import ExecutableProtocol
     from smartsim.launchable.job import Job
     from smartsim.types import LaunchedJobID
 
@@ -189,12 +188,7 @@ def _dispatch(
         def execute_dispatch(generator: Generator, job: Job, idx: int) -> LaunchedJobID:
             args = job.launch_settings.launch_args
             env = job.launch_settings.env_vars
-            # >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
-            # FIXME: Remove this cast after `SmartSimEntity` conforms to
-            #        protocol. For now, live with the "dangerous" type cast
-            # ---------------------------------------------------------------------
-            exe = t.cast("ExecutableProtocol", job.entity)
-            # <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
+            exe = job.entity.as_executable_sequence()
             dispatch = dispatcher.get_dispatch(args)
             try:
                 # Check to see if one of the existing launchers can be

smartsim/settings/arguments/launch/lsf.py

@@ -33,7 +33,6 @@
 from smartsim._core.arguments.shell import ShellLaunchArguments
 from smartsim._core.dispatch import EnvironMappingType, dispatch
 from smartsim._core.shell.shellLauncher import ShellLauncher, ShellLauncherCommand
-from smartsim._core.utils.launcher import ExecutableProtocol
 from smartsim.log import get_logger
 
 from ...common import set_check_input
@@ -44,7 +43,7 @@
 
 def _as_jsrun_command(
     args: ShellLaunchArguments,
-    exe: ExecutableProtocol,
+    exe: t.Sequence[str],
     path: pathlib.Path,
     env: EnvironMappingType,
     stdout_path: pathlib.Path,
@@ -56,7 +55,7 @@ def _as_jsrun_command(
         f"--stdio_stdout={stdout_path}",
         f"--stdio_stderr={stderr_path}",
         "--",
-        *exe.as_program_arguments(),
+        *exe,
     )
     return ShellLauncherCommand(
         env, path, subprocess.DEVNULL, subprocess.DEVNULL, command_tuple