From 4453c2afb24a68eaf1f0819d3bb32c9a7962c401 Mon Sep 17 00:00:00 2001 From: Matt Drozt Date: Wed, 22 May 2024 13:09:06 -0700 Subject: [PATCH 01/19] Initial implementation of a compound entity and rough draft of ensemble --- smartsim/entity/entity.py | 105 +++++++++++++++++++++++++++++++++- smartsim/entity/strategies.py | 96 ++++++++++++++++++++++++------- 2 files changed, 179 insertions(+), 22 deletions(-) diff --git a/smartsim/entity/entity.py b/smartsim/entity/entity.py index b68ea017fd..8580ddc4cc 100644 --- a/smartsim/entity/entity.py +++ b/smartsim/entity/entity.py @@ -24,13 +24,60 @@ # 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 abc +import copy +import itertools +import pathlib import typing as t +from smartsim.entity import strategies +from smartsim.entity.files import EntityFiles + if t.TYPE_CHECKING: - # pylint: disable-next=unused-import + import os + import smartsim.settings.base +# >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> +# TODO: Mocks to be removed later +# ------------------------------------------------------------------------------ +# pylint: disable=multiple-statements + + +class _Mock: + def __init__(self, *_: t.Any, **__: t.Any): ... + def __getattr__(self, _: str) -> "_Mock": + return _Mock() + + +# Remove with merge of #579 +# https://github.com/CrayLabs/SmartSim/pull/579 +class Application(_Mock): ... + + +# Remove with merge of #603 +# https://github.com/CrayLabs/SmartSim/pull/603 +class Job(_Mock): ... + + +# Remove with merge of #599 +# https://github.com/CrayLabs/SmartSim/pull/599 +class JobGroup(_Mock): ... + + +# Remove with merge of #587 +# https://github.com/CrayLabs/SmartSim/pull/587 +class LaunchSettings(_Mock): ... + + +# pylint: enable=multiple-statements +# <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< + + class TelemetryConfiguration: """A base class for configuraing telemetry production behavior on existing `SmartSimEntity` subclasses. Any class that will have @@ -118,3 +165,59 @@ def set_path(self, path: str) -> None: def __repr__(self) -> str: return self.name + + +class CompoundEntity(abc.ABC): + @abc.abstractmethod + def as_jobs(self, settings: LaunchSettings) -> t.Collection[Job]: ... + def as_job_group(self, settings: LaunchSettings) -> JobGroup: + return JobGroup(self.as_jobs(settings)) + + +# TODO: If we like this design, we need to: +# 1) Move this to the `smartsim._core.entity.ensemble` module +# 2) Decide what to do witht the original `Ensemble` impl +class Ensemble(CompoundEntity): + def __init__( + self, + name: str, + exe: str | os.PathLike[str], + exe_args: t.Sequence[str] | None = None, + files: EntityFiles | None = None, + parameters: t.Mapping[str, t.Sequence[str]] | None = None, + permutation_strategy: str | strategies.TPermutationStrategy = "all_perm", + max_permutations: int = 0, + replicas: int = 1, + ) -> None: + self.name = name + self.exe = pathlib.Path(exe) + self.exe_args = list(exe_args) if exe_args else [] + self.files = copy.deepcopy(files) if files else EntityFiles() + self.parameters = dict(parameters) if parameters else {} + self.permutation_strategy = permutation_strategy + self.max_permutations = max_permutations + self.replicas = replicas + + def _create_applications(self) -> tuple[Application, ...]: + permutation_strategy = strategies.resolve(self.permutation_strategy) + permutations = permutation_strategy(self.parameters, self.max_permutations) + permutations = permutations if permutations else [{}] + permutations_ = itertools.chain.from_iterable( + itertools.repeat(permutation, self.replicas) for permutation in permutations + ) + return tuple( + Application( + name=f"{self.name}-{i}", + exe=self.exe, + exe_args=self.exe_args, + files=self.files, + params=permutation, + ) + for i, permutation in enumerate(permutations_) + ) + + def as_jobs(self, settings: LaunchSettings) -> tuple[Job, ...]: + apps = self._create_applications() + if not apps: + raise ValueError("There are no members as part of this ensemble") + return tuple(Job(app, settings) for app in apps) diff --git a/smartsim/entity/strategies.py b/smartsim/entity/strategies.py index 2af88b58e7..f8f91fa044 100644 --- a/smartsim/entity/strategies.py +++ b/smartsim/entity/strategies.py @@ -25,40 +25,94 @@ # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. # Generation Strategies + +import itertools import random import typing as t -from itertools import product + +from smartsim.error import errors + +TPermutationStrategy = t.Callable[ + [t.Mapping[str, t.Sequence[str]], int], list[dict[str, str]] +] + +_REGISTERED_STRATEGIES: t.Final[dict[str, TPermutationStrategy]] = {} + + +def _register(name: str) -> t.Callable[ + [TPermutationStrategy], + TPermutationStrategy, +]: + def _impl(fn: TPermutationStrategy) -> TPermutationStrategy: + if name in _REGISTERED_STRATEGIES: + raise ValueError( + f"A strategy with the name '{name}' has already been registered" + ) + _REGISTERED_STRATEGIES[name] = fn + return fn + + return _impl + + +def resolve(strategy: str | TPermutationStrategy) -> TPermutationStrategy: + if callable(strategy): + return _make_safe_custom_strategy(strategy) + try: + return _REGISTERED_STRATEGIES[strategy] + except KeyError: + raise ValueError( + f"Failed to find an ensembling strategy by the name of '{strategy}'." + f"All known strategies are:\n{', '.join(_REGISTERED_STRATEGIES)}" + ) from None + + +def _make_safe_custom_strategy(fn: TPermutationStrategy) -> TPermutationStrategy: + def _impl( + params: t.Mapping[str, t.Sequence[str]], n_permutations: int + ) -> list[dict[str, str]]: + try: + permutations = fn(params, n_permutations) + except Exception as e: + raise errors.UserStrategyError(str(fn)) from e + if not isinstance(permutations, list) or not all( + isinstance(permutation, dict) for permutation in permutations + ): + raise errors.UserStrategyError(str(fn)) + return permutations + + return _impl # create permutations of all parameters # single model if parameters only have one value +@_register("all_perm") 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 permutation in perms: - temp_model = dict(zip(param_names, permutation)) - all_permutations.append(temp_model) - return all_permutations + params: t.Mapping[str, t.Sequence[str]], + _n_permutations: int = 0, + # ^^^^^^^^^^^^^ + # TODO: Really don't like that this attr is ignored, but going to leave it + # as the original impl for now. Will change if requested! +) -> list[dict[str, str]]: + permutations = itertools.product(*params.values()) + return [dict(zip(params, permutation)) for permutation in permutations] +@_register("step") 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 param_value in zip(*param_values): - permutations.append(dict(zip(param_names, param_value))) - return permutations + params: t.Mapping[str, t.Sequence[str]], _n_permutations: int = 0 +) -> list[dict[str, str]]: + steps = zip(*params.values()) + return [dict(zip(params, step)) for step in steps] +@_register("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]]: - permutations = create_all_permutations(param_names, param_values) + params: t.Mapping[str, t.Sequence[str]], n_permutations: int = 0 +) -> list[dict[str, str]]: + permutations = create_all_permutations(params, 0) - # sample from available permutations if n_models is specified - if n_models and n_models < len(permutations): - permutations = random.sample(permutations, n_models) + # sample from available permutations if n_permutations is specified + if 0 < n_permutations < len(permutations): + permutations = random.sample(permutations, n_permutations) return permutations From 3b5a42078eb7fcc255b350e5beb2bff1f63ce9c1 Mon Sep 17 00:00:00 2001 From: Matt Drozt Date: Wed, 29 May 2024 13:54:25 -0700 Subject: [PATCH 02/19] Add tests --- tests/test_ensemble.py | 132 +++++++++++++++++++++++++++ tests/test_permutation_strategies.py | 126 +++++++++++++++++++++++++ 2 files changed, 258 insertions(+) create mode 100644 tests/test_ensemble.py create mode 100644 tests/test_permutation_strategies.py diff --git a/tests/test_ensemble.py b/tests/test_ensemble.py new file mode 100644 index 0000000000..c3f04c8d86 --- /dev/null +++ b/tests/test_ensemble.py @@ -0,0 +1,132 @@ +# BSD 2-Clause License +# +# Copyright (c) 2021-2024, Hewlett Packard Enterprise +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are met: +# +# 1. Redistributions of source code must retain the above copyright notice, this +# list of conditions and the following disclaimer. +# +# 2. Redistributions in binary form must reproduce the above copyright notice, +# this list of conditions and the following disclaimer in the documentation +# and/or other materials provided with the distribution. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE +# DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE +# FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL +# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR +# SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER +# 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 operator +import itertools + +import pytest + +from smartsim.entity import entity +from smartsim.entity.entity import Ensemble +from smartsim.entity import strategies +from smartsim.error import errors + +pytestmark = pytest.mark.group_a + +_PARAMS = {"SPAM": ["a", "b"], "EGGS": ["c", "d"]} +_2_PERM_STRAT = lambda p, n: [{"SPAM": "a", "EGGS": "b"}, {"SPAM": "c", "EGGS": "d"}] + + +@pytest.fixture +def mock_launcher_settings(): + # TODO: Remove this mock with #587 + # https://github.com/CrayLabs/SmartSim/pull/587 + return entity.LaunchSettings() + + +# fmt: off +@pytest.mark.parametrize( + " params, strategy, max_perms, replicas, expected_num_jobs", # Test Name Misc + (pytest.param( None, "all_perm", 0, 1, 1 , id="No Parameters or Replicas") , + pytest.param(_PARAMS, "all_perm", 0, 1, 4 , id="All Permutations") , + pytest.param(_PARAMS, "step", 0, 1, 2 , id="Stepped Params") , + pytest.param(_PARAMS, "random", 0, 1, 4 , id="Random Permutations") , + pytest.param(_PARAMS, "all_perm", 1, 1, 1 , id="All Permutations [Capped Max Permutations]" , marks=pytest.mark.xfail(reason="'all_perm' strategy currently ignores max number permutations'", strict=True)), + pytest.param(_PARAMS, "step", 1, 1, 1 , id="Stepped Params [Capped Max Permutations]" , marks=pytest.mark.xfail(reason="'step' strategy currently ignores max number permutations'", strict=True)), + pytest.param(_PARAMS, "random", 1, 1, 1 , id="Random Permutations [Capped Max Permutations]"), # ^^^^^^^^^^^^^^^^^ + pytest.param( {}, _2_PERM_STRAT, 0, 1, 2 , id="Custom Permutation Strategy") , # TODO: I would argue that we should make these cases pass + pytest.param( {}, "all_perm", 0, 5, 5 , id="Identical Replicas") , + pytest.param(_PARAMS, "all_perm", 0, 2, 8 , id="Replicas of All Permutations") , + pytest.param(_PARAMS, "step", 0, 2, 4 , id="Replicas of Stepped Params") , + pytest.param(_PARAMS, "random", 3, 2, 6 , id="Replicas of Random Permutations") , +)) +# fmt: on +def test_expected_number_of_apps_created( + # Parameterized + params, + strategy, + max_perms, + replicas, + expected_num_jobs, + # Other fixtures + mock_launcher_settings, +): + jobs = Ensemble( + "test_ensemble", + "echo", + ("hello", "world"), + parameters=params, + permutation_strategy=strategy, + max_permutations=max_perms, + replicas=replicas, + ).as_jobs(mock_launcher_settings) + assert len(jobs) == expected_num_jobs + + +def test_ensemble_without_any_members_rasies_when_cast_to_jobs(mock_launcher_settings): + with pytest.raises(ValueError): + Ensemble( + "test_ensemble", + "echo", + ("hello",), + permutation_strategy=lambda p, n: [{}], + replicas=0, + ).as_jobs(mock_launcher_settings) + + +def test_strategy_error_raised_if_a_strategy_that_dne_is_requested(): + with pytest.raises(ValueError): + Ensemble( + "test_ensemble", + "echo", + ("hello",), + permutation_strategy="THIS-STRATEGY-DNE", + )._create_applications() + + +@pytest.mark.xfail( + reason="This needs an implementation for `Application` to be tested", strict=True +) +@pytest.mark.parametrize( + "params", + ( + pytest.param({"SPAM": ["eggs"]}, id="Non-Empty Params"), + pytest.param({}, id="Empty Params"), + pytest.param(None, id="Nullish Params"), + ), +) +def test_replicated_applications_have_eq_deep_copies_of_parameters(params): + apps = Ensemble( + "test_ensemble", "echo", ("hello",), replicas=2, parameters=params + )._create_applications() + assert len(apps) >= 2 # Sanitiy check to make sure the test is valid + assert all(app_1.param == app_2.param for app_1, app_2 in itertools.pairwise(apps)) + assert all( + app_1.params is not app_2.params + for app_1 in apps + for app_2 in apps + if app_1 is not app_2 + ) diff --git a/tests/test_permutation_strategies.py b/tests/test_permutation_strategies.py new file mode 100644 index 0000000000..82867246a5 --- /dev/null +++ b/tests/test_permutation_strategies.py @@ -0,0 +1,126 @@ +# BSD 2-Clause License +# +# Copyright (c) 2021-2024, Hewlett Packard Enterprise +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are met: +# +# 1. Redistributions of source code must retain the above copyright notice, this +# list of conditions and the following disclaimer. +# +# 2. Redistributions in binary form must reproduce the above copyright notice, +# this list of conditions and the following disclaimer in the documentation +# and/or other materials provided with the distribution. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE +# DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE +# FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL +# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR +# SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER +# 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 pytest + +from smartsim.entity import strategies +from smartsim.error import errors + +pytestmark = pytest.mark.group_a + +def test_strategy_registration(monkeypatch): + monkeypatch.setattr(strategies, "_REGISTERED_STRATEGIES", {}) + assert not strategies._REGISTERED_STRATEGIES + + new_strat = lambda params, nmax: [] + strategies._register("new_strat")(new_strat) + assert strategies._REGISTERED_STRATEGIES == {"new_strat": new_strat} + + +def test_strategies_cannot_be_overwritten(monkeypatch): + monkeypatch.setattr( + strategies, "_REGISTERED_STRATEGIES", {"some-strategy": lambda params, nmax: []} + ) + with pytest.raises(ValueError): + strategies._register("some-strategy")(lambda params, nmax: []) + + +def test_strategy_retreval(monkeypatch): + new_strat_a = lambda params, nmax: [] + new_strat_b = lambda params, nmax: [] + + monkeypatch.setattr( + strategies, + "_REGISTERED_STRATEGIES", + {"new_strat_a": new_strat_a, "new_strat_b": new_strat_b}, + ) + assert strategies.resolve("new_strat_a") == new_strat_a + assert strategies.resolve("new_strat_b") == new_strat_b + + +def test_user_strategy_error_raised_when_attempting_to_get_unknown_strat(): + with pytest.raises(ValueError): + strategies.resolve("NOT-REGISTERED") + + +def broken_strategy(p, n): + raise Exception("This custom strategy raised an error") + + +@pytest.mark.parametrize( + "strategy", + ( + pytest.param(broken_strategy, id="Strategy raises during execution"), + pytest.param(lambda params, nmax: 123, id="Does not return a list"), + pytest.param( + lambda params, nmax: [1, 2, 3], id="Does not return a list of dicts" + ), + ), +) +def test_custom_strategy_raises_user_strategy_error_if_something_goes_wrong(strategy): + with pytest.raises(errors.UserStrategyError): + strategies.resolve(strategy)({"SPAM": ["EGGS"]}, 123) + + +@pytest.mark.parametrize( + "strategy, expected_output", + ( + pytest.param( + strategies.create_all_permutations, + ( + {"SPAM": "a", "EGGS": "c"}, + {"SPAM": "a", "EGGS": "d"}, + {"SPAM": "b", "EGGS": "c"}, + {"SPAM": "b", "EGGS": "d"}, + ), + id="All Permutations", + ), + pytest.param( + strategies.step_values, + ( + {"SPAM": "a", "EGGS": "c"}, + {"SPAM": "b", "EGGS": "d"}, + ), + id="Step Values", + ), + pytest.param( + strategies.random_permutations, + ( + {"SPAM": "a", "EGGS": "c"}, + {"SPAM": "a", "EGGS": "d"}, + {"SPAM": "b", "EGGS": "c"}, + {"SPAM": "b", "EGGS": "d"}, + ), + id="Uncapped Random Permutations", + ), + ), +) +def test_strategy_returns_expected_set(strategy, expected_output): + params = {"SPAM": ["a", "b"], "EGGS": ["c", "d"]} + output = list(strategy(params)) + assert len(output) == len(expected_output) + assert all(item in expected_output for item in output) + assert all(item in output for item in expected_output) From 84e02dc86840cedb67b9947270041337eaa2fa31 Mon Sep 17 00:00:00 2001 From: Matt Drozt Date: Wed, 29 May 2024 14:01:50 -0700 Subject: [PATCH 03/19] Existing ensemble impl follows new API --- smartsim/entity/ensemble.py | 44 +++++-------------------------------- 1 file changed, 5 insertions(+), 39 deletions(-) diff --git a/smartsim/entity/ensemble.py b/smartsim/entity/ensemble.py index e3ddf38cf4..74ce32af12 100644 --- a/smartsim/entity/ensemble.py +++ b/smartsim/entity/ensemble.py @@ -45,14 +45,12 @@ from .entity import SmartSimEntity from .entityList import EntityList from .model import Model -from .strategies import create_all_permutations, random_permutations, step_values +from .strategies import ( + create_all_permutations, random_permutations, step_values, TPermutationStrategy +) 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[Model]): """``Ensemble`` is a group of ``Model`` instances that can @@ -125,11 +123,9 @@ def _initialize_entities(self, **kwargs: t.Any) -> None: # the ensemble and assign run_settings to each member if self.params: if self.run_settings and self.exe: - param_names, params = self._read_model_parameters() - # Compute all combinations of model parameters and arguments n_models = kwargs.get("n_models", 0) - all_model_params = strategy(param_names, params, n_models) + all_model_params = strategy(self.params, n_models) if not isinstance(all_model_params, list): raise UserStrategyError(strategy) @@ -297,7 +293,7 @@ def print_attached_files(self) -> None: print(self.attached_files_table) @staticmethod - def _set_strategy(strategy: str) -> StrategyFunction: + def _set_strategy(strategy: str) -> TPermutationStrategy: """Set the permutation strategy for generating models within the ensemble @@ -317,36 +313,6 @@ def _set_strategy(strategy: str) -> StrategyFunction: f"Permutation strategy given is not supported: {strategy}" ) - 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 - - :raises TypeError: if params are of the wrong type - :return: param names and values for permutation strategy - """ - - if not isinstance(self.params, dict): - raise TypeError( - "Ensemble initialization argument 'params' must be of type dict" - ) - - param_names: t.List[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([str(val)]) - else: - raise TypeError( - "Incorrect type for ensemble parameters\n" - + "Must be list, int, or string." - ) - return param_names, parameters - def add_ml_model( self, name: str, From 844f714a12ae3ef054ec734a9532ca2006100060 Mon Sep 17 00:00:00 2001 From: Matt Drozt Date: Wed, 29 May 2024 15:15:39 -0700 Subject: [PATCH 04/19] Style --- smartsim/entity/ensemble.py | 5 ++++- tests/test_ensemble.py | 4 ---- tests/test_permutation_strategies.py | 1 + 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/smartsim/entity/ensemble.py b/smartsim/entity/ensemble.py index 74ce32af12..fb343627c4 100644 --- a/smartsim/entity/ensemble.py +++ b/smartsim/entity/ensemble.py @@ -46,7 +46,10 @@ from .entityList import EntityList from .model import Model from .strategies import ( - create_all_permutations, random_permutations, step_values, TPermutationStrategy + TPermutationStrategy, + create_all_permutations, + random_permutations, + step_values, ) logger = get_logger(__name__) diff --git a/tests/test_ensemble.py b/tests/test_ensemble.py index c3f04c8d86..ab7202e250 100644 --- a/tests/test_ensemble.py +++ b/tests/test_ensemble.py @@ -24,15 +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 operator import itertools import pytest -from smartsim.entity import entity from smartsim.entity.entity import Ensemble -from smartsim.entity import strategies -from smartsim.error import errors pytestmark = pytest.mark.group_a diff --git a/tests/test_permutation_strategies.py b/tests/test_permutation_strategies.py index 82867246a5..a86c5a9629 100644 --- a/tests/test_permutation_strategies.py +++ b/tests/test_permutation_strategies.py @@ -31,6 +31,7 @@ pytestmark = pytest.mark.group_a + def test_strategy_registration(monkeypatch): monkeypatch.setattr(strategies, "_REGISTERED_STRATEGIES", {}) assert not strategies._REGISTERED_STRATEGIES From 338186a809fa113ae2743dfda9fbbb4e3a53135a Mon Sep 17 00:00:00 2001 From: Matt Drozt Date: Wed, 29 May 2024 15:40:30 -0700 Subject: [PATCH 05/19] Whoops --- smartsim/entity/strategies.py | 2 ++ tests/test_ensemble.py | 1 + 2 files changed, 3 insertions(+) diff --git a/smartsim/entity/strategies.py b/smartsim/entity/strategies.py index f8f91fa044..f351d50714 100644 --- a/smartsim/entity/strategies.py +++ b/smartsim/entity/strategies.py @@ -26,6 +26,8 @@ # Generation Strategies +from __future__ import annotations + import itertools import random import typing as t diff --git a/tests/test_ensemble.py b/tests/test_ensemble.py index ab7202e250..3006e8c76f 100644 --- a/tests/test_ensemble.py +++ b/tests/test_ensemble.py @@ -28,6 +28,7 @@ import pytest +from smartsim.entity import entity from smartsim.entity.entity import Ensemble pytestmark = pytest.mark.group_a From e278ef801a53220bf79a855c7f0dd933cf729600 Mon Sep 17 00:00:00 2001 From: Matt Drozt Date: Fri, 31 May 2024 20:20:38 -0700 Subject: [PATCH 06/19] More linting --- smartsim/entity/ensemble.py | 16 ++++++---------- smartsim/entity/strategies.py | 7 ++++--- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/smartsim/entity/ensemble.py b/smartsim/entity/ensemble.py index 15be943129..def6364e08 100644 --- a/smartsim/entity/ensemble.py +++ b/smartsim/entity/ensemble.py @@ -32,7 +32,6 @@ from tabulate import tabulate from .._core._install.builder import Device -from .._core.utils.helpers import expand_exe_path from ..error import ( EntityExistsError, SmartSimError, @@ -45,13 +44,7 @@ from .entity import SmartSimEntity from .entityList import EntityList from .model import Application -from .strategies import ( - TPermutationStrategy, - create_all_permutations, - random_permutations, -) from .strategies import resolve as resolve_strategy -from .strategies import step_values logger = get_logger(__name__) @@ -174,7 +167,8 @@ def _initialize_entities(self, **kwargs: t.Any) -> None: ) application.enable_key_prefixing() logger.debug( - f"Created ensemble member: {application_name} in {self.name}" + "Created ensemble member: " + f"{application_name} in {self.name}" ) self.add_application(application) else: @@ -200,7 +194,8 @@ def add_application(self, application: Application) -> None: """ if not isinstance(application, Application): raise TypeError( - f"Argument to add_application was of type {type(application)}, not Application" + f"Argument to add_application was of type {type(application)}, " + " not Application" ) # "in" operator uses application name for __eq__ if application in self.entities: @@ -237,7 +232,8 @@ def enable_key_prefixing(self) -> None: application.enable_key_prefixing() def query_key_prefixing(self) -> bool: - """Inquire as to whether each application within the ensemble will prefix their keys + """Inquire as to whether each application within the ensemble will + prefix their keys :returns: True if all applications have key prefixing enabled, False otherwise """ diff --git a/smartsim/entity/strategies.py b/smartsim/entity/strategies.py index d9374e787e..08251949ea 100644 --- a/smartsim/entity/strategies.py +++ b/smartsim/entity/strategies.py @@ -28,6 +28,7 @@ from __future__ import annotations +import functools import itertools import random import typing as t @@ -47,9 +48,8 @@ def _register(name: str) -> t.Callable[ ]: def _impl(fn: TPermutationStrategy) -> TPermutationStrategy: if name in _REGISTERED_STRATEGIES: - raise ValueError( - f"A strategy with the name '{name}' has already been registered" - ) + msg = f"A strategy with the name '{name}' has already been registered" + raise ValueError(msg) _REGISTERED_STRATEGIES[name] = fn return fn @@ -69,6 +69,7 @@ def resolve(strategy: str | TPermutationStrategy) -> TPermutationStrategy: def _make_safe_custom_strategy(fn: TPermutationStrategy) -> TPermutationStrategy: + @functools.wraps(fn) def _impl( params: t.Mapping[str, t.Sequence[str]], n_permutations: int ) -> list[dict[str, str]]: From 502415472f8cb9fa246f7544e7153fa45059c4d0 Mon Sep 17 00:00:00 2001 From: Matt Drozt Date: Fri, 31 May 2024 20:21:57 -0700 Subject: [PATCH 07/19] Fix circular import with new modules --- .pylintrc | 2 +- smartsim/entity/_mock.py | 58 +++++++++++++++++++ smartsim/entity/_new_ensemble.py | 93 ++++++++++++++++++++++++++++++ smartsim/entity/entity.py | 98 ++------------------------------ smartsim/entity/model.py | 9 ++- tests/test_ensemble.py | 43 +++++++------- 6 files changed, 183 insertions(+), 120 deletions(-) create mode 100644 smartsim/entity/_mock.py create mode 100644 smartsim/entity/_new_ensemble.py diff --git a/.pylintrc b/.pylintrc index aa378d0399..34580db3b6 100644 --- a/.pylintrc +++ b/.pylintrc @@ -167,7 +167,7 @@ max-module-lines=1000 # Allow the body of a class to be on the same line as the declaration if body # contains single statement. -single-line-class-stmt=no +single-line-class-stmt=yes # Allow the body of an if to be on the same line as the test if there is no # else. diff --git a/smartsim/entity/_mock.py b/smartsim/entity/_mock.py new file mode 100644 index 0000000000..203c86dc81 --- /dev/null +++ b/smartsim/entity/_mock.py @@ -0,0 +1,58 @@ +# BSD 2-Clause License +# +# Copyright (c) 2021-2024, Hewlett Packard Enterprise +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are met: +# +# 1. Redistributions of source code must retain the above copyright notice, this +# list of conditions and the following disclaimer. +# +# 2. Redistributions in binary form must reproduce the above copyright notice, +# this list of conditions and the following disclaimer in the documentation +# and/or other materials provided with the distribution. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE +# DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE +# FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL +# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR +# SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER +# 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. + +"""This module contains stubs of functionality that is not currently +implemented. + +THIS WHOLE MODULE SHOULD BE REMOVED IN FUTURE!! +""" + +from __future__ import annotations + +import typing as t + + +class Mock: + """Base mock class""" + + def __init__(self, *_: t.Any, **__: t.Any): ... + def __getattr__(self, _: str) -> Mock: + return Mock() + + +# Remove with merge of #603 +# https://github.com/CrayLabs/SmartSim/pull/603 +class Job(Mock): ... + + +# Remove with merge of #599 +# https://github.com/CrayLabs/SmartSim/pull/599 +class JobGroup(Mock): ... + + +# Remove with merge of #587 +# https://github.com/CrayLabs/SmartSim/pull/587 +class LaunchSettings(Mock): ... diff --git a/smartsim/entity/_new_ensemble.py b/smartsim/entity/_new_ensemble.py new file mode 100644 index 0000000000..b6281c4631 --- /dev/null +++ b/smartsim/entity/_new_ensemble.py @@ -0,0 +1,93 @@ +# BSD 2-Clause License +# +# Copyright (c) 2021-2024, Hewlett Packard Enterprise +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are met: +# +# 1. Redistributions of source code must retain the above copyright notice, this +# list of conditions and the following disclaimer. +# +# 2. Redistributions in binary form must reproduce the above copyright notice, +# this list of conditions and the following disclaimer in the documentation +# and/or other materials provided with the distribution. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE +# DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE +# FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL +# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR +# SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER +# 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 copy +import itertools +import os +import typing as t + +# TODO: We are using the mocks defined `mock` module. We should use the correct +# module when these types are actually defined when PRs #587 and #603 are +# merged. +# https://github.com/CrayLabs/SmartSim/pull/587 +# https://github.com/CrayLabs/SmartSim/pull/603 +from smartsim.entity import _mock, entity, strategies +from smartsim.entity.files import EntityFiles +from smartsim.entity.model import Application + + +# TODO: If we like this design, we need to: +# 1) Move this to the `smartsim._core.entity.ensemble` module +# 2) Decide what to do witht the original `Ensemble` impl +class Ensemble(entity.CompoundEntity): + def __init__( + self, + name: str, + exe: str | os.PathLike[str], + exe_args: t.Sequence[str] | None = None, + files: EntityFiles | None = None, + parameters: t.Mapping[str, t.Sequence[str]] | None = None, + permutation_strategy: str | strategies.TPermutationStrategy = "all_perm", + max_permutations: int = 0, + replicas: int = 1, + ) -> None: + self.name = name + self.exe = os.fspath(exe) + self.exe_args = list(exe_args) if exe_args else [] + self.files = copy.deepcopy(files) if files else EntityFiles() + self.parameters = dict(parameters) if parameters else {} + self.permutation_strategy = permutation_strategy + self.max_permutations = max_permutations + self.replicas = replicas + + def _create_applications(self) -> tuple[Application, ...]: + permutation_strategy = strategies.resolve(self.permutation_strategy) + permutations = permutation_strategy(self.parameters, self.max_permutations) + permutations = permutations if permutations else [{}] + permutations_ = itertools.chain.from_iterable( + itertools.repeat(permutation, self.replicas) for permutation in permutations + ) + return tuple( + Application( + name=f"{self.name}-{i}", + exe=self.exe, + run_settings=_mock.Mock(), # type: ignore[arg-type] + # ^^^^^^^^^^^^^^^^^^^ + # FIXME: remove this constructor arg! It should not exist!! + exe_args=self.exe_args, + files=self.files, + params=permutation, + ) + for i, permutation in enumerate(permutations_) + ) + + def as_jobs(self, settings: _mock.LaunchSettings) -> tuple[_mock.Job, ...]: + apps = self._create_applications() + if not apps: + raise ValueError("There are no members as part of this ensemble") + return tuple(_mock.Job(app, settings) for app in apps) diff --git a/smartsim/entity/entity.py b/smartsim/entity/entity.py index 77549dc35d..c13cd95afa 100644 --- a/smartsim/entity/entity.py +++ b/smartsim/entity/entity.py @@ -24,52 +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. - from __future__ import annotations import abc -import copy -import itertools -import os import typing as t -from smartsim.entity import strategies -from smartsim.entity.files import EntityFiles -from smartsim.entity.model import Application - if t.TYPE_CHECKING: import smartsim.settings.base - - -# >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> -# TODO: Mocks to be removed later -# ------------------------------------------------------------------------------ -# pylint: disable=multiple-statements - - -class _Mock: - def __init__(self, *_: t.Any, **__: t.Any): ... - def __getattr__(self, _: str) -> "_Mock": - return _Mock() - - -# Remove with merge of #603 -# https://github.com/CrayLabs/SmartSim/pull/603 -class Job(_Mock): ... - - -# Remove with merge of #599 -# https://github.com/CrayLabs/SmartSim/pull/599 -class JobGroup(_Mock): ... - - -# Remove with merge of #587 -# https://github.com/CrayLabs/SmartSim/pull/587 -class LaunchSettings(_Mock): ... - - -# pylint: enable=multiple-statements -# <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< + from smartsim.entity import _mock class TelemetryConfiguration: @@ -163,58 +125,6 @@ def __repr__(self) -> str: class CompoundEntity(abc.ABC): @abc.abstractmethod - def as_jobs(self, settings: LaunchSettings) -> t.Collection[Job]: ... - def as_job_group(self, settings: LaunchSettings) -> JobGroup: - return JobGroup(self.as_jobs(settings)) - - -# TODO: If we like this design, we need to: -# 1) Move this to the `smartsim._core.entity.ensemble` module -# 2) Decide what to do witht the original `Ensemble` impl -class Ensemble(CompoundEntity): - def __init__( - self, - name: str, - exe: str | os.PathLike[str], - exe_args: t.Sequence[str] | None = None, - files: EntityFiles | None = None, - parameters: t.Mapping[str, t.Sequence[str]] | None = None, - permutation_strategy: str | strategies.TPermutationStrategy = "all_perm", - max_permutations: int = 0, - replicas: int = 1, - ) -> None: - self.name = name - self.exe = os.fspath(exe) - self.exe_args = list(exe_args) if exe_args else [] - self.files = copy.deepcopy(files) if files else EntityFiles() - self.parameters = dict(parameters) if parameters else {} - self.permutation_strategy = permutation_strategy - self.max_permutations = max_permutations - self.replicas = replicas - - def _create_applications(self) -> tuple[Application, ...]: - permutation_strategy = strategies.resolve(self.permutation_strategy) - permutations = permutation_strategy(self.parameters, self.max_permutations) - permutations = permutations if permutations else [{}] - permutations_ = itertools.chain.from_iterable( - itertools.repeat(permutation, self.replicas) for permutation in permutations - ) - return tuple( - Application( - name=f"{self.name}-{i}", - exe=self.exe, - run_settings=_Mock(), # type: ignore[arg-type] - # ^^^^^^^^^^^^^^^^^^^ - # FIXME: remove this constructor arg! It should not exist!! - exe_args=self.exe_args, - files=self.files, - params=permutation, - ) - for i, permutation in enumerate(permutations_) - ) - - def as_jobs(self, settings: LaunchSettings) -> tuple[Job, ...]: - apps = self._create_applications() - if not apps: - raise ValueError("There are no members as part of this ensemble") - return tuple(Job(app, settings) for app in apps) + def as_jobs(self, settings: _mock.LaunchSettings) -> t.Collection[_mock.Job]: ... + def as_job_group(self, settings: _mock.LaunchSettings) -> _mock.JobGroup: + return _mock.JobGroup(self.as_jobs(settings)) diff --git a/smartsim/entity/model.py b/smartsim/entity/model.py index 218991581f..9f31c1e011 100644 --- a/smartsim/entity/model.py +++ b/smartsim/entity/model.py @@ -39,11 +39,14 @@ from .._core.utils.helpers import cat_arg_and_value, expand_exe_path from ..error import EntityExistsError, SSUnsupportedError from ..log import get_logger -from ..settings.base import BatchSettings, RunSettings from .dbobject import DBModel, DBScript from .entity import SmartSimEntity from .files import EntityFiles +if t.TYPE_CHECKING: + from ..settings.base import BatchSettings, RunSettings + + logger = get_logger(__name__) @@ -55,12 +58,12 @@ def __init__( self, name: str, exe: str, - run_settings: RunSettings, + run_settings: "RunSettings", params: t.Optional[t.Dict[str, str]] = None, exe_args: t.Optional[t.List[str]] = None, path: t.Optional[str] = getcwd(), params_as_args: t.Optional[t.List[str]] = None, - batch_settings: t.Optional[BatchSettings] = None, + batch_settings: t.Optional["BatchSettings"] = None, files: t.Optional[EntityFiles] = None, ): """Initialize a ``Application`` diff --git a/tests/test_ensemble.py b/tests/test_ensemble.py index 3006e8c76f..32d440ce4a 100644 --- a/tests/test_ensemble.py +++ b/tests/test_ensemble.py @@ -28,12 +28,12 @@ import pytest -from smartsim.entity import entity -from smartsim.entity.entity import Ensemble +from smartsim.entity import _mock +from smartsim.entity._new_ensemble import Ensemble pytestmark = pytest.mark.group_a -_PARAMS = {"SPAM": ["a", "b"], "EGGS": ["c", "d"]} +_2x2_PARAMS = {"SPAM": ["a", "b"], "EGGS": ["c", "d"]} _2_PERM_STRAT = lambda p, n: [{"SPAM": "a", "EGGS": "b"}, {"SPAM": "c", "EGGS": "d"}] @@ -41,24 +41,24 @@ def mock_launcher_settings(): # TODO: Remove this mock with #587 # https://github.com/CrayLabs/SmartSim/pull/587 - return entity.LaunchSettings() + return _mock.LaunchSettings() # fmt: off @pytest.mark.parametrize( - " params, strategy, max_perms, replicas, expected_num_jobs", # Test Name Misc - (pytest.param( None, "all_perm", 0, 1, 1 , id="No Parameters or Replicas") , - pytest.param(_PARAMS, "all_perm", 0, 1, 4 , id="All Permutations") , - pytest.param(_PARAMS, "step", 0, 1, 2 , id="Stepped Params") , - pytest.param(_PARAMS, "random", 0, 1, 4 , id="Random Permutations") , - pytest.param(_PARAMS, "all_perm", 1, 1, 1 , id="All Permutations [Capped Max Permutations]" , marks=pytest.mark.xfail(reason="'all_perm' strategy currently ignores max number permutations'", strict=True)), - pytest.param(_PARAMS, "step", 1, 1, 1 , id="Stepped Params [Capped Max Permutations]" , marks=pytest.mark.xfail(reason="'step' strategy currently ignores max number permutations'", strict=True)), - pytest.param(_PARAMS, "random", 1, 1, 1 , id="Random Permutations [Capped Max Permutations]"), # ^^^^^^^^^^^^^^^^^ - pytest.param( {}, _2_PERM_STRAT, 0, 1, 2 , id="Custom Permutation Strategy") , # TODO: I would argue that we should make these cases pass - pytest.param( {}, "all_perm", 0, 5, 5 , id="Identical Replicas") , - pytest.param(_PARAMS, "all_perm", 0, 2, 8 , id="Replicas of All Permutations") , - pytest.param(_PARAMS, "step", 0, 2, 4 , id="Replicas of Stepped Params") , - pytest.param(_PARAMS, "random", 3, 2, 6 , id="Replicas of Random Permutations") , + " params, strategy, max_perms, replicas, expected_num_jobs", # Test Name Misc + (pytest.param( None, "all_perm", 0, 1, 1 , id="No Parameters or Replicas") , + pytest.param(_2x2_PARAMS, "all_perm", 0, 1, 4 , id="All Permutations") , + pytest.param(_2x2_PARAMS, "step", 0, 1, 2 , id="Stepped Params") , + pytest.param(_2x2_PARAMS, "random", 0, 1, 4 , id="Random Permutations") , + pytest.param(_2x2_PARAMS, "all_perm", 1, 1, 1 , id="All Permutations [Capped Max Permutations]" , marks=pytest.mark.xfail(reason="'all_perm' strategy currently ignores max number permutations'", strict=True)), + pytest.param(_2x2_PARAMS, "step", 1, 1, 1 , id="Stepped Params [Capped Max Permutations]" , marks=pytest.mark.xfail(reason="'step' strategy currently ignores max number permutations'", strict=True)), + pytest.param(_2x2_PARAMS, "random", 1, 1, 1 , id="Random Permutations [Capped Max Permutations]"), # ^^^^^^^^^^^^^^^^^ + pytest.param( {}, _2_PERM_STRAT, 0, 1, 2 , id="Custom Permutation Strategy") , # TODO: I would argue that we should make these cases pass + pytest.param( {}, "all_perm", 0, 5, 5 , id="Identical Replicas") , + pytest.param(_2x2_PARAMS, "all_perm", 0, 2, 8 , id="Replicas of All Permutations") , + pytest.param(_2x2_PARAMS, "step", 0, 2, 4 , id="Replicas of Stepped Params") , + pytest.param(_2x2_PARAMS, "random", 3, 2, 6 , id="Replicas of Random Permutations") , )) # fmt: on def test_expected_number_of_apps_created( @@ -104,9 +104,6 @@ def test_strategy_error_raised_if_a_strategy_that_dne_is_requested(): )._create_applications() -@pytest.mark.xfail( - reason="This needs an implementation for `Application` to be tested", strict=True -) @pytest.mark.parametrize( "params", ( @@ -117,10 +114,12 @@ def test_strategy_error_raised_if_a_strategy_that_dne_is_requested(): ) def test_replicated_applications_have_eq_deep_copies_of_parameters(params): apps = Ensemble( - "test_ensemble", "echo", ("hello",), replicas=2, parameters=params + "test_ensemble", "echo", ("hello",), replicas=4, parameters=params )._create_applications() assert len(apps) >= 2 # Sanitiy check to make sure the test is valid - assert all(app_1.param == app_2.param for app_1, app_2 in itertools.pairwise(apps)) + assert all( + app_1.params == app_2.params for app_1, app_2 in itertools.pairwise(apps) + ) assert all( app_1.params is not app_2.params for app_1 in apps From e34623cabd54a1b1879abd7f387479f80969afdf Mon Sep 17 00:00:00 2001 From: Matt Drozt Date: Mon, 3 Jun 2024 09:42:47 -0700 Subject: [PATCH 08/19] Py3.9 does not have `itertools.pairwise` --- tests/test_ensemble.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/tests/test_ensemble.py b/tests/test_ensemble.py index 32d440ce4a..df4cd1b42a 100644 --- a/tests/test_ensemble.py +++ b/tests/test_ensemble.py @@ -24,8 +24,6 @@ # 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 pytest from smartsim.entity import _mock @@ -113,13 +111,13 @@ def test_strategy_error_raised_if_a_strategy_that_dne_is_requested(): ), ) def test_replicated_applications_have_eq_deep_copies_of_parameters(params): - apps = Ensemble( - "test_ensemble", "echo", ("hello",), replicas=4, parameters=params - )._create_applications() - assert len(apps) >= 2 # Sanitiy check to make sure the test is valid - assert all( - app_1.params == app_2.params for app_1, app_2 in itertools.pairwise(apps) + apps = list( + Ensemble( + "test_ensemble", "echo", ("hello",), replicas=4, parameters=params + )._create_applications() ) + assert len(apps) >= 2 # Sanitiy check to make sure the test is valid + assert all(app_1.params == app_2.params for app_1 in apps for app_2 in apps) assert all( app_1.params is not app_2.params for app_1 in apps From 3fe3044cfd46308a1b5e2c2d7001abd3d048da13 Mon Sep 17 00:00:00 2001 From: Matt Drozt Date: Wed, 5 Jun 2024 15:36:27 -0700 Subject: [PATCH 09/19] Address reviewer feedback --- smartsim/entity/_new_ensemble.py | 7 ++ smartsim/entity/ensemble.py | 23 +++---- smartsim/entity/entity.py | 8 +++ smartsim/entity/strategies.py | 107 ++++++++++++++++++++++++++++++- 4 files changed, 129 insertions(+), 16 deletions(-) diff --git a/smartsim/entity/_new_ensemble.py b/smartsim/entity/_new_ensemble.py index b6281c4631..2043f4f673 100644 --- a/smartsim/entity/_new_ensemble.py +++ b/smartsim/entity/_new_ensemble.py @@ -45,6 +45,10 @@ # 1) Move this to the `smartsim._core.entity.ensemble` module # 2) Decide what to do witht the original `Ensemble` impl class Ensemble(entity.CompoundEntity): + """Entity to help parameterize the creation multiple application + instances. + """ + def __init__( self, name: str, @@ -66,6 +70,9 @@ def __init__( self.replicas = replicas def _create_applications(self) -> tuple[Application, ...]: + """Concretize the ensemble attributes into a collection of + application instances. + """ permutation_strategy = strategies.resolve(self.permutation_strategy) permutations = permutation_strategy(self.parameters, self.max_permutations) permutations = permutations if permutations else [{}] diff --git a/smartsim/entity/ensemble.py b/smartsim/entity/ensemble.py index def6364e08..bfbc405139 100644 --- a/smartsim/entity/ensemble.py +++ b/smartsim/entity/ensemble.py @@ -32,18 +32,14 @@ from tabulate import tabulate from .._core._install.builder import Device -from ..error import ( - EntityExistsError, - SmartSimError, - SSUnsupportedError, - UserStrategyError, -) +from ..error import EntityExistsError, SmartSimError, SSUnsupportedError from ..log import get_logger from ..settings.base import BatchSettings, RunSettings from .dbobject import DBModel, DBScript from .entity import SmartSimEntity from .entityList import EntityList from .model import Application +from .strategies import TPermutationStrategy from .strategies import resolve as resolve_strategy logger = get_logger(__name__) @@ -64,7 +60,7 @@ def __init__( 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", + perm_strat: t.Union[str, TPermutationStrategy] = "all_perm", **kwargs: t.Any, ) -> None: """Initialize an Ensemble of Application instances. @@ -105,14 +101,19 @@ def applications(self) -> t.Collection[Application]: """An alias for a shallow copy of the ``entities`` attribute""" return list(self.entities) - def _initialize_entities(self, **kwargs: t.Any) -> None: + def _initialize_entities( + self, + *, + perm_strat: t.Union[str, TPermutationStrategy] = "all_perm", + **kwargs: t.Any, + ) -> None: """Initialize all the applications within the ensemble based on the parameters passed to the ensemble and the permutation strategy given at init. :raises UserStrategyError: if user generation strategy fails """ - strategy = resolve_strategy(kwargs.pop("perm_strat")) + strategy = resolve_strategy(perm_strat) replicas = kwargs.pop("replicas", None) self.replicas = replicas @@ -123,12 +124,8 @@ def _initialize_entities(self, **kwargs: t.Any) -> None: # Compute all combinations of application parameters and arguments n_applications = kwargs.get("n_applications", 0) all_application_params = strategy(self.params, n_applications) - if not isinstance(all_application_params, list): - raise UserStrategyError(strategy) for i, param_set in enumerate(all_application_params): - if not isinstance(param_set, dict): - raise UserStrategyError(strategy) run_settings = deepcopy(self.run_settings) application_name = "_".join((self.name, str(i))) application = Application( diff --git a/smartsim/entity/entity.py b/smartsim/entity/entity.py index c13cd95afa..0683be5e2e 100644 --- a/smartsim/entity/entity.py +++ b/smartsim/entity/entity.py @@ -124,6 +124,14 @@ def __repr__(self) -> str: class CompoundEntity(abc.ABC): + """An interface to create different types of collections of launchables + from a single set of launch settings. + + Objects that implement this interface describe how to turn their entities + into a collection of jobs and this interface will handle coercion into + other collections for jobs with slightly different launching behavior. + """ + @abc.abstractmethod def as_jobs(self, settings: _mock.LaunchSettings) -> t.Collection[_mock.Job]: ... def as_job_group(self, settings: _mock.LaunchSettings) -> _mock.JobGroup: diff --git a/smartsim/entity/strategies.py b/smartsim/entity/strategies.py index 08251949ea..3144b8092e 100644 --- a/smartsim/entity/strategies.py +++ b/smartsim/entity/strategies.py @@ -35,10 +35,12 @@ from smartsim.error import errors -TPermutationStrategy = t.Callable[ +# Type alias for the shape of a permutation strategy callable +TPermutationStrategy: t.TypeAlias = t.Callable[ [t.Mapping[str, t.Sequence[str]], int], list[dict[str, str]] ] +# Map of globally registered strategy names to registered strategy callables _REGISTERED_STRATEGIES: t.Final[dict[str, TPermutationStrategy]] = {} @@ -46,7 +48,22 @@ def _register(name: str) -> t.Callable[ [TPermutationStrategy], TPermutationStrategy, ]: + """Create a decorator to globally register a permutation strategy under a + given name. + + :param name: The name under which to register a strategy + :return: A decorator to register a permutation strategy function + """ + def _impl(fn: TPermutationStrategy) -> TPermutationStrategy: + """Add a strategy function to the globally registered strategies under + the `name` caught in the closure. + + :param fn: A permutation strategy + :returns: The original strategy, unaltered + :raises ValueError: A strategy under name caught in the closure has + already been registered + """ if name in _REGISTERED_STRATEGIES: msg = f"A strategy with the name '{name}' has already been registered" raise ValueError(msg) @@ -57,8 +74,20 @@ def _impl(fn: TPermutationStrategy) -> TPermutationStrategy: def resolve(strategy: str | TPermutationStrategy) -> TPermutationStrategy: + """Look-up or sanitize a permutation strategy: + + - When `strategy` is a `str` it will look for a globally registered + strategy function by that name. + + - When `strategy` is a `callable` it is will return a sanitized + strategy function. + + :param strategy: The name of a registered strategy or a custom + permutation strategy + :return: A valid permutation strategy callable + """ if callable(strategy): - return _make_safe_custom_strategy(strategy) + return _make_sanitized_custom_strategy(strategy) try: return _REGISTERED_STRATEGIES[strategy] except KeyError: @@ -68,7 +97,22 @@ def resolve(strategy: str | TPermutationStrategy) -> TPermutationStrategy: ) from None -def _make_safe_custom_strategy(fn: TPermutationStrategy) -> TPermutationStrategy: +def _make_sanitized_custom_strategy(fn: TPermutationStrategy) -> TPermutationStrategy: + """Take a callable that satisfies the shape of a permutation strategy and + return a sanitized version for future callers. + + The sanitized version of the permutation strategy will intercept any + exceptions raised by the original permutation and re-raise a + `UserStrategyError`. + + The sanitized version will also check the type of the value returned from + the original callable, and if it does conform to the expected return type, + a `UserStrategyError` will be raised. + + :param fn: A custom user strategy function + :return: A sanitized version of the custom strategy function + """ + @functools.wraps(fn) def _impl( params: t.Mapping[str, t.Sequence[str]], n_permutations: int @@ -96,6 +140,31 @@ def create_all_permutations( # TODO: Really don't like that this attr is ignored, but going to leave it # as the original impl for now. Will change if requested! ) -> list[dict[str, str]]: + """Take a mapping parameters to possible values and return a sequence of + all possible permutations of those parameters. + + For example calling: + + .. highlight:: python + .. code-block:: python + + create_all_permutations({"A": ["1", "2"], + "B": ["3", "4"]}) + + Would result in the following permutations (not necessarily in this order): + + .. highlight:: python + .. code-block:: python + + [{"A": "1", "B": "3"}, + {"A": "1", "B": "4"}, + {"A": "2", "B": "3"}, + {"A": "2", "B": "4"}] + + :param params: A mapping of parameter names to possible values + :param _n_permutations: + :return: A sequence of mappings of all possible permutations + """ permutations = itertools.product(*params.values()) return [dict(zip(params, permutation)) for permutation in permutations] @@ -104,6 +173,30 @@ def create_all_permutations( def step_values( params: t.Mapping[str, t.Sequence[str]], _n_permutations: int = 0 ) -> list[dict[str, str]]: + """Take a mapping parameters to possible values and return a sequence of + stepped values until a possible values sequence runs out of possible + values. + + For example calling: + + .. highlight:: python + .. code-block:: python + + step_values({"A": ["1", "2"], + "B": ["3", "4"]}) + + Would result in the following permutations: + + .. highlight:: python + .. code-block:: python + + [{"A": "1", "B": "3"}, + {"A": "2", "B": "4"}] + + :param params: A mapping of parameter names to possible values + :param _n_permutations: + :return: A sequence of mappings of stepped values + """ steps = zip(*params.values()) return [dict(zip(params, step)) for step in steps] @@ -112,6 +205,14 @@ def step_values( def random_permutations( params: t.Mapping[str, t.Sequence[str]], n_permutations: int = 0 ) -> list[dict[str, str]]: + """Take a mapping parameters to possible values and return a sequence of + length `n_permutations` sampled randomly from all possible permutations + + :param params: A mapping of parameter names to possible values + :param n_permutations: The maximum number of permutations to sample from + the sequence of all permutations + :return: A sequence of mappings of sampled permutations + """ permutations = create_all_permutations(params, 0) # sample from available permutations if n_permutations is specified From a4635a08e7442c9120f9d7f80aa06f2655848a02 Mon Sep 17 00:00:00 2001 From: Matt Drozt Date: Wed, 5 Jun 2024 15:42:52 -0700 Subject: [PATCH 10/19] Pylint approved type name --- smartsim/entity/_new_ensemble.py | 2 +- smartsim/entity/ensemble.py | 6 +++--- smartsim/entity/strategies.py | 14 +++++++------- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/smartsim/entity/_new_ensemble.py b/smartsim/entity/_new_ensemble.py index 2043f4f673..2696fff8a5 100644 --- a/smartsim/entity/_new_ensemble.py +++ b/smartsim/entity/_new_ensemble.py @@ -56,7 +56,7 @@ def __init__( exe_args: t.Sequence[str] | None = None, files: EntityFiles | None = None, parameters: t.Mapping[str, t.Sequence[str]] | None = None, - permutation_strategy: str | strategies.TPermutationStrategy = "all_perm", + permutation_strategy: str | strategies.PermutationStrategyType = "all_perm", max_permutations: int = 0, replicas: int = 1, ) -> None: diff --git a/smartsim/entity/ensemble.py b/smartsim/entity/ensemble.py index bfbc405139..e5504df264 100644 --- a/smartsim/entity/ensemble.py +++ b/smartsim/entity/ensemble.py @@ -39,7 +39,7 @@ from .entity import SmartSimEntity from .entityList import EntityList from .model import Application -from .strategies import TPermutationStrategy +from .strategies import PermutationStrategyType from .strategies import resolve as resolve_strategy logger = get_logger(__name__) @@ -60,7 +60,7 @@ def __init__( params_as_args: t.Optional[t.List[str]] = None, batch_settings: t.Optional[BatchSettings] = None, run_settings: t.Optional[RunSettings] = None, - perm_strat: t.Union[str, TPermutationStrategy] = "all_perm", + perm_strat: t.Union[str, PermutationStrategyType] = "all_perm", **kwargs: t.Any, ) -> None: """Initialize an Ensemble of Application instances. @@ -104,7 +104,7 @@ def applications(self) -> t.Collection[Application]: def _initialize_entities( self, *, - perm_strat: t.Union[str, TPermutationStrategy] = "all_perm", + perm_strat: t.Union[str, PermutationStrategyType] = "all_perm", **kwargs: t.Any, ) -> None: """Initialize all the applications within the ensemble based diff --git a/smartsim/entity/strategies.py b/smartsim/entity/strategies.py index 3144b8092e..e0a6b3acc6 100644 --- a/smartsim/entity/strategies.py +++ b/smartsim/entity/strategies.py @@ -36,17 +36,17 @@ from smartsim.error import errors # Type alias for the shape of a permutation strategy callable -TPermutationStrategy: t.TypeAlias = t.Callable[ +PermutationStrategyType: t.TypeAlias = t.Callable[ [t.Mapping[str, t.Sequence[str]], int], list[dict[str, str]] ] # Map of globally registered strategy names to registered strategy callables -_REGISTERED_STRATEGIES: t.Final[dict[str, TPermutationStrategy]] = {} +_REGISTERED_STRATEGIES: t.Final[dict[str, PermutationStrategyType]] = {} def _register(name: str) -> t.Callable[ - [TPermutationStrategy], - TPermutationStrategy, + [PermutationStrategyType], + PermutationStrategyType, ]: """Create a decorator to globally register a permutation strategy under a given name. @@ -55,7 +55,7 @@ def _register(name: str) -> t.Callable[ :return: A decorator to register a permutation strategy function """ - def _impl(fn: TPermutationStrategy) -> TPermutationStrategy: + def _impl(fn: PermutationStrategyType) -> PermutationStrategyType: """Add a strategy function to the globally registered strategies under the `name` caught in the closure. @@ -73,7 +73,7 @@ def _impl(fn: TPermutationStrategy) -> TPermutationStrategy: return _impl -def resolve(strategy: str | TPermutationStrategy) -> TPermutationStrategy: +def resolve(strategy: str | PermutationStrategyType) -> PermutationStrategyType: """Look-up or sanitize a permutation strategy: - When `strategy` is a `str` it will look for a globally registered @@ -97,7 +97,7 @@ def resolve(strategy: str | TPermutationStrategy) -> TPermutationStrategy: ) from None -def _make_sanitized_custom_strategy(fn: TPermutationStrategy) -> TPermutationStrategy: +def _make_sanitized_custom_strategy(fn: PermutationStrategyType) -> PermutationStrategyType: """Take a callable that satisfies the shape of a permutation strategy and return a sanitized version for future callers. From afa2ea651a0c6bd87583cee139fa9278dc3975b4 Mon Sep 17 00:00:00 2001 From: amandarichardsonn <30413257+amandarichardsonn@users.noreply.github.com> Date: Mon, 24 Jun 2024 13:44:39 -0700 Subject: [PATCH 11/19] New pr combinations (#2) Ensemble creation strategy implemented. [ reviewed by @MattToast ] [ committed by @amandarichardsonn ] --- smartsim/entity/_new_ensemble.py | 22 +++- smartsim/entity/strategies.py | 152 ++++++++++++--------- tests/test_ensemble.py | 190 +++++++++++++++++++++------ tests/test_permutation_strategies.py | 112 +++++++++++++--- 4 files changed, 350 insertions(+), 126 deletions(-) diff --git a/smartsim/entity/_new_ensemble.py b/smartsim/entity/_new_ensemble.py index 2696fff8a5..ee554486b9 100644 --- a/smartsim/entity/_new_ensemble.py +++ b/smartsim/entity/_new_ensemble.py @@ -39,6 +39,7 @@ from smartsim.entity import _mock, entity, strategies from smartsim.entity.files import EntityFiles from smartsim.entity.model import Application +from smartsim.entity.strategies import ParamSet # TODO: If we like this design, we need to: @@ -54,17 +55,21 @@ def __init__( name: str, exe: str | os.PathLike[str], exe_args: t.Sequence[str] | None = None, + exe_arg_parameters: t.Mapping[str, t.Sequence[t.Sequence[str]]] | None = None, files: EntityFiles | None = None, - parameters: t.Mapping[str, t.Sequence[str]] | None = None, + file_parameters: t.Mapping[str, t.Sequence[str]] | None = None, permutation_strategy: str | strategies.PermutationStrategyType = "all_perm", - max_permutations: int = 0, + max_permutations: int = -1, replicas: int = 1, ) -> None: self.name = name self.exe = os.fspath(exe) self.exe_args = list(exe_args) if exe_args else [] + self.exe_arg_parameters = ( + copy.deepcopy(exe_arg_parameters) if exe_arg_parameters else {} + ) self.files = copy.deepcopy(files) if files else EntityFiles() - self.parameters = dict(parameters) if parameters else {} + self.file_parameters = dict(file_parameters) if file_parameters else {} self.permutation_strategy = permutation_strategy self.max_permutations = max_permutations self.replicas = replicas @@ -74,10 +79,12 @@ def _create_applications(self) -> tuple[Application, ...]: application instances. """ permutation_strategy = strategies.resolve(self.permutation_strategy) - permutations = permutation_strategy(self.parameters, self.max_permutations) - permutations = permutations if permutations else [{}] + combinations = permutation_strategy( + self.file_parameters, self.exe_arg_parameters, self.max_permutations + ) + combinations = combinations if combinations else [ParamSet({}, {})] permutations_ = itertools.chain.from_iterable( - itertools.repeat(permutation, self.replicas) for permutation in permutations + itertools.repeat(permutation, self.replicas) for permutation in combinations ) return tuple( Application( @@ -88,7 +95,8 @@ def _create_applications(self) -> tuple[Application, ...]: # FIXME: remove this constructor arg! It should not exist!! exe_args=self.exe_args, files=self.files, - params=permutation, + params=permutation.params, + params_as_args=permutation.exe_args, ) for i, permutation in enumerate(permutations_) ) diff --git a/smartsim/entity/strategies.py b/smartsim/entity/strategies.py index edce1424c3..ac51478e28 100644 --- a/smartsim/entity/strategies.py +++ b/smartsim/entity/strategies.py @@ -32,12 +32,25 @@ import itertools import random import typing as t +from dataclasses import dataclass, field from smartsim.error import errors + +@dataclass(frozen=True) +class ParamSet: + """ + Represents a set of file parameters and execution arguments as parameters. + """ + + params: dict[str, str] = field(default_factory=dict) + exe_args: dict[str, list[str]] = field(default_factory=dict) + + # Type alias for the shape of a permutation strategy callable -PermutationStrategyType: t.TypeAlias = t.Callable[ - [t.Mapping[str, t.Sequence[str]], int], list[dict[str, str]] +PermutationStrategyType = t.Callable[ + [t.Mapping[str, t.Sequence[str]], t.Mapping[str, t.Sequence[t.Sequence[str]]], int], + list[ParamSet], ] # Map of globally registered strategy names to registered strategy callables @@ -117,14 +130,16 @@ def _make_sanitized_custom_strategy( @functools.wraps(fn) def _impl( - params: t.Mapping[str, t.Sequence[str]], n_permutations: int - ) -> list[dict[str, str]]: + params: t.Optional[t.Mapping[str, t.Sequence[str]]], + exe_args: t.Optional[t.Mapping[str, t.Sequence[t.Sequence[str]]]], + n_permutations: int = 0, + ) -> list[ParamSet]: try: - permutations = fn(params, n_permutations) + permutations = fn(params, exe_args, n_permutations) except Exception as e: raise errors.UserStrategyError(str(fn)) from e if not isinstance(permutations, list) or not all( - isinstance(permutation, dict) for permutation in permutations + isinstance(permutation, ParamSet) for permutation in permutations ): raise errors.UserStrategyError(str(fn)) return permutations @@ -132,93 +147,106 @@ def _impl( return _impl -# create permutations of all parameters -# single application if parameters only have one value @_register("all_perm") def create_all_permutations( params: t.Mapping[str, t.Sequence[str]], - _n_permutations: int = 0, - # ^^^^^^^^^^^^^ - # TODO: Really don't like that this attr is ignored, but going to leave it - # as the original impl for now. Will change if requested! -) -> list[dict[str, str]]: - """Take a mapping parameters to possible values and return a sequence of + exe_arg: t.Mapping[str, t.Sequence[t.Sequence[str]]], + n_permutations: int = -1, +) -> list[ParamSet]: + """Take two mapping parameters to possible values and return a sequence of all possible permutations of those parameters. - For example calling: - .. highlight:: python .. code-block:: python - - create_all_permutations({"A": ["1", "2"], - "B": ["3", "4"]}) - + create_all_permutations({"SPAM": ["a", "b"], + "EGGS": ["c", "d"]}, + {"EXE": [["a"], ["b", "c"]], + "ARGS": [["d"], ["e", "f"]]}, + 1 + ) Would result in the following permutations (not necessarily in this order): - .. highlight:: python .. code-block:: python - - [{"A": "1", "B": "3"}, - {"A": "1", "B": "4"}, - {"A": "2", "B": "3"}, - {"A": "2", "B": "4"}] - - :param params: A mapping of parameter names to possible values - :param _n_permutations: - :return: A sequence of mappings of all possible permutations + [ParamSet(params={'SPAM': 'a', 'EGGS': 'c'}, + exe_args={'EXE': ['a'], 'ARGS': ['d']})] + :param file_params: A mapping of file parameter names to possible values + :param exe_arg_params: A mapping of exe arg parameter names to possible values + :param n_permutations: The maximum number of permutations to sample from + the sequence of all permutations + :return: A sequence of ParamSets of all possible permutations """ - permutations = itertools.product(*params.values()) - return [dict(zip(params, permutation)) for permutation in permutations] + file_params_permutations = itertools.product(*params.values()) + param_zip = ( + dict(zip(params, permutation)) for permutation in file_params_permutations + ) + exe_arg_params_permutations = itertools.product(*exe_arg.values()) + exe_arg_zip = ( + dict(zip(exe_arg, permutation)) for permutation in exe_arg_params_permutations + ) + combinations = itertools.product(param_zip, exe_arg_zip) + param_set = (ParamSet(file_param, exe_arg) for file_param, exe_arg in combinations) + if n_permutations >= 0: + param_set = itertools.islice(param_set, n_permutations) + return list(param_set) @_register("step") def step_values( - params: t.Mapping[str, t.Sequence[str]], _n_permutations: int = 0 -) -> list[dict[str, str]]: - """Take a mapping parameters to possible values and return a sequence of + params: t.Mapping[str, t.Sequence[str]], + exe_args: t.Mapping[str, t.Sequence[t.Sequence[str]]], + n_permutations: int = -1, +) -> list[ParamSet]: + """Take two mapping parameters to possible values and return a sequence of stepped values until a possible values sequence runs out of possible values. - For example calling: - .. highlight:: python .. code-block:: python - - step_values({"A": ["1", "2"], - "B": ["3", "4"]}) - + step_values({"SPAM": ["a", "b"], + "EGGS": ["c", "d"]}, + {"EXE": [["a"], ["b", "c"]], + "ARGS": [["d"], ["e", "f"]]}, + 1 + ) Would result in the following permutations: - .. highlight:: python .. code-block:: python - - [{"A": "1", "B": "3"}, - {"A": "2", "B": "4"}] - - :param params: A mapping of parameter names to possible values - :param _n_permutations: - :return: A sequence of mappings of stepped values + [ParamSet(params={'SPAM': 'a', 'EGGS': 'c'}, + exe_args={'EXE': ['a'], 'ARGS': ['d']})] + :param file_params: A mapping of file parameter names to possible values + :param exe_arg_params: A mapping of exe arg parameter names to possible values + :param _n_permutations: The maximum number of permutations to sample from + the sequence of step permutations + :return: A sequence of ParamSets of stepped values """ - steps = zip(*params.values()) - return [dict(zip(params, step)) for step in steps] + param_zip = zip(*params.values()) + param_zip = (dict(zip(params, step)) for step in param_zip) + exe_arg_zip = zip(*exe_args.values()) + exe_arg_zip = (dict(zip(exe_args, step)) for step in exe_arg_zip) + param_set = ( + ParamSet(file_param, exe_arg) + for (file_param, exe_arg) in zip(param_zip, exe_arg_zip) + ) + if n_permutations >= 0: + param_set = itertools.islice(param_set, n_permutations) + return list(param_set) @_register("random") def random_permutations( - params: t.Mapping[str, t.Sequence[str]], n_permutations: int = 0 -) -> list[dict[str, str]]: - """Take a mapping parameters to possible values and return a sequence of + params: t.Mapping[str, t.Sequence[str]], + exe_args: t.Mapping[str, t.Sequence[t.Sequence[str]]], + n_permutations: int = -1, +) -> list[ParamSet]: + """Take two mapping parameters to possible values and return a sequence of length `n_permutations` sampled randomly from all possible permutations - - :param params: A mapping of parameter names to possible values + :param file_params: A mapping of file parameter names to possible values + :param exe_arg_params: A mapping of exe arg parameter names to possible values :param n_permutations: The maximum number of permutations to sample from the sequence of all permutations - :return: A sequence of mappings of sampled permutations + :return: A sequence of ParamSets of sampled permutations """ - permutations = create_all_permutations(params, 0) - - # sample from available permutations if n_permutations is specified - if 0 < n_permutations < len(permutations): + permutations = create_all_permutations(params, exe_args, -1) + if 0 <= n_permutations < len(permutations): permutations = random.sample(permutations, n_permutations) - return permutations diff --git a/tests/test_ensemble.py b/tests/test_ensemble.py index df4cd1b42a..b74ca5f159 100644 --- a/tests/test_ensemble.py +++ b/tests/test_ensemble.py @@ -24,15 +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 itertools +import typing as t + import pytest from smartsim.entity import _mock from smartsim.entity._new_ensemble import Ensemble +from smartsim.entity.strategies import ParamSet pytestmark = pytest.mark.group_a _2x2_PARAMS = {"SPAM": ["a", "b"], "EGGS": ["c", "d"]} -_2_PERM_STRAT = lambda p, n: [{"SPAM": "a", "EGGS": "b"}, {"SPAM": "c", "EGGS": "d"}] +_2x2_EXE_ARG = {"EXE": [["a"], ["b", "c"]], "ARGS": [["d"], ["e", "f"]]} + + +def user_created_function( + file_params: t.Mapping[str, t.Sequence[str]], + exe_arg_params: t.Mapping[str, t.Sequence[t.Sequence[str]]], + n_permutations: int = 0, +) -> list[ParamSet]: + return [ParamSet({}, {})] @pytest.fixture @@ -42,52 +54,25 @@ def mock_launcher_settings(): return _mock.LaunchSettings() -# fmt: off -@pytest.mark.parametrize( - " params, strategy, max_perms, replicas, expected_num_jobs", # Test Name Misc - (pytest.param( None, "all_perm", 0, 1, 1 , id="No Parameters or Replicas") , - pytest.param(_2x2_PARAMS, "all_perm", 0, 1, 4 , id="All Permutations") , - pytest.param(_2x2_PARAMS, "step", 0, 1, 2 , id="Stepped Params") , - pytest.param(_2x2_PARAMS, "random", 0, 1, 4 , id="Random Permutations") , - pytest.param(_2x2_PARAMS, "all_perm", 1, 1, 1 , id="All Permutations [Capped Max Permutations]" , marks=pytest.mark.xfail(reason="'all_perm' strategy currently ignores max number permutations'", strict=True)), - pytest.param(_2x2_PARAMS, "step", 1, 1, 1 , id="Stepped Params [Capped Max Permutations]" , marks=pytest.mark.xfail(reason="'step' strategy currently ignores max number permutations'", strict=True)), - pytest.param(_2x2_PARAMS, "random", 1, 1, 1 , id="Random Permutations [Capped Max Permutations]"), # ^^^^^^^^^^^^^^^^^ - pytest.param( {}, _2_PERM_STRAT, 0, 1, 2 , id="Custom Permutation Strategy") , # TODO: I would argue that we should make these cases pass - pytest.param( {}, "all_perm", 0, 5, 5 , id="Identical Replicas") , - pytest.param(_2x2_PARAMS, "all_perm", 0, 2, 8 , id="Replicas of All Permutations") , - pytest.param(_2x2_PARAMS, "step", 0, 2, 4 , id="Replicas of Stepped Params") , - pytest.param(_2x2_PARAMS, "random", 3, 2, 6 , id="Replicas of Random Permutations") , -)) -# fmt: on -def test_expected_number_of_apps_created( - # Parameterized - params, - strategy, - max_perms, - replicas, - expected_num_jobs, - # Other fixtures - mock_launcher_settings, -): +def test_ensemble_user_created_strategy(mock_launcher_settings): jobs = Ensemble( "test_ensemble", "echo", ("hello", "world"), - parameters=params, - permutation_strategy=strategy, - max_permutations=max_perms, - replicas=replicas, + permutation_strategy=user_created_function, ).as_jobs(mock_launcher_settings) - assert len(jobs) == expected_num_jobs + assert len(jobs) == 1 -def test_ensemble_without_any_members_rasies_when_cast_to_jobs(mock_launcher_settings): +def test_ensemble_without_any_members_raises_when_cast_to_jobs(mock_launcher_settings): with pytest.raises(ValueError): Ensemble( "test_ensemble", "echo", - ("hello",), - permutation_strategy=lambda p, n: [{}], + ("hello", "world"), + file_parameters=_2x2_PARAMS, + permutation_strategy="random", + max_permutations=30, replicas=0, ).as_jobs(mock_launcher_settings) @@ -113,7 +98,7 @@ def test_strategy_error_raised_if_a_strategy_that_dne_is_requested(): def test_replicated_applications_have_eq_deep_copies_of_parameters(params): apps = list( Ensemble( - "test_ensemble", "echo", ("hello",), replicas=4, parameters=params + "test_ensemble", "echo", ("hello",), replicas=4, file_parameters=params )._create_applications() ) assert len(apps) >= 2 # Sanitiy check to make sure the test is valid @@ -124,3 +109,134 @@ def test_replicated_applications_have_eq_deep_copies_of_parameters(params): for app_2 in apps if app_1 is not app_2 ) + + +# fmt: off +@pytest.mark.parametrize( + " params, exe_arg_params, max_perms, replicas, expected_num_jobs", + (pytest.param(_2x2_PARAMS, _2x2_EXE_ARG, 30, 1, 16 , id="Set max permutation high"), + pytest.param(_2x2_PARAMS, _2x2_EXE_ARG, -1, 1, 16 , id="Set max permutation negative"), + pytest.param(_2x2_PARAMS, _2x2_EXE_ARG, 0, 1, 1 , id="Set max permutation zero"), + pytest.param(_2x2_PARAMS, None, 4, 1, 4 , id="No exe arg params or Replicas"), + pytest.param( None, _2x2_EXE_ARG, 4, 1, 4 , id="No Parameters or Replicas"), + pytest.param( None, None, 4, 1, 1 , id="No Parameters, Exe_Arg_Param or Replicas"), + pytest.param(_2x2_PARAMS, _2x2_EXE_ARG, 1, 1, 1 , id="Set max permutation to lowest"), + pytest.param(_2x2_PARAMS, _2x2_EXE_ARG, 6, 2, 12 , id="Set max permutation, set replicas"), + pytest.param( {}, _2x2_EXE_ARG, 6, 2, 8 , id="Set params as dict, set max permutations and replicas"), + pytest.param(_2x2_PARAMS, {}, 6, 2, 8 , id="Set params as dict, set max permutations and replicas"), + pytest.param( {}, {}, 6, 2, 2 , id="Set params as dict, set max permutations and replicas") +)) +# fmt: on +def test_all_perm_strategy( + # Parameterized + params, + exe_arg_params, + max_perms, + replicas, + expected_num_jobs, + # Other fixtures + mock_launcher_settings, +): + jobs = Ensemble( + "test_ensemble", + "echo", + ("hello", "world"), + file_parameters=params, + exe_arg_parameters=exe_arg_params, + permutation_strategy="all_perm", + max_permutations=max_perms, + replicas=replicas, + ).as_jobs(mock_launcher_settings) + assert len(jobs) == expected_num_jobs + + +def test_all_perm_strategy_contents(): + jobs = Ensemble( + "test_ensemble", + "echo", + ("hello", "world"), + file_parameters=_2x2_PARAMS, + exe_arg_parameters=_2x2_EXE_ARG, + permutation_strategy="all_perm", + max_permutations=16, + replicas=1, + ).as_jobs(mock_launcher_settings) + assert len(jobs) == 16 + + +# fmt: off +@pytest.mark.parametrize( + " params, exe_arg_params, max_perms, replicas, expected_num_jobs", + (pytest.param(_2x2_PARAMS, _2x2_EXE_ARG, 30, 1, 2 , id="Set max permutation high"), + pytest.param(_2x2_PARAMS, _2x2_EXE_ARG, -1, 1, 2 , id="Set max permutation negtive"), + pytest.param(_2x2_PARAMS, _2x2_EXE_ARG, 0, 1, 1 , id="Set max permutation zero"), + pytest.param(_2x2_PARAMS, None, 4, 1, 1 , id="No exe arg params or Replicas"), + pytest.param( None, _2x2_EXE_ARG, 4, 1, 1 , id="No Parameters or Replicas"), + pytest.param( None, None, 4, 1, 1 , id="No Parameters, Exe_Arg_Param or Replicas"), + pytest.param(_2x2_PARAMS, _2x2_EXE_ARG, 1, 1, 1 , id="Set max permutation to lowest"), + pytest.param(_2x2_PARAMS, _2x2_EXE_ARG, 6, 2, 4 , id="Set max permutation, set replicas"), + pytest.param( {}, _2x2_EXE_ARG, 6, 2, 2 , id="Set params as dict, set max permutations and replicas"), + pytest.param(_2x2_PARAMS, {}, 6, 2, 2 , id="Set params as dict, set max permutations and replicas"), + pytest.param( {}, {}, 6, 2, 2 , id="Set params as dict, set max permutations and replicas") +)) +# fmt: on +def test_step_strategy( + # Parameterized + params, + exe_arg_params, + max_perms, + replicas, + expected_num_jobs, + # Other fixtures + mock_launcher_settings, +): + jobs = Ensemble( + "test_ensemble", + "echo", + ("hello", "world"), + file_parameters=params, + exe_arg_parameters=exe_arg_params, + permutation_strategy="step", + max_permutations=max_perms, + replicas=replicas, + ).as_jobs(mock_launcher_settings) + assert len(jobs) == expected_num_jobs + + +# fmt: off +@pytest.mark.parametrize( + " params, exe_arg_params, max_perms, replicas, expected_num_jobs", + (pytest.param(_2x2_PARAMS, _2x2_EXE_ARG, 30, 1, 16 , id="Set max permutation high"), + pytest.param(_2x2_PARAMS, _2x2_EXE_ARG, -1, 1, 16 , id="Set max permutation negative"), + pytest.param(_2x2_PARAMS, _2x2_EXE_ARG, 0, 1, 1 , id="Set max permutation zero"), + pytest.param(_2x2_PARAMS, None, 4, 1, 4 , id="No exe arg params or Replicas"), + pytest.param( None, _2x2_EXE_ARG, 4, 1, 4 , id="No Parameters or Replicas"), + pytest.param( None, None, 4, 1, 1 , id="No Parameters, Exe_Arg_Param or Replicas"), + pytest.param(_2x2_PARAMS, _2x2_EXE_ARG, 1, 1, 1 , id="Set max permutation to lowest"), + pytest.param(_2x2_PARAMS, _2x2_EXE_ARG, 6, 2, 12 , id="Set max permutation, set replicas"), + pytest.param( {}, _2x2_EXE_ARG, 6, 2, 8 , id="Set params as dict, set max permutations and replicas"), + pytest.param(_2x2_PARAMS, {}, 6, 2, 8 , id="Set params as dict, set max permutations and replicas"), + pytest.param( {}, {}, 6, 2, 2 , id="Set params as dict, set max permutations and replicas") +)) +# fmt: on +def test_random_strategy( + # Parameterized + params, + exe_arg_params, + max_perms, + replicas, + expected_num_jobs, + # Other fixtures + mock_launcher_settings, +): + jobs = Ensemble( + "test_ensemble", + "echo", + ("hello", "world"), + file_parameters=params, + exe_arg_parameters=exe_arg_params, + permutation_strategy="random", + max_permutations=max_perms, + replicas=replicas, + ).as_jobs(mock_launcher_settings) + assert len(jobs) == expected_num_jobs diff --git a/tests/test_permutation_strategies.py b/tests/test_permutation_strategies.py index a86c5a9629..2bf5496ead 100644 --- a/tests/test_permutation_strategies.py +++ b/tests/test_permutation_strategies.py @@ -24,9 +24,12 @@ # 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 dataclasses + import pytest from smartsim.entity import strategies +from smartsim.entity.strategies import ParamSet from smartsim.error import errors pytestmark = pytest.mark.group_a @@ -36,22 +39,24 @@ def test_strategy_registration(monkeypatch): monkeypatch.setattr(strategies, "_REGISTERED_STRATEGIES", {}) assert not strategies._REGISTERED_STRATEGIES - new_strat = lambda params, nmax: [] + new_strat = lambda params, exe_args, nmax: [] strategies._register("new_strat")(new_strat) assert strategies._REGISTERED_STRATEGIES == {"new_strat": new_strat} def test_strategies_cannot_be_overwritten(monkeypatch): monkeypatch.setattr( - strategies, "_REGISTERED_STRATEGIES", {"some-strategy": lambda params, nmax: []} + strategies, + "_REGISTERED_STRATEGIES", + {"some-strategy": lambda params, exe_args, nmax: []}, ) with pytest.raises(ValueError): - strategies._register("some-strategy")(lambda params, nmax: []) + strategies._register("some-strategy")(lambda params, exe_args, nmax: []) def test_strategy_retreval(monkeypatch): - new_strat_a = lambda params, nmax: [] - new_strat_b = lambda params, nmax: [] + new_strat_a = lambda params, exe_args, nmax: [] + new_strat_b = lambda params, exe_args, nmax: [] monkeypatch.setattr( strategies, @@ -67,7 +72,7 @@ def test_user_strategy_error_raised_when_attempting_to_get_unknown_strat(): strategies.resolve("NOT-REGISTERED") -def broken_strategy(p, n): +def broken_strategy(p, n, e): raise Exception("This custom strategy raised an error") @@ -75,15 +80,16 @@ def broken_strategy(p, n): "strategy", ( pytest.param(broken_strategy, id="Strategy raises during execution"), - pytest.param(lambda params, nmax: 123, id="Does not return a list"), + pytest.param(lambda params, exe_args, nmax: 123, id="Does not return a list"), pytest.param( - lambda params, nmax: [1, 2, 3], id="Does not return a list of dicts" + lambda params, exe_args, nmax: [1, 2, 3], + id="Does not return a list of dicts", ), ), ) def test_custom_strategy_raises_user_strategy_error_if_something_goes_wrong(strategy): with pytest.raises(errors.UserStrategyError): - strategies.resolve(strategy)({"SPAM": ["EGGS"]}, 123) + strategies.resolve(strategy)({"SPAM": ["EGGS"]}, {"SPAM": [["EGGS"]]}, 123) @pytest.mark.parametrize( @@ -92,28 +98,87 @@ def test_custom_strategy_raises_user_strategy_error_if_something_goes_wrong(stra pytest.param( strategies.create_all_permutations, ( - {"SPAM": "a", "EGGS": "c"}, - {"SPAM": "a", "EGGS": "d"}, - {"SPAM": "b", "EGGS": "c"}, - {"SPAM": "b", "EGGS": "d"}, + [ + ParamSet( + params={"SPAM": "a", "EGGS": "c"}, exe_args={"EXE": ["a"]} + ), + ParamSet( + params={"SPAM": "a", "EGGS": "c"}, + exe_args={"EXE": ["b", "c"]}, + ), + ParamSet( + params={"SPAM": "a", "EGGS": "d"}, exe_args={"EXE": ["a"]} + ), + ParamSet( + params={"SPAM": "a", "EGGS": "d"}, + exe_args={"EXE": ["b", "c"]}, + ), + ParamSet( + params={"SPAM": "b", "EGGS": "c"}, exe_args={"EXE": ["a"]} + ), + ParamSet( + params={"SPAM": "b", "EGGS": "c"}, + exe_args={"EXE": ["b", "c"]}, + ), + ParamSet( + params={"SPAM": "b", "EGGS": "d"}, exe_args={"EXE": ["a"]} + ), + ParamSet( + params={"SPAM": "b", "EGGS": "d"}, + exe_args={"EXE": ["b", "c"]}, + ), + ] ), id="All Permutations", ), pytest.param( strategies.step_values, ( - {"SPAM": "a", "EGGS": "c"}, - {"SPAM": "b", "EGGS": "d"}, + [ + ParamSet( + params={"SPAM": "a", "EGGS": "c"}, exe_args={"EXE": ["a"]} + ), + ParamSet( + params={"SPAM": "b", "EGGS": "d"}, + exe_args={"EXE": ["b", "c"]}, + ), + ] ), id="Step Values", ), pytest.param( strategies.random_permutations, ( - {"SPAM": "a", "EGGS": "c"}, - {"SPAM": "a", "EGGS": "d"}, - {"SPAM": "b", "EGGS": "c"}, - {"SPAM": "b", "EGGS": "d"}, + [ + ParamSet( + params={"SPAM": "a", "EGGS": "c"}, exe_args={"EXE": ["a"]} + ), + ParamSet( + params={"SPAM": "a", "EGGS": "c"}, + exe_args={"EXE": ["b", "c"]}, + ), + ParamSet( + params={"SPAM": "a", "EGGS": "d"}, exe_args={"EXE": ["a"]} + ), + ParamSet( + params={"SPAM": "a", "EGGS": "d"}, + exe_args={"EXE": ["b", "c"]}, + ), + ParamSet( + params={"SPAM": "b", "EGGS": "c"}, exe_args={"EXE": ["a"]} + ), + ParamSet( + params={"SPAM": "b", "EGGS": "c"}, + exe_args={"EXE": ["b", "c"]}, + ), + ParamSet( + params={"SPAM": "b", "EGGS": "d"}, exe_args={"EXE": ["a"]} + ), + ParamSet( + params={"SPAM": "b", "EGGS": "d"}, + exe_args={"EXE": ["b", "c"]}, + ), + ] ), id="Uncapped Random Permutations", ), @@ -121,7 +186,14 @@ def test_custom_strategy_raises_user_strategy_error_if_something_goes_wrong(stra ) def test_strategy_returns_expected_set(strategy, expected_output): params = {"SPAM": ["a", "b"], "EGGS": ["c", "d"]} - output = list(strategy(params)) + exe_args = {"EXE": [["a"], ["b", "c"]]} + output = list(strategy(params, exe_args, 50)) assert len(output) == len(expected_output) assert all(item in expected_output for item in output) assert all(item in output for item in expected_output) + + +def test_param_set_is_frozen(): + param = ParamSet("set1", "set2") + with pytest.raises(dataclasses.FrozenInstanceError): + param.exe_args = "change" From 02af76f2e15fa7e3ba22734f50e63672363ee799 Mon Sep 17 00:00:00 2001 From: Matt Drozt Date: Tue, 25 Jun 2024 10:12:57 -0700 Subject: [PATCH 12/19] Fix strategy type errors --- smartsim/entity/strategies.py | 36 ++++++++++++++++++++++------------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/smartsim/entity/strategies.py b/smartsim/entity/strategies.py index ac51478e28..e3a2527a52 100644 --- a/smartsim/entity/strategies.py +++ b/smartsim/entity/strategies.py @@ -130,9 +130,9 @@ def _make_sanitized_custom_strategy( @functools.wraps(fn) def _impl( - params: t.Optional[t.Mapping[str, t.Sequence[str]]], - exe_args: t.Optional[t.Mapping[str, t.Sequence[t.Sequence[str]]]], - n_permutations: int = 0, + params: t.Mapping[str, t.Sequence[str]], + exe_args: t.Mapping[str, t.Sequence[t.Sequence[str]]], + n_permutations: int = -1, ) -> list[ParamSet]: try: permutations = fn(params, exe_args, n_permutations) @@ -179,12 +179,19 @@ def create_all_permutations( param_zip = ( dict(zip(params, permutation)) for permutation in file_params_permutations ) + exe_arg_params_permutations = itertools.product(*exe_arg.values()) + exe_arg_params_permutations_ = ( + tuple(map(list, sequence)) for sequence in exe_arg_params_permutations + ) exe_arg_zip = ( - dict(zip(exe_arg, permutation)) for permutation in exe_arg_params_permutations + dict(zip(exe_arg, permutation)) for permutation in exe_arg_params_permutations_ ) + combinations = itertools.product(param_zip, exe_arg_zip) - param_set = (ParamSet(file_param, exe_arg) for file_param, exe_arg in combinations) + param_set: t.Iterable[ParamSet] = ( + ParamSet(file_param, exe_arg) for file_param, exe_arg in combinations + ) if n_permutations >= 0: param_set = itertools.islice(param_set, n_permutations) return list(param_set) @@ -212,20 +219,23 @@ def step_values( .. highlight:: python .. code-block:: python [ParamSet(params={'SPAM': 'a', 'EGGS': 'c'}, - exe_args={'EXE': ['a'], 'ARGS': ['d']})] + exe_args={'EXE': ['a'], 'ARGS': ['d']})] :param file_params: A mapping of file parameter names to possible values :param exe_arg_params: A mapping of exe arg parameter names to possible values - :param _n_permutations: The maximum number of permutations to sample from + :param n_permutations: The maximum number of permutations to sample from the sequence of step permutations :return: A sequence of ParamSets of stepped values """ - param_zip = zip(*params.values()) - param_zip = (dict(zip(params, step)) for step in param_zip) - exe_arg_zip = zip(*exe_args.values()) - exe_arg_zip = (dict(zip(exe_args, step)) for step in exe_arg_zip) - param_set = ( + param_zip: t.Iterable[tuple[str, ...]] = zip(*params.values()) + param_zip_ = (dict(zip(params, step)) for step in param_zip) + + exe_arg_zip: t.Iterable[tuple[t.Sequence[str], ...]] = zip(*exe_args.values()) + exe_arg_zip_ = (map(list, sequence) for sequence in exe_arg_zip) + exe_arg_zip__ = (dict(zip(exe_args, step)) for step in exe_arg_zip_) + + param_set: t.Iterable[ParamSet] = ( ParamSet(file_param, exe_arg) - for (file_param, exe_arg) in zip(param_zip, exe_arg_zip) + for file_param, exe_arg in zip(param_zip_, exe_arg_zip__) ) if n_permutations >= 0: param_set = itertools.islice(param_set, n_permutations) From 838589298485a67472a67739358faa343438dfc7 Mon Sep 17 00:00:00 2001 From: Matt Drozt Date: Tue, 25 Jun 2024 10:13:42 -0700 Subject: [PATCH 13/19] Remove mocks of now implemented classes --- smartsim/_core/utils/helpers.py | 1 - smartsim/entity/_mock.py | 2 ++ smartsim/entity/_new_ensemble.py | 8 +++++--- tests/test_ensemble.py | 6 +++--- 4 files changed, 10 insertions(+), 7 deletions(-) diff --git a/smartsim/_core/utils/helpers.py b/smartsim/_core/utils/helpers.py index 70f52bc4e1..a1c0d7aa24 100644 --- a/smartsim/_core/utils/helpers.py +++ b/smartsim/_core/utils/helpers.py @@ -122,7 +122,6 @@ def expand_exe_path(exe: str) -> str: # which returns none if not found in_path = which(exe) - print(f"hmm what is this: {in_path}") if not in_path: if os.path.isfile(exe) and os.access(exe, os.X_OK): return os.path.abspath(exe) diff --git a/smartsim/entity/_mock.py b/smartsim/entity/_mock.py index e5788e1490..8a0d7ed8fc 100644 --- a/smartsim/entity/_mock.py +++ b/smartsim/entity/_mock.py @@ -41,3 +41,5 @@ class Mock: def __init__(self, *_: t.Any, **__: t.Any): ... def __getattr__(self, _: str) -> Mock: return Mock() + + __deepcopy__: t.ClassVar[t.Literal[None]] = None diff --git a/smartsim/entity/_new_ensemble.py b/smartsim/entity/_new_ensemble.py index 39dfe8c75d..2c288395bf 100644 --- a/smartsim/entity/_new_ensemble.py +++ b/smartsim/entity/_new_ensemble.py @@ -89,13 +89,15 @@ def _create_applications(self) -> tuple[Application, ...]: Application( name=f"{self.name}-{i}", exe=self.exe, - run_settings=_mock.Mock(), # type: ignore[arg-type] - # ^^^^^^^^^^^^^^^^^^^ + run_settings=_mock.Mock(), + # ^^^^^^^^^^^^^^^^^^^^^^^ # FIXME: remove this constructor arg! It should not exist!! exe_args=self.exe_args, files=self.files, params=permutation.params, - params_as_args=permutation.exe_args, + params_as_args=permutation.exe_args, # type: ignore[arg-type] + # ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + # FIXME: this is the wrong type on Application! ) for i, permutation in enumerate(permutations_) ) diff --git a/tests/test_ensemble.py b/tests/test_ensemble.py index b74ca5f159..2756102b81 100644 --- a/tests/test_ensemble.py +++ b/tests/test_ensemble.py @@ -32,6 +32,8 @@ from smartsim.entity import _mock from smartsim.entity._new_ensemble import Ensemble from smartsim.entity.strategies import ParamSet +from smartsim.settings.launchCommand import LauncherType +from smartsim.settings.launchSettings import LaunchSettings pytestmark = pytest.mark.group_a @@ -49,9 +51,7 @@ def user_created_function( @pytest.fixture def mock_launcher_settings(): - # TODO: Remove this mock with #587 - # https://github.com/CrayLabs/SmartSim/pull/587 - return _mock.LaunchSettings() + return LaunchSettings(LauncherType.Local, {}, {}) def test_ensemble_user_created_strategy(mock_launcher_settings): From 4f93de5efb3335b4192e04cff3f7f885d9885187 Mon Sep 17 00:00:00 2001 From: Matt Drozt Date: Tue, 25 Jun 2024 13:38:38 -0500 Subject: [PATCH 14/19] Remove extra space --- smartsim/entity/ensemble.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/smartsim/entity/ensemble.py b/smartsim/entity/ensemble.py index 61cd1349d4..953b33e64f 100644 --- a/smartsim/entity/ensemble.py +++ b/smartsim/entity/ensemble.py @@ -192,7 +192,7 @@ def add_application(self, application: Application) -> None: if not isinstance(application, Application): raise TypeError( f"Argument to add_application was of type {type(application)}, " - " not Application" + "not Application" ) # "in" operator uses application name for __eq__ if application in self.entities: From 14f669988ae3e7dadc4d72763b40fe70d3dcc63e Mon Sep 17 00:00:00 2001 From: Matt Drozt Date: Tue, 25 Jun 2024 13:43:39 -0700 Subject: [PATCH 15/19] Deepcopy mock for python 3.9 and 3.10 --- smartsim/entity/_mock.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/smartsim/entity/_mock.py b/smartsim/entity/_mock.py index 8a0d7ed8fc..8f1043ed3c 100644 --- a/smartsim/entity/_mock.py +++ b/smartsim/entity/_mock.py @@ -40,6 +40,7 @@ class Mock: def __init__(self, *_: t.Any, **__: t.Any): ... def __getattr__(self, _: str) -> Mock: - return Mock() + return type(self)() - __deepcopy__: t.ClassVar[t.Literal[None]] = None + def __deepcopy__(self, _: dict[t.Any, t.Any]) -> Mock: + return type(self)() From 0b8a148f44153b67dcd8f3b7fe92d95d53d628b6 Mon Sep 17 00:00:00 2001 From: Matt Drozt Date: Tue, 25 Jun 2024 15:32:57 -0700 Subject: [PATCH 16/19] Ensembles have a path, that path is `test_dir` in tests --- smartsim/entity/_new_ensemble.py | 8 ++++++++ tests/test_ensemble.py | 24 +++++++++++++++++++----- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/smartsim/entity/_new_ensemble.py b/smartsim/entity/_new_ensemble.py index 2c288395bf..7c6e870f80 100644 --- a/smartsim/entity/_new_ensemble.py +++ b/smartsim/entity/_new_ensemble.py @@ -29,6 +29,7 @@ import copy import itertools import os +import os.path import typing as t from smartsim.entity import _mock, entity, strategies @@ -55,6 +56,7 @@ def __init__( exe: str | os.PathLike[str], exe_args: t.Sequence[str] | None = None, exe_arg_parameters: t.Mapping[str, t.Sequence[t.Sequence[str]]] | None = None, + path: str | os.PathLike[str] | None = None, files: EntityFiles | None = None, file_parameters: t.Mapping[str, t.Sequence[str]] | None = None, permutation_strategy: str | strategies.PermutationStrategyType = "all_perm", @@ -67,6 +69,11 @@ def __init__( self.exe_arg_parameters = ( copy.deepcopy(exe_arg_parameters) if exe_arg_parameters else {} ) + self.path = os.fspath(path) if path is not None else os.getcwd() + # ^^^^^^^^^^^ + # TODO: Copied from the original implementation, but I'm not sure that + # I like this default. Shouldn't it be something under an + # experiment directory? If so, how it injected?? self.files = copy.deepcopy(files) if files else EntityFiles() self.file_parameters = dict(file_parameters) if file_parameters else {} self.permutation_strategy = permutation_strategy @@ -93,6 +100,7 @@ def _create_applications(self) -> tuple[Application, ...]: # ^^^^^^^^^^^^^^^^^^^^^^^ # FIXME: remove this constructor arg! It should not exist!! exe_args=self.exe_args, + path=os.path.join(self.path, self.name), files=self.files, params=permutation.params, params_as_args=permutation.exe_args, # type: ignore[arg-type] diff --git a/tests/test_ensemble.py b/tests/test_ensemble.py index 2756102b81..9149ea4d43 100644 --- a/tests/test_ensemble.py +++ b/tests/test_ensemble.py @@ -54,22 +54,26 @@ def mock_launcher_settings(): return LaunchSettings(LauncherType.Local, {}, {}) -def test_ensemble_user_created_strategy(mock_launcher_settings): +def test_ensemble_user_created_strategy(mock_launcher_settings, test_dir): jobs = Ensemble( "test_ensemble", "echo", ("hello", "world"), + path=test_dir, permutation_strategy=user_created_function, ).as_jobs(mock_launcher_settings) assert len(jobs) == 1 -def test_ensemble_without_any_members_raises_when_cast_to_jobs(mock_launcher_settings): +def test_ensemble_without_any_members_raises_when_cast_to_jobs( + mock_launcher_settings, test_dir +): with pytest.raises(ValueError): Ensemble( "test_ensemble", "echo", ("hello", "world"), + path=test_dir, file_parameters=_2x2_PARAMS, permutation_strategy="random", max_permutations=30, @@ -77,12 +81,13 @@ def test_ensemble_without_any_members_raises_when_cast_to_jobs(mock_launcher_set ).as_jobs(mock_launcher_settings) -def test_strategy_error_raised_if_a_strategy_that_dne_is_requested(): +def test_strategy_error_raised_if_a_strategy_that_dne_is_requested(test_dir): with pytest.raises(ValueError): Ensemble( "test_ensemble", "echo", ("hello",), + path=test_dir, permutation_strategy="THIS-STRATEGY-DNE", )._create_applications() @@ -95,10 +100,15 @@ def test_strategy_error_raised_if_a_strategy_that_dne_is_requested(): pytest.param(None, id="Nullish Params"), ), ) -def test_replicated_applications_have_eq_deep_copies_of_parameters(params): +def test_replicated_applications_have_eq_deep_copies_of_parameters(params, test_dir): apps = list( Ensemble( - "test_ensemble", "echo", ("hello",), replicas=4, file_parameters=params + "test_ensemble", + "echo", + ("hello",), + path=test_dir, + replicas=4, + file_parameters=params, )._create_applications() ) assert len(apps) >= 2 # Sanitiy check to make sure the test is valid @@ -136,11 +146,13 @@ def test_all_perm_strategy( expected_num_jobs, # Other fixtures mock_launcher_settings, + test_dir, ): jobs = Ensemble( "test_ensemble", "echo", ("hello", "world"), + path=test_dir, file_parameters=params, exe_arg_parameters=exe_arg_params, permutation_strategy="all_perm", @@ -189,11 +201,13 @@ def test_step_strategy( expected_num_jobs, # Other fixtures mock_launcher_settings, + test_dir, ): jobs = Ensemble( "test_ensemble", "echo", ("hello", "world"), + path=test_dir, file_parameters=params, exe_arg_parameters=exe_arg_params, permutation_strategy="step", From fd2f39f4958c5a5ce8c1020600c7c6429b47a564 Mon Sep 17 00:00:00 2001 From: Matt Drozt Date: Tue, 25 Jun 2024 15:33:25 -0700 Subject: [PATCH 17/19] Don't hard code launcher --- tests/test_ensemble.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/test_ensemble.py b/tests/test_ensemble.py index 9149ea4d43..c43b178085 100644 --- a/tests/test_ensemble.py +++ b/tests/test_ensemble.py @@ -32,7 +32,6 @@ from smartsim.entity import _mock from smartsim.entity._new_ensemble import Ensemble from smartsim.entity.strategies import ParamSet -from smartsim.settings.launchCommand import LauncherType from smartsim.settings.launchSettings import LaunchSettings pytestmark = pytest.mark.group_a @@ -50,8 +49,8 @@ def user_created_function( @pytest.fixture -def mock_launcher_settings(): - return LaunchSettings(LauncherType.Local, {}, {}) +def mock_launcher_settings(wlmutils): + return LaunchSettings(wlmutils.get_test_launcher(), {}, {}) def test_ensemble_user_created_strategy(mock_launcher_settings, test_dir): From 36af08321316e85b3465fd44ed53ff537fe1dacb Mon Sep 17 00:00:00 2001 From: Matt Drozt Date: Wed, 3 Jul 2024 12:54:19 -0500 Subject: [PATCH 18/19] Remove old ens impl --- smartsim/entity/_new_ensemble.py | 117 ------- smartsim/entity/ensemble.py | 558 ++++--------------------------- tests/test_ensemble.py | 2 +- 3 files changed, 70 insertions(+), 607 deletions(-) delete mode 100644 smartsim/entity/_new_ensemble.py diff --git a/smartsim/entity/_new_ensemble.py b/smartsim/entity/_new_ensemble.py deleted file mode 100644 index 7c6e870f80..0000000000 --- a/smartsim/entity/_new_ensemble.py +++ /dev/null @@ -1,117 +0,0 @@ -# BSD 2-Clause License -# -# Copyright (c) 2021-2024, Hewlett Packard Enterprise -# All rights reserved. -# -# Redistribution and use in source and binary forms, with or without -# modification, are permitted provided that the following conditions are met: -# -# 1. Redistributions of source code must retain the above copyright notice, this -# list of conditions and the following disclaimer. -# -# 2. Redistributions in binary form must reproduce the above copyright notice, -# this list of conditions and the following disclaimer in the documentation -# and/or other materials provided with the distribution. -# -# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" -# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE -# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE -# DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE -# FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL -# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR -# SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER -# 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 copy -import itertools -import os -import os.path -import typing as t - -from smartsim.entity import _mock, entity, strategies -from smartsim.entity.files import EntityFiles -from smartsim.entity.model import Application -from smartsim.entity.strategies import ParamSet -from smartsim.launchable.job import Job - -if t.TYPE_CHECKING: - from smartsim.settings.launchSettings import LaunchSettings - - -# TODO: If we like this design, we need to: -# 1) Move this to the `smartsim._core.entity.ensemble` module -# 2) Decide what to do witht the original `Ensemble` impl -class Ensemble(entity.CompoundEntity): - """Entity to help parameterize the creation multiple application - instances. - """ - - def __init__( - self, - name: str, - exe: str | os.PathLike[str], - exe_args: t.Sequence[str] | None = None, - exe_arg_parameters: t.Mapping[str, t.Sequence[t.Sequence[str]]] | None = None, - path: str | os.PathLike[str] | None = None, - files: EntityFiles | None = None, - file_parameters: t.Mapping[str, t.Sequence[str]] | None = None, - permutation_strategy: str | strategies.PermutationStrategyType = "all_perm", - max_permutations: int = -1, - replicas: int = 1, - ) -> None: - self.name = name - self.exe = os.fspath(exe) - self.exe_args = list(exe_args) if exe_args else [] - self.exe_arg_parameters = ( - copy.deepcopy(exe_arg_parameters) if exe_arg_parameters else {} - ) - self.path = os.fspath(path) if path is not None else os.getcwd() - # ^^^^^^^^^^^ - # TODO: Copied from the original implementation, but I'm not sure that - # I like this default. Shouldn't it be something under an - # experiment directory? If so, how it injected?? - self.files = copy.deepcopy(files) if files else EntityFiles() - self.file_parameters = dict(file_parameters) if file_parameters else {} - self.permutation_strategy = permutation_strategy - self.max_permutations = max_permutations - self.replicas = replicas - - def _create_applications(self) -> tuple[Application, ...]: - """Concretize the ensemble attributes into a collection of - application instances. - """ - permutation_strategy = strategies.resolve(self.permutation_strategy) - combinations = permutation_strategy( - self.file_parameters, self.exe_arg_parameters, self.max_permutations - ) - combinations = combinations if combinations else [ParamSet({}, {})] - permutations_ = itertools.chain.from_iterable( - itertools.repeat(permutation, self.replicas) for permutation in combinations - ) - return tuple( - Application( - name=f"{self.name}-{i}", - exe=self.exe, - run_settings=_mock.Mock(), - # ^^^^^^^^^^^^^^^^^^^^^^^ - # FIXME: remove this constructor arg! It should not exist!! - exe_args=self.exe_args, - path=os.path.join(self.path, self.name), - files=self.files, - params=permutation.params, - params_as_args=permutation.exe_args, # type: ignore[arg-type] - # ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - # FIXME: this is the wrong type on Application! - ) - for i, permutation in enumerate(permutations_) - ) - - def as_jobs(self, settings: LaunchSettings) -> tuple[Job, ...]: - apps = self._create_applications() - if not apps: - raise ValueError("There are no members as part of this ensemble") - return tuple(Job(app, settings) for app in apps) diff --git a/smartsim/entity/ensemble.py b/smartsim/entity/ensemble.py index 953b33e64f..517d331615 100644 --- a/smartsim/entity/ensemble.py +++ b/smartsim/entity/ensemble.py @@ -24,511 +24,91 @@ # 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.path as osp -import typing as t -from copy import deepcopy -from os import getcwd +from __future__ import annotations -from tabulate import tabulate +import copy +import itertools +import os +import os.path +import typing as t -from .._core._install.builder import Device -from ..error import EntityExistsError, SmartSimError, SSUnsupportedError -from ..log import get_logger -from ..settings import BatchSettings, RunSettings -from .dbobject import FSModel, FSScript -from .entity import SmartSimEntity -from .entityList import EntityList -from .model import Application -from .strategies import PermutationStrategyType -from .strategies import resolve as resolve_strategy +from smartsim.entity import _mock, entity, strategies +from smartsim.entity.files import EntityFiles +from smartsim.entity.model import Application +from smartsim.entity.strategies import ParamSet +from smartsim.launchable.job import Job -logger = get_logger(__name__) +if t.TYPE_CHECKING: + from smartsim.settings.launchSettings import LaunchSettings -class Ensemble(EntityList[Application]): - """``Ensemble`` is a group of ``Application`` instances that can - be treated as a reference to a single instance. +class Ensemble(entity.CompoundEntity): + """Entity to help parameterize the creation multiple application + instances. """ def __init__( self, name: str, - params: t.Optional[t.Dict[str, t.Any]] = None, - exe: t.Optional[str] = None, - exe_args: t.Optional[t.List[str]] = None, - path: t.Optional[str] = getcwd(), - params_as_args: t.Optional[t.List[str]] = None, - batch_settings: t.Optional[BatchSettings] = None, - run_settings: t.Optional[RunSettings] = None, - perm_strat: t.Union[str, PermutationStrategyType] = "all_perm", - **kwargs: t.Any, - ) -> None: - """Initialize an Ensemble of Application instances. - - The kwargs argument can be used to pass custom input - parameters to the permutation strategy. - - :param name: name of the ensemble - :param exe: executable to run - :param exe_args: executable arguments - :param params: parameters to expand into ``Application`` members - :param params_as_args: list of params that should be used as command - line arguments to the ``Application`` member executables and not written - to generator files - :param batch_settings: describes settings for ``Ensemble`` as batch workload - :param run_settings: describes how each ``Application`` should be executed - :param replicas: number of ``Application`` replicas to create - a keyword - argument of kwargs - :param perm_strategy: strategy for expanding ``params`` into - ``Application`` instances from params argument - options are "all_perm", "step", "random" - or a callable function. - :return: ``Ensemble`` instance - """ - self.exe = exe or "" - self.exe_args = exe_args or [] - self.params = params or {} - self.params_as_args = params_as_args or [] - self._key_prefixing_enabled = True - self.batch_settings = batch_settings - self.run_settings = run_settings - self.replicas: str - - super().__init__(name, path=str(path), perm_strat=perm_strat, **kwargs) - - @property - def applications(self) -> t.Collection[Application]: - """An alias for a shallow copy of the ``entities`` attribute""" - return list(self.entities) - - def _initialize_entities( - self, - *, - perm_strat: t.Union[str, PermutationStrategyType] = "all_perm", - **kwargs: t.Any, + exe: str | os.PathLike[str], + exe_args: t.Sequence[str] | None = None, + exe_arg_parameters: t.Mapping[str, t.Sequence[t.Sequence[str]]] | None = None, + path: str | os.PathLike[str] | None = None, + files: EntityFiles | None = None, + file_parameters: t.Mapping[str, t.Sequence[str]] | None = None, + permutation_strategy: str | strategies.PermutationStrategyType = "all_perm", + max_permutations: int = -1, + replicas: int = 1, ) -> None: - """Initialize all the applications within the ensemble based - on the parameters passed to the ensemble and the permutation - strategy given at init. - - :raises UserStrategyError: if user generation strategy fails - """ - strategy = resolve_strategy(perm_strat) - replicas = kwargs.pop("replicas", None) - self.replicas = replicas - - # if a ensemble has parameters and run settings, create - # the ensemble and assign run_settings to each member - if self.params: - if self.run_settings and self.exe: - # Compute all combinations of application parameters and arguments - n_applications = kwargs.get("n_applications", 0) - all_application_params = strategy(self.params, n_applications) - - for i, param_set in enumerate(all_application_params): - run_settings = deepcopy(self.run_settings) - application_name = "_".join((self.name, str(i))) - application = Application( - name=application_name, - exe=self.exe, - exe_args=self.exe_args, - params=param_set, - path=osp.join(self.path, application_name), - run_settings=run_settings, - params_as_args=self.params_as_args, - ) - application.enable_key_prefixing() - application.params_to_args() - logger.debug( - f"Created ensemble member: {application_name} in {self.name}" - ) - self.add_application(application) - # cannot generate applications without run settings - else: - raise SmartSimError( - "Ensembles without 'params' or 'replicas' argument to " - "expand into members cannot be given run settings" - ) - else: - if self.run_settings and self.exe: - if replicas: - for i in range(replicas): - application_name = "_".join((self.name, str(i))) - application = Application( - name=application_name, - params={}, - exe=self.exe, - exe_args=self.exe_args, - path=osp.join(self.path, application_name), - run_settings=deepcopy(self.run_settings), - ) - application.enable_key_prefixing() - logger.debug( - "Created ensemble member: " - f"{application_name} in {self.name}" - ) - self.add_application(application) - else: - raise SmartSimError( - "Ensembles without 'params' or 'replicas' argument to " - "expand into members cannot be given run settings" - ) - # if no params, no run settings and no batch settings, error because we - # don't know how to run the ensemble - elif not self.batch_settings: - raise SmartSimError( - "Ensemble must be provided batch settings or run settings" - ) - else: - logger.info("Empty ensemble created for batch launch") - - def add_application(self, application: Application) -> None: - """Add a application to this ensemble - - :param application: application instance to be added - :raises TypeError: if application is not an instance of ``Application`` - :raises EntityExistsError: if application already exists in this ensemble - """ - if not isinstance(application, Application): - raise TypeError( - f"Argument to add_application was of type {type(application)}, " - "not Application" - ) - # "in" operator uses application name for __eq__ - if application in self.entities: - raise EntityExistsError( - f"Application {application.name} already exists in ensemble {self.name}" - ) - - if self._fs_models: - self._extend_entity_fs_models(application, self._fs_models) - if self._fs_scripts: - self._extend_entity_fs_scripts(application, self._fs_scripts) - - self.entities.append(application) - - def register_incoming_entity(self, incoming_entity: SmartSimEntity) -> None: - """Register future communication between entities. - - Registers the named data sources that this entity - has access to by storing the key_prefix associated - with that entity - - Only python clients can have multiple incoming connections - - :param incoming_entity: The entity that data will be received from - """ - for application in self.applications: - application.register_incoming_entity(incoming_entity) - - def enable_key_prefixing(self) -> None: - """If called, each application within this ensemble will prefix its key with its - own application name. - """ - for application in self.applications: - application.enable_key_prefixing() - - def query_key_prefixing(self) -> bool: - """Inquire as to whether each application within the ensemble will - prefix their keys - - :returns: True if all applications have key prefixing enabled, False otherwise - """ - return all( - application.query_key_prefixing() for application in self.applications + self.name = name + self.exe = os.fspath(exe) + self.exe_args = list(exe_args) if exe_args else [] + self.exe_arg_parameters = ( + copy.deepcopy(exe_arg_parameters) if exe_arg_parameters else {} ) + self.path = os.fspath(path) if path is not None else os.getcwd() + # ^^^^^^^^^^^ + # TODO: Copied from the original implementation, but I'm not sure that + # I like this default. Shouldn't it be something under an + # experiment directory? If so, how it injected?? + self.files = copy.deepcopy(files) if files else EntityFiles() + self.file_parameters = dict(file_parameters) if file_parameters else {} + self.permutation_strategy = permutation_strategy + self.max_permutations = max_permutations + self.replicas = replicas - 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 application within the ensemble for generation - - Attach files needed for the entity that, upon generation, - will be located in the path of the entity. - - During generation, files "to_copy" are copied into - the path of the entity, and files "to_symlink" are - symlinked into the path of the entity. - - Files "to_configure" are text based application input files where - parameters for the application are set. Note that only applications - support the "to_configure" field. These files must have - fields tagged that correspond to the values the user - would like to change. The tag is settable but defaults - to a semicolon e.g. THERMO = ;10; - - :param to_copy: files to copy - :param to_symlink: files to symlink - :param to_configure: input files with tagged parameters - """ - for application in self.applications: - application.attach_generator_files( - to_copy=to_copy, to_symlink=to_symlink, to_configure=to_configure - ) - - @property - def attached_files_table(self) -> str: - """Return a plain-text table with information about files - attached to applications belonging to this ensemble. - - :returns: A table of all files attached to all applications - """ - if not self.applications: - return "The ensemble is empty, no files to show." - - table = tabulate( - [ - [application.name, application.attached_files_table] - for application in self.applications - ], - headers=["Application name", "Files"], - tablefmt="grid", - ) - - return table - - def print_attached_files(self) -> None: - """Print table of attached files to std out""" - print(self.attached_files_table) - - def add_ml_model( - self, - name: str, - backend: str, - model: t.Optional[bytes] = None, - model_path: t.Optional[str] = None, - device: str = Device.CPU.value.upper(), - devices_per_node: int = 1, - first_device: int = 0, - batch_size: int = 0, - min_batch_size: int = 0, - min_batch_timeout: int = 0, - tag: str = "", - inputs: t.Optional[t.List[str]] = None, - outputs: t.Optional[t.List[str]] = None, - ) -> None: - """A TF, TF-lite, PT, or ONNX model to load into the fs at runtime - - Each ML Model added will be loaded into a - feature store (converged or not) prior to the execution - of every entity belonging to this ensemble - - One of either model (in memory representation) or model_path (file) - must be provided - - :param name: key to store model under - :param model: model in memory - :param model_path: serialized model - :param backend: name of the backend (TORCH, TF, TFLITE, ONNX) - :param device: name of device for execution - :param devices_per_node: number of GPUs per node in multiGPU nodes - :param first_device: first device in multi-GPU nodes to use for execution, - defaults to 0; ignored if devices_per_node is 1 - :param batch_size: batch size for execution - :param min_batch_size: minimum batch size for model execution - :param min_batch_timeout: time to wait for minimum batch size - :param tag: additional tag for model information - :param inputs: model inputs (TF only) - :param outputs: model outupts (TF only) - """ - fs_model = FSModel( - name=name, - backend=backend, - model=model, - model_file=model_path, - device=device, - devices_per_node=devices_per_node, - first_device=first_device, - batch_size=batch_size, - min_batch_size=min_batch_size, - min_batch_timeout=min_batch_timeout, - tag=tag, - inputs=inputs, - outputs=outputs, - ) - dupe = next( - ( - fs_model.name - for ensemble_ml_model in self._fs_models - if ensemble_ml_model.name == fs_model.name - ), - None, - ) - if dupe: - raise SSUnsupportedError( - f'An ML Model with name "{fs_model.name}" already exists' - ) - self._fs_models.append(fs_model) - for entity in self.applications: - self._extend_entity_fs_models(entity, [fs_model]) - - def add_script( - self, - name: str, - script: t.Optional[str] = None, - script_path: t.Optional[str] = None, - device: str = Device.CPU.value.upper(), - devices_per_node: int = 1, - first_device: int = 0, - ) -> None: - """TorchScript to launch with every entity belonging to this ensemble - - Each script added to the application will be loaded into an - feature store (converged or not) prior to the execution - of every entity belonging to this ensemble - - Device selection is either "GPU" or "CPU". If many devices are - present, a number can be passed for specification e.g. "GPU:1". - - Setting ``devices_per_node=N``, with N greater than one will result - in the application being stored in the first N devices of type ``device``. - - One of either script (in memory string representation) or script_path (file) - must be provided - - :param name: key to store script under - :param script: TorchScript code - :param script_path: path to TorchScript code - :param device: device for script execution - :param devices_per_node: number of devices on each host - :param first_device: first device to use on each host + def _create_applications(self) -> tuple[Application, ...]: + """Concretize the ensemble attributes into a collection of + application instances. """ - fs_script = FSScript( - name=name, - script=script, - script_path=script_path, - device=device, - devices_per_node=devices_per_node, - first_device=first_device, + permutation_strategy = strategies.resolve(self.permutation_strategy) + combinations = permutation_strategy( + self.file_parameters, self.exe_arg_parameters, self.max_permutations ) - dupe = next( - ( - fs_script.name - for ensemble_script in self._fs_scripts - if ensemble_script.name == fs_script.name - ), - None, + combinations = combinations if combinations else [ParamSet({}, {})] + permutations_ = itertools.chain.from_iterable( + itertools.repeat(permutation, self.replicas) for permutation in combinations ) - if dupe: - raise SSUnsupportedError( - f'A Script with name "{fs_script.name}" already exists' + return tuple( + Application( + name=f"{self.name}-{i}", + exe=self.exe, + run_settings=_mock.Mock(), + # ^^^^^^^^^^^^^^^^^^^^^^^ + # FIXME: remove this constructor arg! It should not exist!! + exe_args=self.exe_args, + path=os.path.join(self.path, self.name), + files=self.files, + params=permutation.params, + params_as_args=permutation.exe_args, # type: ignore[arg-type] + # ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + # FIXME: this is the wrong type on Application! ) - self._fs_scripts.append(fs_script) - for entity in self.applications: - self._extend_entity_fs_scripts(entity, [fs_script]) - - def add_function( - self, - name: str, - function: t.Optional[str] = None, - device: str = Device.CPU.value.upper(), - devices_per_node: int = 1, - first_device: int = 0, - ) -> None: - """TorchScript function to launch with every entity belonging to this ensemble - - Each script function to the application will be loaded into a - non-converged feature store prior to the execution - of every entity belonging to this ensemble. - - For converged feature stores, the :meth:`add_script` method should be used. - - Device selection is either "GPU" or "CPU". If many devices are - present, a number can be passed for specification e.g. "GPU:1". - - Setting ``devices_per_node=N``, with N greater than one will result - in the script being stored in the first N devices of type ``device``; - alternatively, setting ``first_device=M`` will result in the script - being stored on nodes M through M + N - 1. - - :param name: key to store function under - :param function: TorchScript code - :param device: device for script execution - :param devices_per_node: number of devices on each host - :param first_device: first device to use on each host - """ - fs_script = FSScript( - name=name, - script=function, - device=device, - devices_per_node=devices_per_node, - first_device=first_device, - ) - dupe = next( - ( - fs_script.name - for ensemble_script in self._fs_scripts - if ensemble_script.name == fs_script.name - ), - None, + for i, permutation in enumerate(permutations_) ) - if dupe: - raise SSUnsupportedError( - f'A Script with name "{fs_script.name}" already exists' - ) - self._fs_scripts.append(fs_script) - for entity in self.applications: - self._extend_entity_fs_scripts(entity, [fs_script]) - @staticmethod - def _extend_entity_fs_models( - application: Application, fs_models: t.List[FSModel] - ) -> None: - """ - Ensures that the Machine Learning model names being added to the Ensemble - are unique. - - This static method checks if the provided ML model names already exist in - the Ensemble. An SSUnsupportedError is raised if any duplicate names are - found. Otherwise, it appends the given list of FSModel to the Ensemble. - - :param application: SmartSim Application object. - :param fs_models: List of FSModels to append to the Ensemble. - """ - for add_ml_model in fs_models: - dupe = next( - ( - fs_model.name - for fs_model in application.fs_models - if fs_model.name == add_ml_model.name - ), - None, - ) - if dupe: - raise SSUnsupportedError( - f'An ML Model with name "{add_ml_model.name}" already exists' - ) - application.add_ml_model_object(add_ml_model) - - @staticmethod - def _extend_entity_fs_scripts( - application: Application, fs_scripts: t.List[FSScript] - ) -> None: - """ - Ensures that the script/function names being added to the Ensemble are unique. - - This static method checks if the provided script/function names already exist - in the Ensemble. An SSUnsupportedError is raised if any duplicate names - are found. Otherwise, it appends the given list of FSScripts to the - Ensemble. - - :param application: SmartSim Application object. - :param fs_scripts: List of FSScripts to append to the Ensemble. - """ - for add_script in fs_scripts: - dupe = next( - ( - add_script.name - for fs_script in application.fs_scripts - if fs_script.name == add_script.name - ), - None, - ) - if dupe: - raise SSUnsupportedError( - f'A Script with name "{add_script.name}" already exists' - ) - application.add_script_object(add_script) + def as_jobs(self, settings: LaunchSettings) -> tuple[Job, ...]: + apps = self._create_applications() + if not apps: + raise ValueError("There are no members as part of this ensemble") + return tuple(Job(app, settings) for app in apps) diff --git a/tests/test_ensemble.py b/tests/test_ensemble.py index c43b178085..3f170dfcb2 100644 --- a/tests/test_ensemble.py +++ b/tests/test_ensemble.py @@ -30,7 +30,7 @@ import pytest from smartsim.entity import _mock -from smartsim.entity._new_ensemble import Ensemble +from smartsim.entity.ensemble import Ensemble from smartsim.entity.strategies import ParamSet from smartsim.settings.launchSettings import LaunchSettings From 022888a08d41c602425371160d2245b6a7af4325 Mon Sep 17 00:00:00 2001 From: Matt Drozt Date: Tue, 9 Jul 2024 16:33:01 -0500 Subject: [PATCH 19/19] Address reviewer comments --- tests/test_permutation_strategies.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/tests/test_permutation_strategies.py b/tests/test_permutation_strategies.py index 2bf5496ead..b14514c99b 100644 --- a/tests/test_permutation_strategies.py +++ b/tests/test_permutation_strategies.py @@ -37,10 +37,14 @@ def test_strategy_registration(monkeypatch): monkeypatch.setattr(strategies, "_REGISTERED_STRATEGIES", {}) - assert not strategies._REGISTERED_STRATEGIES + assert strategies._REGISTERED_STRATEGIES == {} new_strat = lambda params, exe_args, nmax: [] - strategies._register("new_strat")(new_strat) + decorator = strategies._register("new_strat") + assert strategies._REGISTERED_STRATEGIES == {} + + ret_val = decorator(new_strat) + assert ret_val is new_strat assert strategies._REGISTERED_STRATEGIES == {"new_strat": new_strat} @@ -83,13 +87,13 @@ def broken_strategy(p, n, e): pytest.param(lambda params, exe_args, nmax: 123, id="Does not return a list"), pytest.param( lambda params, exe_args, nmax: [1, 2, 3], - id="Does not return a list of dicts", + id="Does not return a list of ParamSet", ), ), ) def test_custom_strategy_raises_user_strategy_error_if_something_goes_wrong(strategy): with pytest.raises(errors.UserStrategyError): - strategies.resolve(strategy)({"SPAM": ["EGGS"]}, {"SPAM": [["EGGS"]]}, 123) + strategies.resolve(strategy)({"SPAM": ["EGGS"]}, {"HELLO": [["WORLD"]]}, 123) @pytest.mark.parametrize(