From 3749882e5a0e82a33251138202938c762de9bca8 Mon Sep 17 00:00:00 2001 From: Sean Bryan Date: Tue, 8 Aug 2023 11:08:59 +1000 Subject: [PATCH] Add patch remove feature Currently, `benchcab` does not support removing namelist parameters from the base namelist file. For example, legacy CABLE branches may not be compatible with a namelist parameter in the base namelist file. This change adds a `patch_remove` key which will remove a given set of namelist parameters for a specific branch. Fixes #92 --- benchcab/bench_config.py | 8 ++++++ benchcab/repository.py | 2 ++ benchcab/task.py | 38 ++++++++++++++++++++++++++++ docs/user_guide/config_options.md | 11 ++++++++ tests/common.py | 2 ++ tests/test_bench_config.py | 15 +++++++++++ tests/test_task.py | 42 +++++++++++++++++++++++++++++++ 7 files changed, 118 insertions(+) diff --git a/benchcab/bench_config.py b/benchcab/bench_config.py index d902ee2a..a92037b9 100644 --- a/benchcab/bench_config.py +++ b/benchcab/bench_config.py @@ -88,6 +88,14 @@ def check_config(config: dict): f"The 'patch' field in realisation '{branch_id}' must be a " "dictionary that is compatible with the f90nml python package." ) + # the "patch_remove" key is optional + if "patch_remove" in branch_config and not isinstance( + branch_config["patch_remove"], dict + ): + raise TypeError( + f"The 'patch_remove' field in realisation '{branch_id}' must be a " + "dictionary that is compatible with the f90nml python package." + ) # the "build_script" key is optional if "build_script" in branch_config and not isinstance( branch_config["build_script"], str diff --git a/benchcab/repository.py b/benchcab/repository.py index fb92df74..987e43c5 100644 --- a/benchcab/repository.py +++ b/benchcab/repository.py @@ -25,6 +25,7 @@ def __init__( name: Optional[str] = None, revision: Optional[int] = None, patch: Optional[dict] = None, + patch_remove: Optional[dict] = None, build_script: Optional[str] = None, repo_id: Optional[int] = None, ) -> None: @@ -32,6 +33,7 @@ def __init__( self.name = name if name else self.path.name self.revision = revision self.patch = patch + self.patch_remove = patch_remove self.build_script = build_script self._repo_id = repo_id diff --git a/benchcab/task.py b/benchcab/task.py index b91d3731..ba5be25b 100644 --- a/benchcab/task.py +++ b/benchcab/task.py @@ -43,6 +43,20 @@ def deep_update(mapping: Dict[KeyType, Any], *updating_mappings: Dict[KeyType, A # fmt: on +def deep_del( + mapping: Dict[KeyType, Any], *updating_mappings: Dict[KeyType, Any] +) -> Dict[KeyType, Any]: + """Deletes all key-value 'leaf nodes' in `mapping` specified by `updating_mappings`.""" + updated_mapping = mapping.copy() + for updating_mapping in updating_mappings: + for key, value in updating_mapping.items(): + if isinstance(updated_mapping[key], dict) and isinstance(value, dict): + updated_mapping[key] = deep_del(updated_mapping[key], value) + else: + del updated_mapping[key] + return updated_mapping + + def patch_namelist(nml_path: Path, patch: dict): """Writes a namelist patch specified by `patch` to `nml_path`. @@ -59,6 +73,23 @@ def patch_namelist(nml_path: Path, patch: dict): f90nml.write(deep_update(nml, patch), nml_path) +def patch_remove_namelist(nml_path: Path, patch_remove: dict): + """Removes a subset of namelist parameters specified by `patch_remove` from `nml_path`. + + The `patch_remove` dictionary must comply with the `f90nml` api. + """ + + nml = f90nml.read(nml_path) + # remove namelist file as f90nml cannot write to an existing file + nml_path.unlink() + try: + f90nml.write(deep_del(nml, patch_remove), nml_path) + except KeyError as exc: + raise KeyError( + f"Namelist parameters specified in `patch_remove` do not exist in {nml_path.name}." + ) from exc + + f90_logical_repr = {True: ".true.", False: ".false."} @@ -167,6 +198,13 @@ def setup_task(self, verbose=False): ) patch_namelist(nml_path, self.repo.patch) + if self.repo.patch_remove: + if verbose: + print( + f" Removing branch specific configurations from CABLE namelist file {nml_path}" + ) + patch_remove_namelist(nml_path, self.repo.patch_remove) + def clean_task(self, verbose=False): """Cleans output files, namelist files, log files and cable executables if they exist.""" diff --git a/docs/user_guide/config_options.md b/docs/user_guide/config_options.md index 0bfaeb6f..6695a06a 100644 --- a/docs/user_guide/config_options.md +++ b/docs/user_guide/config_options.md @@ -78,6 +78,12 @@ The different running modes of `benchcab` are solely dependent on the options us : The `patch` key must be a dictionary like data structure that is compliant with the [`f90nml`][f90nml-github] python package. : This key is **optional** and can be omitted from the config file (in which case `patch` does not modify the namelist file). +#### `patch_remove` + +: Specifies branch-specific namelist settings to be removed from the `cable.nml` namelist settings. The `patch_remove` key is similar to [`patch`](#`patch`) in the that these settings will be removed from the namelist file for this branch for all science configurations run by `benchcab`. +: The `patch_remove` key must be a dictionary like data structure that is compliant with the [`f90nml`][f90nml-github] python package. +: This key is **optional** and can be omitted from the config file (in which case `patch_remove` does not modify the namelist file). + Example: ```yaml realisations: [ @@ -93,6 +99,11 @@ realisations: [ } } } + patch_remove: { + cable: { + soilparmnew: nil + } + } } ] ``` diff --git a/tests/common.py b/tests/common.py index 73bc5a61..92830b5c 100644 --- a/tests/common.py +++ b/tests/common.py @@ -27,6 +27,7 @@ def get_mock_config() -> dict: "revision": 9000, "path": "trunk", "patch": {}, + "patch_remove": {}, "build_script": "", }, { @@ -34,6 +35,7 @@ def get_mock_config() -> dict: "revision": -1, "path": "branches/Users/sean/my-branch", "patch": {"cable": {"cable_user": {"ENABLE_SOME_FEATURE": False}}}, + "patch_remove": {}, "build_script": "", }, ], diff --git a/tests/test_bench_config.py b/tests/test_bench_config.py index 505e7103..110ec934 100644 --- a/tests/test_bench_config.py +++ b/tests/test_bench_config.py @@ -32,6 +32,11 @@ def test_check_config(): config["realisations"][0].pop("patch") check_config(config) + # Success case: branch configuration with missing patch_remove key + config = get_mock_config() + config["realisations"][0].pop("patch_remove") + check_config(config) + # Success case: test config when realisations contains more than two keys config = get_mock_config() config["realisations"].append({"path": "path/to/my_new_branch"}) @@ -169,6 +174,16 @@ def test_check_config(): config["realisations"][1]["patch"] = r"cable_user%ENABLE_SOME_FEATURE = .FALSE." check_config(config) + # Failure case: type of patch_remove key is not a dictionary + with pytest.raises( + TypeError, + match="The 'patch_remove' field in realisation '1' must be a dictionary that is " + "compatible with the f90nml python package.", + ): + config = get_mock_config() + config["realisations"][1]["patch_remove"] = r"cable_user%ENABLE_SOME_FEATURE" + check_config(config) + # Failure case: type of build_script key is not a string with pytest.raises( TypeError, match="The 'build_script' field in realisation '1' must be a string." diff --git a/tests/test_task.py b/tests/test_task.py index 5b31ae68..0db4fcc8 100644 --- a/tests/test_task.py +++ b/tests/test_task.py @@ -9,6 +9,7 @@ from benchcab.task import ( patch_namelist, + patch_remove_namelist, get_fluxsite_tasks, get_fluxsite_comparisons, get_comparison_name, @@ -221,6 +222,47 @@ def test_patch_namelist(): assert f90nml.read(nml_path) == prev +def test_patch_remove_namelist(): + """Tests for `patch_remove_namelist()`.""" + + nml_path = MOCK_CWD / "test.nml" + + # Success case: remove a namelist parameter from derrived type + nml = {"cable": {"cable_user": {"some_parameter": True}}} + f90nml.write(nml, nml_path) + patch_remove_namelist(nml_path, nml) + assert not f90nml.read(nml_path)["cable"] + nml_path.unlink() + + # Success case: test existing namelist parameters are preserved + # when removing a namelist parameter + to_remove = {"cable": {"cable_user": {"new_feature": True}}} + nml = {"cable": {"cable_user": {"some_parameter": True, "new_feature": True}}} + f90nml.write(nml, nml_path) + patch_remove_namelist(nml_path, to_remove) + assert f90nml.read(nml_path) == {"cable": {"cable_user": {"some_parameter": True}}} + nml_path.unlink() + + # Success case: empty patch_remove does nothing + nml = {"cable": {"cable_user": {"some_parameter": True}}} + f90nml.write(nml, nml_path) + patch_remove_namelist(nml_path, {}) + assert f90nml.read(nml_path) == nml + nml_path.unlink() + + # Failure case: patch_remove should raise KeyError when namelist parameters don't exist in + # nml_path + to_remove = {"cable": {"foo": {"bar": True}}} + nml = {"cable": {"cable_user": {"some_parameter": True, "new_feature": True}}} + f90nml.write(nml, nml_path) + with pytest.raises( + KeyError, + match=f"Namelist parameters specified in `patch_remove` do not exist in {nml_path.name}.", + ): + patch_remove_namelist(nml_path, to_remove) + nml_path.unlink(missing_ok=True) + + def test_setup_task(): """Tests for `setup_task()`."""