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

Add log to file of parameters #353

Merged
merged 11 commits into from
Sep 28, 2023
63 changes: 58 additions & 5 deletions smartsim/_core/generation/generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,10 @@
import typing as t

from distutils import dir_util # pylint: disable=deprecated-module
from logging import INFO, DEBUG
from os import mkdir, path, symlink
from os.path import join, relpath
from tabulate import tabulate

from ...entity import Model, TaggedFilesHierarchy
from ...log import get_logger
Expand All @@ -49,7 +52,9 @@ class Generator:
and writing into configuration files as well.
"""

def __init__(self, gen_path: str, overwrite: bool = False) -> None:
def __init__(
self, gen_path: str, overwrite: bool = False, verbose: bool = True
) -> None:
"""Initialize a generator object

if overwrite is true, replace any existing
Expand All @@ -59,12 +64,18 @@ def __init__(self, gen_path: str, overwrite: bool = False) -> None:
is false, raises EntityExistsError when there is a name
collision between entities.

:param gen_path: Path in which files need to be generated
:type gen_path: str
:param overwrite: toggle entity replacement, defaults to False
:type overwrite: bool, optional
:param verbose: Whether generation information should be logged to std out
:type verbose: bool, optional
"""
self._writer = ModelWriter()
self.gen_path = gen_path
self.log_file = join(gen_path, "param_settings.txt")
MattToast marked this conversation as resolved.
Show resolved Hide resolved
self.overwrite = overwrite
self.log_level = DEBUG if not verbose else INFO

def generate_experiment(self, *args: t.Any) -> None:
"""Run ensemble and experiment file structure generation
Expand Down Expand Up @@ -129,7 +140,9 @@ def _gen_exp_dir(self) -> None:
# keep exists ok for race conditions on NFS
pathlib.Path(self.gen_path).mkdir(exist_ok=True)
else:
logger.info("Working in previously created experiment")
logger.log(
level=self.log_level, msg="Working in previously created experiment"
)

def _gen_orc_dir(self, orchestrator: t.Optional[Orchestrator]) -> None:
"""Create the directory that will hold the error, output and
Expand Down Expand Up @@ -249,10 +262,50 @@ def _build_tagged_files(tagged: TaggedFilesHierarchy) -> None:

# write in changes to configurations
if isinstance(entity, Model):
logger.debug(
f"Configuring model {entity.name} with params {entity.params}"
files_to_params = self._writer.configure_tagged_model_files(
to_write, entity.params
)
self._writer.configure_tagged_model_files(to_write, entity.params)
self._log_params(entity, files_to_params)

def _log_params(
self, entity: Model, files_to_params: t.Dict[str, t.Dict[str, str]]
) -> None:
"""Log which files were modified during generation

and what values were set to the parameters

:param entity: the model being generated
:type entity: Model
:param files_to_params: a dict connecting each file to its parameter settings
:type files_to_params: t.Dict[str, t.Dict[str, str]]
"""
used_params: t.Dict[str, str] = {}
file_to_tables: t.Dict[str, str] = {}
for file, params in files_to_params.items():
used_params.update(params)
table = tabulate([[param, value] for param, value in params.items()])
file_to_tables[relpath(file, self.gen_path)] = table

if used_params:
used_params_str = ", ".join(
[f"{name}={value}" for name, value in used_params.items()]
)
logger.log(
level=self.log_level,
msg=f"Configured model {entity.name} with params {used_params_str}",
)
with open(self.log_file, mode="a", encoding="utf-8") as logfile:
Copy link
Member

Choose a reason for hiding this comment

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

Quick sanity check for my understanding:

If I have this setup to an experiment:

exp  = Experiment("my_exp")

rs1 = exp.create_run_settings(...)
ens1 = exp.create_ensemble("my_ens_1", run_settings=rs1, params={"SPAM": [...], "EGGS": [...]})
ens1.attach_generator_files(to_configure=[...])


rs2 = exp.create_run_settings(...)
ens2 = exp.create_ensemble("my_ens_2", run_settings=rs2, params={"FOO": [...], "BAR": [...]})
ens2.attach_generator_files(to_configure=[...])

exp.generate(ens1, ens2)

This will generate one single param_settings.txt in the experiment root, correct? Would it be worth the effort to make multiple param_settings.txts instead of/in addition to this file in the ensemble roots?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I am in favor of the additional ones, but I'd like to keep the experiment-wide one in the root, for a quicker inspection.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added them.

logfile.write(f"Model name: {entity.name}" + "\n")
file_table = tabulate(
[[file, table] for file, table in file_to_tables.items()],
MattToast marked this conversation as resolved.
Show resolved Hide resolved
headers=["File name", "Parameters"],
)
logfile.write(file_table + "\n\n")
else:
logger.log(
level=self.log_level,
msg=f"Configured model {entity.name} with no parameters",
)

@staticmethod
def _copy_entity_files(entity: Model) -> None:
Expand Down
27 changes: 19 additions & 8 deletions smartsim/_core/generation/modelwriter.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,21 +63,27 @@ def configure_tagged_model_files(
tagged_files: t.List[str],
params: t.Dict[str, str],
make_missing_tags_fatal: bool = False,
) -> None:
) -> t.Dict[str, t.Dict[str, str]]:
"""Read, write and configure tagged files attached to a Model
instance.

:param tagged_files: list of paths to tagged files
:type model: list[str]
:param params: model parameters
:type params: dict[str, str]
:param make_missing_tags_fatal: blow up if a tag is missing
:param make_missing_tags_fatal: raise an error if a tag is missing
:type make_missing_tags_fatal: bool
:returns: A dict connecting each file to its parameter settings
:rtype: dict[str,dict[str,str]]
"""
files_to_tags: t.Dict[str, t.Dict[str, str]] = {}
for tagged_file in tagged_files:
self._set_lines(tagged_file)
self._replace_tags(params, make_missing_tags_fatal)
used_tags = self._replace_tags(params, make_missing_tags_fatal)
self._write_changes(tagged_file)
files_to_tags[tagged_file] = used_tags

return files_to_tags

def _set_lines(self, file_path: str) -> None:
"""Set the lines for the modelwrtter to iterate over
Expand All @@ -104,18 +110,23 @@ def _write_changes(self, file_path: str) -> None:
except (IOError, OSError) as e:
raise ParameterWriterError(file_path, read=False) from e

def _replace_tags(self, params: t.Dict[str, str], make_fatal: bool = False) -> None:
"""Replace the tagged within the tagged file attached to this
def _replace_tags(
self, params: t.Dict[str, str], make_fatal: bool = False
) -> t.Dict[str, str]:
"""Replace the tagged parameters within the file attached to this
model. The tag defaults to ";"

:param model: The model instance
:type model: Model
:param make_fatal: (Optional) Set to True to force a fatal error
if a tag is not matched
:type make_fatal: bool
:returns: A dict of parameter names and values set for the file
:rtype: dict[str,str]
"""
edited = []
unused_tags: t.Dict[str, t.List[int]] = {}
used_params: t.Dict[str, str] = {}
for i, line in enumerate(self.lines):
search = re.search(self.regex, line)
if search:
Expand All @@ -126,6 +137,7 @@ def _replace_tags(self, params: t.Dict[str, str], make_fatal: bool = False) -> N
new_val = str(params[previous_value])
new_line = re.sub(self.regex, new_val, line, 1)
search = re.search(self.regex, new_line)
used_params[previous_value] = new_val
if not search:
edited.append(new_line)
else:
Expand All @@ -143,13 +155,12 @@ def _replace_tags(self, params: t.Dict[str, str], make_fatal: bool = False) -> N
else:
edited.append(line)
for tag, value in unused_tags.items():
missing_tag_message = (
f"Unused tag {tag} on line(s): {str(value)}"
)
missing_tag_message = f"Unused tag {tag} on line(s): {str(value)}"
if make_fatal:
raise SmartSimError(missing_tag_message)
logger.warning(missing_tag_message)
self.lines = edited
return used_params

def _is_ensemble_spec(
self, tagged_line: str, model_params: t.Dict[str, str]
Expand Down
10 changes: 8 additions & 2 deletions smartsim/experiment.py
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,11 @@ def stop(self, *args: t.Any) -> None:
raise

def generate(
self, *args: t.Any, tag: t.Optional[str] = None, overwrite: bool = False
self,
*args: t.Any,
tag: t.Optional[str] = None,
overwrite: bool = False,
verbose: bool = False,
) -> None:
"""Generate the file structure for an ``Experiment``

Expand All @@ -251,9 +255,11 @@ def generate(
:param overwrite: overwrite existing folders and contents,
defaults to False
:type overwrite: bool, optional
:param verbose: log parameter settings to std out
:type verbose: bool
"""
try:
generator = Generator(self.exp_path, overwrite=overwrite)
generator = Generator(self.exp_path, overwrite=overwrite, verbose=verbose)
if tag:
generator.set_tag(tag)
generator.generate_experiment(*args)
Expand Down
32 changes: 32 additions & 0 deletions tests/test_configs/log_params_truth.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
Model name: dir_test_0
File name Parameters
-------------------------- ------------
dir_test/dir_test_0/in.atm ------ --
THERMO 10
STEPS 10
------ --

Model name: dir_test_1
File name Parameters
-------------------------- ------------
dir_test/dir_test_1/in.atm ------ --
THERMO 10
STEPS 20
------ --

Model name: dir_test_2
File name Parameters
-------------------------- ------------
dir_test/dir_test_2/in.atm ------ --
THERMO 20
STEPS 10
------ --

Model name: dir_test_3
File name Parameters
-------------------------- ------------
dir_test/dir_test_3/in.atm ------ --
THERMO 20
STEPS 20
------ --

19 changes: 19 additions & 0 deletions tests/test_generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
# 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 filecmp
from os import path as osp

import pytest
Expand Down Expand Up @@ -265,6 +266,24 @@ def test_multiple_tags(fileutils):
)


def test_generation_log(fileutils):
"""Test that an error is issued when a tag is unused and make_fatal is True"""

test_dir = fileutils.make_test_dir()
exp = Experiment("gen-log-test", test_dir, launcher="local")

params = {"THERMO": [10, 20], "STEPS": [10, 20]}
ensemble = exp.create_ensemble("dir_test", params=params, run_settings=rs)
conf_file = fileutils.get_test_dir_path("in.atm")
ensemble.attach_generator_files(to_configure=conf_file)

exp.generate(ensemble, verbose=True)
assert filecmp.cmp(
osp.join(test_dir, "param_settings.txt"),
fileutils.get_test_dir_path("log_params_truth.txt"),
)


def test_config_dir(fileutils):
"""Test the generation and configuration of models with
tagged files that are directories with subdirectories and files
Expand Down
25 changes: 22 additions & 3 deletions tests/test_modelwriter.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,13 @@
import pytest

from smartsim._core.generation.modelwriter import ModelWriter
from smartsim.error.errors import ParameterWriterError
from smartsim.error.errors import ParameterWriterError, SmartSimError
from smartsim.settings import RunSettings

mw_run_settings = RunSettings("python", exe_args="sleep.py")


def test_write_easy_configs(fileutils):

test_dir = fileutils.make_test_dir()

param_dict = {
Expand Down Expand Up @@ -69,7 +68,6 @@ def test_write_easy_configs(fileutils):


def test_write_med_configs(fileutils):

test_dir = fileutils.make_test_dir()

param_dict = {
Expand Down Expand Up @@ -144,3 +142,24 @@ def test_mw_error_2():
writer = ModelWriter()
with pytest.raises(ParameterWriterError):
writer._write_changes("[not/a/path]")


def test_write_mw_error_3(fileutils):
test_dir = fileutils.make_test_dir()

param_dict = {
"5": 10, # MOM_input
}

conf_path = fileutils.get_test_dir_path("easy/marked/")
correct_path = fileutils.get_test_dir_path("easy/correct/")
# copy confs to gen directory
dir_util.copy_tree(conf_path, test_dir)
assert path.isdir(test_dir)

# init modelwriter
writer = ModelWriter()
with pytest.raises(SmartSimError):
MattToast marked this conversation as resolved.
Show resolved Hide resolved
writer.configure_tagged_model_files(
glob(test_dir + "/*"), param_dict, make_missing_tags_fatal=True
)