From 2eaa9f331e3263d25c1dd78d5aac117316370a6d Mon Sep 17 00:00:00 2001 From: Douglas Jacobsen Date: Mon, 14 Oct 2024 14:08:55 -0600 Subject: [PATCH 1/4] Update concretize to be additive This commit updates the `workspace concretize` action to allow additive / subsequent calls to concretize new experiments that have been added to the workspace since the last concretization. This can also be used to add new package definitions related to modifiers that were added to the workspace. --- lib/ramble/ramble/cmd/workspace.py | 13 +++-- lib/ramble/ramble/workspace/workspace.py | 61 +++++++++++------------- 2 files changed, 37 insertions(+), 37 deletions(-) diff --git a/lib/ramble/ramble/cmd/workspace.py b/lib/ramble/ramble/cmd/workspace.py index 83ccd6610..3bb17d4a4 100644 --- a/lib/ramble/ramble/cmd/workspace.py +++ b/lib/ramble/ramble/cmd/workspace.py @@ -414,6 +414,14 @@ def workspace_concretize_setup_parser(subparser): help="Remove unused software and experiment templates from workspace config", required=False, ) + subparser.add_argument( + "--quiet", + "-q", + dest="quiet", + action="store_true", + help="Silently ignore conflicting package definitions", + required=False, + ) def workspace_concretize(args): @@ -423,11 +431,8 @@ def workspace_concretize(args): logger.debug("Simplifying workspace config") ws.simplify() else: - if args.force_concretize: - ws.force_concretize = True - logger.debug("Concretizing workspace") - ws.concretize() + ws.concretize(force=args.force_concretize, quiet=args.quiet) def workspace_run_pipeline(args, pipeline): diff --git a/lib/ramble/ramble/workspace/workspace.py b/lib/ramble/ramble/workspace/workspace.py index 7abd8067b..9246a49b5 100644 --- a/lib/ramble/ramble/workspace/workspace.py +++ b/lib/ramble/ramble/workspace/workspace.py @@ -469,7 +469,6 @@ def __init__(self, root, dry_run=False, read_default_template=True): self.txlock = lk.Lock(self._transaction_lock_path) self.dry_run = dry_run self.repeat_success_strict = True - self.force_concretize = False self.read_default_template = read_default_template self.configs = ramble.config.ConfigScope("workspace", self.config_dir) @@ -1106,25 +1105,19 @@ def yaml_add_comment_before_key( workspace_dict[namespace.ramble][namespace.application] = apps_dict print(f"\n{syaml.dump(workspace_dict)}") - def concretize(self): - full_software_dict = self.get_software_dict() + def concretize(self, force=False, quiet=False): + """Concretize software definitions for defined experiments - if not self.force_concretize: - try: - if ( - full_software_dict[namespace.packages] - or full_software_dict[namespace.environments] - ): - raise RambleWorkspaceError( - "Cannot concretize an already concretized " - "workspace. To overwrite the current configuration " - "with the default software configuration, use " - "'ramble workspace concretize -f'." - ) - except KeyError: - pass + Extract suggested software for experiments defined in a workspace, and + ensure the software environments are defined properly. + + Args: + force (bool): Whether to overwrite conflicting definitions of named packages or not + quiet (bool): Whether to silently ignore conflicts or not - full_software_dict = syaml.syaml_dict() + + """ + full_software_dict = self.get_software_dict() if ( namespace.packages not in full_software_dict @@ -1159,7 +1152,7 @@ def concretize(self): for compiler_dict in compiler_dicts: for comp, info in compiler_dict.items(): if fnmatch.fnmatch(app_inst.package_manager.name, info["package_manager"]): - if comp not in packages_dict: + if comp not in packages_dict or force: packages_dict[comp] = syaml.syaml_dict() packages_dict[comp]["pkg_spec"] = info["pkg_spec"] ramble.config.add( @@ -1183,20 +1176,19 @@ def concretize(self): ramble.config.add( config_path, scope=self.ws_file_config_scope_name() ) - elif not specs_equiv(info, packages_dict[comp]): + elif not quiet and not specs_equiv(info, packages_dict[comp]): logger.debug(f" Spec 1: {str(info)}") logger.debug(f" Spec 2: {str(packages_dict[comp])}") raise RambleConflictingDefinitionError( - f"Compiler {comp} defined in multiple conflicting ways" + f"Compiler {comp} would be defined defined " + "in multiple conflicting ways" ) - add_env = False - if env_name not in environments_dict: - new_env = syaml.syaml_dict() - new_env[namespace.packages] = [] - logger.debug(f"Trying to define packages for {env_name}") - app_packages = new_env[namespace.packages] + app_packages = [] + if env_name in environments_dict: + if namespace.packages in environments_dict[env_name]: + app_packages = environments_dict[env_name][namespace.packages].copy() software_dicts = [app_inst.software_specs] for mod_inst in app_inst._modifier_instances: @@ -1206,8 +1198,7 @@ def concretize(self): for spec_name, info in software_dict.items(): if fnmatch.fnmatch(app_inst.package_manager.name, info["package_manager"]): logger.debug(f" Found spec: {spec_name}") - if spec_name not in packages_dict: - add_env = True + if spec_name not in packages_dict or force: packages_dict[spec_name] = syaml.syaml_dict() packages_dict[spec_name]["pkg_spec"] = info["pkg_spec"] if "compiler_spec" in info and info["compiler_spec"]: @@ -1215,18 +1206,22 @@ def concretize(self): if "compiler" in info and info["compiler"]: packages_dict[spec_name]["compiler"] = info["compiler"] - elif not specs_equiv(info, packages_dict[spec_name]): + elif not quiet and not specs_equiv(info, packages_dict[spec_name]): logger.debug(f" Spec 1: {str(info)}") logger.debug(f" Spec 2: {str(packages_dict[spec_name])}") raise RambleConflictingDefinitionError( - f"Package {spec_name} defined in multiple conflicting ways" + f"Package {spec_name} would be defined in multiple " + "conflicting ways" ) if spec_name not in app_packages: app_packages.append(spec_name) - if add_env: - environments_dict[env_name] = new_env.copy() + if app_packages: + if env_name not in environments_dict: + environments_dict[env_name] = syaml.syaml_dict() + + environments_dict[env_name][namespace.packages] = app_packages.copy() ramble.config.config.update_config( "software", full_software_dict, scope=self.ws_file_config_scope_name() From a193d2d3b9ef15b7edfa0f7023ebdf44c1b84eba Mon Sep 17 00:00:00 2001 From: Douglas Jacobsen Date: Mon, 14 Oct 2024 15:27:10 -0600 Subject: [PATCH 2/4] Add tests for additive concretization --- lib/ramble/ramble/test/cmd/workspace.py | 13 ++-- .../ramble/test/cmd/workspace_concretize.py | 64 +++++++++++++++++++ 2 files changed, 71 insertions(+), 6 deletions(-) create mode 100644 lib/ramble/ramble/test/cmd/workspace_concretize.py diff --git a/lib/ramble/ramble/test/cmd/workspace.py b/lib/ramble/ramble/test/cmd/workspace.py index b5aba306f..7459a2cc0 100644 --- a/lib/ramble/ramble/test/cmd/workspace.py +++ b/lib/ramble/ramble/test/cmd/workspace.py @@ -598,7 +598,7 @@ def test_concretize_concrete_config(): software: packages: zlib: - pkg_spec: 'zlib' + pkg_spec: 'zlib@1.3' environments: zlib: packages: @@ -618,7 +618,7 @@ def test_concretize_concrete_config(): with pytest.raises(ramble.workspace.RambleWorkspaceError) as e: workspace("concretize", global_args=["-w", workspace_name]) - assert "Cannot concretize an already concretized workspace." in e + assert "Package zlib would be defined in multiple conflicting ways" in e def test_force_concretize(): @@ -672,12 +672,13 @@ def test_force_concretize(): ws1._re_read() - with pytest.raises(ramble.workspace.RambleWorkspaceError) as e: - workspace("concretize", global_args=["-w", workspace_name]) - assert "Cannot concretize an already concretized workspace." in e - workspace("concretize", "-f", global_args=["-w", workspace_name]) + assert search_files_for_string([config_path], "zlib:") is True + assert search_files_for_string([config_path], "zlib-test") is True + + workspace("concretize", "--simplify", global_args=["-w", workspace_name]) + assert search_files_for_string([config_path], "zlib:") is True assert search_files_for_string([config_path], "zlib-test") is False diff --git a/lib/ramble/ramble/test/cmd/workspace_concretize.py b/lib/ramble/ramble/test/cmd/workspace_concretize.py new file mode 100644 index 000000000..19a3b5091 --- /dev/null +++ b/lib/ramble/ramble/test/cmd/workspace_concretize.py @@ -0,0 +1,64 @@ +# Copyright 2022-2024 The Ramble Authors +# +# Licensed under the Apache License, Version 2.0 or the MIT license +# , at your +# option. This file may not be copied, modified, or distributed +# except according to those terms. + +import os +import pytest + +import ramble.workspace +from ramble.main import RambleCommand + +# everything here uses the mock_workspace_path +pytestmark = pytest.mark.usefixtures("mutable_config", "mutable_mock_workspace_path") + +workspace = RambleCommand("workspace") + + +def test_workspace_concretize_additive(request): + workspace_name = request.node.name + + ws = ramble.workspace.create(workspace_name) + global_args = ["-w", workspace_name] + + workspace( + "generate-config", "gromacs", "-p", "spack", "--wf", "water_*", global_args=global_args + ) + workspace("concretize", "-q", global_args=global_args) + + with open(ws.config_file_path) as f: + content = f.read() + assert "spack_gromacs" in content + assert "gcc9" in content + assert "wrfv4" not in content + assert "intel-oneapi-vtune" not in content + + workspace("generate-config", "wrfv4", "-p", "spack", global_args=global_args) + workspace("concretize", "-q", global_args=global_args) + + with open(ws.config_file_path) as f: + content = f.read() + assert "spack_gromacs" in content + assert "gcc9" in content + assert "wrfv4" in content + assert "intel-oneapi-vtune" not in content + + modifiers_path = os.path.join(ws.config_dir, "modifiers.yaml") + + with open(modifiers_path, "w+") as f: + f.write( + """modifiers: +- name: intel-aps""" + ) + + workspace("concretize", "-q", global_args=global_args) + + with open(ws.config_file_path) as f: + content = f.read() + assert "spack_gromacs" in content + assert "gcc9" in content + assert "wrfv4" in content + assert "intel-oneapi-vtune" in content From b0173d9266eb0eaa90a60235998af150a51d9c4c Mon Sep 17 00:00:00 2001 From: Douglas Jacobsen Date: Mon, 14 Oct 2024 15:58:53 -0600 Subject: [PATCH 3/4] Update bash completion --- share/ramble/ramble-completion.bash | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/share/ramble/ramble-completion.bash b/share/ramble/ramble-completion.bash index 275b91b3a..508ad6306 100755 --- a/share/ramble/ramble-completion.bash +++ b/share/ramble/ramble-completion.bash @@ -669,7 +669,7 @@ _ramble_workspace_create() { } _ramble_workspace_concretize() { - RAMBLE_COMPREPLY="-h --help -f --force-concretize --simplify" + RAMBLE_COMPREPLY="-h --help -f --force-concretize --simplify --quiet -q" } _ramble_workspace_setup() { From 73e830c126b366e19e9fb4ec07a86b482f3c1375 Mon Sep 17 00:00:00 2001 From: Douglas Jacobsen Date: Wed, 16 Oct 2024 15:42:34 -0600 Subject: [PATCH 4/4] Address reviewer feedback --- lib/ramble/ramble/workspace/workspace.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/ramble/ramble/workspace/workspace.py b/lib/ramble/ramble/workspace/workspace.py index 9246a49b5..2bb0ed7df 100644 --- a/lib/ramble/ramble/workspace/workspace.py +++ b/lib/ramble/ramble/workspace/workspace.py @@ -1180,8 +1180,7 @@ def concretize(self, force=False, quiet=False): logger.debug(f" Spec 1: {str(info)}") logger.debug(f" Spec 2: {str(packages_dict[comp])}") raise RambleConflictingDefinitionError( - f"Compiler {comp} would be defined defined " - "in multiple conflicting ways" + f"Compiler {comp} would be defined " "in multiple conflicting ways" ) logger.debug(f"Trying to define packages for {env_name}")