Skip to content

Commit

Permalink
Address some reviewer feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
MattToast committed Jul 24, 2024
1 parent f0bf80f commit 7a0b893
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 9 deletions.
4 changes: 4 additions & 0 deletions smartsim/experiment.py
Original file line number Diff line number Diff line change
Expand Up @@ -199,11 +199,15 @@ def _start(job: Job) -> LaunchedJobID:
# <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
dispatch = dispatcher.get_dispatch(args)
try:
# Check to see if one of the existing launchers can be
# configured to handle the launch arguments ...
launch_config = dispatch.configure_first_compatible_launcher(
from_available_launchers=self._active_launchers,
with_settings=args,
)
except errors.LauncherNotFoundError:
# ... otherwise create a new launcher that _can_ handle the
# launch arguments and configure _that_ one
launch_config = dispatch.create_new_launcher_configuration(
for_experiment=self, with_settings=args
)
Expand Down
19 changes: 12 additions & 7 deletions smartsim/settings/arguments/launchArguments.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
from __future__ import annotations

import copy
import textwrap
import typing as t
from abc import ABC, abstractmethod

Expand All @@ -38,16 +39,15 @@


class LaunchArguments(ABC):
"""Abstract base class that defines all generic launcher
argument methods that are not supported. It is the
responsibility of child classes for each launcher to translate
the input parameter to a properly formatted launcher argument.
"""Abstract base class for launcher arguments. It is the responsibility of
child classes for each launcher to add methods to set input parameters and
to maintain valid state between parameters set by a user.
"""

def __init__(self, launch_args: t.Dict[str, str | None] | None) -> None:
"""Initialize a new `LaunchArguments` instance.
:param launch_args: A mapping of arguments to values to pre-initialize
:param launch_args: A mapping of arguments to (optional) values
"""
self._launch_args = copy.deepcopy(launch_args) or {}

Expand Down Expand Up @@ -95,5 +95,10 @@ def format_env_vars(
return None

def __str__(self) -> str: # pragma: no-cover
string = f"\nLaunch Arguments:\n{fmt_dict(self._launch_args)}"
return string
return textwrap.dedent(f"""\
Launch Arguments:
Launcher: {self.launcher_str()}
Name: {type(self).__name__}
Arguments:
{fmt_dict(self._launch_args)}
""")
35 changes: 33 additions & 2 deletions smartsim/settings/dispatch.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ def get_dispatch(
dispatch_ = self._dispatch_registry.get(args, None)
if dispatch_ is None:
raise TypeError(
f"No dispatch for `{type(args).__name__}` has been registered "
f"No dispatch for `{args.__name__}` has been registered "
f"has been registered with {type(self).__name__} `{self}`"
)
# Note the sleight-of-hand here: we are secretly casting a type of
Expand All @@ -160,24 +160,45 @@ def get_dispatch(
@t.final
@dataclasses.dataclass(frozen=True)
class _DispatchRegistration(t.Generic[_DispatchableT, _LaunchableT]):
"""An entry into the `Dispatcher`'s dispatch registry. This class is simply
a wrapper around a launcher and how to format a `_DispatchableT` instance
to be launched by the afore mentioned launcher.
"""

formatter: _FormatterType[_DispatchableT, _LaunchableT]
launcher_type: type[LauncherProtocol[_LaunchableT]]

def _is_compatible_launcher(self, launcher: LauncherProtocol[t.Any]) -> bool:
# Disabling because we want to match the the type of the dispatch
# Disabling because we want to match the type of the dispatch
# *exactly* as specified by the user
# pylint: disable-next=unidiomatic-typecheck
return type(launcher) is self.launcher_type

def create_new_launcher_configuration(
self, for_experiment: Experiment, with_settings: _DispatchableT
) -> _LaunchConfigType:
"""Create a new instance of a launcher for an experiment that the
provided settings where set to dispatch to, and configure it with the
provided launch settings.
:param for_experiment: The experiment responsible creating the launcher
:param with_settings: The settings with which to configure the newly
created launcher
:returns: A configured launcher
"""
launcher = self.launcher_type.create(for_experiment)
return self.create_adapter_from_launcher(launcher, with_settings)

def create_adapter_from_launcher(
self, launcher: LauncherProtocol[_LaunchableT], settings: _DispatchableT
) -> _LaunchConfigType:
"""Creates configured launcher from an existing launcher using the provided settings
:param launcher: A launcher that the type of `settings` has been
configured to dispatch to.
:param settings: A settings with which to configure the launcher.
:returns: A configured launcher.
"""
if not self._is_compatible_launcher(launcher):
raise TypeError(
f"Cannot create launcher adapter from launcher `{launcher}` "
Expand All @@ -195,6 +216,16 @@ def configure_first_compatible_launcher(
with_settings: _DispatchableT,
from_available_launchers: t.Iterable[LauncherProtocol[t.Any]],
) -> _LaunchConfigType:
"""Configure the first compatible adapter launch to launch with the
provided settings. Launchers are iterated and discarded from the
iterator until the iterator is exhausted.
:param with_settings: The settings with which to configure the launcher
:param from_available_launchers: An iterable that yields launcher instances
:raises errors.LauncherNotFoundError: No compatible launcher was
yielded from the provided iterator.
:returns: A launcher configured with the provided settings.
"""
launcher = helpers.first(self._is_compatible_launcher, from_available_launchers)
if launcher is None:
raise errors.LauncherNotFoundError(
Expand Down

0 comments on commit 7a0b893

Please sign in to comment.