From ad99ebfbe9b83917ad079e7ee9e28bd761c79576 Mon Sep 17 00:00:00 2001 From: Stefan Verhoeven Date: Thu, 5 Oct 2023 10:01:48 +0200 Subject: [PATCH] Tests for eWaterCycleModel (#378) * Replace test_abstract with base/test_model * Add more tests for base.model module * Dont depend on fake models from grpc4bmi * Make type of eWaterCycleModel.parameters property an ItemsView To be more inline with pymt See https://github.com/eWaterCycle/ewatercycle/pull/365#discussion_r1328434215 * Use local var instead of prop * Fix more tests * More and better tests * Use public interface where possible * Update model.py * Sort imports --- CHANGELOG.md | 1 + src/ewatercycle/base/model.py | 32 +- src/ewatercycle/plugins/hype/model.py | 6 +- src/ewatercycle/plugins/lisflood/model.py | 6 +- src/ewatercycle/plugins/marrmot/model.py | 10 +- src/ewatercycle/plugins/pcrglobwb/model.py | 6 +- src/ewatercycle/plugins/wflow/model.py | 6 +- src/ewatercycle/testing/fake_models.py | 134 +++++++ tests/plugins/hype/test_model.py | 24 +- tests/plugins/lisflood/test_model.py | 27 +- tests/plugins/marrmot/test_model_m01.py | 4 +- tests/plugins/marrmot/test_model_m14.py | 4 +- tests/plugins/wflow/test_model.py | 17 +- tests/src/base/__init__.py | 1 + tests/src/base/test_model.py | 397 +++++++++++++++++++ tests/src/fake_models.py | 432 --------------------- tests/src/models/test_abstract.py | 339 ---------------- tests/src/test_container.py | 38 +- 18 files changed, 633 insertions(+), 851 deletions(-) create mode 100644 tests/src/base/__init__.py create mode 100644 tests/src/base/test_model.py delete mode 100644 tests/src/fake_models.py delete mode 100644 tests/src/models/test_abstract.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 3db9779e..c48b5e43 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ Formatted as described on [https://keepachangelog.com](https://keepachangelog.co - Forcing ((#365)[https://github.com/eWaterCycle/ewatercycle/pull/365]): - Instead of modifying an existing recipe now builds a ESMValTool recipe from scratch using a fluent interface - DefaultForcing has overridable class methods for each step of the forcing generation process (build_recipe, run_recipe, recipe_output_to_forcing_arguments). +- eWaterCycleModel.parameters property type is ItemsView instead of dict. ### Deprecated diff --git a/src/ewatercycle/base/model.py b/src/ewatercycle/base/model.py index a2c62bb4..35a38609 100644 --- a/src/ewatercycle/base/model.py +++ b/src/ewatercycle/base/model.py @@ -4,6 +4,7 @@ import datetime import inspect import logging +from collections.abc import ItemsView from datetime import timezone from pathlib import Path from typing import Annotated, Any, Iterable, Optional, Type, cast @@ -14,7 +15,6 @@ import yaml from cftime import num2pydate from grpc4bmi.bmi_optionaldest import OptionalDestBmi -from grpc4bmi.reserve import reserve_values, reserve_values_at_indices from pydantic import ( BaseModel, BeforeValidator, @@ -88,9 +88,9 @@ def _make_bmi_instance(self) -> OptionalDestBmi: # where it is {}.items() # TODO is this OK? @property - def parameters(self) -> dict[str, Any]: + def parameters(self) -> ItemsView[str, Any]: """Display the model's parameters and their values.""" - return {} + return {}.items() def setup(self, *, cfg_dir: str | None = None, **kwargs) -> tuple[str, str]: """Perform model setup. @@ -136,9 +136,10 @@ def _make_cfg_dir(self, cfg_dir: Optional[str] = None, **kwargs) -> Path: def _make_cfg_file(self, **kwargs): """Create new config file and return its path.""" cfg_file = self._cfg_dir / "config.yaml" - self.parameters.update(**kwargs) + myparameters = dict(list(self.parameters)) + myparameters.update(**kwargs) with cfg_file.open(mode="w") as file: - yaml.dump({k: v for k, v in self.parameters}, file) + yaml.dump({k: v for k, v in myparameters}, file) return cfg_file @@ -167,7 +168,10 @@ def initialize(self, config_file: str) -> None: self._bmi.initialize(config_file) def finalize(self) -> None: - """Perform tear-down tasks for the model.""" + """Perform tear-down tasks for the model. + + After finalization, the model should not be used anymore. + """ self._bmi.finalize() del self._bmi @@ -181,10 +185,7 @@ def get_value(self, name: str) -> np.ndarray: Args: name: Name of variable """ - if isinstance(self._bmi, OptionalDestBmi): - return self._bmi.get_value(name) - dest = reserve_values(self._bmi, name) - return self._bmi.get_value(name, dest) + return self._bmi.get_value(name) def get_value_at_coords( self, name, lat: Iterable[float], lon: Iterable[float] @@ -198,10 +199,7 @@ def get_value_at_coords( """ indices = self._coords_to_indices(name, lat, lon) indices = np.array(indices) - if isinstance(self._bmi, OptionalDestBmi): - return self._bmi.get_value_at_indices(name, indices) - dest = reserve_values_at_indices(self._bmi, name, indices) - return self._bmi.get_value_at_indices(name, dest, indices) + return self._bmi.get_value_at_indices(name, indices) def set_value(self, name: str, value: np.ndarray) -> None: """Specify a new value for a model variable. @@ -273,9 +271,9 @@ def get_value_as_xarray(self, name: str) -> xr.DataArray: data=np.reshape( self.get_value(name), ( + 1, shape[0], shape[1], - 1, ), ), coords={ @@ -371,6 +369,10 @@ def end_time_as_datetime(self) -> datetime.datetime: @property def time_as_datetime(self) -> datetime.datetime: """Current time of the model as a datetime object'.""" + # TODO some bmi implementations like Wflow.jl returns 'd' + # which can not be converted to a datetime object + # as nupmy2date expects a + # `