Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve parameter handling with and without generation #107

Merged
merged 5 commits into from
Nov 16, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 26 additions & 7 deletions smartsim/entity/ensemble.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ def __init__(
self,
name,
params,
params_as_args=None,
batch_settings=None,
run_settings=None,
perm_strat="all_perm",
Expand All @@ -63,6 +64,10 @@ def __init__(
:type name: str
:param params: parameters to expand into ``Model`` members
:type params: dict[str, Any]
:param params_as_args: list of params which should be used as command line arguments
to the ``Model`` member executables and not written to generator
files
:type arg_params: list[str]
:param batch_settings: describes settings for ``Ensemble`` as batch workload
:type batch_settings: BatchSettings, optional
:param run_settings: describes how each ``Model`` should be executed
Expand All @@ -78,6 +83,7 @@ def __init__(
:rtype: ``Ensemble``
"""
self.params = init_default({}, params, dict)
self.params_as_args = init_default({}, params_as_args, (list,str))
self._key_prefixing_enabled = True
self.batch_settings = init_default({}, batch_settings, BatchSettings)
self.run_settings = init_default({}, run_settings, RunSettings)
Expand All @@ -96,32 +102,42 @@ def _initialize_entities(self, **kwargs):
"""
strategy = self._set_strategy(kwargs.pop("perm_strat"))
replicas = kwargs.pop("replicas", None)

# if a ensemble has parameters and run settings, create
# the ensemble and copy run_settings to each member
# the ensemble and assign run_settings to each member
if self.params:
if self.run_settings:
names, params = self._read_model_parameters()
all_model_params = strategy(names, params, **kwargs)
param_names, params = self._read_model_parameters()

# Compute all combinations of model parameters and arguments
all_model_params = strategy(
param_names, params, **kwargs
)
if not isinstance(all_model_params, list):
raise UserStrategyError(strategy)

for i, param_set in enumerate(all_model_params):
if not isinstance(param_set, dict):
raise UserStrategyError(strategy)

run_settings = deepcopy(self.run_settings)
model_name = "_".join((self.name, str(i)))
model = Model(
model_name,
param_set,
self.path,
run_settings=deepcopy(self.run_settings),
run_settings=run_settings,
params_as_args=self.params_as_args,
)
model.enable_key_prefixing()
model.params_to_args()
logger.debug(
f"Created ensemble member: {model_name} in {self.name}"
)
self.add_model(model)
# cannot generate models without run settings
else:
raise SmartSimError(
"Ensembles supplied with 'params' argument must be provided run settings"
"Ensembles without 'params' or 'replicas' argument to expand into members cannot be given run settings"
)
else:
if self.run_settings:
Expand Down Expand Up @@ -152,6 +168,7 @@ def _initialize_entities(self, **kwargs):
else:
logger.info("Empty ensemble created for batch launch")


def add_model(self, model):
"""Add a model to this ensemble

Expand Down Expand Up @@ -260,10 +277,12 @@ def _read_model_parameters(self):
:return: param names and values for permutation strategy
:rtype: tuple[list, list]
"""

if not isinstance(self.params, dict):
raise TypeError(
"Ensemble initialization argument 'params' must be of type dict"
)

param_names = []
parameters = []
for name, val in self.params.items():
Expand All @@ -278,4 +297,4 @@ def _read_model_parameters(self):
"Incorrect type for ensemble parameters\n"
+ "Must be list, int, or string."
)
return param_names, parameters
return param_names, parameters
25 changes: 22 additions & 3 deletions smartsim/entity/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,27 +24,34 @@
# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

from smartsim.error.errors import SSConfigError
from ..error import EntityExistsError
from ..utils.helpers import init_default
from ..utils.helpers import cat_arg_and_value, init_default
from .entity import SmartSimEntity
from .files import EntityFiles


class Model(SmartSimEntity):
def __init__(self, name, params, path, run_settings):
def __init__(self, name, params, path, run_settings, params_as_args=None):
"""Initialize a model entity within Smartsim

:param name: name of the model
:type name: str
:param params: model parameters for writing into configuration files.
:param params: model parameters for writing into configuration files or
to be passed as command line arguments to executable.
:type params: dict
:param path: path to output, error, and configuration files
:type path: str
:param run_settings: launcher settings specified in the experiment
:type run_settings: RunSettings
:param params_as_args: list of parameters which have to be
interpreted as command line arguments to
be added to run_settings
:type params_as_args: list[str]
"""
super().__init__(name, path, run_settings)
self.params = params
self.params_as_args = params_as_args
self.incoming_entities = []
self._key_prefixing_enabled = False
self.files = None
Expand Down Expand Up @@ -111,6 +118,18 @@ def attach_generator_files(self, to_copy=None, to_symlink=None, to_configure=Non
to_configure = init_default([], to_configure, (list, str))
self.files = EntityFiles(to_configure, to_copy, to_symlink)

def params_to_args(self):
"""Convert parameters to command line arguments and update run settings.
"""
for param in self.params_as_args:
if not param in self.params:
raise SSConfigError(f"Tried to convert {param} to command line argument " +
f"for Model {self.name}, but its value was not found in model params")
if self.run_settings is None:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will this case ever happen?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Spartee it seems like we don't check whether run_settings is None, thus a very bad user might initialize a Model with run_settings=None. I don't know if there is any reason why we are allowing this, otherwise we might want to guard against it in another part of the code?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yea I think step creation would just fail, but when we go to the server arch I think thisll be more important.

raise SSConfigError(f"Tried to configure command line parameter for Model {self.name}, " +
"but no RunSettings are set.")
self.run_settings.add_exe_args(cat_arg_and_value(param, self.params[param]))

def __eq__(self, other):
if self.name == other.name:
return True
Expand Down
37 changes: 21 additions & 16 deletions smartsim/generation/modelwriter.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
class ModelWriter:
def __init__(self):
self.tag = ";"
self.regex = "(;.+;)"
self.regex = "(;[^;]+;)"
self.lines = []

def set_tag(self, tag, regex=None):
Expand Down Expand Up @@ -108,21 +108,26 @@ def _replace_tags(self, params):
for i, line in enumerate(self.lines):
search = re.search(self.regex, line)
if search:
tagged_line = search.group(0)
previous_value = self._get_prev_value(tagged_line)
if self._is_ensemble_spec(tagged_line, params):
new_val = str(params[previous_value])
new_line = re.sub(self.regex, new_val, line)
edited.append(new_line)

# if a tag is found but is not in this model's configurations
# put in placeholder value
else:
tag = tagged_line.split(self.tag)[1]
if tag not in unused_tags:
unused_tags[tag] = []
unused_tags[tag].append(i + 1)
edited.append(re.sub(self.regex, previous_value, line))
while search:
tagged_line = search.group(0)
previous_value = self._get_prev_value(tagged_line)
if self._is_ensemble_spec(tagged_line, params):
new_val = str(params[previous_value])
new_line = re.sub(self.regex, new_val, line, 1)
search = re.search(self.regex, new_line)
if not search:
edited.append(new_line)
else:
line = new_line

# if a tag is found but is not in this model's configurations
# put in placeholder value
else:
tag = tagged_line.split(self.tag)[1]
if tag not in unused_tags:
unused_tags[tag] = []
unused_tags[tag].append(i + 1)
edited.append(re.sub(self.regex, previous_value, line))
else:
edited.append(line)
for tag in unused_tags:
Expand Down
33 changes: 33 additions & 0 deletions smartsim/utils/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,3 +141,36 @@ def delete_elements(dictionary, key_list):
for key in key_list:
if key in dictionary:
del dictionary[key]


def cat_arg_and_value(arg_name, value):
"""Concatenate a command line argument and its value

This function returns ``arg_name`` and ``value
concatenated in the best possible way for a command
line execution, namely:
- if arg_name starts with `--` (e.g. `--arg`):
`arg_name=value` is returned (i.e. `--arg=val`)
- if arg_name starts with `-` (e.g. `-a`):
`arg_name value` is returned (i.e. `-a val`)
- if arg_name does not start with `-` and it is a
long option (e.g. `arg`):
`--arg_name=value` (i.e., `--arg=val`)
- if arg_name does not start with `-` and it is a
short option (e.g. `a`):
`-arg_name=value` (i.e., `-a val`)

:param arg_name: the command line argument name
:type arg_name: str
:param value: the command line argument value
:type value: str
"""

if arg_name.startswith("--"):
return "=".join((arg_name, str(value)))
elif arg_name.startswith("-"):
return " ".join((arg_name, str(value)))
elif len(arg_name) == 1:
return " ".join(("-" + arg_name, str(value)))
else:
return "=".join(("--" + arg_name, str(value)))
1 change: 1 addition & 0 deletions tests/backends/run_sklearn_onnx.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from sklearn.ensemble import RandomForestRegressor
from sklearn.linear_model import LinearRegression
from sklearn.model_selection import train_test_split

from smartredis import Client


Expand Down
2 changes: 1 addition & 1 deletion tests/backends/run_tf.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import os

import numpy as np
from smartredis import Client
from tensorflow import keras

from smartredis import Client
from smartsim.tf import freeze_model


Expand Down
1 change: 1 addition & 0 deletions tests/backends/run_torch.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import torch
import torch.nn as nn
import torch.nn.functional as F

from smartredis import Client


Expand Down
1 change: 1 addition & 0 deletions tests/test_configs/multi_tags_template.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
echo "My two parameters are ;port; and ;password;, OK?"
1 change: 1 addition & 0 deletions tests/test_configs/smartredis/consumer.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import numpy as np
import torch
import torch.nn as nn

from smartredis import Client

if __name__ == "__main__":
Expand Down
1 change: 1 addition & 0 deletions tests/test_configs/smartredis/producer.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import numpy as np
import torch
import torch.nn as nn

from smartredis import Client


Expand Down
91 changes: 91 additions & 0 deletions tests/test_ensemble.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from copy import deepcopy

import pytest

from smartsim import Experiment
Expand Down Expand Up @@ -102,6 +104,95 @@ def test_user_strategy():
assert ensemble.entities[1].params == model_2_params


# ----- Model arguments -------------------------------------


def test_arg_params():
"""Test parameterized exe arguments"""
params = {"H": [5, 6], "g_param": ["a", "b"]}

# Copy rs to avoid modifying referenced object
rs_copy = deepcopy(rs)
rs_orig_args = rs_copy.exe_args
ensemble = Ensemble(
"step",
params=params,
params_as_args=list(params.keys()),
run_settings=rs_copy,
perm_strat="step",
)
assert len(ensemble) == 2

exe_args_0 = rs_orig_args + ["-H", "5", "--g_param=a"]
assert ensemble.entities[0].run_settings.exe_args == exe_args_0

exe_args_1 = rs_orig_args + ["-H", "6", "--g_param=b"]
assert ensemble.entities[1].run_settings.exe_args == exe_args_1


def test_arg_and_model_params_step():
"""Test parameterized exe arguments combined with
model parameters and step strategy
"""
params = {"H": [5, 6], "g_param": ["a", "b"], "h": [5, 6], "g": [7, 8]}

# Copy rs to avoid modifying referenced object
rs_copy = deepcopy(rs)
rs_orig_args = rs_copy.exe_args
ensemble = Ensemble(
"step", params, params_as_args=["H", "g_param"], run_settings=rs_copy, perm_strat="step"
)
assert len(ensemble) == 2

exe_args_0 = rs_orig_args + ["-H", "5", "--g_param=a"]
assert ensemble.entities[0].run_settings.exe_args == exe_args_0

exe_args_1 = rs_orig_args + ["-H", "6", "--g_param=b"]
assert ensemble.entities[1].run_settings.exe_args == exe_args_1

model_1_params = {"H": 5, "g_param": "a", "h": 5, "g": 7}
assert ensemble.entities[0].params == model_1_params

model_2_params = {"H": 6, "g_param": "b", "h": 6, "g": 8}
assert ensemble.entities[1].params == model_2_params


def test_arg_and_model_params_all_perms():
"""Test parameterized exe arguments combined with
model parameters and all_perm strategy
"""
params = {"h": [5, 6], "g_param": ["a", "b"]}

# Copy rs to avoid modifying referenced object
rs_copy = deepcopy(rs)
rs_orig_args = rs_copy.exe_args
ensemble = Ensemble(
"step",
params,
params_as_args=["g_param"],
run_settings=rs_copy,
perm_strat="all_perm",
)
assert len(ensemble) == 4

exe_args_0 = rs_orig_args + ["--g_param=a"]
assert ensemble.entities[0].run_settings.exe_args == exe_args_0
assert ensemble.entities[2].run_settings.exe_args == exe_args_0

exe_args_1 = rs_orig_args + ["--g_param=b"]
assert ensemble.entities[1].run_settings.exe_args == exe_args_1
assert ensemble.entities[3].run_settings.exe_args == exe_args_1

model_0_params = {"g_param": "a", "h": 5}
assert ensemble.entities[0].params == model_0_params
model_1_params = {"g_param": "b", "h": 5}
assert ensemble.entities[1].params == model_1_params
model_2_params = {"g_param": "a", "h": 6}
assert ensemble.entities[2].params == model_2_params
model_3_params = {"g_param": "b", "h": 6}
assert ensemble.entities[3].params == model_3_params


# ----- Error Handling --------------------------------------

# unknown permuation strategy
Expand Down
Loading