From d7db386aef14f93ad94907cbdf15b3510b17b410 Mon Sep 17 00:00:00 2001 From: Lin Guo Date: Fri, 17 May 2024 18:46:06 -0700 Subject: [PATCH] Configure spack install dir based on `env_name` This helps with saving redundant spack concretization, when experiments under different workloads are actually using the same software environment. Some more contexts (courtesy of Bob and Doug): * Historically the spack env was rendered during workspace render time, that's why the existing namespacing. * That's changed in https://github.com/GoogleCloudPlatform/ramble/pull/433/. * And it's safe to assume same `env_name` always 1-1 maps to a spack env. There's a check to guarantee that: https://github.com/GoogleCloudPlatform/ramble/blob/f4e0e06c7f4ab0efd5d05000cb7ebec7b3ad33b2/lib/ramble/ramble/software_environments.py#L371-L374. Tested: tested with the following: * A new created workspace with gromacs ran fine. * (backward-compt) An existing workspace created from the current develop head ran fine after running `ramble workspace setup` under the current branch. --- lib/ramble/ramble/expander.py | 10 -- lib/ramble/ramble/experiment_set.py | 7 +- .../end_to_end/dryrun_copies_external_env.py | 2 +- .../test/end_to_end/experiment_excludes.py | 2 +- .../test/end_to_end/experiment_repeats.py | 2 +- .../ramble/test/end_to_end/explicit_zips.py | 2 +- .../test/end_to_end/package_manager_config.py | 2 +- .../package_manager_requirements.py | 2 +- .../ramble/test/end_to_end/spack_env_cache.py | 119 ++++++++++++++++++ .../test/end_to_end/test_configvar_dry_run.py | 2 +- .../ramble/test/end_to_end/wrfv4_dry_run.py | 2 +- .../ramble/test/modifier_application.py | 2 +- .../mock_modifier_spack_configs.py | 2 +- .../mock_spack_modifier.py | 6 +- .../modifier_helpers.py | 16 +-- 15 files changed, 143 insertions(+), 35 deletions(-) create mode 100644 lib/ramble/ramble/test/end_to_end/spack_env_cache.py diff --git a/lib/ramble/ramble/expander.py b/lib/ramble/ramble/expander.py index 61cdcc1ca..c0508f02d 100644 --- a/lib/ramble/ramble/expander.py +++ b/lib/ramble/ramble/expander.py @@ -318,7 +318,6 @@ def __init__(self, variables, experiment_set, no_expand_vars=set()): self._application_namespace = None self._workload_namespace = None self._experiment_namespace = None - self._env_namespace = None self._env_path = None self._application_input_dir = None @@ -388,15 +387,6 @@ def experiment_namespace(self): return self._experiment_namespace - @property - def env_namespace(self): - if not self._env_namespace: - var = self.expansion_str(self._keywords.env_name) + \ - '.' + self.expansion_str(self._keywords.workload_name) - self._env_namespace = self.expand_var(var) - - return self._env_namespace - @property def env_path(self): if not self._env_path: diff --git a/lib/ramble/ramble/experiment_set.py b/lib/ramble/ramble/experiment_set.py index 18e6677df..3e692d050 100644 --- a/lib/ramble/ramble/experiment_set.py +++ b/lib/ramble/ramble/experiment_set.py @@ -262,10 +262,9 @@ def _prepare_experiment(self, exp_template_name, variables, context, repeats): Returns: (Application): Instance of an application class for this experiment """ - variables[self.keywords.env_path] = \ - os.path.join(self._workspace.software_dir, - Expander.expansion_str(self.keywords.env_name) + '.' + - Expander.expansion_str(self.keywords.workload_name)) + variables[self.keywords.env_path] = os.path.join( + self._workspace.software_dir, Expander.expansion_str(self.keywords.env_name) + ) experiment_suffix = '' # After generating the base experiment, append the index to repeat experiments diff --git a/lib/ramble/ramble/test/end_to_end/dryrun_copies_external_env.py b/lib/ramble/ramble/test/end_to_end/dryrun_copies_external_env.py index f0ff685da..19adea545 100644 --- a/lib/ramble/ramble/test/end_to_end/dryrun_copies_external_env.py +++ b/lib/ramble/ramble/test/end_to_end/dryrun_copies_external_env.py @@ -77,7 +77,7 @@ def test_dryrun_copies_external_env(mutable_config, mutable_mock_workspace_path, setup_pipeline = setup_cls(ws, filters) setup_pipeline.run() - env_file = os.path.join(ws.software_dir, 'wrfv4.CONUS_12km', 'spack.yaml') + env_file = os.path.join(ws.software_dir, 'wrfv4', 'spack.yaml') assert os.path.exists(env_file) diff --git a/lib/ramble/ramble/test/end_to_end/experiment_excludes.py b/lib/ramble/ramble/test/end_to_end/experiment_excludes.py index aa4dd44cf..428cdccd3 100644 --- a/lib/ramble/ramble/test/end_to_end/experiment_excludes.py +++ b/lib/ramble/ramble/test/end_to_end/experiment_excludes.py @@ -153,7 +153,7 @@ def test_wrfv4_exclusions(mutable_config, mutable_mock_workspace_path): out_files, 'Would download https://www2.mmm.ucar.edu/wrf/users/benchmark/v422/v42_bench_conus12km.tar.gz') # noqa # Test software directories - software_dirs = ['wrfv4.CONUS_12km', 'wrfv4-portable.CONUS_12km'] + software_dirs = ['wrfv4', 'wrfv4-portable'] software_base_dir = os.path.join(ws1.root, ramble.workspace.workspace_software_path) assert os.path.exists(software_base_dir) for software_dir in software_dirs: diff --git a/lib/ramble/ramble/test/end_to_end/experiment_repeats.py b/lib/ramble/ramble/test/end_to_end/experiment_repeats.py index ab1efd98b..df769b09c 100644 --- a/lib/ramble/ramble/test/end_to_end/experiment_repeats.py +++ b/lib/ramble/ramble/test/end_to_end/experiment_repeats.py @@ -109,7 +109,7 @@ def test_gromacs_repeats(mutable_config, mutable_mock_workspace_path): out_files, 'Would download https://ftp.gromacs.org/pub/benchmarks/water_GMX50_bare.tar.gz') # noqa # Test software directories - software_dirs = ['gromacs.water_gmx50', 'gromacs.water_bare'] + software_dirs = ['gromacs'] software_base_dir = os.path.join(ws1.root, ramble.workspace.workspace_software_path) assert os.path.exists(software_base_dir) for software_dir in software_dirs: diff --git a/lib/ramble/ramble/test/end_to_end/explicit_zips.py b/lib/ramble/ramble/test/end_to_end/explicit_zips.py index dee33eb4c..92bf0f67d 100644 --- a/lib/ramble/ramble/test/end_to_end/explicit_zips.py +++ b/lib/ramble/ramble/test/end_to_end/explicit_zips.py @@ -147,7 +147,7 @@ def test_wrfv4_explicit_zips(mutable_config, mutable_mock_workspace_path): out_files, 'Would download https://www2.mmm.ucar.edu/wrf/users/benchmark/v422/v42_bench_conus12km.tar.gz') # noqa # Test software directories - software_dirs = ['wrfv4.CONUS_12km', 'wrfv4-portable.CONUS_12km'] + software_dirs = ['wrfv4', 'wrfv4-portable'] software_base_dir = os.path.join(ws1.root, ramble.workspace.workspace_software_path) assert os.path.exists(software_base_dir) for software_dir in software_dirs: diff --git a/lib/ramble/ramble/test/end_to_end/package_manager_config.py b/lib/ramble/ramble/test/end_to_end/package_manager_config.py index e9389859b..a7ed0c910 100644 --- a/lib/ramble/ramble/test/end_to_end/package_manager_config.py +++ b/lib/ramble/ramble/test/end_to_end/package_manager_config.py @@ -59,7 +59,7 @@ def test_package_manager_config_zlib(mock_applications): workspace('setup', '--dry-run', global_args=['-w', workspace_name]) - spack_yaml = os.path.join(ws.software_dir, 'zlib-configs.ensure_installed', + spack_yaml = os.path.join(ws.software_dir, 'zlib-configs', 'spack.yaml') assert os.path.isfile(spack_yaml) diff --git a/lib/ramble/ramble/test/end_to_end/package_manager_requirements.py b/lib/ramble/ramble/test/end_to_end/package_manager_requirements.py index c5ca813e8..2eb62e24a 100644 --- a/lib/ramble/ramble/test/end_to_end/package_manager_requirements.py +++ b/lib/ramble/ramble/test/end_to_end/package_manager_requirements.py @@ -64,7 +64,7 @@ def test_package_manager_requirements_zlib(mock_applications, mock_modifiers): workspace('setup', global_args=['-w', workspace_name]) - spack_yaml = os.path.join(ws.software_dir, 'zlib-configs.ensure_installed', + spack_yaml = os.path.join(ws.software_dir, 'zlib-configs', 'spack.yaml') assert os.path.isfile(spack_yaml) diff --git a/lib/ramble/ramble/test/end_to_end/spack_env_cache.py b/lib/ramble/ramble/test/end_to_end/spack_env_cache.py new file mode 100644 index 000000000..7131cbf0d --- /dev/null +++ b/lib/ramble/ramble/test/end_to_end/spack_env_cache.py @@ -0,0 +1,119 @@ +# 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 +import ramble.config +import ramble.software_environments +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_spack_env_cache(): + test_config = """ +ramble: + variables: + mpi_command: 'mpirun -n {n_ranks} -ppn {processes_per_node}' + batch_submit: '{execute_experiment}' + processes_per_node: '1' + applications: + gromacs: + workloads: + water_bare: + experiments: + test1: + variables: + n_nodes: '1' + test2: + variables: + n_nodes: '2' + env_name: 'g2' + test3: + variables: + n_nodes: '3' + water_gmx50: + experiments: + test4: + variables: + n_nodes: '1' + spack: + packages: + intel-mpi: + spack_spec: intel-oneapi-mpi@2021.11.0 + gromacs: + spack_spec: gromacs + environments: + gromacs: + packages: + - gromacs + - intel-mpi + g2: + packages: + - gromacs + - intel-mpi +""" + workspace_name = 'test-spack-env-cache' + ws = ramble.workspace.create(workspace_name) + ws.write() + + config_path = os.path.join(ws.config_dir, ramble.workspace.config_file_name) + + with open(config_path, 'w+') as f: + f.write(test_config) + + ws._re_read() + + workspace( + 'setup', + '--dry-run', + global_args=['-w', workspace_name], + ) + + # spack env should be present only at the env_name level. + assert os.path.exists(os.path.join(ws.software_dir, 'gromacs')) + assert os.path.exists(os.path.join(ws.software_dir, 'g2')) + assert not os.path.exists(os.path.join(ws.software_dir, 'g2.water_bare')) + + # First encounter of an env_name (test1 -> gromacs, test2 -> g2) requires spack usage. + test1_log = os.path.join(ws.log_dir, 'setup.latest', 'gromacs.water_bare.test1.out') + with open(test1_log, 'r') as f: + content = f.read() + assert 'spack install' in content + assert 'spack concretize' in content + + test2_log = os.path.join(ws.log_dir, 'setup.latest', 'gromacs.water_bare.test2.out') + with open(test2_log, 'r') as f: + content = f.read() + assert 'spack install' in content + assert 'spack concretize' in content + + # Envs should already exist and can skip spack calls. + test3_log = os.path.join(ws.log_dir, 'setup.latest', 'gromacs.water_bare.test3.out') + with open(test3_log, 'r') as f: + content = f.read() + assert 'spack install' not in content + assert 'spack concretize' not in content + + test4_log = os.path.join( + ws.log_dir, 'setup.latest', 'gromacs.water_gmx50.test4.out' + ) + with open(test4_log, 'r') as f: + content = f.read() + assert 'spack install' not in content + assert 'spack concretize' not in content diff --git a/lib/ramble/ramble/test/end_to_end/test_configvar_dry_run.py b/lib/ramble/ramble/test/end_to_end/test_configvar_dry_run.py index 57b919236..d05f647c3 100644 --- a/lib/ramble/ramble/test/end_to_end/test_configvar_dry_run.py +++ b/lib/ramble/ramble/test/end_to_end/test_configvar_dry_run.py @@ -91,7 +91,7 @@ def test_configvar_dry_run(mutable_config, mutable_mock_workspace_path): workspace('setup', '--dry-run', global_args=['-w', workspace_name]) - software_dir = 'openfoam.motorbike' + software_dir = 'openfoam' software_base_dir = os.path.join(ws.root, ramble.workspace.workspace_software_path) assert os.path.exists(software_base_dir) diff --git a/lib/ramble/ramble/test/end_to_end/wrfv4_dry_run.py b/lib/ramble/ramble/test/end_to_end/wrfv4_dry_run.py index 2ad18d632..734a98bee 100644 --- a/lib/ramble/ramble/test/end_to_end/wrfv4_dry_run.py +++ b/lib/ramble/ramble/test/end_to_end/wrfv4_dry_run.py @@ -136,7 +136,7 @@ def test_wrfv4_dry_run(mutable_config, mutable_mock_workspace_path): out_files, 'Would download https://www2.mmm.ucar.edu/wrf/users/benchmark/v422/v42_bench_conus12km.tar.gz') # noqa # Test software directories - software_dirs = ['wrfv4.CONUS_12km', 'wrfv4-portable.CONUS_12km'] + software_dirs = ['wrfv4', 'wrfv4-portable'] software_base_dir = os.path.join(ws1.root, ramble.workspace.workspace_software_path) assert os.path.exists(software_base_dir) for software_dir in software_dirs: diff --git a/lib/ramble/ramble/test/modifier_application.py b/lib/ramble/ramble/test/modifier_application.py index e9d3ba1f0..b45ed70e1 100644 --- a/lib/ramble/ramble/test/modifier_application.py +++ b/lib/ramble/ramble/test/modifier_application.py @@ -77,7 +77,7 @@ def test_wrfv4_aps_test(mutable_config, mutable_mock_workspace_path): workspace('setup', '--dry-run', global_args=['-w', workspace_name]) - software_path = os.path.join(ws1.software_dir, 'wrfv4.CONUS_12km', 'spack.yaml') + software_path = os.path.join(ws1.software_dir, 'wrfv4', 'spack.yaml') with open(software_path, 'r') as f: assert 'intel-oneapi-vtune' in f.read() diff --git a/lib/ramble/ramble/test/modifier_functionality/mock_modifier_spack_configs.py b/lib/ramble/ramble/test/modifier_functionality/mock_modifier_spack_configs.py index d504cbfaf..98e7c1a29 100644 --- a/lib/ramble/ramble/test/modifier_functionality/mock_modifier_spack_configs.py +++ b/lib/ramble/ramble/test/modifier_functionality/mock_modifier_spack_configs.py @@ -53,7 +53,7 @@ def test_gromacs_mock_spack_config_mod(mutable_mock_workspace_path, assert os.path.isfile(exp_script) - spack_yaml = os.path.join(ws1.software_dir, 'gromacs.water_bare', + spack_yaml = os.path.join(ws1.software_dir, 'gromacs', 'spack.yaml') assert os.path.isfile(spack_yaml) diff --git a/lib/ramble/ramble/test/modifier_functionality/mock_spack_modifier.py b/lib/ramble/ramble/test/modifier_functionality/mock_spack_modifier.py index 65c3e8b02..4a0c5e835 100644 --- a/lib/ramble/ramble/test/modifier_functionality/mock_spack_modifier.py +++ b/lib/ramble/ramble/test/modifier_functionality/mock_spack_modifier.py @@ -40,9 +40,9 @@ def test_gromacs_dry_run_mock_spack_mod(mutable_mock_workspace_path, ] software_tests = [ - ('gromacs.water_bare', 'mod_package1@1.1'), - ('gromacs.water_bare', 'mod_package2@1.1'), - ('gromacs.water_bare', 'gromacs'), + ('gromacs', 'mod_package1@1.1'), + ('gromacs', 'mod_package2@1.1'), + ('gromacs', 'gromacs'), ] with ramble.workspace.create(workspace_name) as ws1: diff --git a/lib/ramble/ramble/test/modifier_functionality/modifier_helpers.py b/lib/ramble/ramble/test/modifier_functionality/modifier_helpers.py index 0d264a364..5eedd26e1 100644 --- a/lib/ramble/ramble/test/modifier_functionality/modifier_helpers.py +++ b/lib/ramble/ramble/test/modifier_functionality/modifier_helpers.py @@ -56,8 +56,8 @@ def intel_aps_modifier(): def intel_aps_answer(): expected_software = [ - ('gromacs.water_bare', 'intel-oneapi-vtune'), - ('gromacs.water_bare', 'gromacs') + ('gromacs', 'intel-oneapi-vtune'), + ('gromacs', 'gromacs') ] expected_strs = [ 'aps -c mpi', @@ -76,7 +76,7 @@ def lscpu_modifier(): def lscpu_answer(): expected_software = [ - ('gromacs.water_bare', 'gromacs'), + ('gromacs', 'gromacs'), ] expected_strs = [ 'lscpu', @@ -94,7 +94,7 @@ def env_var_append_paths_modifier(): def env_var_append_paths_modifier_answer(): expected_software = [ - ('gromacs.water_bare', 'gromacs'), + ('gromacs', 'gromacs'), ] expected_strs = [ 'export test_var="${test_var}:test_val"' @@ -112,7 +112,7 @@ def env_var_append_vars_modifier(): def env_var_append_vars_modifier_answer(): expected_software = [ - ('gromacs.water_bare', 'gromacs'), + ('gromacs', 'gromacs'), ] expected_strs = [ 'export test_var="${test_var},test_val"', @@ -130,7 +130,7 @@ def env_var_prepend_paths_modifier(): def env_var_prepend_paths_modifier_answer(): expected_software = [ - ('gromacs.water_bare', 'gromacs'), + ('gromacs', 'gromacs'), ] expected_strs = [ 'export test_var="test_val:${test_var}"', @@ -148,7 +148,7 @@ def env_var_set_modifier(): def env_var_set_modifier_answer(): expected_software = [ - ('gromacs.water_bare', 'gromacs'), + ('gromacs', 'gromacs'), ] expected_strs = [ 'export test_var=test_val', @@ -167,7 +167,7 @@ def env_var_unset_modifier(): def env_var_unset_modifier_answer(): expected_software = [ - ('gromacs.water_bare', 'gromacs'), + ('gromacs', 'gromacs'), ] expected_strs = [ 'unset test_var',