Forcing builder #365
Forcing builder #365
Conversation
This make base package much cleaner
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
Got
Traceback (most recent call last):
File "/home/verhoes/git/eWaterCycle/ewatercycle/src/ewatercycle/esmvaltool/diagnostic/copy.py", line 7, in <module>
from esmvaltool.diag_scripts.shared import (
File "/home/verhoes/mambaforge/envs/ewatercycle/lib/python3.10/site-packages/esmvaltool/diag_scripts/shared/__init__.py", line 2, in <module>
from . import io, iris_helpers, names, plot
File "/home/verhoes/mambaforge/envs/ewatercycle/lib/python3.10/site-packages/esmvaltool/diag_scripts/shared/io.py", line 5, in <module>
from pprint import pformat
File "/home/verhoes/mambaforge/envs/ewatercycle/lib/python3.10/pprint.py", line 38, in <module>
import dataclasses as _dataclasses
File "/home/verhoes/mambaforge/envs/ewatercycle/lib/python3.10/dataclasses.py", line 3, in <module>
import copy
File "/home/verhoes/git/eWaterCycle/ewatercycle/src/ewatercycle/esmvaltool/diagnostic/copy.py", line 7, in <module>
from esmvaltool.diag_scripts.shared import (
ImportError: cannot import name 'ProvenanceLogger' from partially initialized module 'esmvaltool.diag_scripts.shared' (most likely due to a circular import) (/home/verhoes/mambaforge/envs/ewatercycle/lib/python3.10/site-packages/esmvaltool/diag_scripts/shared/__init__.py)
|
I tried to use the following dataset definition to use CMIP data (as that can be automatically downloaded by esmvaltool. It failed on this error: ErrorFile [~/micromamba/envs/ewatercycle310/lib/python3.10/site-packages/esmvalcore/_recipe/check.py:110](https://file+.vscode-resource.vscode-cdn.net/home/bart/git/ewatercycle/~/micromamba/envs/ewatercycle310/lib/python3.10/site-packages/esmvalcore/_recipe/check.py:110), in variable(var, required_keys)
104 missing = required - set(var)
105 if missing:
106 raise RecipeError(
107 f"Missing keys {missing} in\n"
108 f"{pformat(var)}\n"
109 "for variable {var['variable_group']} in diagnostic "
--> 110 f"{var['diagnostic']}")
KeyError: 'diagnostic'Datasetfrom ewatercycle.esmvaltool.models import Dataset
cmip_dataset = Dataset(**{
"dataset": "EC-Earth3",
"project": "CMIP6",
"grid": "gr",
"exp": ["historical",],
"ensemble": "r6i1p1f1",
"start_year": 2000,
"end_year": 2022,
})forcing = GenericDistributedForcing.generate(
dataset=cmip_dataset,
start_time="2020-01-01T00:00:00Z",
end_time="2020-01-01T00:00:00Z",
shape="/home/bart/git/ewatercycle/src/ewatercycle/testing/data/Rhine/Rhine.shp",
) |
+ more docs + fix tests
Had to make Dataset.mip optional otherwise tdps var with mip=day throws esmvaltool error
'*' in forcing files was caused by version attribute in Dataset being silently forgotten. By allowing extra attributes the generate file has version value instead of `*`.
|
Your CMIP example now works and is part of docstring at https://ewatercycle--365.org.readthedocs.build/en/365/autoapi/ewatercycle/base/forcing/index.html#ewatercycle.base.forcing.GenericDistributedForcing |
|
See https://github.com/eWaterCycle/infra/blob/main/README.md#mount-dcache-on-local-machine how to mount ERA data locally. |
There was a problem hiding this comment.
Awesome work! I really love that we now have standard forcings for generic and lumped models, and that we can support much more ESMValTool functionality, most importantly any dataset.
Since Bart already tried it out, I just quickly skimmed over the code and left a couple of small comments.
tests/src/esmvaltool/test_models.py
Outdated
| @@ -0,0 +1,65 @@ | |||
| from pathlib import Path | |||
There was a problem hiding this comment.
I found the name of this file test_models to be quite confusing. Can we make it more pydantic-specific?
There was a problem hiding this comment.
I think you are confused these models with hydrological models
I could rename the module to
- test_schema
- test_recipe_models
- test_io
- something else?
That models are implemented in pydantic should not be important to consumers so I dont like adding pydantic in the module name.
To make consistent we should also rename ewatercycle.esmvaltool.models .
I like schema.
@Peter9192 do you have a preference or suggestion?
To be more inline with pymt See #365 (comment)
When we talk about models we mean hydrological models.
|
Kudos, SonarCloud Quality Gate passed! |
|
Thanks for testing and reviewing. |
* 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 #365 (comment) * Use local var instead of prop * Fix more tests * More and better tests * Use public interface where possible * Update model.py * Sort imports
Use fluent builder pattern to construct esmvaltool recipes.
Recipes for models with esmvaltool recipes are build using builder and existing diagnostic.
TODO