From 94daccd5335b0f8ea770da61a44af80cc7055861 Mon Sep 17 00:00:00 2001 From: Amanda Richardson Date: Wed, 28 Aug 2024 23:00:12 -0500 Subject: [PATCH 01/13] init --- tests/test_generator.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/test_generator.py b/tests/test_generator.py index e440227798..a8bb1a6f23 100644 --- a/tests/test_generator.py +++ b/tests/test_generator.py @@ -45,6 +45,8 @@ def get_gen_symlink_dir(fileutils): def get_gen_configure_dir(fileutils): yield fileutils.get_test_conf_path(osp.join("generator_files", "tag_dir_template")) +def get_gen_file(fileutils, filename): + return fileutils.get_test_conf_path(osp.join("generator_files", filename)) @pytest.fixture def generator_instance(test_dir) -> Generator: @@ -375,3 +377,18 @@ def _check_generated(param_0, param_1, dir): _check_generated(1, 3, os.path.join(jobs_dir, "ensemble-name-3-3", "run")) _check_generated(0, 2, os.path.join(jobs_dir, "ensemble-name-0-0", "run")) ids.clear() + + +def test_this(fileutils): + # Directory of files to configure + """Test the generation and configuration of applications with + tagged files that are directories with subdirectories and files + """ + config = get_gen_file(fileutils, "tag_dir_template") + script = fileutils.get_test_conf_path("sleep.py") + #print(list(os.walk(config))) + print(list(os.walk(script))) + # for root, dirs, files in os.walk(config): + # print(list(os.walk(config))) + # # for name in dirs: + # # os.rmdir(os.path.join(root, name)) \ No newline at end of file From 77a6a3de9ae0d2404771afa2bb91b21fadd47239 Mon Sep 17 00:00:00 2001 From: Amanda Richardson Date: Thu, 5 Sep 2024 18:11:21 -0500 Subject: [PATCH 02/13] tests passing, mypy pass, styled --- smartsim/_core/entrypoints/file_operations.py | 88 ++++- smartsim/_core/generation/generator.py | 54 ++- smartsim/entity/__init__.py | 1 - smartsim/entity/files.py | 144 -------- tests/test_file_operations.py | 150 +++++++- tests/test_generator.py | 334 ++++++++++-------- 6 files changed, 420 insertions(+), 351 deletions(-) diff --git a/smartsim/_core/entrypoints/file_operations.py b/smartsim/_core/entrypoints/file_operations.py index 618d305710..944c98a378 100644 --- a/smartsim/_core/entrypoints/file_operations.py +++ b/smartsim/_core/entrypoints/file_operations.py @@ -62,6 +62,16 @@ def _make_substitution( ) +def _prepare_param_dict(param_dict: str) -> dict[str, t.Any]: + decoded_dict = base64.b64decode(param_dict) + encoded_param_dict = pickle.loads(decoded_dict) + if not encoded_param_dict: + raise ValueError("param dictionary is empty") + if not isinstance(encoded_param_dict, dict): + raise TypeError("param dict is not a valid dictionary") + return encoded_param_dict + + def _replace_tags_in( item: str, substitutions: t.Sequence[Callable[[str], str]], @@ -153,11 +163,11 @@ def symlink(parsed_args: argparse.Namespace) -> None: os.symlink(parsed_args.source, parsed_args.dest) -def configure(parsed_args: argparse.Namespace) -> None: +def configure_file(parsed_args: argparse.Namespace) -> None: """Set, search and replace the tagged parameters for the - configure operation within tagged files attached to an entity. + configure_file operation within tagged files attached to an entity. - User-formatted files can be attached using the `configure` argument. + User-formatted files can be attached using the `configure_file` argument. These files will be modified during ``Application`` generation to replace tagged sections in the user-formatted files with values from the `params` initializer argument used during ``Application`` creation: @@ -166,27 +176,21 @@ def configure(parsed_args: argparse.Namespace) -> None: .. highlight:: bash .. code-block:: bash python -m smartsim._core.entrypoints.file_operations \ - configure /absolute/file/source/pat /absolute/file/dest/path \ + configure_file /absolute/file/source/pat /absolute/file/dest/path \ tag_deliminator param_dict /absolute/file/source/path: The tagged files the search and replace operations to be performed upon /absolute/file/dest/path: The destination for configured files to be written to. - tag_delimiter: tag for the configure operation to search for, defaults to + tag_delimiter: tag for the configure_file operation to search for, defaults to semi-colon e.g. ";" param_dict: A dict of parameter names and values set for the file """ tag_delimiter = parsed_args.tag_delimiter - decoded_dict = base64.b64decode(parsed_args.param_dict) - param_dict = pickle.loads(decoded_dict) - - if not param_dict: - raise ValueError("param dictionary is empty") - if not isinstance(param_dict, dict): - raise TypeError("param dict is not a valid dictionary") + param_dict = _prepare_param_dict(parsed_args.param_dict) substitutions = tuple( _make_substitution(k, v, tag_delimiter) for k, v in param_dict.items() @@ -201,6 +205,54 @@ def configure(parsed_args: argparse.Namespace) -> None: file_stream.writelines(lines) +def configure_directory(parsed_args: argparse.Namespace) -> None: + """Set, search and replace the tagged parameters for the + configure_directory operation within tagged directories attached to an entity. + + User-formatted directories can be attached using the `configure_directory` argument. + These files within the directory will be modified during ``Application`` generation to replace + tagged sections in the user-formatted files with values from the `params` + initializer argument used during ``Application`` creation: + + Sample usage: + .. highlight:: bash + .. code-block:: bash + python -m smartsim._core.entrypoints.file_operations \ + configure_directory /absolute/file/source/path /absolute/file/dest/path \ + tag_deliminator param_dict + + /absolute/file/source/path: The tagged directories the search and replace operations + to be performed upon + /absolute/file/dest/path: The destination for configured directories to be + written to. + tag_delimiter: tag for the configure_directory operation to search for, defaults to + semi-colon e.g. ";" + param_dict: A dict of parameter names and values set for the file + + """ + tag_delimiter = parsed_args.tag_delimiter + param_dict = _prepare_param_dict(parsed_args.param_dict) + + substitutions = tuple( + _make_substitution(k, v, tag_delimiter) for k, v in param_dict.items() + ) + for dirpath, _, filenames in os.walk(parsed_args.dest): + if filenames: + for file_name in filenames: + # Set the lines to iterate over + with open( + os.path.join(dirpath, file_name), "r+", encoding="utf-8" + ) as file_stream: + lines = [ + _replace_tags_in(line, substitutions) for line in file_stream + ] + # write configured file to destination specified + with open( + os.path.join(dirpath, file_name), "w+", encoding="utf-8" + ) as file_stream: + file_stream.writelines(lines) + + def get_parser() -> argparse.ArgumentParser: """Instantiate a parser to process command line arguments @@ -235,8 +287,16 @@ def get_parser() -> argparse.ArgumentParser: symlink_parser.add_argument("dest", type=_abspath) # Subparser for configure op - configure_parser = subparsers.add_parser("configure") - configure_parser.set_defaults(func=configure) + configure_parser = subparsers.add_parser("configure_file") + configure_parser.set_defaults(func=configure_file) + configure_parser.add_argument("source", type=_abspath) + configure_parser.add_argument("dest", type=_abspath) + configure_parser.add_argument("tag_delimiter", type=str, default=";") + configure_parser.add_argument("param_dict", type=str) + + # Subparser for configure op + configure_parser = subparsers.add_parser("configure_directory") + configure_parser.set_defaults(func=configure_directory) configure_parser.add_argument("source", type=_abspath) configure_parser.add_argument("dest", type=_abspath) configure_parser.add_argument("tag_delimiter", type=str, default=";") diff --git a/smartsim/_core/generation/generator.py b/smartsim/_core/generation/generator.py index 801af116ce..1a5493a58d 100644 --- a/smartsim/_core/generation/generator.py +++ b/smartsim/_core/generation/generator.py @@ -36,7 +36,7 @@ from os import mkdir, path from os.path import join -from ...entity import Application, TaggedFilesHierarchy +from ...entity import Application from ...entity.files import EntityFiles from ...launchable import Job from ...log import get_logger @@ -240,7 +240,7 @@ def _symlink_files(files: t.Union[EntityFiles, None], dest: pathlib.Path) -> Non @staticmethod def _write_tagged_files( files: t.Union[EntityFiles, None], - params: t.Mapping[str, str], + params: t.Mapping[str, t.Any], dest: pathlib.Path, ) -> None: """Read, configure and write the tagged input files for @@ -250,47 +250,39 @@ def _write_tagged_files( :param app: The Application attached to the Job :param dest: Path to the Jobs run directory """ - # Return if no files are attached if files is None: return if files.tagged: to_write = [] - - def _build_tagged_files(tagged: TaggedFilesHierarchy) -> None: - """Using a TaggedFileHierarchy, reproduce the tagged file - directory structure - - :param tagged: a TaggedFileHierarchy to be built as a - directory structure - """ - for file in tagged.files: - dst_path = path.join(dest, tagged.base, path.basename(file)) - shutil.copyfile(file, dst_path) + for file in files.tagged: + if os.path.isfile(file): + dst_path = os.path.join(dest, os.path.basename(file)) + dst_path = shutil.copyfile(file, dst_path) to_write.append(dst_path) - - for tagged_dir in tagged.dirs: - mkdir(path.join(dest, tagged.base, path.basename(tagged_dir.base))) - _build_tagged_files(tagged_dir) - - if files.tagged_hierarchy: - _build_tagged_files(files.tagged_hierarchy) - - # Pickle the dictionary + elif os.path.isdir(file): + dst_path = shutil.copytree(file, dest, dirs_exist_ok=True) + to_write.append(dst_path) + else: + raise ValueError(f"Invalid path: {file}") + tag_delimiter = ";" pickled_dict = pickle.dumps(params) - # Default tag delimiter - tag = ";" - # Encode the pickled dictionary with Base64 encoded_dict = base64.b64encode(pickled_dict).decode("ascii") - for dest_path in to_write: + for file_sys_path in to_write: + if os.path.isdir(file_sys_path): + file_entrypoint = "configure_directory" + elif os.path.isfile(file_sys_path): + file_entrypoint = "configure" + else: + raise ValueError(f"Invalid path: {file_sys_path}") subprocess.run( args=[ sys.executable, "-m", "smartsim._core.entrypoints.file_operations", - "configure", - dest_path, - dest_path, - tag, + file_entrypoint, + file_sys_path, + file_sys_path, + tag_delimiter, encoded_dict, ] ) diff --git a/smartsim/entity/__init__.py b/smartsim/entity/__init__.py index 7ffa290b2c..d497d11b1c 100644 --- a/smartsim/entity/__init__.py +++ b/smartsim/entity/__init__.py @@ -30,4 +30,3 @@ from .ensemble import Ensemble from .entity import SmartSimEntity, TelemetryConfiguration from .entityList import EntityList, EntitySequence -from .files import TaggedFilesHierarchy diff --git a/smartsim/entity/files.py b/smartsim/entity/files.py index 9ec86a68b5..86a721221a 100644 --- a/smartsim/entity/files.py +++ b/smartsim/entity/files.py @@ -66,7 +66,6 @@ def __init__( self.tagged = tagged or [] self.copy = copy or [] self.link = symlink or [] - self.tagged_hierarchy = None self._check_files() def _check_files(self) -> None: @@ -82,10 +81,6 @@ def _check_files(self) -> None: self.copy = self._type_check_files(self.copy, "Copyable") self.link = self._type_check_files(self.link, "Symlink") - self.tagged_hierarchy = TaggedFilesHierarchy.from_list_paths( - self.tagged, dir_contents_to_base=True - ) - for i, value in enumerate(self.copy): self.copy[i] = self._check_path(value) @@ -147,142 +142,3 @@ def __str__(self) -> str: return "No file attached to this entity." return tabulate(values, headers=["Strategy", "Files"], tablefmt="grid") - - -class TaggedFilesHierarchy: - """The TaggedFilesHierarchy represents a directory - containing potentially tagged files and subdirectories. - - TaggedFilesHierarchy.base is the directory path from - the the root of the generated file structure - - TaggedFilesHierarchy.files is a collection of paths to - files that need to be copied to directory that the - TaggedFilesHierarchy represents - - TaggedFilesHierarchy.dirs is a collection of child - TaggedFilesHierarchy, each representing a subdirectory - that needs to generated - - By performing a depth first search over the entire - hierarchy starting at the root directory structure, the - tagged file directory structure can be replicated - """ - - def __init__(self, parent: t.Optional[t.Any] = None, subdir_name: str = "") -> None: - """Initialize a TaggedFilesHierarchy - - :param parent: The parent hierarchy of the new hierarchy, - must be None if creating a root hierarchy, - must be provided if creating a subhierachy - :param subdir_name: Name of subdirectory representd by the new hierarchy, - must be "" if creating a root hierarchy, - must be any valid dir name if subhierarchy, - invalid names are ".", ".." or contain path seperators - :raises ValueError: if given a subdir_name without a parent, - if given a parent without a subdir_name, - or if the subdir_name is invalid - """ - if parent is None and subdir_name: - raise ValueError( - "TaggedFilesHierarchies should not have a subdirectory name without a" - + " parent" - ) - if parent is not None and not subdir_name: - raise ValueError( - "Child TaggedFilesHierarchies must have a subdirectory name" - ) - if subdir_name in {".", ".."} or path.sep in subdir_name: - raise ValueError( - "Child TaggedFilesHierarchies subdirectory names must not contain" - + " path seperators or be reserved dirs '.' or '..'" - ) - - if parent: - parent.dirs.add(self) - - self._base: str = path.join(parent.base, subdir_name) if parent else "" - self.parent: t.Any = parent - self.files: t.Set[str] = set() - self.dirs: t.Set[TaggedFilesHierarchy] = set() - - @property - def base(self) -> str: - """Property to ensure that self.base is read-only""" - return self._base - - @classmethod - def from_list_paths( - cls, path_list: t.List[str], dir_contents_to_base: bool = False - ) -> t.Any: - """Given a list of absolute paths to files and dirs, create and return - a TaggedFilesHierarchy instance representing the file hierarchy of - tagged files. All files in the path list will be placed in the base of - the file hierarchy. - - :param path_list: list of absolute paths to tagged files or dirs - containing tagged files - :param dir_contents_to_base: When a top level dir is encountered, if - this value is truthy, files in the dir are - put into the base hierarchy level. - Otherwise, a new sub level is created for - the dir - :return: A built tagged file hierarchy for the given files - """ - tagged_file_hierarchy = cls() - if dir_contents_to_base: - new_paths = [] - for tagged_path in path_list: - if os.path.isdir(tagged_path): - new_paths += [ - os.path.join(tagged_path, file) - for file in os.listdir(tagged_path) - ] - else: - new_paths.append(tagged_path) - path_list = new_paths - tagged_file_hierarchy._add_paths(path_list) - return tagged_file_hierarchy - - def _add_file(self, file: str) -> None: - """Add a file to the current level in the file hierarchy - - :param file: absoute path to a file to add to the hierarchy - """ - self.files.add(file) - - def _add_dir(self, dir_path: str) -> None: - """Add a dir contianing tagged files by creating a new sub level in the - tagged file hierarchy. All paths within the directroy are added to the - the new level sub level tagged file hierarchy - - :param dir: absoute path to a dir to add to the hierarchy - """ - tagged_file_hierarchy = TaggedFilesHierarchy(self, path.basename(dir_path)) - # pylint: disable-next=protected-access - tagged_file_hierarchy._add_paths( - [path.join(dir_path, file) for file in os.listdir(dir_path)] - ) - - def _add_paths(self, paths: t.List[str]) -> None: - """Takes a list of paths and iterates over it, determining if each - path is to a file or a dir and then appropriatly adding it to the - TaggedFilesHierarchy. - - :param paths: list of paths to files or dirs to add to the hierarchy - :raises ValueError: if link to dir is found - :raises FileNotFoundError: if path does not exist - """ - for candidate in paths: - candidate = os.path.abspath(candidate) - if os.path.isdir(candidate): - if os.path.islink(candidate): - raise ValueError( - "Tagged directories and thier subdirectories cannot be links" - + " to prevent circular directory structures" - ) - self._add_dir(candidate) - elif os.path.isfile(candidate): - self._add_file(candidate) - else: - raise FileNotFoundError(f"File or Directory {candidate} not found") diff --git a/tests/test_file_operations.py b/tests/test_file_operations.py index 564399fd0c..6302702e91 100644 --- a/tests/test_file_operations.py +++ b/tests/test_file_operations.py @@ -42,6 +42,10 @@ pytestmark = pytest.mark.group_a +def get_gen_file(fileutils, filename): + return fileutils.get_test_conf_path(osp.join("generator_files", filename)) + + def test_symlink_files(test_dir): """ Test operation to symlink files @@ -496,8 +500,8 @@ def test_remove_op_not_absolute(): pytest.param({}, "ValueError", id="empty dict"), ], ) -def test_configure_op(test_dir, fileutils, param_dict, error_type): - """Test configure operation with correct parameter dictionary, empty dicitonary, and an incorrect type""" +def test_configure_file_op(test_dir, fileutils, param_dict, error_type): + """Test configure_file operation with correct parameter dictionary, empty dicitonary, and an incorrect type""" tag = ";" @@ -525,28 +529,28 @@ def test_configure_op(test_dir, fileutils, param_dict, error_type): # Run configure op on test files for tagged_file in tagged_files: parser = get_parser() - cmd = f"configure {tagged_file} {tagged_file} {tag} {encoded_dict}" + cmd = f"configure_file {tagged_file} {tagged_file} {tag} {encoded_dict}" args = cmd.split() ns = parser.parse_args(args) if error_type == "ValueError": with pytest.raises(ValueError) as ex: - file_operations.configure(ns) + file_operations.configure_file(ns) assert "param dictionary is empty" in ex.value.args[0] elif error_type == "TypeError": with pytest.raises(TypeError) as ex: - file_operations.configure(ns) + file_operations.configure_file(ns) assert "param dict is not a valid dictionary" in ex.value.args[0] else: - file_operations.configure(ns) + file_operations.configure_file(ns) if error_type == "None": for written, correct in zip(tagged_files, correct_files): assert filecmp.cmp(written, correct) -def test_configure_invalid_tags(fileutils, test_dir): - """Test configure operation with an invalid tag""" +def test_configure_file_invalid_tags(fileutils, test_dir): + """Test configure_file operation with an invalid tag""" generator_files = pathlib.Path(fileutils.get_test_conf_path("generator_files")) tagged_file = generator_files / "easy/marked/invalidtag.txt" correct_file = generator_files / "easy/correct/invalidtag.txt" @@ -561,17 +565,17 @@ def test_configure_invalid_tags(fileutils, test_dir): # Encode the pickled dictionary with Base64 encoded_dict = base64.b64encode(pickled_dict).decode("ascii") parser = get_parser() - cmd = f"configure {tagged_file} {target_file} {tag} {encoded_dict}" + cmd = f"configure_file {tagged_file} {target_file} {tag} {encoded_dict}" args = cmd.split() ns = parser.parse_args(args) - file_operations.configure(ns) + file_operations.configure_file(ns) assert filecmp.cmp(correct_file, target_file) -def test_configure_not_absolute(): +def test_configure_file_not_absolute(): """Test that ValueError is raised when tagged files - given to configure op are not absolute paths + given to configure_file op are not absolute paths """ tagged_file = ".." @@ -583,7 +587,91 @@ def test_configure_not_absolute(): # Encode the pickled dictionary with Base64 encoded_dict = base64.b64encode(pickled_dict) parser = get_parser() - cmd = f"configure {tagged_file} {tagged_file} {tag} {encoded_dict}" + cmd = f"configure_file {tagged_file} {tagged_file} {tag} {encoded_dict}" + args = cmd.split() + + with pytest.raises(SystemExit) as e: + parser.parse_args(args) + + assert isinstance(e.value.__context__, argparse.ArgumentError) + assert "invalid _abspath value" in e.value.__context__.message + + +@pytest.mark.parametrize( + ["param_dict", "error_type"], + [ + pytest.param( + {"PARAM0": "param_value_1", "PARAM1": "param_value_2"}, + "None", + id="correct dict", + ), + pytest.param( + ["list", "of", "values"], + "TypeError", + id="incorrect dict", + ), + pytest.param({}, "ValueError", id="empty dict"), + ], +) +def test_configure_directory(test_dir, fileutils, param_dict, error_type): + """Test configure_directory operation with correct parameter dictionary, empty dicitonary, and an incorrect type""" + tag = ";" + config = get_gen_file(fileutils, "tag_dir_template") + path = shutil.copytree(config, test_dir, dirs_exist_ok=True) + assert osp.isdir(path) + + # Pickle the dictionary + pickled_dict = pickle.dumps(param_dict) + # Encode the pickled dictionary with Base64 + encoded_dict = base64.b64encode(pickled_dict).decode("ascii") + + parser = get_parser() + cmd = f"configure_directory {path} {path} {tag} {encoded_dict}" + args = cmd.split() + ns = parser.parse_args(args) + + if error_type == "ValueError": + with pytest.raises(ValueError) as ex: + file_operations.configure_directory(ns) + assert "param dictionary is empty" in ex.value.args[0] + elif error_type == "TypeError": + with pytest.raises(TypeError) as ex: + file_operations.configure_directory(ns) + assert "param dict is not a valid dictionary" in ex.value.args[0] + else: + file_operations.configure_directory(ns) + if error_type == "None": + + def _check_generated(param_0, param_1): + assert osp.isdir(osp.join(path, "nested_0")) + assert osp.isdir(osp.join(path, "nested_1")) + + with open(osp.join(path, "nested_0", "tagged_0.sh")) as f: + line = f.readline() + assert line.strip() == f'echo "Hello with parameter 0 = {param_0}"' + + with open(osp.join(path, "nested_1", "tagged_1.sh")) as f: + line = f.readline() + assert line.strip() == f'echo "Hello with parameter 1 = {param_1}"' + + _check_generated("param_value_1", "param_value_2") + + +def test_configure_directory_not_absolute(): + """Test that ValueError is raised when tagged directories + given to configure op are not absolute paths + """ + + tagged_directory = ".." + tag = ";" + param_dict = {"5": 10} + # Pickle the dictionary + pickled_dict = pickle.dumps(param_dict) + + # Encode the pickled dictionary with Base64 + encoded_dict = base64.b64encode(pickled_dict) + parser = get_parser() + cmd = f"configure_directory {tagged_directory} {tagged_directory} {tag} {encoded_dict}" args = cmd.split() with pytest.raises(SystemExit) as e: @@ -653,8 +741,38 @@ def test_parser_copy(): assert ns.dest == dest_path -def test_parser_configure_parse(): - """Test that the parser succeeds when receiving expected args for the configure operation""" +def test_parser_configure_file_parse(): + """Test that the parser succeeds when receiving expected args for the configure_file operation""" + parser = get_parser() + + src_path = pathlib.Path("/absolute/file/src/path") + dest_path = pathlib.Path("/absolute/file/dest/path") + tag_delimiter = ";" + + param_dict = { + "5": 10, + "FIRST": "SECOND", + "17": 20, + "65": "70", + "placeholder": "group leftupper region", + "1200": "120", + } + + pickled_dict = pickle.dumps(param_dict) + encoded_dict = base64.b64encode(pickled_dict) + + cmd = f"configure_file {src_path} {dest_path} {tag_delimiter} {encoded_dict}" + args = cmd.split() + ns = parser.parse_args(args) + + assert ns.source == src_path + assert ns.dest == dest_path + assert ns.tag_delimiter == tag_delimiter + assert ns.param_dict == str(encoded_dict) + + +def test_parser_configure_directory_parse(): + """Test that the parser succeeds when receiving expected args for the configure_directory operation""" parser = get_parser() src_path = pathlib.Path("/absolute/file/src/path") @@ -673,7 +791,7 @@ def test_parser_configure_parse(): pickled_dict = pickle.dumps(param_dict) encoded_dict = base64.b64encode(pickled_dict) - cmd = f"configure {src_path} {dest_path} {tag_delimiter} {encoded_dict}" + cmd = f"configure_directory {src_path} {dest_path} {tag_delimiter} {encoded_dict}" args = cmd.split() ns = parser.parse_args(args) diff --git a/tests/test_generator.py b/tests/test_generator.py index f40d8cc87a..193c0cbd6d 100644 --- a/tests/test_generator.py +++ b/tests/test_generator.py @@ -1,8 +1,12 @@ +import builtins +import contextlib import filecmp +import io import itertools import os import pathlib import random +from contextlib import contextmanager from glob import glob from os import listdir from os import path as osp @@ -11,9 +15,10 @@ from smartsim import Experiment from smartsim._core.generation.generator import Generator -from smartsim.entity import Application, Ensemble +from smartsim.entity import Application, Ensemble, entity from smartsim.entity.files import EntityFiles from smartsim.launchable import Job +from smartsim.launchable.basejob import BaseJob from smartsim.settings import LaunchSettings # TODO Add JobGroup tests when JobGroup becomes a Launchable @@ -45,168 +50,157 @@ def get_gen_symlink_dir(fileutils): def get_gen_configure_dir(fileutils): yield fileutils.get_test_conf_path(osp.join("generator_files", "tag_dir_template")) + def get_gen_file(fileutils, filename): return fileutils.get_test_conf_path(osp.join("generator_files", filename)) + @pytest.fixture def generator_instance(test_dir) -> Generator: """Fixture to create an instance of Generator.""" root = pathlib.Path(test_dir, "temp_id") + os.mkdir(root) yield Generator(root=root) -def test_log_file_path(generator_instance): - """Test if the log_file function returns the correct log path.""" - base_path = "/tmp" - expected_path = osp.join(base_path, "smartsim_params.txt") - assert generator_instance._log_file(base_path) == pathlib.Path(expected_path) +class EchoHelloWorldEntity(entity.SmartSimEntity): + """A simple smartsim entity that meets the `ExecutableProtocol` protocol""" + def __init__(self): + self.name = "entity_name" -def test_generate_job_directory(test_dir, wlmutils, generator_instance): - """Test Generator.generate_job""" - # Create Job - launch_settings = LaunchSettings(wlmutils.get_test_launcher()) - app = Application("app_name", exe="python") - job = Job(app, launch_settings) - # Mock id - run_id = "temp_id" - # Call Generator.generate_job - job_run_path, _, _ = generator_instance.generate_job(job, 0) - assert isinstance(job_run_path, pathlib.Path) - expected_run_path = ( - pathlib.Path(test_dir) - / run_id - / f"{job.__class__.__name__.lower()}s" - / f"{app.name}-{0}" + +class MockJob(BaseJob): + """Mock Job for testing.""" + + def __init__(self): + self.name = "test_job" + self.index = 1 + self.entity = EchoHelloWorldEntity() + + def get_launch_steps(self): + raise NotImplementedError + + +# UNIT TESTS + + +def test_init_generator(generator_instance, test_dir): + """Test Generator init""" + assert generator_instance.root == pathlib.Path(test_dir) / "temp_id" + + +def test_generate_job_root(generator_instance): + """Test Generator._generate_job_root returns correct path""" + mock_job = MockJob() + root_path = generator_instance._generate_job_root(mock_job, mock_job.index) + expected_path = ( + generator_instance.root + / f"{MockJob.__name__.lower()}s" + / f"{mock_job.name}-{mock_job.index}" + ) + assert root_path == expected_path + + +def test_generate_run_path(generator_instance): + """Test Generator._generate_run_path returns correct path""" + mock_job = MockJob() + run_path = generator_instance._generate_run_path(mock_job, mock_job.index) + expected_path = ( + generator_instance.root + / f"{MockJob.__name__.lower()}s" + / f"{mock_job.name}-{mock_job.index}" / "run" ) - assert job_run_path == expected_run_path - expected_log_path = ( - pathlib.Path(test_dir) - / run_id - / f"{job.__class__.__name__.lower()}s" - / f"{app.name}-{0}" + assert run_path == expected_path + + +def test_generate_log_path(generator_instance): + """Test Generator._generate_log_path returns correct path""" + mock_job = MockJob() + log_path = generator_instance._generate_log_path(mock_job, mock_job.index) + expected_path = ( + generator_instance.root + / f"{MockJob.__name__.lower()}s" + / f"{mock_job.name}-{mock_job.index}" / "log" ) - assert osp.isdir(expected_run_path) - assert osp.isdir(expected_log_path) - # Assert smartsim params file created - assert osp.isfile(osp.join(expected_log_path, "smartsim_params.txt")) - # Assert smartsim params correctly written to - with open(expected_log_path / "smartsim_params.txt", "r") as file: - content = file.read() - assert "Generation start date and time:" in content + assert log_path == expected_path -def test_exp_private_generate_method(wlmutils, test_dir, generator_instance): - """Test that Job directory was created from Experiment._generate.""" - # Create Experiment - exp = Experiment(name="experiment_name", exp_path=test_dir) - # Create Job - app = Application("name", "python") - launch_settings = LaunchSettings(wlmutils.get_test_launcher()) - job = Job(app, launch_settings) - # Generate Job directory - job_index = 1 - job_execution_path, _, _ = exp._generate(generator_instance, job, job_index) - # Assert Job run directory exists - assert osp.isdir(job_execution_path) - # Assert Job log directory exists - head, _ = os.path.split(job_execution_path) - expected_log_path = pathlib.Path(head) / "log" - assert osp.isdir(expected_log_path) - +def test_log_file_path(generator_instance): + """Test Generator._log_file returns correct path""" + base_path = "/tmp" + expected_path = osp.join(base_path, "smartsim_params.txt") + assert generator_instance._log_file(base_path) == pathlib.Path(expected_path) -def test_generate_copy_file(generator_instance, fileutils, wlmutils): - """Test that attached copy files are copied into Job directory""" - # Create the Job and attach copy generator file - launch_settings = LaunchSettings(wlmutils.get_test_launcher()) - app = Application("name", "python") - script = fileutils.get_test_conf_path("sleep.py") - app.attach_generator_files(to_copy=script) - job = Job(app, launch_settings) - # Create the experiment - path, _, _ = generator_instance.generate_job(job, 1) - expected_file = pathlib.Path(path) / "sleep.py" - assert osp.isfile(expected_file) +def test_output_files(generator_instance): + """Test Generator._output_files returns err/out paths""" + base_path = pathlib.Path("/tmp") + out_file_path, err_file_path = generator_instance._output_files( + base_path, MockJob().name + ) + assert out_file_path == base_path / f"{MockJob().name}.out" + assert err_file_path == base_path / f"{MockJob().name}.err" -def test_generate_copy_directory(wlmutils, get_gen_copy_dir, generator_instance): - # Create the Job and attach generator file - launch_settings = LaunchSettings(wlmutils.get_test_launcher()) - app = Application("name", "python") - app.attach_generator_files(to_copy=get_gen_copy_dir) - job = Job(app, launch_settings) +def test_copy_file(generator_instance, fileutils): + """Test Generator._copy_files helper function with file list""" + script = fileutils.get_test_conf_path("sleep.py") + files = EntityFiles(copy=script) + generator_instance._copy_files(files, generator_instance.root) + expected_file = generator_instance.root / "sleep.py" + assert osp.isfile(expected_file) - # Call Generator.generate_job - path, _, _ = generator_instance.generate_job(job, 1) - expected_folder = path / "to_copy_dir" - assert osp.isdir(expected_folder) +def test_copy_directory(get_gen_copy_dir, generator_instance): + """Test Generator._copy_files helper function with directory""" + files = EntityFiles(copy=get_gen_copy_dir) + generator_instance._copy_files(files, generator_instance.root) + copied_folder = generator_instance.root / os.path.basename(get_gen_copy_dir) + assert osp.isdir(copied_folder) -def test_generate_symlink_directory(wlmutils, generator_instance, get_gen_symlink_dir): - # Create the Job and attach generator file - launch_settings = LaunchSettings(wlmutils.get_test_launcher()) - app = Application("name", "python") - # Attach directory to Application - app.attach_generator_files(to_symlink=get_gen_symlink_dir) - # Create Job - job = Job(app, launch_settings) - # Call Generator.generate_job - path, _, _ = generator_instance.generate_job(job, 1) - expected_folder = path / "to_symlink_dir" - assert osp.isdir(expected_folder) - assert expected_folder.is_symlink() - assert os.fspath(expected_folder.resolve()) == osp.realpath(get_gen_symlink_dir) - # Combine symlinked file list and original file list for comparison +def test_symlink_directory(generator_instance, get_gen_symlink_dir): + """Test Generator._symlink_files helper function with directory""" + files = EntityFiles(symlink=get_gen_symlink_dir) + generator_instance._symlink_files(files, generator_instance.root) + symlinked_folder = generator_instance.root / os.path.basename(get_gen_symlink_dir) + assert osp.isdir(symlinked_folder) + assert symlinked_folder.is_symlink() + assert os.fspath(symlinked_folder.resolve()) == osp.realpath(get_gen_symlink_dir) for written, correct in itertools.zip_longest( - listdir(get_gen_symlink_dir), listdir(expected_folder) + listdir(get_gen_symlink_dir), listdir(symlinked_folder) ): - # For each pair, check if the filenames are equal assert written == correct -def test_generate_symlink_file(get_gen_symlink_dir, wlmutils, generator_instance): - # Create the Job and attach generator file - launch_settings = LaunchSettings(wlmutils.get_test_launcher()) - app = Application("name", "python") - # Path of directory to symlink - symlink_dir = get_gen_symlink_dir - # Get a list of all files in the directory - symlink_files = sorted(glob(symlink_dir + "/*")) - # Attach directory to Application - app.attach_generator_files(to_symlink=symlink_files) - # Create Job - job = Job(app, launch_settings) - - # Call Generator.generate_job - path, _, _ = generator_instance.generate_job(job, 1) - expected_file = path / "mock2.txt" - assert osp.isfile(expected_file) - assert expected_file.is_symlink() - assert os.fspath(expected_file.resolve()) == osp.join( +def test_symlink_file(get_gen_symlink_dir, generator_instance): + """Test Generator._symlink_files helper function with file list""" + symlink_files = sorted(glob(get_gen_symlink_dir + "/*")) + files = EntityFiles(symlink=symlink_files) + generator_instance._symlink_files(files, generator_instance.root) + symlinked_file = generator_instance.root / os.path.basename(symlink_files[0]) + assert osp.isfile(symlinked_file) + assert symlinked_file.is_symlink() + assert os.fspath(symlinked_file.resolve()) == osp.join( osp.realpath(get_gen_symlink_dir), "mock2.txt" ) -def test_generate_configure(fileutils, wlmutils, generator_instance): - # Directory of files to configure +def test_write_tagged_file(fileutils, generator_instance): + """Test Generator._write_tagged_files helper function with file list""" conf_path = fileutils.get_test_conf_path( osp.join("generator_files", "easy", "marked/") ) - # Retrieve a list of files for configuration tagged_files = sorted(glob(conf_path + "/*")) - # Retrieve directory of files to compare after Experiment.generate_experiment completion correct_path = fileutils.get_test_conf_path( osp.join("generator_files", "easy", "correct/") ) - # Retrieve list of files in correctly tagged directory for comparison correct_files = sorted(glob(correct_path + "/*")) - # Initialize a Job - launch_settings = LaunchSettings(wlmutils.get_test_launcher()) - param_dict = { + files = EntityFiles(tagged=tagged_files) + param_set = { "5": 10, "FIRST": "SECOND", "17": 20, @@ -215,19 +209,85 @@ def test_generate_configure(fileutils, wlmutils, generator_instance): "1200": "120", "VALID": "valid", } - app = Application("name_1", "python", file_parameters=param_dict) - app.attach_generator_files(to_configure=tagged_files) - job = Job(app, launch_settings) - - # Call Generator.generate_job - path, _, _ = generator_instance.generate_job(job, 0) - # Retrieve the list of configured files in the test directory - configured_files = sorted(glob(str(path) + "/*")) - # Use filecmp.cmp to check that the corresponding files are equal + generator_instance._write_tagged_files( + files=files, params=param_set, dest=generator_instance.root + ) + configured_files = sorted(glob(os.path.join(generator_instance.root) + "/*")) for written, correct in itertools.zip_longest(configured_files, correct_files): assert filecmp.cmp(written, correct) +def test_write_tagged_directory(fileutils, generator_instance): + """Test Generator._write_tagged_files helper function with directory path""" + config = get_gen_file(fileutils, "tag_dir_template") + files = EntityFiles(tagged=config) + param_set = {"PARAM0": "param_value_1", "PARAM1": "param_value_2"} + generator_instance._write_tagged_files( + files=files, params=param_set, dest=generator_instance.root + ) + + def _check_generated(param_0, param_1): + assert osp.isdir(osp.join(generator_instance.root, "nested_0")) + assert osp.isdir(osp.join(generator_instance.root, "nested_1")) + + with open(osp.join(generator_instance.root, "nested_0", "tagged_0.sh")) as f: + line = f.readline() + assert line.strip() == f'echo "Hello with parameter 0 = {param_0}"' + + with open(osp.join(generator_instance.root, "nested_1", "tagged_1.sh")) as f: + line = f.readline() + assert line.strip() == f'echo "Hello with parameter 1 = {param_1}"' + + _check_generated("param_value_1", "param_value_2") + + +def test_generate_job(generator_instance, monkeypatch: pytest.MonkeyPatch): + """Test Generator.generate_job returns correct paths and calls correct functions and writes to params file.""" + monkeypatch.setattr( + Generator, "_generate_run_path", lambda self, job, job_index: "/tmp_run" + ) + log_path = generator_instance.root / "tmp_log" + os.mkdir(log_path) + monkeypatch.setattr( + Generator, "_generate_log_path", lambda self, job, job_index: log_path + ) + monkeypatch.setattr(Generator, "_output_files", lambda self, x, y: ("out", "err")) + monkeypatch.setattr( + Generator, "_build_operations", lambda self, job, job_path: None + ) + job_path, out_file, err_file = generator_instance.generate_job( + MockJob(), MockJob().index + ) + assert job_path == "/tmp_run" + assert out_file == "out" + assert err_file == "err" + with open(log_path / "smartsim_params.txt", "r") as file: + content = file.read() + assert "Generation start date and time:" in content + + +# INTEGRATED TESTS + + +def test_exp_private_generate_method(wlmutils, test_dir, generator_instance): + """Test that Job directory was created from Experiment._generate.""" + # Create Experiment + exp = Experiment(name="experiment_name", exp_path=test_dir) + # Create Job + app = Application("name", "python") + launch_settings = LaunchSettings(wlmutils.get_test_launcher()) + job = Job(app, launch_settings) + # Generate Job directory + job_index = 1 + job_execution_path, _, _ = exp._generate(generator_instance, job, job_index) + # Assert Job run directory exists + assert osp.isdir(job_execution_path) + # Assert Job log directory exists + head, _ = os.path.split(job_execution_path) + expected_log_path = pathlib.Path(head) / "log" + assert osp.isdir(expected_log_path) + + def test_exp_private_generate_method_ensemble(test_dir, wlmutils, generator_instance): """Test that Job directory was created from Experiment.""" ensemble = Ensemble("ensemble-name", "echo", replicas=2) @@ -353,7 +413,6 @@ def test_generate_ensemble_configure( job_list = ensemble.as_jobs(launch_settings) exp = Experiment(name="exp_name", exp_path=test_dir) id = exp.start(*job_list) - print(id) run_dir = listdir(test_dir) jobs_dir = os.path.join(test_dir, run_dir[0], "jobs") @@ -375,18 +434,3 @@ def _check_generated(param_0, param_1, dir): _check_generated(1, 3, os.path.join(jobs_dir, "ensemble-name-3-3", "run")) _check_generated(0, 2, os.path.join(jobs_dir, "ensemble-name-0-0", "run")) ids.clear() - - -def test_this(fileutils): - # Directory of files to configure - """Test the generation and configuration of applications with - tagged files that are directories with subdirectories and files - """ - config = get_gen_file(fileutils, "tag_dir_template") - script = fileutils.get_test_conf_path("sleep.py") - #print(list(os.walk(config))) - print(list(os.walk(script))) - # for root, dirs, files in os.walk(config): - # print(list(os.walk(config))) - # # for name in dirs: - # # os.rmdir(os.path.join(root, name)) \ No newline at end of file From abf42a541d99e8d752e128e6ccc1cf3c754e71d7 Mon Sep 17 00:00:00 2001 From: Amanda Richardson Date: Thu, 5 Sep 2024 18:22:59 -0500 Subject: [PATCH 03/13] small fixes --- smartsim/_core/generation/generator.py | 16 ++++------- smartsim/entity/files.py | 1 - tests/test_generator.py | 39 ++++++++++++++++++++------ 3 files changed, 35 insertions(+), 21 deletions(-) diff --git a/smartsim/_core/generation/generator.py b/smartsim/_core/generation/generator.py index 1a5493a58d..cb510156f7 100644 --- a/smartsim/_core/generation/generator.py +++ b/smartsim/_core/generation/generator.py @@ -33,8 +33,6 @@ import sys import typing as t from datetime import datetime -from os import mkdir, path -from os.path import join from ...entity import Application from ...entity.files import EntityFiles @@ -136,20 +134,15 @@ def generate_job( :param log_path: The path to the "log" directory for the job instance. """ - # Generate ../job_name/run directory job_path = self._generate_run_path(job, job_index) - # Generate ../job_name/log directory log_path = self._generate_log_path(job, job_index) - # Create and write to the parameter settings file with open(self._log_file(log_path), mode="w", encoding="utf-8") as log_file: dt_string = datetime.now().strftime("%d/%m/%Y %H:%M:%S") log_file.write(f"Generation start date and time: {dt_string}\n") - # Create output files out_file, err_file = self._output_files(log_path, job.entity.name) - # Perform file system operations on attached files self._build_operations(job, job_path) return job_path, out_file, err_file @@ -173,7 +166,7 @@ def _build_operations(cls, job: Job, job_path: pathlib.Path) -> None: def _copy_files(files: t.Union[EntityFiles, None], dest: pathlib.Path) -> None: """Perform copy file sys operations on a list of files. - :param app: The Application attached to the Job + :param files: The paths to copy :param dest: Path to the Jobs run directory """ # Return if no files are attached @@ -213,7 +206,7 @@ def _copy_files(files: t.Union[EntityFiles, None], dest: pathlib.Path) -> None: def _symlink_files(files: t.Union[EntityFiles, None], dest: pathlib.Path) -> None: """Perform symlink file sys operations on a list of files. - :param app: The Application attached to the Job + :param files: The paths to symlink :param dest: Path to the Jobs run directory """ # Return if no files are attached @@ -247,7 +240,8 @@ def _write_tagged_files( a Job instance. This function specifically deals with the tagged files attached to an entity. - :param app: The Application attached to the Job + :param files: The paths to configure + :param params: A dictionary of params :param dest: Path to the Jobs run directory """ if files is None: @@ -271,7 +265,7 @@ def _write_tagged_files( if os.path.isdir(file_sys_path): file_entrypoint = "configure_directory" elif os.path.isfile(file_sys_path): - file_entrypoint = "configure" + file_entrypoint = "configure_file" else: raise ValueError(f"Invalid path: {file_sys_path}") subprocess.run( diff --git a/smartsim/entity/files.py b/smartsim/entity/files.py index 86a721221a..08143fbfc2 100644 --- a/smartsim/entity/files.py +++ b/smartsim/entity/files.py @@ -23,7 +23,6 @@ # 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 os import typing as t from os import path diff --git a/tests/test_generator.py b/tests/test_generator.py index 193c0cbd6d..25f9981f3e 100644 --- a/tests/test_generator.py +++ b/tests/test_generator.py @@ -1,12 +1,34 @@ -import builtins -import contextlib +# 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 filecmp -import io import itertools import os import pathlib import random -from contextlib import contextmanager from glob import glob from os import listdir from os import path as osp @@ -27,7 +49,6 @@ ids = set() - def random_id(): while True: num = str(random.randint(1, 100)) @@ -51,10 +72,6 @@ def get_gen_configure_dir(fileutils): yield fileutils.get_test_conf_path(osp.join("generator_files", "tag_dir_template")) -def get_gen_file(fileutils, filename): - return fileutils.get_test_conf_path(osp.join("generator_files", filename)) - - @pytest.fixture def generator_instance(test_dir) -> Generator: """Fixture to create an instance of Generator.""" @@ -63,6 +80,10 @@ def generator_instance(test_dir) -> Generator: yield Generator(root=root) +def get_gen_file(fileutils, filename): + return fileutils.get_test_conf_path(osp.join("generator_files", filename)) + + class EchoHelloWorldEntity(entity.SmartSimEntity): """A simple smartsim entity that meets the `ExecutableProtocol` protocol""" From b670ea5674f13e02f2f98a28d2e0fa3a103d8644 Mon Sep 17 00:00:00 2001 From: Amanda Richardson Date: Mon, 9 Sep 2024 15:34:43 -0500 Subject: [PATCH 04/13] generator styling --- tests/test_generator.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_generator.py b/tests/test_generator.py index 25f9981f3e..49bfcaf070 100644 --- a/tests/test_generator.py +++ b/tests/test_generator.py @@ -49,6 +49,7 @@ ids = set() + def random_id(): while True: num = str(random.randint(1, 100)) From 1bd78b7f21bd000bda913aae40c207aa870d4079 Mon Sep 17 00:00:00 2001 From: Amanda Richardson Date: Wed, 11 Sep 2024 13:12:50 -0500 Subject: [PATCH 05/13] attempt to fix failing github workflow --- smartsim/_core/entrypoints/file_operations.py | 40 +++++++++++-------- tests/test_generator.py | 29 ++++++++------ 2 files changed, 40 insertions(+), 29 deletions(-) diff --git a/smartsim/_core/entrypoints/file_operations.py b/smartsim/_core/entrypoints/file_operations.py index 944c98a378..d21543471f 100644 --- a/smartsim/_core/entrypoints/file_operations.py +++ b/smartsim/_core/entrypoints/file_operations.py @@ -49,7 +49,7 @@ def _abspath(input_path: str) -> pathlib.Path: """Helper function to check that paths are absolute""" path = pathlib.Path(input_path) if not path.is_absolute(): - raise ValueError(f"path `{path}` must be absolute") + raise ValueError(f"Path `{path}` must be absolute.") return path @@ -63,13 +63,19 @@ def _make_substitution( def _prepare_param_dict(param_dict: str) -> dict[str, t.Any]: + """Decode and deserialize a base64-encoded parameter dictionary. + + This function takes a base64-encoded string representation of a dictionary, + decodes it, and then deserializes it using pickle. It performs validation + to ensure the resulting object is a non-empty dictionary. + """ decoded_dict = base64.b64decode(param_dict) - encoded_param_dict = pickle.loads(decoded_dict) - if not encoded_param_dict: + deserialized_dict = pickle.loads(decoded_dict) + if not deserialized_dict: raise ValueError("param dictionary is empty") - if not isinstance(encoded_param_dict, dict): + if not isinstance(deserialized_dict, dict): raise TypeError("param dict is not a valid dictionary") - return encoded_param_dict + return deserialized_dict def _replace_tags_in( @@ -176,7 +182,7 @@ def configure_file(parsed_args: argparse.Namespace) -> None: .. highlight:: bash .. code-block:: bash python -m smartsim._core.entrypoints.file_operations \ - configure_file /absolute/file/source/pat /absolute/file/dest/path \ + configure_file /absolute/file/source/path /absolute/file/dest/path \ tag_deliminator param_dict /absolute/file/source/path: The tagged files the search and replace operations @@ -189,20 +195,22 @@ def configure_file(parsed_args: argparse.Namespace) -> None: """ tag_delimiter = parsed_args.tag_delimiter - param_dict = _prepare_param_dict(parsed_args.param_dict) - substitutions = tuple( _make_substitution(k, v, tag_delimiter) for k, v in param_dict.items() ) - - # Set the lines to iterate over - with open(parsed_args.source, "r+", encoding="utf-8") as file_stream: - lines = [_replace_tags_in(line, substitutions) for line in file_stream] - - # write configured file to destination specified - with open(parsed_args.dest, "w+", encoding="utf-8") as file_stream: - file_stream.writelines(lines) + try: + # Set the lines to iterate over + with open(parsed_args.source, "r+", encoding="utf-8") as file_stream: + try: + lines = [_replace_tags_in(line, substitutions) for line in file_stream] + except IOError as e: + raise e from None + # write configured file to destination specified + with open(parsed_args.dest, "w+", encoding="utf-8") as file_stream: + file_stream.writelines(lines) + except (FileNotFoundError, PermissionError) as e: + raise e from None def configure_directory(parsed_args: argparse.Namespace) -> None: diff --git a/tests/test_generator.py b/tests/test_generator.py index 49bfcaf070..62b3370dfb 100644 --- a/tests/test_generator.py +++ b/tests/test_generator.py @@ -91,6 +91,9 @@ class EchoHelloWorldEntity(entity.SmartSimEntity): def __init__(self): self.name = "entity_name" + def as_program_arguments(self): + return ("echo", "Hello", "World!") + class MockJob(BaseJob): """Mock Job for testing.""" @@ -184,6 +187,19 @@ def test_copy_directory(get_gen_copy_dir, generator_instance): assert osp.isdir(copied_folder) +def test_symlink_file(get_gen_symlink_dir, generator_instance): + """Test Generator._symlink_files helper function with file list""" + symlink_files = sorted(glob(get_gen_symlink_dir + "/*")) + files = EntityFiles(symlink=symlink_files) + generator_instance._symlink_files(files, generator_instance.root) + symlinked_file = generator_instance.root / os.path.basename(symlink_files[0]) + assert osp.isfile(symlinked_file) + assert symlinked_file.is_symlink() + assert os.fspath(symlinked_file.resolve()) == osp.join( + osp.realpath(get_gen_symlink_dir), "mock2.txt" + ) + + def test_symlink_directory(generator_instance, get_gen_symlink_dir): """Test Generator._symlink_files helper function with directory""" files = EntityFiles(symlink=get_gen_symlink_dir) @@ -198,19 +214,6 @@ def test_symlink_directory(generator_instance, get_gen_symlink_dir): assert written == correct -def test_symlink_file(get_gen_symlink_dir, generator_instance): - """Test Generator._symlink_files helper function with file list""" - symlink_files = sorted(glob(get_gen_symlink_dir + "/*")) - files = EntityFiles(symlink=symlink_files) - generator_instance._symlink_files(files, generator_instance.root) - symlinked_file = generator_instance.root / os.path.basename(symlink_files[0]) - assert osp.isfile(symlinked_file) - assert symlinked_file.is_symlink() - assert os.fspath(symlinked_file.resolve()) == osp.join( - osp.realpath(get_gen_symlink_dir), "mock2.txt" - ) - - def test_write_tagged_file(fileutils, generator_instance): """Test Generator._write_tagged_files helper function with file list""" conf_path = fileutils.get_test_conf_path( From 7a048e2aed829015e1e9920955f2e7a32baacd08 Mon Sep 17 00:00:00 2001 From: Amanda Richardson Date: Tue, 17 Sep 2024 18:18:08 -0500 Subject: [PATCH 06/13] addressed all comments --- smartsim/_core/entrypoints/file_operations.py | 106 ++--- smartsim/_core/generation/generator.py | 142 +++---- smartsim/experiment.py | 17 +- tests/test_experiment.py | 5 +- tests/test_file_operations.py | 73 ++-- tests/test_generator.py | 374 +++++++++--------- 6 files changed, 336 insertions(+), 381 deletions(-) diff --git a/smartsim/_core/entrypoints/file_operations.py b/smartsim/_core/entrypoints/file_operations.py index d21543471f..a714eff6a4 100644 --- a/smartsim/_core/entrypoints/file_operations.py +++ b/smartsim/_core/entrypoints/file_operations.py @@ -71,10 +71,10 @@ def _prepare_param_dict(param_dict: str) -> dict[str, t.Any]: """ decoded_dict = base64.b64decode(param_dict) deserialized_dict = pickle.loads(decoded_dict) - if not deserialized_dict: - raise ValueError("param dictionary is empty") if not isinstance(deserialized_dict, dict): raise TypeError("param dict is not a valid dictionary") + if not deserialized_dict: + raise ValueError("param dictionary is empty") return deserialized_dict @@ -86,6 +86,23 @@ def _replace_tags_in( return functools.reduce(lambda a, fn: fn(a), substitutions, item) +def _process_file( + substitutions: t.Sequence[Callable[[str], str]], + source: pathlib.Path, + destination: pathlib.Path, +) -> None: + """ + Process a source file by replacing tags with specified substitutions and + write the result to a destination file. + """ + # Set the lines to iterate over + with open(source, "r+", encoding="utf-8") as file_stream: + lines = [_replace_tags_in(line, substitutions) for line in file_stream] + # write configured file to destination specified + with open(destination, "w+", encoding="utf-8") as file_stream: + file_stream.writelines(lines) + + def move(parsed_args: argparse.Namespace) -> None: """Move a source file or directory to another location. If dest is an existing directory or a symlink to a directory, then the srouce will @@ -169,7 +186,7 @@ def symlink(parsed_args: argparse.Namespace) -> None: os.symlink(parsed_args.source, parsed_args.dest) -def configure_file(parsed_args: argparse.Namespace) -> None: +def configure(parsed_args: argparse.Namespace) -> None: """Set, search and replace the tagged parameters for the configure_file operation within tagged files attached to an entity. @@ -193,50 +210,6 @@ def configure_file(parsed_args: argparse.Namespace) -> None: semi-colon e.g. ";" param_dict: A dict of parameter names and values set for the file - """ - tag_delimiter = parsed_args.tag_delimiter - param_dict = _prepare_param_dict(parsed_args.param_dict) - substitutions = tuple( - _make_substitution(k, v, tag_delimiter) for k, v in param_dict.items() - ) - try: - # Set the lines to iterate over - with open(parsed_args.source, "r+", encoding="utf-8") as file_stream: - try: - lines = [_replace_tags_in(line, substitutions) for line in file_stream] - except IOError as e: - raise e from None - # write configured file to destination specified - with open(parsed_args.dest, "w+", encoding="utf-8") as file_stream: - file_stream.writelines(lines) - except (FileNotFoundError, PermissionError) as e: - raise e from None - - -def configure_directory(parsed_args: argparse.Namespace) -> None: - """Set, search and replace the tagged parameters for the - configure_directory operation within tagged directories attached to an entity. - - User-formatted directories can be attached using the `configure_directory` argument. - These files within the directory will be modified during ``Application`` generation to replace - tagged sections in the user-formatted files with values from the `params` - initializer argument used during ``Application`` creation: - - Sample usage: - .. highlight:: bash - .. code-block:: bash - python -m smartsim._core.entrypoints.file_operations \ - configure_directory /absolute/file/source/path /absolute/file/dest/path \ - tag_deliminator param_dict - - /absolute/file/source/path: The tagged directories the search and replace operations - to be performed upon - /absolute/file/dest/path: The destination for configured directories to be - written to. - tag_delimiter: tag for the configure_directory operation to search for, defaults to - semi-colon e.g. ";" - param_dict: A dict of parameter names and values set for the file - """ tag_delimiter = parsed_args.tag_delimiter param_dict = _prepare_param_dict(parsed_args.param_dict) @@ -244,21 +217,20 @@ def configure_directory(parsed_args: argparse.Namespace) -> None: substitutions = tuple( _make_substitution(k, v, tag_delimiter) for k, v in param_dict.items() ) - for dirpath, _, filenames in os.walk(parsed_args.dest): - if filenames: + if parsed_args.source.is_dir(): + for dirpath, _, filenames in os.walk(parsed_args.source): + new_dir_dest = dirpath.replace( + str(parsed_args.source), str(parsed_args.dest), 1 + ) + os.makedirs(new_dir_dest, exist_ok=True) for file_name in filenames: - # Set the lines to iterate over - with open( - os.path.join(dirpath, file_name), "r+", encoding="utf-8" - ) as file_stream: - lines = [ - _replace_tags_in(line, substitutions) for line in file_stream - ] - # write configured file to destination specified - with open( - os.path.join(dirpath, file_name), "w+", encoding="utf-8" - ) as file_stream: - file_stream.writelines(lines) + src_file = os.path.join(dirpath, file_name) + dst_file = os.path.join(new_dir_dest, file_name) + print(type(substitutions)) + _process_file(substitutions, src_file, dst_file) + else: + dst_file = parsed_args.dest / os.path.basename(parsed_args.source) + _process_file(substitutions, parsed_args.source, dst_file) def get_parser() -> argparse.ArgumentParser: @@ -295,16 +267,8 @@ def get_parser() -> argparse.ArgumentParser: symlink_parser.add_argument("dest", type=_abspath) # Subparser for configure op - configure_parser = subparsers.add_parser("configure_file") - configure_parser.set_defaults(func=configure_file) - configure_parser.add_argument("source", type=_abspath) - configure_parser.add_argument("dest", type=_abspath) - configure_parser.add_argument("tag_delimiter", type=str, default=";") - configure_parser.add_argument("param_dict", type=str) - - # Subparser for configure op - configure_parser = subparsers.add_parser("configure_directory") - configure_parser.set_defaults(func=configure_directory) + configure_parser = subparsers.add_parser("configure") + configure_parser.set_defaults(func=configure) configure_parser.add_argument("source", type=_abspath) configure_parser.add_argument("dest", type=_abspath) configure_parser.add_argument("tag_delimiter", type=str, default=";") diff --git a/smartsim/_core/generation/generator.py b/smartsim/_core/generation/generator.py index cb510156f7..aa1acfce32 100644 --- a/smartsim/_core/generation/generator.py +++ b/smartsim/_core/generation/generator.py @@ -28,13 +28,12 @@ import os import pathlib import pickle -import shutil import subprocess import sys import typing as t +from collections import namedtuple from datetime import datetime -from ...entity import Application from ...entity.files import EntityFiles from ...launchable import Job from ...log import get_logger @@ -43,12 +42,29 @@ logger.propagate = False +@t.runtime_checkable +class _GenerableProtocol(t.Protocol): + """A protocol that defines a generable entity with a method to retrieve + a list of file names. This protocol is used to ensure that functions + implementing it provide a `files` and `file_parameters` property. + """ + + files: t.Union[EntityFiles, None] + file_parameters: t.Mapping[str, str] + + +Job_Path = namedtuple("Job_Path", ["run_path", "out_path", "err_path"]) + + class Generator: """The primary job of the Generator is to create the directory and file structure for a SmartSim Job. The Generator is also responsible for writing and configuring files into the Job directory. """ + run = "run" + log = "log" + def __init__(self, root: pathlib.Path) -> None: """Initialize a Generator object @@ -79,7 +95,7 @@ def _generate_run_path(self, job: Job, job_index: int) -> pathlib.Path: :param job_index (int): The index of the Job instance (used for naming). :returns: The path to the "run" directory for the Job instance. """ - path = self._generate_job_root(job, job_index) / "run" + path = self._generate_job_root(job, job_index) / self.run path.mkdir(exist_ok=False, parents=True) return pathlib.Path(path) @@ -91,7 +107,9 @@ def _generate_log_path(self, job: Job, job_index: int) -> pathlib.Path: :param job_index: The index of the Job instance (used for naming). :returns: The path to the "log" directory for the Job instance. """ - path = self._generate_job_root(job, job_index) / "log" + path = ( + self._generate_job_root(job, job_index) / self.log + ) # module or class level constant path.mkdir(exist_ok=False, parents=True) return pathlib.Path(path) @@ -107,16 +125,28 @@ def _log_file(log_path: pathlib.Path) -> pathlib.Path: return pathlib.Path(log_path) / "smartsim_params.txt" @staticmethod - def _output_files( - log_path: pathlib.Path, job_name: str - ) -> t.Tuple[pathlib.Path, pathlib.Path]: + def _out_file(log_path: pathlib.Path, job_name: str) -> pathlib.Path: + """Return the path to the output file. + + :param log_path: Path to log directory + :param job_name: Name of Job + :returns: Path to output file + """ out_file_path = log_path / f"{job_name}.out" + return out_file_path + + @staticmethod + def _err_file(log_path: pathlib.Path, job_name: str) -> pathlib.Path: + """Return the path to the error file. + + :param log_path: Path to log directory + :param job_name: Name of Job + :returns: Path to error file + """ err_file_path = log_path / f"{job_name}.err" - return out_file_path, err_file_path + return err_file_path - def generate_job( - self, job: Job, job_index: int - ) -> t.Tuple[pathlib.Path, pathlib.Path, pathlib.Path]: + def generate_job(self, job: Job, job_index: int) -> Job_Path: """Write and configure input files for a Job. To have files or directories present in the created Job @@ -141,11 +171,12 @@ def generate_job( dt_string = datetime.now().strftime("%d/%m/%Y %H:%M:%S") log_file.write(f"Generation start date and time: {dt_string}\n") - out_file, err_file = self._output_files(log_path, job.entity.name) + out_file = self._out_file(log_path, job.entity.name) + err_file = self._err_file(log_path, job.entity.name) self._build_operations(job, job_path) - return job_path, out_file, err_file + return Job_Path(job_path, out_file, err_file) @classmethod def _build_operations(cls, job: Job, job_path: pathlib.Path) -> None: @@ -157,10 +188,12 @@ def _build_operations(cls, job: Job, job_path: pathlib.Path) -> None: :param job: The Job to perform file ops on attached entity files :param job_path: Path to the Jobs run directory """ - app = t.cast(Application, job.entity) - cls._copy_files(app.files, job_path) - cls._symlink_files(app.files, job_path) - cls._write_tagged_files(app.files, app.file_parameters, job_path) + if isinstance(job.entity, _GenerableProtocol): + cls._copy_files(job.entity.files, job_path) + cls._symlink_files(job.entity.files, job_path) + cls._write_tagged_files( + job.entity.files, job.entity.file_parameters, job_path + ) @staticmethod def _copy_files(files: t.Union[EntityFiles, None], dest: pathlib.Path) -> None: @@ -169,38 +202,25 @@ def _copy_files(files: t.Union[EntityFiles, None], dest: pathlib.Path) -> None: :param files: The paths to copy :param dest: Path to the Jobs run directory """ - # Return if no files are attached if files is None: return for src in files.copy: + args = [ + sys.executable, + "-m", + "smartsim._core.entrypoints.file_operations", + "copy", + src, + ] + destination = str(dest) if os.path.isdir(src): - # Remove basename of source base_source_name = os.path.basename(src) - # Attach source basename to destination - new_dst_path = os.path.join(dest, base_source_name) - # Copy source contents to new destination path - subprocess.run( - args=[ - sys.executable, - "-m", - "smartsim._core.entrypoints.file_operations", - "copy", - src, - new_dst_path, - "--dirs_exist_ok", - ] - ) + destination = os.path.join(dest, base_source_name) + args.append(str(destination)) + args.append("--dirs_exist_ok") else: - subprocess.run( - args=[ - sys.executable, - "-m", - "smartsim._core.entrypoints.file_operations", - "copy", - src, - dest, - ] - ) + args.append(str(dest)) + subprocess.run(args) @staticmethod def _symlink_files(files: t.Union[EntityFiles, None], dest: pathlib.Path) -> None: @@ -209,7 +229,6 @@ def _symlink_files(files: t.Union[EntityFiles, None], dest: pathlib.Path) -> Non :param files: The paths to symlink :param dest: Path to the Jobs run directory """ - # Return if no files are attached if files is None: return for src in files.link: @@ -217,7 +236,6 @@ def _symlink_files(files: t.Union[EntityFiles, None], dest: pathlib.Path) -> Non normalized_path = os.path.normpath(src) # Get the parent directory (last folder) parent_dir = os.path.basename(normalized_path) - # Create destination new_dest = os.path.join(str(dest), parent_dir) subprocess.run( args=[ @@ -233,49 +251,31 @@ def _symlink_files(files: t.Union[EntityFiles, None], dest: pathlib.Path) -> Non @staticmethod def _write_tagged_files( files: t.Union[EntityFiles, None], - params: t.Mapping[str, t.Any], + params: t.Mapping[str, str], dest: pathlib.Path, ) -> None: - """Read, configure and write the tagged input files for - a Job instance. This function specifically deals with the tagged - files attached to an entity. + """Read, configure and write the tagged files and directories for + a Job instance. - :param files: The paths to configure + :param files: Paths to configure :param params: A dictionary of params :param dest: Path to the Jobs run directory """ if files is None: return if files.tagged: - to_write = [] - for file in files.tagged: - if os.path.isfile(file): - dst_path = os.path.join(dest, os.path.basename(file)) - dst_path = shutil.copyfile(file, dst_path) - to_write.append(dst_path) - elif os.path.isdir(file): - dst_path = shutil.copytree(file, dest, dirs_exist_ok=True) - to_write.append(dst_path) - else: - raise ValueError(f"Invalid path: {file}") tag_delimiter = ";" pickled_dict = pickle.dumps(params) encoded_dict = base64.b64encode(pickled_dict).decode("ascii") - for file_sys_path in to_write: - if os.path.isdir(file_sys_path): - file_entrypoint = "configure_directory" - elif os.path.isfile(file_sys_path): - file_entrypoint = "configure_file" - else: - raise ValueError(f"Invalid path: {file_sys_path}") + for path in files.tagged: subprocess.run( args=[ sys.executable, "-m", "smartsim._core.entrypoints.file_operations", - file_entrypoint, - file_sys_path, - file_sys_path, + "configure", + path, + dest, tag_delimiter, encoded_dict, ] diff --git a/smartsim/experiment.py b/smartsim/experiment.py index ea7cccc3d7..cd269ad77c 100644 --- a/smartsim/experiment.py +++ b/smartsim/experiment.py @@ -44,6 +44,7 @@ from smartsim.status import InvalidJobStatus, JobStatus from ._core import Generator, Manifest, previewrenderer +from ._core.generation.generator import Job_Path from .entity import TelemetryConfiguration from .error import SmartSimError from .log import ctx_exp_path, get_logger, method_contextualizer @@ -210,8 +211,10 @@ def execute_dispatch(generator: Generator, job: Job, idx: int) -> LaunchedJobID: for_experiment=self, with_arguments=args ) # Generate the job directory and return the generated job path - job_execution_path, out, err = self._generate(generator, job, idx) - id_ = launch_config.start(exe, job_execution_path, env, out, err) + job_paths = self._generate(generator, job, idx) + id_ = launch_config.start( + exe, job_paths.run_path, env, job_paths.out_path, job_paths.err_path + ) # Save the underlying launcher instance and launched job id. That # way we do not need to spin up a launcher instance for each # individual job, and the experiment can monitor job statuses. @@ -255,9 +258,7 @@ def get_status( return tuple(stats) @_contextualize - def _generate( - self, generator: Generator, job: Job, job_index: int - ) -> t.Tuple[pathlib.Path, pathlib.Path, pathlib.Path]: + def _generate(self, generator: Generator, job: Job, job_index: int) -> Job_Path: """Generate the directory structure and files for a ``Job`` If files or directories are attached to an ``Application`` object @@ -269,12 +270,12 @@ def _generate( run and log directory. :param job: The Job instance for which the output is generated. :param job_index: The index of the Job instance (used for naming). - :returns: The path to the generated output for the Job instance. + :returns: The paths to the generated output for the Job instance. :raises: A SmartSimError if an error occurs during the generation process. """ try: - job_path, out, err = generator.generate_job(job, job_index) - return (job_path, out, err) + job_paths = generator.generate_job(job, job_index) + return job_paths except SmartSimError as e: logger.error(e) raise diff --git a/tests/test_experiment.py b/tests/test_experiment.py index 39f2b9b114..635aff88b0 100644 --- a/tests/test_experiment.py +++ b/tests/test_experiment.py @@ -36,6 +36,7 @@ from smartsim._core import dispatch from smartsim._core.control.launch_history import LaunchHistory +from smartsim._core.generation.generator import Job_Path from smartsim._core.utils.launcher import LauncherProtocol, create_job_id from smartsim.entity import entity from smartsim.experiment import Experiment @@ -57,7 +58,9 @@ def experiment(monkeypatch, test_dir, dispatcher): monkeypatch.setattr( exp, "_generate", - lambda gen, job, idx: ("/tmp/job", "/tmp/job/out.txt", "/tmp/job/err.txt"), + lambda generator, job, idx: Job_Path( + "/tmp/job", "/tmp/job/out.txt", "/tmp/job/err.txt" + ), ) yield exp diff --git a/tests/test_file_operations.py b/tests/test_file_operations.py index 6302702e91..327eb74286 100644 --- a/tests/test_file_operations.py +++ b/tests/test_file_operations.py @@ -30,7 +30,6 @@ import os import pathlib import pickle -import shutil from glob import glob from os import path as osp @@ -501,22 +500,15 @@ def test_remove_op_not_absolute(): ], ) def test_configure_file_op(test_dir, fileutils, param_dict, error_type): - """Test configure_file operation with correct parameter dictionary, empty dicitonary, and an incorrect type""" + """Test configure file operation with correct parameter dictionary, empty dicitonary, and an incorrect type""" tag = ";" - conf_path = fileutils.get_test_conf_path( - osp.join("generator_files", "easy", "marked/") - ) # retrieve files to compare after test correct_path = fileutils.get_test_conf_path( osp.join("generator_files", "easy", "correct/") ) - # copy files to test directory - shutil.copytree(conf_path, test_dir, dirs_exist_ok=True) - assert osp.isdir(test_dir) - tagged_files = sorted(glob(test_dir + "/*")) correct_files = sorted(glob(correct_path + "/*")) @@ -529,20 +521,20 @@ def test_configure_file_op(test_dir, fileutils, param_dict, error_type): # Run configure op on test files for tagged_file in tagged_files: parser = get_parser() - cmd = f"configure_file {tagged_file} {tagged_file} {tag} {encoded_dict}" + cmd = f"configure {tagged_file} {tagged_file} {tag} {encoded_dict}" args = cmd.split() ns = parser.parse_args(args) if error_type == "ValueError": with pytest.raises(ValueError) as ex: - file_operations.configure_file(ns) + file_operations.configure(ns) assert "param dictionary is empty" in ex.value.args[0] elif error_type == "TypeError": with pytest.raises(TypeError) as ex: - file_operations.configure_file(ns) + file_operations.configure(ns) assert "param dict is not a valid dictionary" in ex.value.args[0] else: - file_operations.configure_file(ns) + file_operations.configure(ns) if error_type == "None": for written, correct in zip(tagged_files, correct_files): @@ -550,11 +542,11 @@ def test_configure_file_op(test_dir, fileutils, param_dict, error_type): def test_configure_file_invalid_tags(fileutils, test_dir): - """Test configure_file operation with an invalid tag""" + """Test configure file operation with an invalid tag""" generator_files = pathlib.Path(fileutils.get_test_conf_path("generator_files")) tagged_file = generator_files / "easy/marked/invalidtag.txt" correct_file = generator_files / "easy/correct/invalidtag.txt" - target_file = pathlib.Path(test_dir, "target.txt") + target_file = pathlib.Path(test_dir, "invalidtag.txt") tag = ";" param_dict = {"VALID": "valid"} @@ -565,17 +557,17 @@ def test_configure_file_invalid_tags(fileutils, test_dir): # Encode the pickled dictionary with Base64 encoded_dict = base64.b64encode(pickled_dict).decode("ascii") parser = get_parser() - cmd = f"configure_file {tagged_file} {target_file} {tag} {encoded_dict}" + cmd = f"configure {tagged_file} {test_dir} {tag} {encoded_dict}" args = cmd.split() ns = parser.parse_args(args) - file_operations.configure_file(ns) + file_operations.configure(ns) assert filecmp.cmp(correct_file, target_file) def test_configure_file_not_absolute(): """Test that ValueError is raised when tagged files - given to configure_file op are not absolute paths + given to configure file op are not absolute paths """ tagged_file = ".." @@ -587,7 +579,7 @@ def test_configure_file_not_absolute(): # Encode the pickled dictionary with Base64 encoded_dict = base64.b64encode(pickled_dict) parser = get_parser() - cmd = f"configure_file {tagged_file} {tagged_file} {tag} {encoded_dict}" + cmd = f"configure {tagged_file} {tagged_file} {tag} {encoded_dict}" args = cmd.split() with pytest.raises(SystemExit) as e: @@ -614,11 +606,9 @@ def test_configure_file_not_absolute(): ], ) def test_configure_directory(test_dir, fileutils, param_dict, error_type): - """Test configure_directory operation with correct parameter dictionary, empty dicitonary, and an incorrect type""" + """Test configure directory operation with correct parameter dictionary, empty dicitonary, and an incorrect type""" tag = ";" config = get_gen_file(fileutils, "tag_dir_template") - path = shutil.copytree(config, test_dir, dirs_exist_ok=True) - assert osp.isdir(path) # Pickle the dictionary pickled_dict = pickle.dumps(param_dict) @@ -626,35 +616,30 @@ def test_configure_directory(test_dir, fileutils, param_dict, error_type): encoded_dict = base64.b64encode(pickled_dict).decode("ascii") parser = get_parser() - cmd = f"configure_directory {path} {path} {tag} {encoded_dict}" + cmd = f"configure {config} {test_dir} {tag} {encoded_dict}" args = cmd.split() ns = parser.parse_args(args) if error_type == "ValueError": with pytest.raises(ValueError) as ex: - file_operations.configure_directory(ns) + file_operations.configure(ns) assert "param dictionary is empty" in ex.value.args[0] elif error_type == "TypeError": with pytest.raises(TypeError) as ex: - file_operations.configure_directory(ns) + file_operations.configure(ns) assert "param dict is not a valid dictionary" in ex.value.args[0] else: - file_operations.configure_directory(ns) - if error_type == "None": - - def _check_generated(param_0, param_1): - assert osp.isdir(osp.join(path, "nested_0")) - assert osp.isdir(osp.join(path, "nested_1")) - - with open(osp.join(path, "nested_0", "tagged_0.sh")) as f: - line = f.readline() - assert line.strip() == f'echo "Hello with parameter 0 = {param_0}"' + file_operations.configure(ns) + assert osp.isdir(osp.join(test_dir, "nested_0")) + assert osp.isdir(osp.join(test_dir, "nested_1")) - with open(osp.join(path, "nested_1", "tagged_1.sh")) as f: - line = f.readline() - assert line.strip() == f'echo "Hello with parameter 1 = {param_1}"' + with open(osp.join(test_dir, "nested_0", "tagged_0.sh")) as f: + line = f.readline() + assert line.strip() == f'echo "Hello with parameter 0 = param_value_1"' - _check_generated("param_value_1", "param_value_2") + with open(osp.join(test_dir, "nested_1", "tagged_1.sh")) as f: + line = f.readline() + assert line.strip() == f'echo "Hello with parameter 1 = param_value_2"' def test_configure_directory_not_absolute(): @@ -671,7 +656,7 @@ def test_configure_directory_not_absolute(): # Encode the pickled dictionary with Base64 encoded_dict = base64.b64encode(pickled_dict) parser = get_parser() - cmd = f"configure_directory {tagged_directory} {tagged_directory} {tag} {encoded_dict}" + cmd = f"configure {tagged_directory} {tagged_directory} {tag} {encoded_dict}" args = cmd.split() with pytest.raises(SystemExit) as e: @@ -742,7 +727,7 @@ def test_parser_copy(): def test_parser_configure_file_parse(): - """Test that the parser succeeds when receiving expected args for the configure_file operation""" + """Test that the parser succeeds when receiving expected args for the configure file operation""" parser = get_parser() src_path = pathlib.Path("/absolute/file/src/path") @@ -761,7 +746,7 @@ def test_parser_configure_file_parse(): pickled_dict = pickle.dumps(param_dict) encoded_dict = base64.b64encode(pickled_dict) - cmd = f"configure_file {src_path} {dest_path} {tag_delimiter} {encoded_dict}" + cmd = f"configure {src_path} {dest_path} {tag_delimiter} {encoded_dict}" args = cmd.split() ns = parser.parse_args(args) @@ -772,7 +757,7 @@ def test_parser_configure_file_parse(): def test_parser_configure_directory_parse(): - """Test that the parser succeeds when receiving expected args for the configure_directory operation""" + """Test that the parser succeeds when receiving expected args for the configure directory operation""" parser = get_parser() src_path = pathlib.Path("/absolute/file/src/path") @@ -791,7 +776,7 @@ def test_parser_configure_directory_parse(): pickled_dict = pickle.dumps(param_dict) encoded_dict = base64.b64encode(pickled_dict) - cmd = f"configure_directory {src_path} {dest_path} {tag_delimiter} {encoded_dict}" + cmd = f"configure {src_path} {dest_path} {tag_delimiter} {encoded_dict}" args = cmd.split() ns = parser.parse_args(args) diff --git a/tests/test_generator.py b/tests/test_generator.py index 62b3370dfb..daac45a291 100644 --- a/tests/test_generator.py +++ b/tests/test_generator.py @@ -28,7 +28,7 @@ import itertools import os import pathlib -import random +import unittest.mock from glob import glob from os import listdir from os import path as osp @@ -37,10 +37,9 @@ from smartsim import Experiment from smartsim._core.generation.generator import Generator -from smartsim.entity import Application, Ensemble, entity +from smartsim.entity import Ensemble, entity from smartsim.entity.files import EntityFiles from smartsim.launchable import Job -from smartsim.launchable.basejob import BaseJob from smartsim.settings import LaunchSettings # TODO Add JobGroup tests when JobGroup becomes a Launchable @@ -50,12 +49,11 @@ ids = set() +_ID_GENERATOR = (str(i) for i in itertools.count()) + + def random_id(): - while True: - num = str(random.randint(1, 100)) - if num not in ids: - ids.add(num) - return num + return next(_ID_GENERATOR) @pytest.fixture @@ -74,14 +72,14 @@ def get_gen_configure_dir(fileutils): @pytest.fixture -def generator_instance(test_dir) -> Generator: +def generator_instance(test_dir: str) -> Generator: """Fixture to create an instance of Generator.""" root = pathlib.Path(test_dir, "temp_id") os.mkdir(root) yield Generator(root=root) -def get_gen_file(fileutils, filename): +def get_gen_file(fileutils, filename: str): return fileutils.get_test_conf_path(osp.join("generator_files", filename)) @@ -90,104 +88,171 @@ class EchoHelloWorldEntity(entity.SmartSimEntity): def __init__(self): self.name = "entity_name" + self.files = None + self.file_parameters = None def as_program_arguments(self): return ("echo", "Hello", "World!") + def files(): + return ["file_path"] -class MockJob(BaseJob): - """Mock Job for testing.""" - - def __init__(self): - self.name = "test_job" - self.index = 1 - self.entity = EchoHelloWorldEntity() - def get_launch_steps(self): - raise NotImplementedError +@pytest.fixture +def mock_job() -> unittest.mock.MagicMock: + """Fixture to create a mock Job.""" + job = unittest.mock.MagicMock( + **{ + "entity": EchoHelloWorldEntity(), + "name": "test_job", + "get_launch_steps": unittest.mock.MagicMock( + side_effect=lambda: NotImplementedError() + ), + }, + spec=Job, + ) + yield job # UNIT TESTS -def test_init_generator(generator_instance, test_dir): +def test_init_generator(generator_instance: Generator, test_dir: str): """Test Generator init""" assert generator_instance.root == pathlib.Path(test_dir) / "temp_id" -def test_generate_job_root(generator_instance): +def test_generate_job_root( + generator_instance: Generator, mock_job: unittest.mock.MagicMock +): """Test Generator._generate_job_root returns correct path""" - mock_job = MockJob() - root_path = generator_instance._generate_job_root(mock_job, mock_job.index) + mock_index = 1 + root_path = generator_instance._generate_job_root(mock_job, mock_index) expected_path = ( generator_instance.root - / f"{MockJob.__name__.lower()}s" - / f"{mock_job.name}-{mock_job.index}" + / f"{mock_job.__class__.__name__.lower()}s" + / f"{mock_job.name}-{mock_index}" ) assert root_path == expected_path -def test_generate_run_path(generator_instance): +def test_generate_run_path( + test_dir: str, + mock_job: unittest.mock.MagicMock, + generator_instance: Generator, + monkeypatch: pytest.MonkeyPatch, +): """Test Generator._generate_run_path returns correct path""" - mock_job = MockJob() - run_path = generator_instance._generate_run_path(mock_job, mock_job.index) - expected_path = ( - generator_instance.root - / f"{MockJob.__name__.lower()}s" - / f"{mock_job.name}-{mock_job.index}" - / "run" + mock_index = 1 + monkeypatch.setattr( + Generator, + "_generate_job_root", + lambda self, job, job_index: pathlib.Path(test_dir), ) - assert run_path == expected_path + run_path = generator_instance._generate_run_path(mock_job, mock_index) + expected_run_path = pathlib.Path(test_dir) / "run" + assert run_path == expected_run_path -def test_generate_log_path(generator_instance): +def test_generate_log_path( + test_dir: str, + mock_job: unittest.mock.MagicMock, + generator_instance: Generator, + monkeypatch: pytest.MonkeyPatch, +): """Test Generator._generate_log_path returns correct path""" - mock_job = MockJob() - log_path = generator_instance._generate_log_path(mock_job, mock_job.index) - expected_path = ( - generator_instance.root - / f"{MockJob.__name__.lower()}s" - / f"{mock_job.name}-{mock_job.index}" - / "log" + mock_index = 1 + monkeypatch.setattr( + Generator, + "_generate_job_root", + lambda self, job, job_index: pathlib.Path(test_dir), ) - assert log_path == expected_path + log_path = generator_instance._generate_log_path(mock_job, mock_index) + expected_log_path = pathlib.Path(test_dir) / "log" + assert log_path == expected_log_path -def test_log_file_path(generator_instance): +def test_log_file_path(test_dir: str, generator_instance: Generator): """Test Generator._log_file returns correct path""" - base_path = "/tmp" - expected_path = osp.join(base_path, "smartsim_params.txt") - assert generator_instance._log_file(base_path) == pathlib.Path(expected_path) + expected_path = pathlib.Path(test_dir) / "smartsim_params.txt" + assert generator_instance._log_file(test_dir) == expected_path -def test_output_files(generator_instance): - """Test Generator._output_files returns err/out paths""" - base_path = pathlib.Path("/tmp") - out_file_path, err_file_path = generator_instance._output_files( - base_path, MockJob().name - ) - assert out_file_path == base_path / f"{MockJob().name}.out" - assert err_file_path == base_path / f"{MockJob().name}.err" +def test_out_file( + test_dir: str, generator_instance: Generator, mock_job: unittest.mock.MagicMock +): + """Test Generator._out_file returns out path""" + out_file_path = generator_instance._out_file(pathlib.Path(test_dir), mock_job.name) + assert out_file_path == pathlib.Path(test_dir) / f"{mock_job.name}.out" + + +def test_err_file( + test_dir: str, generator_instance: Generator, mock_job: unittest.mock.MagicMock +): + """Test Generator._err_file returns err path""" + err_file_path = generator_instance._err_file(pathlib.Path(test_dir), mock_job.name) + assert err_file_path == pathlib.Path(test_dir) / f"{mock_job.name}.err" + + +def test_generate_job( + mock_job: unittest.mock.MagicMock, + generator_instance: Generator, + monkeypatch: pytest.MonkeyPatch, +): + """Test Generator.generate_job returns correct paths and calls correct functions and writes to params file.""" + mock_index = 1 + job_paths = generator_instance.generate_job(mock_job, mock_index) + assert job_paths.run_path.name == Generator.run + assert job_paths.out_path.name == f"{mock_job.entity.name}.out" + assert job_paths.err_path.name == f"{mock_job.entity.name}.err" -def test_copy_file(generator_instance, fileutils): - """Test Generator._copy_files helper function with file list""" +def test_build_operations( + mock_job: unittest.mock.MagicMock, generator_instance: Generator, test_dir: str +): + """Test Generator._build_operations calls correct helper functions""" + with ( + unittest.mock.patch( + "smartsim._core.generation.Generator._copy_files" + ) as mock_copy_files, + unittest.mock.patch( + "smartsim._core.generation.Generator._symlink_files" + ) as mock_symlink_files, + unittest.mock.patch( + "smartsim._core.generation.Generator._write_tagged_files" + ) as mock_write_tagged_files, + ): + generator_instance._build_operations(mock_job, pathlib.Path(test_dir)) + mock_copy_files.assert_called_once() + mock_symlink_files.assert_called_once() + mock_write_tagged_files.assert_called_once() + + +def test_copy_file(generator_instance: Generator, fileutils): + """Test Generator._copy_files helper function with file""" script = fileutils.get_test_conf_path("sleep.py") + file = os.path.basename(script) files = EntityFiles(copy=script) generator_instance._copy_files(files, generator_instance.root) - expected_file = generator_instance.root / "sleep.py" + expected_file = ( + generator_instance.root / file + ) # parameterize this test - create a fixture that will loop through all the files in the directory - verify with more than 1 assert osp.isfile(expected_file) -def test_copy_directory(get_gen_copy_dir, generator_instance): +def test_copy_directory(get_gen_copy_dir, generator_instance: Generator): """Test Generator._copy_files helper function with directory""" - files = EntityFiles(copy=get_gen_copy_dir) - generator_instance._copy_files(files, generator_instance.root) + files = EntityFiles( + copy=get_gen_copy_dir + ) # parameterize for multiple directories, # create a fixture that creates a directory of files, validate that generate matches, validate with an empty directory, multiple dirs or so on + generator_instance._copy_files( + files, generator_instance.root + ) # fixture that says generate random files copied_folder = generator_instance.root / os.path.basename(get_gen_copy_dir) assert osp.isdir(copied_folder) -def test_symlink_file(get_gen_symlink_dir, generator_instance): +def test_symlink_file(get_gen_symlink_dir, generator_instance: Generator): """Test Generator._symlink_files helper function with file list""" symlink_files = sorted(glob(get_gen_symlink_dir + "/*")) files = EntityFiles(symlink=symlink_files) @@ -200,7 +265,7 @@ def test_symlink_file(get_gen_symlink_dir, generator_instance): ) -def test_symlink_directory(generator_instance, get_gen_symlink_dir): +def test_symlink_directory(generator_instance: Generator, get_gen_symlink_dir): """Test Generator._symlink_files helper function with directory""" files = EntityFiles(symlink=get_gen_symlink_dir) generator_instance._symlink_files(files, generator_instance.root) @@ -214,7 +279,7 @@ def test_symlink_directory(generator_instance, get_gen_symlink_dir): assert written == correct -def test_write_tagged_file(fileutils, generator_instance): +def test_write_tagged_file(fileutils, generator_instance: Generator): """Test Generator._write_tagged_files helper function with file list""" conf_path = fileutils.get_test_conf_path( osp.join("generator_files", "easy", "marked/") @@ -237,117 +302,51 @@ def test_write_tagged_file(fileutils, generator_instance): generator_instance._write_tagged_files( files=files, params=param_set, dest=generator_instance.root ) - configured_files = sorted(glob(os.path.join(generator_instance.root) + "/*")) + configured_files = sorted(glob(os.path.join(generator_instance.root, "*"))) for written, correct in itertools.zip_longest(configured_files, correct_files): assert filecmp.cmp(written, correct) -def test_write_tagged_directory(fileutils, generator_instance): +def test_write_tagged_directory(fileutils, generator_instance: Generator): """Test Generator._write_tagged_files helper function with directory path""" config = get_gen_file(fileutils, "tag_dir_template") - files = EntityFiles(tagged=config) + files = EntityFiles(tagged=[config]) param_set = {"PARAM0": "param_value_1", "PARAM1": "param_value_2"} generator_instance._write_tagged_files( files=files, params=param_set, dest=generator_instance.root ) - def _check_generated(param_0, param_1): - assert osp.isdir(osp.join(generator_instance.root, "nested_0")) - assert osp.isdir(osp.join(generator_instance.root, "nested_1")) - - with open(osp.join(generator_instance.root, "nested_0", "tagged_0.sh")) as f: - line = f.readline() - assert line.strip() == f'echo "Hello with parameter 0 = {param_0}"' - - with open(osp.join(generator_instance.root, "nested_1", "tagged_1.sh")) as f: - line = f.readline() - assert line.strip() == f'echo "Hello with parameter 1 = {param_1}"' - - _check_generated("param_value_1", "param_value_2") + assert osp.isdir(osp.join(generator_instance.root, "nested_0")) + assert osp.isdir(osp.join(generator_instance.root, "nested_1")) + with open(osp.join(generator_instance.root, "nested_0", "tagged_0.sh")) as f: + line = f.readline() + assert line.strip() == f'echo "Hello with parameter 0 = param_value_1"' -def test_generate_job(generator_instance, monkeypatch: pytest.MonkeyPatch): - """Test Generator.generate_job returns correct paths and calls correct functions and writes to params file.""" - monkeypatch.setattr( - Generator, "_generate_run_path", lambda self, job, job_index: "/tmp_run" - ) - log_path = generator_instance.root / "tmp_log" - os.mkdir(log_path) - monkeypatch.setattr( - Generator, "_generate_log_path", lambda self, job, job_index: log_path - ) - monkeypatch.setattr(Generator, "_output_files", lambda self, x, y: ("out", "err")) - monkeypatch.setattr( - Generator, "_build_operations", lambda self, job, job_path: None - ) - job_path, out_file, err_file = generator_instance.generate_job( - MockJob(), MockJob().index - ) - assert job_path == "/tmp_run" - assert out_file == "out" - assert err_file == "err" - with open(log_path / "smartsim_params.txt", "r") as file: - content = file.read() - assert "Generation start date and time:" in content + with open(osp.join(generator_instance.root, "nested_1", "tagged_1.sh")) as f: + line = f.readline() + assert line.strip() == f'echo "Hello with parameter 1 = param_value_2"' # INTEGRATED TESTS -def test_exp_private_generate_method(wlmutils, test_dir, generator_instance): - """Test that Job directory was created from Experiment._generate.""" - # Create Experiment +def test_exp_private_generate_method( + mock_job: unittest.mock.MagicMock, test_dir: str, generator_instance: Generator +): + """Test that Experiment._generate returns expected tuple.""" + mock_index = 1 exp = Experiment(name="experiment_name", exp_path=test_dir) - # Create Job - app = Application("name", "python") - launch_settings = LaunchSettings(wlmutils.get_test_launcher()) - job = Job(app, launch_settings) - # Generate Job directory - job_index = 1 - job_execution_path, _, _ = exp._generate(generator_instance, job, job_index) - # Assert Job run directory exists - assert osp.isdir(job_execution_path) - # Assert Job log directory exists - head, _ = os.path.split(job_execution_path) - expected_log_path = pathlib.Path(head) / "log" - assert osp.isdir(expected_log_path) - - -def test_exp_private_generate_method_ensemble(test_dir, wlmutils, generator_instance): - """Test that Job directory was created from Experiment.""" - ensemble = Ensemble("ensemble-name", "echo", replicas=2) - launch_settings = LaunchSettings(wlmutils.get_test_launcher()) - job_list = ensemble.as_jobs(launch_settings) - exp = Experiment(name="exp_name", exp_path=test_dir) - for i, job in enumerate(job_list): - job_run_path, _, _ = exp._generate(generator_instance, job, i) - head, _ = os.path.split(job_run_path) - expected_log_path = pathlib.Path(head) / "log" - assert osp.isdir(job_run_path) - assert osp.isdir(pathlib.Path(expected_log_path)) + job_paths = exp._generate(generator_instance, mock_job, mock_index) + assert osp.isdir(job_paths.run_path) + assert job_paths.out_path.name == f"{mock_job.entity.name}.out" + assert job_paths.err_path.name == f"{mock_job.entity.name}.err" -def test_generate_ensemble_directory(wlmutils, generator_instance): - ensemble = Ensemble("ensemble-name", "echo", replicas=2) - launch_settings = LaunchSettings(wlmutils.get_test_launcher()) - job_list = ensemble.as_jobs(launch_settings) - for i, job in enumerate(job_list): - # Call Generator.generate_job - path, _, _ = generator_instance.generate_job(job, i) - # Assert run directory created - assert osp.isdir(path) - # Assert smartsim params file created - head, _ = os.path.split(path) - expected_log_path = pathlib.Path(head) / "log" - assert osp.isdir(expected_log_path) - assert osp.isfile(osp.join(expected_log_path, "smartsim_params.txt")) - # Assert smartsim params correctly written to - with open(expected_log_path / "smartsim_params.txt", "r") as file: - content = file.read() - assert "Generation start date and time:" in content - - -def test_generate_ensemble_directory_start(test_dir, wlmutils, monkeypatch): +def test_generate_ensemble_directory_start( + test_dir: str, wlmutils, monkeypatch: pytest.Monkeypatch +): + """Test that Experiment._generate returns expected tuple.""" monkeypatch.setattr( "smartsim._core.dispatch._LauncherAdapter.start", lambda launch, exe, job_execution_path, env, out, err: random_id(), @@ -358,17 +357,19 @@ def test_generate_ensemble_directory_start(test_dir, wlmutils, monkeypatch): exp = Experiment(name="exp_name", exp_path=test_dir) exp.start(*job_list) run_dir = listdir(test_dir) - jobs_dir = os.path.join(test_dir, run_dir[0], "jobs") - job_dir = listdir(jobs_dir) - for ensemble_dir in job_dir: - run_path = os.path.join(jobs_dir, ensemble_dir, "run") - log_path = os.path.join(jobs_dir, ensemble_dir, "log") - assert osp.isdir(run_path) - assert osp.isdir(log_path) + jobs_dir_path = pathlib.Path(test_dir) / run_dir[0] / "jobs" + list_of_job_dirs = jobs_dir_path.iterdir() + for job in list_of_job_dirs: + run_path = jobs_dir_path / job / Generator.run + assert run_path.is_dir() + log_path = jobs_dir_path / job / Generator.log + assert log_path.is_dir() ids.clear() -def test_generate_ensemble_copy(test_dir, wlmutils, monkeypatch, get_gen_copy_dir): +def test_generate_ensemble_copy( + test_dir: str, wlmutils, monkeypatch: pytest.Monkeypatch, get_gen_copy_dir +): monkeypatch.setattr( "smartsim._core.dispatch._LauncherAdapter.start", lambda launch, exe, job_execution_path, env, out, err: random_id(), @@ -381,16 +382,16 @@ def test_generate_ensemble_copy(test_dir, wlmutils, monkeypatch, get_gen_copy_di exp = Experiment(name="exp_name", exp_path=test_dir) exp.start(*job_list) run_dir = listdir(test_dir) - jobs_dir = os.path.join(test_dir, run_dir[0], "jobs") - job_dir = listdir(jobs_dir) + jobs_dir = pathlib.Path(test_dir) / run_dir[0] / "jobs" + job_dir = jobs_dir.iterdir() for ensemble_dir in job_dir: - copy_folder_path = os.path.join(jobs_dir, ensemble_dir, "run", "to_copy_dir") - assert osp.isdir(copy_folder_path) + copy_folder_path = jobs_dir / ensemble_dir / Generator.run / "to_copy_dir" + assert copy_folder_path.is_dir() ids.clear() def test_generate_ensemble_symlink( - test_dir, wlmutils, monkeypatch, get_gen_symlink_dir + test_dir: str, wlmutils, monkeypatch: pytest.Monkeypatch, get_gen_symlink_dir ): monkeypatch.setattr( "smartsim._core.dispatch._LauncherAdapter.start", @@ -405,57 +406,58 @@ def test_generate_ensemble_symlink( launch_settings = LaunchSettings(wlmutils.get_test_launcher()) job_list = ensemble.as_jobs(launch_settings) exp = Experiment(name="exp_name", exp_path=test_dir) - exp.start(*job_list) + _ = exp.start(*job_list) run_dir = listdir(test_dir) - jobs_dir = os.path.join(test_dir, run_dir[0], "jobs") - job_dir = listdir(jobs_dir) + jobs_dir = pathlib.Path(test_dir) / run_dir[0] / "jobs" + job_dir = jobs_dir.iterdir() for ensemble_dir in job_dir: sym_file_path = pathlib.Path(jobs_dir) / ensemble_dir / "run" / "to_symlink_dir" - assert osp.isdir(sym_file_path) + assert sym_file_path.is_dir() assert sym_file_path.is_symlink() assert os.fspath(sym_file_path.resolve()) == osp.realpath(get_gen_symlink_dir) ids.clear() def test_generate_ensemble_configure( - test_dir, wlmutils, monkeypatch, get_gen_configure_dir + test_dir: str, wlmutils, monkeypatch: pytest.Monkeypatch, get_gen_configure_dir ): monkeypatch.setattr( "smartsim._core.dispatch._LauncherAdapter.start", lambda launch, exe, job_execution_path, env, out, err: random_id(), ) - params = {"PARAM0": [0, 1], "PARAM1": [2, 3]} - # Retrieve a list of files for configuration + param_set = {"PARAM0": [0, 1], "PARAM1": [2, 3]} tagged_files = sorted(glob(get_gen_configure_dir + "/*")) ensemble = Ensemble( "ensemble-name", "echo", replicas=1, files=EntityFiles(tagged=tagged_files), - file_parameters=params, + file_parameters=param_set, ) launch_settings = LaunchSettings(wlmutils.get_test_launcher()) job_list = ensemble.as_jobs(launch_settings) exp = Experiment(name="exp_name", exp_path=test_dir) - id = exp.start(*job_list) + _ = exp.start(*job_list) run_dir = listdir(test_dir) - jobs_dir = os.path.join(test_dir, run_dir[0], "jobs") + jobs_dir = pathlib.Path(test_dir) / run_dir[0] / "jobs" def _check_generated(param_0, param_1, dir): - assert osp.isdir(dir) - assert osp.isfile(osp.join(dir, "tagged_0.sh")) - assert osp.isfile(osp.join(dir, "tagged_1.sh")) + assert dir.is_dir() + tagged_0 = dir / "tagged_0.sh" + tagged_1 = dir / "tagged_1.sh" + assert tagged_0.is_file() + assert tagged_1.is_file() - with open(osp.join(dir, "tagged_0.sh")) as f: + with open(tagged_0) as f: line = f.readline() assert line.strip() == f'echo "Hello with parameter 0 = {param_0}"' - with open(osp.join(dir, "tagged_1.sh")) as f: + with open(tagged_1) as f: line = f.readline() assert line.strip() == f'echo "Hello with parameter 1 = {param_1}"' - _check_generated(0, 3, os.path.join(jobs_dir, "ensemble-name-1-1", "run")) - _check_generated(1, 2, os.path.join(jobs_dir, "ensemble-name-2-2", "run")) - _check_generated(1, 3, os.path.join(jobs_dir, "ensemble-name-3-3", "run")) - _check_generated(0, 2, os.path.join(jobs_dir, "ensemble-name-0-0", "run")) + _check_generated(0, 3, jobs_dir / "ensemble-name-1-1" / Generator.run) + _check_generated(1, 2, jobs_dir / "ensemble-name-2-2" / Generator.run) + _check_generated(1, 3, jobs_dir / "ensemble-name-3-3" / Generator.run) + _check_generated(0, 2, jobs_dir / "ensemble-name-0-0" / Generator.run) ids.clear() From fdc4671c9cc317b871b6f42e83fda8940db2b713 Mon Sep 17 00:00:00 2001 From: Amanda Richardson Date: Mon, 23 Sep 2024 14:50:10 -0500 Subject: [PATCH 07/13] pushing for review --- smartsim/_core/commands/commandList.py | 6 +- smartsim/_core/generation/generator.py | 283 +++++++++++++++---------- tests/test_generator.py | 147 +++++++------ 3 files changed, 241 insertions(+), 195 deletions(-) diff --git a/smartsim/_core/commands/commandList.py b/smartsim/_core/commands/commandList.py index 34743063e6..0f10208e32 100644 --- a/smartsim/_core/commands/commandList.py +++ b/smartsim/_core/commands/commandList.py @@ -34,9 +34,11 @@ class CommandList(MutableSequence[Command]): """Container for a Sequence of Command objects""" - def __init__(self, commands: t.Union[Command, t.List[Command]]): + def __init__(self, commands: t.Optional[t.Union[Command, t.List[Command]]] = None): """CommandList constructor""" - if isinstance(commands, Command): + if commands is None: + commands = [] + elif isinstance(commands, Command): commands = [commands] self._commands: t.List[Command] = list(commands) diff --git a/smartsim/_core/generation/generator.py b/smartsim/_core/generation/generator.py index aa1acfce32..a0cc0c1f74 100644 --- a/smartsim/_core/generation/generator.py +++ b/smartsim/_core/generation/generator.py @@ -37,6 +37,7 @@ from ...entity.files import EntityFiles from ...launchable import Job from ...log import get_logger +from ..commands import Command, CommandList logger = get_logger(__name__) logger.propagate = False @@ -44,9 +45,9 @@ @t.runtime_checkable class _GenerableProtocol(t.Protocol): - """A protocol that defines a generable entity with a method to retrieve - a list of file names. This protocol is used to ensure that functions - implementing it provide a `files` and `file_parameters` property. + """A protocol that defines a generatable entity with a method to retrieve + a list of file names and params. This protocol is used to ensure that functions + using job.entity continue if attrs file and params are supported. """ files: t.Union[EntityFiles, None] @@ -63,7 +64,9 @@ class Generator: """ run = "run" + """The name of the directory where run-related files are stored.""" log = "log" + """The name of the directory where log files are stored.""" def __init__(self, root: pathlib.Path) -> None: """Initialize a Generator object @@ -76,210 +79,256 @@ def __init__(self, root: pathlib.Path) -> None: self.root = root """The root path under which to generate files""" - def _generate_job_root(self, job: Job, job_index: int) -> pathlib.Path: - """Generates the root directory for a specific job instance. + def _build_job_base_path(self, job: Job, job_index: int) -> pathlib.Path: + """Build and return a Jobs base directory. The path is created by combining the + root directory with the Job type (derived from the class name), + the name attribute of the Job, and an index to differentiate between multiple + Job runs. - :param job: The Job instance for which the root directory is generated. - :param job_index: The index of the Job instance (used for naming). - :returns: The path to the root directory for the Job instance. + :param job: The job object + :param job_index: The index of the Job + :returns: The built file path for the Job """ job_type = f"{job.__class__.__name__.lower()}s" job_path = self.root / f"{job_type}/{job.name}-{job_index}" return pathlib.Path(job_path) - def _generate_run_path(self, job: Job, job_index: int) -> pathlib.Path: - """Generates the path for the "run" directory within the root directory - of a specific Job instance. + def _build_job_run_path(self, job: Job, job_index: int) -> pathlib.Path: + """Build and return a Jobs run directory. The path is formed by combining + the base directory with the `run` class-level variable, where run specifies + the name of the job's run folder. - :param job (Job): The Job instance for which the path is generated. - :param job_index (int): The index of the Job instance (used for naming). - :returns: The path to the "run" directory for the Job instance. + :param job: The job object + :param job_index: The index of the Job + :returns: The built file path for the Job run folder """ - path = self._generate_job_root(job, job_index) / self.run + path = self._build_job_base_path(job, job_index) / self.run path.mkdir(exist_ok=False, parents=True) return pathlib.Path(path) - def _generate_log_path(self, job: Job, job_index: int) -> pathlib.Path: - """ - Generates the path for the "log" directory within the root directory of a specific Job instance. + def _build_job_log_path(self, job: Job, job_index: int) -> pathlib.Path: + """Build and return a Jobs log directory. The path is formed by combining + the base directory with the `log` class-level variable, where log specifies + the name of the job's log folder. - :param job: The Job instance for which the path is generated. - :param job_index: The index of the Job instance (used for naming). - :returns: The path to the "log" directory for the Job instance. + :param job: The job object + :param job_index: The index of the Job + :returns: The built file path for the Job run folder """ path = ( - self._generate_job_root(job, job_index) / self.log - ) # module or class level constant + self._build_job_base_path(job, job_index) / self.log + ) path.mkdir(exist_ok=False, parents=True) return pathlib.Path(path) @staticmethod - def _log_file(log_path: pathlib.Path) -> pathlib.Path: - """Returns the location of the file - summarizing the parameters used for the generation - of the entity. + def _build_log_file_path(log_path: pathlib.Path) -> pathlib.Path: + """Build and return an entities file summarizing the parameters + used for the generation of the entity. :param log_path: Path to log directory - :returns: Path to file with parameter settings + :returns: The built file path an entities params file """ return pathlib.Path(log_path) / "smartsim_params.txt" @staticmethod - def _out_file(log_path: pathlib.Path, job_name: str) -> pathlib.Path: - """Return the path to the output file. + def _build_out_file_path(log_path: pathlib.Path, job_name: str) -> pathlib.Path: + """Build and return the path to the output file. The path is created by combining + the Jobs log directory with the job name and appending the `.out` extension. :param log_path: Path to log directory - :param job_name: Name of Job - :returns: Path to output file + :param job_name: Name of the Job + :returns: Path to the output file """ out_file_path = log_path / f"{job_name}.out" return out_file_path @staticmethod - def _err_file(log_path: pathlib.Path, job_name: str) -> pathlib.Path: - """Return the path to the error file. + def _build_err_file_path(log_path: pathlib.Path, job_name: str) -> pathlib.Path: + """Build and return the path to the error file. The path is created by combining + the Jobs log directory with the job name and appending the `.err` extension. :param log_path: Path to log directory - :param job_name: Name of Job - :returns: Path to error file + :param job_name: Name of the Job + :returns: Path to the error file """ err_file_path = log_path / f"{job_name}.err" return err_file_path def generate_job(self, job: Job, job_index: int) -> Job_Path: - """Write and configure input files for a Job. - - To have files or directories present in the created Job - directory, such as datasets or input files, call - ``entity.attach_generator_files`` prior to generation. - - Tagged application files are read, checked for input variables to - configure, and written. Input variables to configure are - specified with a tag within the input file itself. - The default tag is surrounding an input value with semicolons. - e.g. ``THERMO=;90;`` - - :param job: The job instance to write and configure files for. - :param job_path: The path to the "run" directory for the job instance. - :param log_path: The path to the "log" directory for the job instance. + """Build and return the Job's run directory, error file and out file. + + This method creates the Job's run and log directories, generates the + `smartsim_params.txt` file to log parameters used for the Job, and sets + up the output and error files for Job execution information. If files are + attached to the Job's entity, it builds file operation commands and executes + them. + + :param job: The job object + :param job_index: The index of the Job + :return: Job's run directory, error file and out file. """ - job_path = self._generate_run_path(job, job_index) - log_path = self._generate_log_path(job, job_index) + job_path = self._build_job_run_path(job, job_index) + log_path = self._build_job_log_path(job, job_index) - with open(self._log_file(log_path), mode="w", encoding="utf-8") as log_file: + with open(self._build_log_file_path(log_path), mode="w", encoding="utf-8") as log_file: dt_string = datetime.now().strftime("%d/%m/%Y %H:%M:%S") log_file.write(f"Generation start date and time: {dt_string}\n") - out_file = self._out_file(log_path, job.entity.name) - err_file = self._err_file(log_path, job.entity.name) + out_file = self._build_out_file_path(log_path, job.entity.name) + err_file = self._build_err_file_path(log_path, job.entity.name) - self._build_operations(job, job_path) + cmd_list = self._build_commands(job, job_path) + if cmd_list: + self._execute_commands(cmd_list) return Job_Path(job_path, out_file, err_file) @classmethod - def _build_operations(cls, job: Job, job_path: pathlib.Path) -> None: - """This method orchestrates file system ops for the attached SmartSim entity. - It processes three types of file system operations: to_copy, to_symlink, and to_configure. - For each type, it calls the corresponding private methods that open a subprocess - to complete each task. - - :param job: The Job to perform file ops on attached entity files - :param job_path: Path to the Jobs run directory + def _build_commands(cls, job: Job, job_path: pathlib.Path) -> t.Optional[CommandList]: + """Build file operation commands for all files attached to a Job's entity. + + This method constructs commands for copying, symlinking, and writing tagged files + associated with the Job's entity. It aggregates these commands into a CommandList + to return. + + :param job: The job object + :param job_path: The file path for the Job run folder + :return: A CommandList containing the file operation commands, or None if the entity + does not support file operations. """ if isinstance(job.entity, _GenerableProtocol): - cls._copy_files(job.entity.files, job_path) - cls._symlink_files(job.entity.files, job_path) - cls._write_tagged_files( - job.entity.files, job.entity.file_parameters, job_path - ) + cmd_list = CommandList() + helpers = [ + cls._copy_files, + cls._symlink_files, + lambda files, path: cls._write_tagged_files(files, job.entity.file_parameters, path) + ] - @staticmethod - def _copy_files(files: t.Union[EntityFiles, None], dest: pathlib.Path) -> None: - """Perform copy file sys operations on a list of files. + for method in helpers: + return_cmd_list = method(job.entity.files, job_path) + if return_cmd_list: + cmd_list.commands.extend(return_cmd_list.commands) - :param files: The paths to copy - :param dest: Path to the Jobs run directory + return cmd_list + return None + + @classmethod + def _execute_commands(cls, cmd_list: CommandList) -> None: + """Execute a list of commands using subprocess. + + This helper function iterates through each command in the provided CommandList + and executes them using the subprocess module. + + :param cmd_list: A CommandList object containing the commands to be executed + """ + for cmd in cmd_list: + subprocess.run(cmd.command) + + @staticmethod + def _copy_files(files: t.Union[EntityFiles, None], dest: pathlib.Path) -> t.Optional[CommandList]: + """Build command to copy files/directories from specified paths to a destination directory. + + This method creates commands to copy files/directories from the source paths provided in the + `files` parameter to the specified destination directory. If the source is a directory, + it copies the directory while allowing existing directories to remain intact. + + :param files: An EntityFiles object containing the paths to copy, or None. + :param dest: The destination path to the Job's run directory. + :return: A CommandList containing the copy commands, or None if no files are provided. """ if files is None: - return + return None + cmd_list = CommandList() for src in files.copy: - args = [ + cmd = Command([ sys.executable, "-m", "smartsim._core.entrypoints.file_operations", "copy", src, - ] + ]) destination = str(dest) if os.path.isdir(src): base_source_name = os.path.basename(src) destination = os.path.join(dest, base_source_name) - args.append(str(destination)) - args.append("--dirs_exist_ok") + cmd.append(str(destination)) + cmd.append("--dirs_exist_ok") else: - args.append(str(dest)) - subprocess.run(args) + cmd.append(str(dest)) + cmd_list.commands.append(cmd) + return cmd_list @staticmethod - def _symlink_files(files: t.Union[EntityFiles, None], dest: pathlib.Path) -> None: - """Perform symlink file sys operations on a list of files. - - :param files: The paths to symlink - :param dest: Path to the Jobs run directory + def _symlink_files(files: t.Union[EntityFiles, None], dest: pathlib.Path) -> t.Optional[CommandList]: + """Build command to symlink files/directories from specified paths to a destination directory. + + This method creates commands to symlink files/directories from the source paths provided in the + `files` parameter to the specified destination directory. If the source is a directory, + it copies the directory while allowing existing directories to remain intact. + + :param files: An EntityFiles object containing the paths to symlink, or None. + :param dest: The destination path to the Job's run directory. + :return: A CommandList containing the symlink commands, or None if no files are provided. """ if files is None: - return + return None + cmd_list = CommandList() for src in files.link: # Normalize the path to remove trailing slashes normalized_path = os.path.normpath(src) # Get the parent directory (last folder) parent_dir = os.path.basename(normalized_path) new_dest = os.path.join(str(dest), parent_dir) - subprocess.run( - args=[ - sys.executable, - "-m", - "smartsim._core.entrypoints.file_operations", - "symlink", - src, - new_dest, - ] - ) + cmd=Command([ + sys.executable, + "-m", + "smartsim._core.entrypoints.file_operations", + "symlink", + src, + new_dest, + ]) + cmd_list.append(cmd) + return cmd_list @staticmethod def _write_tagged_files( files: t.Union[EntityFiles, None], params: t.Mapping[str, str], dest: pathlib.Path, - ) -> None: - """Read, configure and write the tagged files and directories for - a Job instance. - - :param files: Paths to configure + ) -> t.Optional[CommandList]: + """Build command to configure files/directories from specified paths to a destination directory. + + This method processes tagged files by reading their configurations, + serializing the provided parameters, and generating commands to + write these configurations to the destination directory. + + :param files: An EntityFiles object containing the paths to configure, or None. :param params: A dictionary of params - :param dest: Path to the Jobs run directory + :param dest: The destination path to the Job's run directory. + :return: A CommandList containing the configuration commands, or None if no files are provided. """ if files is None: - return + return None + cmd_list = CommandList() if files.tagged: tag_delimiter = ";" pickled_dict = pickle.dumps(params) encoded_dict = base64.b64encode(pickled_dict).decode("ascii") for path in files.tagged: - subprocess.run( - args=[ - sys.executable, - "-m", - "smartsim._core.entrypoints.file_operations", - "configure", - path, - dest, - tag_delimiter, - encoded_dict, - ] - ) + cmd=Command([ + sys.executable, + "-m", + "smartsim._core.entrypoints.file_operations", + "configure", + path, + str(dest), + tag_delimiter, + encoded_dict, + ]) + cmd_list.commands.append(cmd) + return cmd_list # TODO address in ticket 723 # self._log_params(entity, files_to_params) diff --git a/tests/test_generator.py b/tests/test_generator.py index daac45a291..12878b27f3 100644 --- a/tests/test_generator.py +++ b/tests/test_generator.py @@ -36,6 +36,7 @@ import pytest from smartsim import Experiment +from smartsim._core.commands import Command, CommandList from smartsim._core.generation.generator import Generator from smartsim.entity import Ensemble, entity from smartsim.entity.files import EntityFiles @@ -122,12 +123,12 @@ def test_init_generator(generator_instance: Generator, test_dir: str): assert generator_instance.root == pathlib.Path(test_dir) / "temp_id" -def test_generate_job_root( +def test_build_job_base_path( generator_instance: Generator, mock_job: unittest.mock.MagicMock ): - """Test Generator._generate_job_root returns correct path""" + """Test Generator._build_job_base_path returns correct path""" mock_index = 1 - root_path = generator_instance._generate_job_root(mock_job, mock_index) + root_path = generator_instance._build_job_base_path(mock_job, mock_index) expected_path = ( generator_instance.root / f"{mock_job.__class__.__name__.lower()}s" @@ -136,61 +137,65 @@ def test_generate_job_root( assert root_path == expected_path -def test_generate_run_path( +def test_build_job_run_path( test_dir: str, mock_job: unittest.mock.MagicMock, generator_instance: Generator, monkeypatch: pytest.MonkeyPatch, ): - """Test Generator._generate_run_path returns correct path""" + """Test Generator._build_job_run_path returns correct path""" mock_index = 1 monkeypatch.setattr( Generator, - "_generate_job_root", + "_build_job_base_path", lambda self, job, job_index: pathlib.Path(test_dir), ) - run_path = generator_instance._generate_run_path(mock_job, mock_index) + run_path = generator_instance._build_job_run_path(mock_job, mock_index) expected_run_path = pathlib.Path(test_dir) / "run" assert run_path == expected_run_path -def test_generate_log_path( +def test_build_job_log_path( test_dir: str, mock_job: unittest.mock.MagicMock, generator_instance: Generator, monkeypatch: pytest.MonkeyPatch, ): - """Test Generator._generate_log_path returns correct path""" + """Test Generator._build_job_log_path returns correct path""" mock_index = 1 monkeypatch.setattr( Generator, - "_generate_job_root", + "_build_job_base_path", lambda self, job, job_index: pathlib.Path(test_dir), ) - log_path = generator_instance._generate_log_path(mock_job, mock_index) + log_path = generator_instance._build_job_log_path(mock_job, mock_index) expected_log_path = pathlib.Path(test_dir) / "log" assert log_path == expected_log_path -def test_log_file_path(test_dir: str, generator_instance: Generator): - """Test Generator._log_file returns correct path""" +def test_build_log_file_path(test_dir: str, generator_instance: Generator): + """Test Generator._build_log_file_path returns correct path""" expected_path = pathlib.Path(test_dir) / "smartsim_params.txt" - assert generator_instance._log_file(test_dir) == expected_path + assert generator_instance._build_log_file_path(test_dir) == expected_path -def test_out_file( +def test_build_out_file_path( test_dir: str, generator_instance: Generator, mock_job: unittest.mock.MagicMock ): - """Test Generator._out_file returns out path""" - out_file_path = generator_instance._out_file(pathlib.Path(test_dir), mock_job.name) + """Test Generator._build_out_file_path returns out path""" + out_file_path = generator_instance._build_out_file_path( + pathlib.Path(test_dir), mock_job.name + ) assert out_file_path == pathlib.Path(test_dir) / f"{mock_job.name}.out" -def test_err_file( +def test_build_err_file_path( test_dir: str, generator_instance: Generator, mock_job: unittest.mock.MagicMock ): - """Test Generator._err_file returns err path""" - err_file_path = generator_instance._err_file(pathlib.Path(test_dir), mock_job.name) + """Test Generator._build_err_file_path returns err path""" + err_file_path = generator_instance._build_err_file_path( + pathlib.Path(test_dir), mock_job.name + ) assert err_file_path == pathlib.Path(test_dir) / f"{mock_job.name}.err" @@ -199,7 +204,7 @@ def test_generate_job( generator_instance: Generator, monkeypatch: pytest.MonkeyPatch, ): - """Test Generator.generate_job returns correct paths and calls correct functions and writes to params file.""" + """Test Generator.generate_job returns correct paths""" mock_index = 1 job_paths = generator_instance.generate_job(mock_job, mock_index) assert job_paths.run_path.name == Generator.run @@ -207,10 +212,10 @@ def test_generate_job( assert job_paths.err_path.name == f"{mock_job.entity.name}.err" -def test_build_operations( +def test_build_commands( mock_job: unittest.mock.MagicMock, generator_instance: Generator, test_dir: str ): - """Test Generator._build_operations calls correct helper functions""" + """Test Generator._build_commands calls correct helper functions""" with ( unittest.mock.patch( "smartsim._core.generation.Generator._copy_files" @@ -222,61 +227,63 @@ def test_build_operations( "smartsim._core.generation.Generator._write_tagged_files" ) as mock_write_tagged_files, ): - generator_instance._build_operations(mock_job, pathlib.Path(test_dir)) + generator_instance._build_commands(mock_job, pathlib.Path(test_dir)) mock_copy_files.assert_called_once() mock_symlink_files.assert_called_once() mock_write_tagged_files.assert_called_once() +def test_execute_commands(generator_instance: Generator): + """Test Generator._execute_commands subprocess.run""" + with ( + unittest.mock.patch( + "smartsim._core.generation.generator.subprocess.run" + ) as run_process, + ): + cmd_list = CommandList(Command(["test", "command"])) + generator_instance._execute_commands(cmd_list) + run_process.assert_called_once() + + def test_copy_file(generator_instance: Generator, fileutils): """Test Generator._copy_files helper function with file""" script = fileutils.get_test_conf_path("sleep.py") - file = os.path.basename(script) files = EntityFiles(copy=script) - generator_instance._copy_files(files, generator_instance.root) - expected_file = ( - generator_instance.root / file - ) # parameterize this test - create a fixture that will loop through all the files in the directory - verify with more than 1 - assert osp.isfile(expected_file) + cmd_list = generator_instance._copy_files(files, generator_instance.root) + assert isinstance(cmd_list, CommandList) + assert len(cmd_list) == 1 + assert str(generator_instance.root) and script in cmd_list.commands[0].command def test_copy_directory(get_gen_copy_dir, generator_instance: Generator): """Test Generator._copy_files helper function with directory""" - files = EntityFiles( - copy=get_gen_copy_dir - ) # parameterize for multiple directories, # create a fixture that creates a directory of files, validate that generate matches, validate with an empty directory, multiple dirs or so on - generator_instance._copy_files( - files, generator_instance.root - ) # fixture that says generate random files - copied_folder = generator_instance.root / os.path.basename(get_gen_copy_dir) - assert osp.isdir(copied_folder) + files = EntityFiles(copy=get_gen_copy_dir) + cmd_list = generator_instance._copy_files(files, generator_instance.root) + assert isinstance(cmd_list, CommandList) + assert len(cmd_list) == 1 + assert ( + str(generator_instance.root) + and get_gen_copy_dir in cmd_list.commands[0].command + ) def test_symlink_file(get_gen_symlink_dir, generator_instance: Generator): """Test Generator._symlink_files helper function with file list""" symlink_files = sorted(glob(get_gen_symlink_dir + "/*")) files = EntityFiles(symlink=symlink_files) - generator_instance._symlink_files(files, generator_instance.root) - symlinked_file = generator_instance.root / os.path.basename(symlink_files[0]) - assert osp.isfile(symlinked_file) - assert symlinked_file.is_symlink() - assert os.fspath(symlinked_file.resolve()) == osp.join( - osp.realpath(get_gen_symlink_dir), "mock2.txt" - ) + cmd_list = generator_instance._symlink_files(files, generator_instance.root) + assert isinstance(cmd_list, CommandList) + for file, cmd in zip(symlink_files, cmd_list): + assert file in cmd.command def test_symlink_directory(generator_instance: Generator, get_gen_symlink_dir): """Test Generator._symlink_files helper function with directory""" files = EntityFiles(symlink=get_gen_symlink_dir) - generator_instance._symlink_files(files, generator_instance.root) + cmd_list = generator_instance._symlink_files(files, generator_instance.root) symlinked_folder = generator_instance.root / os.path.basename(get_gen_symlink_dir) - assert osp.isdir(symlinked_folder) - assert symlinked_folder.is_symlink() - assert os.fspath(symlinked_folder.resolve()) == osp.realpath(get_gen_symlink_dir) - for written, correct in itertools.zip_longest( - listdir(get_gen_symlink_dir), listdir(symlinked_folder) - ): - assert written == correct + assert isinstance(cmd_list, CommandList) + assert str(symlinked_folder) in cmd_list.commands[0].command def test_write_tagged_file(fileutils, generator_instance: Generator): @@ -285,10 +292,6 @@ def test_write_tagged_file(fileutils, generator_instance: Generator): osp.join("generator_files", "easy", "marked/") ) tagged_files = sorted(glob(conf_path + "/*")) - correct_path = fileutils.get_test_conf_path( - osp.join("generator_files", "easy", "correct/") - ) - correct_files = sorted(glob(correct_path + "/*")) files = EntityFiles(tagged=tagged_files) param_set = { "5": 10, @@ -299,12 +302,12 @@ def test_write_tagged_file(fileutils, generator_instance: Generator): "1200": "120", "VALID": "valid", } - generator_instance._write_tagged_files( + cmd_list = generator_instance._write_tagged_files( files=files, params=param_set, dest=generator_instance.root ) - configured_files = sorted(glob(os.path.join(generator_instance.root, "*"))) - for written, correct in itertools.zip_longest(configured_files, correct_files): - assert filecmp.cmp(written, correct) + assert isinstance(cmd_list, CommandList) + for file, cmd in zip(tagged_files, cmd_list): + assert file in cmd.command def test_write_tagged_directory(fileutils, generator_instance: Generator): @@ -312,20 +315,12 @@ def test_write_tagged_directory(fileutils, generator_instance: Generator): config = get_gen_file(fileutils, "tag_dir_template") files = EntityFiles(tagged=[config]) param_set = {"PARAM0": "param_value_1", "PARAM1": "param_value_2"} - generator_instance._write_tagged_files( + cmd_list = generator_instance._write_tagged_files( files=files, params=param_set, dest=generator_instance.root ) - assert osp.isdir(osp.join(generator_instance.root, "nested_0")) - assert osp.isdir(osp.join(generator_instance.root, "nested_1")) - - with open(osp.join(generator_instance.root, "nested_0", "tagged_0.sh")) as f: - line = f.readline() - assert line.strip() == f'echo "Hello with parameter 0 = param_value_1"' - - with open(osp.join(generator_instance.root, "nested_1", "tagged_1.sh")) as f: - line = f.readline() - assert line.strip() == f'echo "Hello with parameter 1 = param_value_2"' + assert isinstance(cmd_list, CommandList) + assert str(config) in cmd_list.commands[0].command # INTEGRATED TESTS @@ -344,7 +339,7 @@ def test_exp_private_generate_method( def test_generate_ensemble_directory_start( - test_dir: str, wlmutils, monkeypatch: pytest.Monkeypatch + test_dir: str, wlmutils, monkeypatch: pytest.MonkeyPatch ): """Test that Experiment._generate returns expected tuple.""" monkeypatch.setattr( @@ -368,7 +363,7 @@ def test_generate_ensemble_directory_start( def test_generate_ensemble_copy( - test_dir: str, wlmutils, monkeypatch: pytest.Monkeypatch, get_gen_copy_dir + test_dir: str, wlmutils, monkeypatch: pytest.MonkeyPatch, get_gen_copy_dir ): monkeypatch.setattr( "smartsim._core.dispatch._LauncherAdapter.start", @@ -391,7 +386,7 @@ def test_generate_ensemble_copy( def test_generate_ensemble_symlink( - test_dir: str, wlmutils, monkeypatch: pytest.Monkeypatch, get_gen_symlink_dir + test_dir: str, wlmutils, monkeypatch: pytest.MonkeyPatch, get_gen_symlink_dir ): monkeypatch.setattr( "smartsim._core.dispatch._LauncherAdapter.start", @@ -419,7 +414,7 @@ def test_generate_ensemble_symlink( def test_generate_ensemble_configure( - test_dir: str, wlmutils, monkeypatch: pytest.Monkeypatch, get_gen_configure_dir + test_dir: str, wlmutils, monkeypatch: pytest.MonkeyPatch, get_gen_configure_dir ): monkeypatch.setattr( "smartsim._core.dispatch._LauncherAdapter.start", From d689785b16bfa552a4c50c503e269ad38c01f4b6 Mon Sep 17 00:00:00 2001 From: Amanda Richardson Date: Mon, 23 Sep 2024 15:07:53 -0500 Subject: [PATCH 08/13] pushing for review --- smartsim/_core/generation/generator.py | 126 ++++++++++++++----------- tests/test_generator.py | 2 - 2 files changed, 71 insertions(+), 57 deletions(-) diff --git a/smartsim/_core/generation/generator.py b/smartsim/_core/generation/generator.py index a0cc0c1f74..f0278d5cf9 100644 --- a/smartsim/_core/generation/generator.py +++ b/smartsim/_core/generation/generator.py @@ -45,23 +45,19 @@ @t.runtime_checkable class _GenerableProtocol(t.Protocol): - """A protocol that defines a generatable entity with a method to retrieve - a list of file names and params. This protocol is used to ensure that functions - using job.entity continue if attrs file and params are supported. - """ + """Ensures functions using job.entity continue if attrs file and params are supported.""" files: t.Union[EntityFiles, None] file_parameters: t.Mapping[str, str] Job_Path = namedtuple("Job_Path", ["run_path", "out_path", "err_path"]) +"""Paths related to the Jobs execution.""" class Generator: - """The primary job of the Generator is to create the directory and file structure - for a SmartSim Job. The Generator is also responsible for writing and configuring - files into the Job directory. - """ + """The primary responsibility of the Generator class is to create the directory structure + for a SmartSim Job and to build and execute file operation commands.""" run = "run" """The name of the directory where run-related files are stored.""" @@ -71,10 +67,16 @@ class Generator: def __init__(self, root: pathlib.Path) -> None: """Initialize a Generator object - The class handles symlinking, copying, and configuration of files - associated with a Jobs entity. Additionally, it writes entity parameters - used for the specific run into the "smartsim_params.txt" settings file within - the Jobs log folder. + The Generator class constructs a Jobs directory structure, including: + + - The run and log directories + - Output and error files + - The "smartsim_params.txt" settings file + + Additionally, it manages symlinking, copying, and configuring files associated + with a Jobs entity. + + :param root: The Jobs base path """ self.root = root """The root path under which to generate files""" @@ -115,9 +117,7 @@ def _build_job_log_path(self, job: Job, job_index: int) -> pathlib.Path: :param job_index: The index of the Job :returns: The built file path for the Job run folder """ - path = ( - self._build_job_base_path(job, job_index) / self.log - ) + path = self._build_job_base_path(job, job_index) / self.log path.mkdir(exist_ok=False, parents=True) return pathlib.Path(path) @@ -157,7 +157,7 @@ def _build_err_file_path(log_path: pathlib.Path, job_name: str) -> pathlib.Path: def generate_job(self, job: Job, job_index: int) -> Job_Path: """Build and return the Job's run directory, error file and out file. - + This method creates the Job's run and log directories, generates the `smartsim_params.txt` file to log parameters used for the Job, and sets up the output and error files for Job execution information. If files are @@ -172,7 +172,9 @@ def generate_job(self, job: Job, job_index: int) -> Job_Path: job_path = self._build_job_run_path(job, job_index) log_path = self._build_job_log_path(job, job_index) - with open(self._build_log_file_path(log_path), mode="w", encoding="utf-8") as log_file: + with open( + self._build_log_file_path(log_path), mode="w", encoding="utf-8" + ) as log_file: dt_string = datetime.now().strftime("%d/%m/%Y %H:%M:%S") log_file.write(f"Generation start date and time: {dt_string}\n") @@ -186,13 +188,15 @@ def generate_job(self, job: Job, job_index: int) -> Job_Path: return Job_Path(job_path, out_file, err_file) @classmethod - def _build_commands(cls, job: Job, job_path: pathlib.Path) -> t.Optional[CommandList]: + def _build_commands( + cls, job: Job, job_path: pathlib.Path + ) -> t.Optional[CommandList]: """Build file operation commands for all files attached to a Job's entity. This method constructs commands for copying, symlinking, and writing tagged files associated with the Job's entity. It aggregates these commands into a CommandList to return. - + :param job: The job object :param job_path: The file path for the Job run folder :return: A CommandList containing the file operation commands, or None if the entity @@ -203,7 +207,9 @@ def _build_commands(cls, job: Job, job_path: pathlib.Path) -> t.Optional[Command helpers = [ cls._copy_files, cls._symlink_files, - lambda files, path: cls._write_tagged_files(files, job.entity.file_parameters, path) + lambda files, path: cls._write_tagged_files( + files, job.entity.file_parameters, path + ), ] for method in helpers: @@ -213,7 +219,7 @@ def _build_commands(cls, job: Job, job_path: pathlib.Path) -> t.Optional[Command return cmd_list return None - + @classmethod def _execute_commands(cls, cmd_list: CommandList) -> None: """Execute a list of commands using subprocess. @@ -227,13 +233,15 @@ def _execute_commands(cls, cmd_list: CommandList) -> None: subprocess.run(cmd.command) @staticmethod - def _copy_files(files: t.Union[EntityFiles, None], dest: pathlib.Path) -> t.Optional[CommandList]: + def _copy_files( + files: t.Union[EntityFiles, None], dest: pathlib.Path + ) -> t.Optional[CommandList]: """Build command to copy files/directories from specified paths to a destination directory. This method creates commands to copy files/directories from the source paths provided in the `files` parameter to the specified destination directory. If the source is a directory, it copies the directory while allowing existing directories to remain intact. - + :param files: An EntityFiles object containing the paths to copy, or None. :param dest: The destination path to the Job's run directory. :return: A CommandList containing the copy commands, or None if no files are provided. @@ -242,13 +250,15 @@ def _copy_files(files: t.Union[EntityFiles, None], dest: pathlib.Path) -> t.Opti return None cmd_list = CommandList() for src in files.copy: - cmd = Command([ - sys.executable, - "-m", - "smartsim._core.entrypoints.file_operations", - "copy", - src, - ]) + cmd = Command( + [ + sys.executable, + "-m", + "smartsim._core.entrypoints.file_operations", + "copy", + src, + ] + ) destination = str(dest) if os.path.isdir(src): base_source_name = os.path.basename(src) @@ -261,9 +271,11 @@ def _copy_files(files: t.Union[EntityFiles, None], dest: pathlib.Path) -> t.Opti return cmd_list @staticmethod - def _symlink_files(files: t.Union[EntityFiles, None], dest: pathlib.Path) -> t.Optional[CommandList]: + def _symlink_files( + files: t.Union[EntityFiles, None], dest: pathlib.Path + ) -> t.Optional[CommandList]: """Build command to symlink files/directories from specified paths to a destination directory. - + This method creates commands to symlink files/directories from the source paths provided in the `files` parameter to the specified destination directory. If the source is a directory, it copies the directory while allowing existing directories to remain intact. @@ -281,14 +293,16 @@ def _symlink_files(files: t.Union[EntityFiles, None], dest: pathlib.Path) -> t.O # Get the parent directory (last folder) parent_dir = os.path.basename(normalized_path) new_dest = os.path.join(str(dest), parent_dir) - cmd=Command([ - sys.executable, - "-m", - "smartsim._core.entrypoints.file_operations", - "symlink", - src, - new_dest, - ]) + cmd = Command( + [ + sys.executable, + "-m", + "smartsim._core.entrypoints.file_operations", + "symlink", + src, + new_dest, + ] + ) cmd_list.append(cmd) return cmd_list @@ -299,9 +313,9 @@ def _write_tagged_files( dest: pathlib.Path, ) -> t.Optional[CommandList]: """Build command to configure files/directories from specified paths to a destination directory. - - This method processes tagged files by reading their configurations, - serializing the provided parameters, and generating commands to + + This method processes tagged files by reading their configurations, + serializing the provided parameters, and generating commands to write these configurations to the destination directory. :param files: An EntityFiles object containing the paths to configure, or None. @@ -317,21 +331,23 @@ def _write_tagged_files( pickled_dict = pickle.dumps(params) encoded_dict = base64.b64encode(pickled_dict).decode("ascii") for path in files.tagged: - cmd=Command([ - sys.executable, - "-m", - "smartsim._core.entrypoints.file_operations", - "configure", - path, - str(dest), - tag_delimiter, - encoded_dict, - ]) + cmd = Command( + [ + sys.executable, + "-m", + "smartsim._core.entrypoints.file_operations", + "configure", + path, + str(dest), + tag_delimiter, + encoded_dict, + ] + ) cmd_list.commands.append(cmd) return cmd_list - # TODO address in ticket 723 - # self._log_params(entity, files_to_params) + # TODO address in ticket 723 + # self._log_params(entity, files_to_params) # TODO to be refactored in ticket 723 # def _log_params( diff --git a/tests/test_generator.py b/tests/test_generator.py index 12878b27f3..b6ba399059 100644 --- a/tests/test_generator.py +++ b/tests/test_generator.py @@ -24,7 +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 filecmp import itertools import os import pathlib @@ -202,7 +201,6 @@ def test_build_err_file_path( def test_generate_job( mock_job: unittest.mock.MagicMock, generator_instance: Generator, - monkeypatch: pytest.MonkeyPatch, ): """Test Generator.generate_job returns correct paths""" mock_index = 1 From be2c1959376a2c36cd8c7a5865c26eed63f9c239 Mon Sep 17 00:00:00 2001 From: Amanda Richardson Date: Mon, 23 Sep 2024 15:40:00 -0500 Subject: [PATCH 09/13] mypy errors fixed --- smartsim/_core/generation/generator.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/smartsim/_core/generation/generator.py b/smartsim/_core/generation/generator.py index f0278d5cf9..b5d426715b 100644 --- a/smartsim/_core/generation/generator.py +++ b/smartsim/_core/generation/generator.py @@ -202,18 +202,21 @@ def _build_commands( :return: A CommandList containing the file operation commands, or None if the entity does not support file operations. """ - if isinstance(job.entity, _GenerableProtocol): + entity = job.entity + if isinstance(entity, _GenerableProtocol): cmd_list = CommandList() - helpers = [ + helpers: t.List[ + t.Callable[[EntityFiles | None, pathlib.Path], CommandList | None] + ] = [ cls._copy_files, cls._symlink_files, lambda files, path: cls._write_tagged_files( - files, job.entity.file_parameters, path + files, entity.file_parameters, path ), ] for method in helpers: - return_cmd_list = method(job.entity.files, job_path) + return_cmd_list = method(entity.files, job_path) if return_cmd_list: cmd_list.commands.extend(return_cmd_list.commands) From 79c7b658e5f79e7cac6dfaefe9aa90afa3f11417 Mon Sep 17 00:00:00 2001 From: Amanda Richardson Date: Wed, 25 Sep 2024 15:53:49 -0500 Subject: [PATCH 10/13] psuhign updates --- smartsim/_core/generation/generator.py | 67 ++++++++++++++------------ tests/test_generator.py | 24 +++++---- 2 files changed, 51 insertions(+), 40 deletions(-) diff --git a/smartsim/_core/generation/generator.py b/smartsim/_core/generation/generator.py index b5d426715b..0d7c918999 100644 --- a/smartsim/_core/generation/generator.py +++ b/smartsim/_core/generation/generator.py @@ -28,6 +28,7 @@ import os import pathlib import pickle +import time import subprocess import sys import typing as t @@ -52,37 +53,37 @@ class _GenerableProtocol(t.Protocol): Job_Path = namedtuple("Job_Path", ["run_path", "out_path", "err_path"]) -"""Paths related to the Jobs execution.""" +"""Paths related to the Job's execution.""" class Generator: """The primary responsibility of the Generator class is to create the directory structure for a SmartSim Job and to build and execute file operation commands.""" - run = "run" + run_directory = "run" """The name of the directory where run-related files are stored.""" - log = "log" + log_directory = "log" """The name of the directory where log files are stored.""" def __init__(self, root: pathlib.Path) -> None: """Initialize a Generator object - The Generator class constructs a Jobs directory structure, including: + The Generator class constructs a Job's directory structure, including: - The run and log directories - Output and error files - The "smartsim_params.txt" settings file Additionally, it manages symlinking, copying, and configuring files associated - with a Jobs entity. + with a Job's entity. - :param root: The Jobs base path + :param root: The Job's base path """ self.root = root """The root path under which to generate files""" def _build_job_base_path(self, job: Job, job_index: int) -> pathlib.Path: - """Build and return a Jobs base directory. The path is created by combining the + """Build and return a Job's base directory. The path is created by combining the root directory with the Job type (derived from the class name), the name attribute of the Job, and an index to differentiate between multiple Job runs. @@ -96,7 +97,7 @@ def _build_job_base_path(self, job: Job, job_index: int) -> pathlib.Path: return pathlib.Path(job_path) def _build_job_run_path(self, job: Job, job_index: int) -> pathlib.Path: - """Build and return a Jobs run directory. The path is formed by combining + """Build and return a Job's run directory. The path is formed by combining the base directory with the `run` class-level variable, where run specifies the name of the job's run folder. @@ -104,12 +105,11 @@ def _build_job_run_path(self, job: Job, job_index: int) -> pathlib.Path: :param job_index: The index of the Job :returns: The built file path for the Job run folder """ - path = self._build_job_base_path(job, job_index) / self.run - path.mkdir(exist_ok=False, parents=True) + path = self._build_job_base_path(job, job_index) / self.run_directory return pathlib.Path(path) def _build_job_log_path(self, job: Job, job_index: int) -> pathlib.Path: - """Build and return a Jobs log directory. The path is formed by combining + """Build and return a Job's log directory. The path is formed by combining the base directory with the `log` class-level variable, where log specifies the name of the job's log folder. @@ -117,8 +117,7 @@ def _build_job_log_path(self, job: Job, job_index: int) -> pathlib.Path: :param job_index: The index of the Job :returns: The built file path for the Job run folder """ - path = self._build_job_base_path(job, job_index) / self.log - path.mkdir(exist_ok=False, parents=True) + path = self._build_job_base_path(job, job_index) / self.log_directory return pathlib.Path(path) @staticmethod @@ -134,7 +133,7 @@ def _build_log_file_path(log_path: pathlib.Path) -> pathlib.Path: @staticmethod def _build_out_file_path(log_path: pathlib.Path, job_name: str) -> pathlib.Path: """Build and return the path to the output file. The path is created by combining - the Jobs log directory with the job name and appending the `.out` extension. + the Job's log directory with the job name and appending the `.out` extension. :param log_path: Path to log directory :param job_name: Name of the Job @@ -146,7 +145,7 @@ def _build_out_file_path(log_path: pathlib.Path, job_name: str) -> pathlib.Path: @staticmethod def _build_err_file_path(log_path: pathlib.Path, job_name: str) -> pathlib.Path: """Build and return the path to the error file. The path is created by combining - the Jobs log directory with the job name and appending the `.err` extension. + the Job's log directory with the job name and appending the `.err` extension. :param log_path: Path to log directory :param job_name: Name of the Job @@ -172,39 +171,41 @@ def generate_job(self, job: Job, job_index: int) -> Job_Path: job_path = self._build_job_run_path(job, job_index) log_path = self._build_job_log_path(job, job_index) + out_file = self._build_out_file_path(log_path, job.entity.name) + err_file = self._build_err_file_path(log_path, job.entity.name) + + cmd_list = self._build_commands(job, job_path, log_path) + + self._execute_commands(cmd_list) + with open( self._build_log_file_path(log_path), mode="w", encoding="utf-8" ) as log_file: dt_string = datetime.now().strftime("%d/%m/%Y %H:%M:%S") log_file.write(f"Generation start date and time: {dt_string}\n") - out_file = self._build_out_file_path(log_path, job.entity.name) - err_file = self._build_err_file_path(log_path, job.entity.name) - - cmd_list = self._build_commands(job, job_path) - if cmd_list: - self._execute_commands(cmd_list) - return Job_Path(job_path, out_file, err_file) @classmethod def _build_commands( - cls, job: Job, job_path: pathlib.Path - ) -> t.Optional[CommandList]: - """Build file operation commands for all files attached to a Job's entity. + cls, job: Job, job_path: pathlib.Path, log_path: pathlib.Path + ) -> CommandList: + """Build file operation commands for a Job's entity. This method constructs commands for copying, symlinking, and writing tagged files - associated with the Job's entity. It aggregates these commands into a CommandList + associated with the Job's entity. This method builds the constructs the commands to + generate the Job's run and log directory. It aggregates these commands into a CommandList to return. :param job: The job object :param job_path: The file path for the Job run folder - :return: A CommandList containing the file operation commands, or None if the entity - does not support file operations. + :return: A CommandList containing the file operation commands """ + cmd_list = CommandList() + cmd_list.commands.append(cls._mkdir_file(job_path)) + cmd_list.commands.append(cls._mkdir_file(log_path)) entity = job.entity if isinstance(entity, _GenerableProtocol): - cmd_list = CommandList() helpers: t.List[ t.Callable[[EntityFiles | None, pathlib.Path], CommandList | None] ] = [ @@ -220,8 +221,7 @@ def _build_commands( if return_cmd_list: cmd_list.commands.extend(return_cmd_list.commands) - return cmd_list - return None + return cmd_list @classmethod def _execute_commands(cls, cmd_list: CommandList) -> None: @@ -234,6 +234,11 @@ def _execute_commands(cls, cmd_list: CommandList) -> None: """ for cmd in cmd_list: subprocess.run(cmd.command) + + @staticmethod + def _mkdir_file(file_path: pathlib.Path) -> Command: + cmd = Command(["mkdir", "-p", str(file_path)]) + return cmd @staticmethod def _copy_files( diff --git a/tests/test_generator.py b/tests/test_generator.py index b6ba399059..3396ddb437 100644 --- a/tests/test_generator.py +++ b/tests/test_generator.py @@ -205,7 +205,7 @@ def test_generate_job( """Test Generator.generate_job returns correct paths""" mock_index = 1 job_paths = generator_instance.generate_job(mock_job, mock_index) - assert job_paths.run_path.name == Generator.run + assert job_paths.run_path.name == Generator.run_directory assert job_paths.out_path.name == f"{mock_job.entity.name}.out" assert job_paths.err_path.name == f"{mock_job.entity.name}.err" @@ -225,7 +225,7 @@ def test_build_commands( "smartsim._core.generation.Generator._write_tagged_files" ) as mock_write_tagged_files, ): - generator_instance._build_commands(mock_job, pathlib.Path(test_dir)) + generator_instance._build_commands(mock_job, pathlib.Path(test_dir) / generator_instance.run_directory, pathlib.Path(test_dir) / generator_instance.log_directory) mock_copy_files.assert_called_once() mock_symlink_files.assert_called_once() mock_write_tagged_files.assert_called_once() @@ -242,6 +242,12 @@ def test_execute_commands(generator_instance: Generator): generator_instance._execute_commands(cmd_list) run_process.assert_called_once() +def test_mkdir_file(generator_instance: Generator, test_dir: str): + """Test Generator._mkdir_file returns correct type and value""" + cmd = generator_instance._mkdir_file(pathlib.Path(test_dir)) + assert isinstance(cmd, Command) + assert cmd.command == ["mkdir", "-p", test_dir] + def test_copy_file(generator_instance: Generator, fileutils): """Test Generator._copy_files helper function with file""" @@ -353,9 +359,9 @@ def test_generate_ensemble_directory_start( jobs_dir_path = pathlib.Path(test_dir) / run_dir[0] / "jobs" list_of_job_dirs = jobs_dir_path.iterdir() for job in list_of_job_dirs: - run_path = jobs_dir_path / job / Generator.run + run_path = jobs_dir_path / job / Generator.run_directory assert run_path.is_dir() - log_path = jobs_dir_path / job / Generator.log + log_path = jobs_dir_path / job / Generator.log_directory assert log_path.is_dir() ids.clear() @@ -378,7 +384,7 @@ def test_generate_ensemble_copy( jobs_dir = pathlib.Path(test_dir) / run_dir[0] / "jobs" job_dir = jobs_dir.iterdir() for ensemble_dir in job_dir: - copy_folder_path = jobs_dir / ensemble_dir / Generator.run / "to_copy_dir" + copy_folder_path = jobs_dir / ensemble_dir / Generator.run_directory / "to_copy_dir" assert copy_folder_path.is_dir() ids.clear() @@ -449,8 +455,8 @@ def _check_generated(param_0, param_1, dir): line = f.readline() assert line.strip() == f'echo "Hello with parameter 1 = {param_1}"' - _check_generated(0, 3, jobs_dir / "ensemble-name-1-1" / Generator.run) - _check_generated(1, 2, jobs_dir / "ensemble-name-2-2" / Generator.run) - _check_generated(1, 3, jobs_dir / "ensemble-name-3-3" / Generator.run) - _check_generated(0, 2, jobs_dir / "ensemble-name-0-0" / Generator.run) + _check_generated(0, 3, jobs_dir / "ensemble-name-1-1" / Generator.run_directory) + _check_generated(1, 2, jobs_dir / "ensemble-name-2-2" / Generator.run_directory) + _check_generated(1, 3, jobs_dir / "ensemble-name-3-3" / Generator.run_directory) + _check_generated(0, 2, jobs_dir / "ensemble-name-0-0" / Generator.run_directory) ids.clear() From 959df600f97ff3616d5275459775579f3410b07d Mon Sep 17 00:00:00 2001 From: Amanda Richardson Date: Wed, 25 Sep 2024 16:08:28 -0500 Subject: [PATCH 11/13] styling --- smartsim/_core/generation/generator.py | 8 ++++---- smartsim/experiment.py | 3 ++- tests/test_generator.py | 18 +++++++++++++----- 3 files changed, 19 insertions(+), 10 deletions(-) diff --git a/smartsim/_core/generation/generator.py b/smartsim/_core/generation/generator.py index 0d7c918999..8107a83d15 100644 --- a/smartsim/_core/generation/generator.py +++ b/smartsim/_core/generation/generator.py @@ -28,9 +28,9 @@ import os import pathlib import pickle -import time import subprocess import sys +import time import typing as t from collections import namedtuple from datetime import datetime @@ -175,9 +175,9 @@ def generate_job(self, job: Job, job_index: int) -> Job_Path: err_file = self._build_err_file_path(log_path, job.entity.name) cmd_list = self._build_commands(job, job_path, log_path) - + self._execute_commands(cmd_list) - + with open( self._build_log_file_path(log_path), mode="w", encoding="utf-8" ) as log_file: @@ -234,7 +234,7 @@ def _execute_commands(cls, cmd_list: CommandList) -> None: """ for cmd in cmd_list: subprocess.run(cmd.command) - + @staticmethod def _mkdir_file(file_path: pathlib.Path) -> Command: cmd = Command(["mkdir", "-p", str(file_path)]) diff --git a/smartsim/experiment.py b/smartsim/experiment.py index 8e85fec3f2..77ad021def 100644 --- a/smartsim/experiment.py +++ b/smartsim/experiment.py @@ -38,12 +38,13 @@ from smartsim._core import dispatch from smartsim._core.config import CONFIG from smartsim._core.control import interval as _interval +from smartsim._core.control import preview_renderer from smartsim._core.control.launch_history import LaunchHistory as _LaunchHistory from smartsim._core.utils import helpers as _helpers from smartsim.error import errors from smartsim.status import TERMINAL_STATUSES, InvalidJobStatus, JobStatus -from ._core import Generator, Manifest, previewrenderer +from ._core import Generator, Manifest from ._core.generation.generator import Job_Path from .entity import TelemetryConfiguration from .error import SmartSimError diff --git a/tests/test_generator.py b/tests/test_generator.py index 8a673b424f..4c25ccd05f 100644 --- a/tests/test_generator.py +++ b/tests/test_generator.py @@ -38,7 +38,7 @@ from smartsim._core.commands import Command, CommandList from smartsim._core.generation.generator import Generator from smartsim.builders import Ensemble -from smartsim.entity import Application +from smartsim.entity import entity from smartsim.entity.files import EntityFiles from smartsim.launchable import Job from smartsim.settings import LaunchSettings @@ -47,7 +47,7 @@ pytestmark = pytest.mark.group_a -_ID_GENERATOR = (str(i) for i in itertools.count()) +ids = set() _ID_GENERATOR = (str(i) for i in itertools.count()) @@ -92,7 +92,7 @@ def __init__(self): self.files = None self.file_parameters = None - def as_program_arguments(self): + def as_executable_sequence(self): return ("echo", "Hello", "World!") def files(): @@ -226,7 +226,11 @@ def test_build_commands( "smartsim._core.generation.Generator._write_tagged_files" ) as mock_write_tagged_files, ): - generator_instance._build_commands(mock_job, pathlib.Path(test_dir) / generator_instance.run_directory, pathlib.Path(test_dir) / generator_instance.log_directory) + generator_instance._build_commands( + mock_job, + pathlib.Path(test_dir) / generator_instance.run_directory, + pathlib.Path(test_dir) / generator_instance.log_directory, + ) mock_copy_files.assert_called_once() mock_symlink_files.assert_called_once() mock_write_tagged_files.assert_called_once() @@ -243,6 +247,7 @@ def test_execute_commands(generator_instance: Generator): generator_instance._execute_commands(cmd_list) run_process.assert_called_once() + def test_mkdir_file(generator_instance: Generator, test_dir: str): """Test Generator._mkdir_file returns correct type and value""" cmd = generator_instance._mkdir_file(pathlib.Path(test_dir)) @@ -385,7 +390,9 @@ def test_generate_ensemble_copy( jobs_dir = pathlib.Path(test_dir) / run_dir[0] / "jobs" job_dir = jobs_dir.iterdir() for ensemble_dir in job_dir: - copy_folder_path = jobs_dir / ensemble_dir / Generator.run_directory / "to_copy_dir" + copy_folder_path = ( + jobs_dir / ensemble_dir / Generator.run_directory / "to_copy_dir" + ) assert copy_folder_path.is_dir() ids.clear() @@ -415,6 +422,7 @@ def test_generate_ensemble_symlink( assert sym_file_path.is_dir() assert sym_file_path.is_symlink() assert os.fspath(sym_file_path.resolve()) == osp.realpath(get_gen_symlink_dir) + ids.clear() def test_generate_ensemble_configure( From 82322157447daa1c5e4daa159f0887be9ae520a3 Mon Sep 17 00:00:00 2001 From: Amanda Richardson Date: Thu, 26 Sep 2024 10:41:31 -0500 Subject: [PATCH 12/13] mypy fixes --- smartsim/_core/generation/generator.py | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/smartsim/_core/generation/generator.py b/smartsim/_core/generation/generator.py index 8107a83d15..f671d36c7c 100644 --- a/smartsim/_core/generation/generator.py +++ b/smartsim/_core/generation/generator.py @@ -77,7 +77,7 @@ def __init__(self, root: pathlib.Path) -> None: Additionally, it manages symlinking, copying, and configuring files associated with a Job's entity. - :param root: The Job's base path + :param root: Job base path """ self.root = root """The root path under which to generate files""" @@ -88,8 +88,8 @@ def _build_job_base_path(self, job: Job, job_index: int) -> pathlib.Path: the name attribute of the Job, and an index to differentiate between multiple Job runs. - :param job: The job object - :param job_index: The index of the Job + :param job: Job object + :param job_index: Job index :returns: The built file path for the Job """ job_type = f"{job.__class__.__name__.lower()}s" @@ -101,8 +101,8 @@ def _build_job_run_path(self, job: Job, job_index: int) -> pathlib.Path: the base directory with the `run` class-level variable, where run specifies the name of the job's run folder. - :param job: The job object - :param job_index: The index of the Job + :param job: Job object + :param job_index: Job index :returns: The built file path for the Job run folder """ path = self._build_job_base_path(job, job_index) / self.run_directory @@ -113,8 +113,8 @@ def _build_job_log_path(self, job: Job, job_index: int) -> pathlib.Path: the base directory with the `log` class-level variable, where log specifies the name of the job's log folder. - :param job: The job object - :param job_index: The index of the Job + :param job: Job object + :param job_index: Job index :returns: The built file path for the Job run folder """ path = self._build_job_base_path(job, job_index) / self.log_directory @@ -163,8 +163,8 @@ def generate_job(self, job: Job, job_index: int) -> Job_Path: attached to the Job's entity, it builds file operation commands and executes them. - :param job: The job object - :param job_index: The index of the Job + :param job: Job object + :param job_index: Job index :return: Job's run directory, error file and out file. """ @@ -197,7 +197,7 @@ def _build_commands( generate the Job's run and log directory. It aggregates these commands into a CommandList to return. - :param job: The job object + :param job: Job object :param job_path: The file path for the Job run folder :return: A CommandList containing the file operation commands """ @@ -207,7 +207,10 @@ def _build_commands( entity = job.entity if isinstance(entity, _GenerableProtocol): helpers: t.List[ - t.Callable[[EntityFiles | None, pathlib.Path], CommandList | None] + t.Callable[ + [t.Union[EntityFiles, None], pathlib.Path], + t.Union[CommandList, None], + ] ] = [ cls._copy_files, cls._symlink_files, From dfbb53ff7278e7a33bf11146d05e891fd28cbdb6 Mon Sep 17 00:00:00 2001 From: Amanda Richardson Date: Fri, 27 Sep 2024 13:49:38 -0500 Subject: [PATCH 13/13] remove old generator code --- smartsim/_core/generation/generator.py | 47 -------------------------- 1 file changed, 47 deletions(-) diff --git a/smartsim/_core/generation/generator.py b/smartsim/_core/generation/generator.py index f671d36c7c..6d31fe2ce8 100644 --- a/smartsim/_core/generation/generator.py +++ b/smartsim/_core/generation/generator.py @@ -356,50 +356,3 @@ def _write_tagged_files( ) cmd_list.commands.append(cmd) return cmd_list - - # TODO address in ticket 723 - # self._log_params(entity, files_to_params) - - # TODO to be refactored in ticket 723 - # def _log_params( - # self, entity: Application, 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 application being generated - # :param files_to_params: a dict connecting each file to its parameter settings - # """ - # 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(params.items(), headers=["Name", "Value"]) - # 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 application {entity.name} with params {used_params_str}", - # ) - # file_table = tabulate( - # file_to_tables.items(), - # headers=["File name", "Parameters"], - # ) - # log_entry = f"Application name: {entity.name}\n{file_table}\n\n" - # with open(self.log_file, mode="a", encoding="utf-8") as logfile: - # logfile.write(log_entry) - # with open( - # join(entity.path, "smartsim_params.txt"), mode="w", encoding="utf-8" - # ) as local_logfile: - # local_logfile.write(log_entry) - - # else: - # logger.log( - # level=self.log_level, - # msg=f"Configured application {entity.name} with no parameters", - # )