Skip to content

Commit

Permalink
Add patch remove feature
Browse files Browse the repository at this point in the history
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
  • Loading branch information
SeanBryan51 committed Aug 8, 2023
1 parent 79e7fa4 commit 3749882
Show file tree
Hide file tree
Showing 7 changed files with 118 additions and 0 deletions.
8 changes: 8 additions & 0 deletions benchcab/bench_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions benchcab/repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,15 @@ 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:
self.path = Path(path)
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

Expand Down
38 changes: 38 additions & 0 deletions benchcab/task.py
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand All @@ -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."}


Expand Down Expand Up @@ -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(

Check warning on line 203 in benchcab/task.py

View check run for this annotation

Codecov / codecov/patch

benchcab/task.py#L202-L203

Added lines #L202 - L203 were not covered by tests
f" Removing branch specific configurations from CABLE namelist file {nml_path}"
)
patch_remove_namelist(nml_path, self.repo.patch_remove)

Check warning on line 206 in benchcab/task.py

View check run for this annotation

Codecov / codecov/patch

benchcab/task.py#L206

Added line #L206 was not covered by tests

def clean_task(self, verbose=False):
"""Cleans output files, namelist files, log files and cable executables if they exist."""

Expand Down
11 changes: 11 additions & 0 deletions docs/user_guide/config_options.md
Original file line number Diff line number Diff line change
Expand Up @@ -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: [
Expand All @@ -93,6 +99,11 @@ realisations: [
}
}
}
patch_remove: {
cable: {
soilparmnew: nil
}
}
}
]
```
Expand Down
2 changes: 2 additions & 0 deletions tests/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,15 @@ def get_mock_config() -> dict:
"revision": 9000,
"path": "trunk",
"patch": {},
"patch_remove": {},
"build_script": "",
},
{
"name": "v3.0-YP-changes",
"revision": -1,
"path": "branches/Users/sean/my-branch",
"patch": {"cable": {"cable_user": {"ENABLE_SOME_FEATURE": False}}},
"patch_remove": {},
"build_script": "",
},
],
Expand Down
15 changes: 15 additions & 0 deletions tests/test_bench_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"})
Expand Down Expand Up @@ -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."
Expand Down
42 changes: 42 additions & 0 deletions tests/test_task.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

from benchcab.task import (
patch_namelist,
patch_remove_namelist,
get_fluxsite_tasks,
get_fluxsite_comparisons,
get_comparison_name,
Expand Down Expand Up @@ -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()`."""

Expand Down

0 comments on commit 3749882

Please sign in to comment.