Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated model API #354

Merged
merged 27 commits into from
Jul 27, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
e63e7d2
Core proposal for new model interface
Peter9192 Jun 13, 2023
08da73d
Move concept to replace the original api
Peter9192 Jun 14, 2023
0336861
Add class for dealing with container image names and verisons
Peter9192 Jun 14, 2023
20de9df
wip - use new interface for pcrglobwb
Peter9192 Jun 14, 2023
e58c3d5
Use new api in pcrglob and provide default implementation for coords …
Peter9192 Jun 15, 2023
56f8c1a
use optionaldestbmi wrapper
Peter9192 Jun 15, 2023
6541406
fix flake8 and type issues in base/model.py
Peter9192 Jun 15, 2023
ff84bbc
Make ContainerImage subclass of str and add validation to it
Peter9192 Jun 15, 2023
40b80e4
Fix types and flakes in container and pcrglob model
Peter9192 Jun 15, 2023
16b7eb7
add tests for containerimage
Peter9192 Jun 16, 2023
a384061
Use validators to process received parametersets and forcing; pcrglob…
Peter9192 Jun 16, 2023
faf4b07
Move wflow to new model interface
BSchilperoort Jul 24, 2023
ee6352f
Fix typing issues, improve wflow implementation
BSchilperoort Jul 24, 2023
b336979
Move hype to new model api, fix hype tests
BSchilperoort Jul 24, 2023
4cda895
Make cfg dir and file pydantic privateattrs
BSchilperoort Jul 25, 2023
54a7b80
Add type assertion, pydantic (v1) seems unreliable
BSchilperoort Jul 25, 2023
156ad1b
Fix wflow tests
BSchilperoort Jul 25, 2023
b9fae33
Make _make_cfg_dir reusable
BSchilperoort Jul 25, 2023
054a011
Move marrmot to new api, fix tests
BSchilperoort Jul 25, 2023
f9d490a
Improve _make_cfg_dir, fix tests
BSchilperoort Jul 26, 2023
380979c
Add version nrs, start move to post_init
BSchilperoort Jul 26, 2023
2928b59
Add preliminary parameter set version check
BSchilperoort Jul 27, 2023
6bc3cee
Move lisflood to new API, fix tests
BSchilperoort Jul 27, 2023
73e4fee
Merge remote-tracking branch 'origin/main' into defaultmodels
Peter9192 Jul 27, 2023
422550b
fix some tests
Peter9192 Jul 27, 2023
9baf4de
fix setup of pcrglobwb (failing test)
Peter9192 Jul 27, 2023
11617db
fix more tests
Peter9192 Jul 27, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 14 additions & 12 deletions src/ewatercycle/base/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@

import abc
import logging
from datetime import datetime, timezone
import datetime
from datetime import timezone
from pathlib import Path
from typing import Any, Iterable, Type, cast
from typing import Any, Iterable, Optional, Type, cast

import bmipy
import numpy as np
Expand Down Expand Up @@ -86,19 +87,20 @@ def setup(self, *, cfg_dir: str | None = None, **kwargs) -> tuple[str, str]:

return str(self._cfg_file), str(self._cfg_dir)

def _make_cfg_dir(self, cfg_dir):
def _make_cfg_dir(self, cfg_dir: Optional[str] = None, **kwargs) -> Path:
if cfg_dir is not None:
cfg_dir = to_absolute_path(cfg_dir)
cfg_path = to_absolute_path(cfg_dir)
else:
tz = timezone.utc
timestamp = datetime.now(tz).strftime("%Y%m%d_%H%M%S")
cfg_dir = to_absolute_path(
f"ewatercycle_{timestamp}", parent=CFG.output_dir
timestamp = datetime.datetime.now(tz).strftime("%Y%m%d_%H%M%S")
folder_prefix = kwargs.get("folder_prefix", "ewatercycle")
cfg_path = to_absolute_path(
f"{folder_prefix}_{timestamp}", parent=CFG.output_dir
)

cfg_dir.mkdir(parents=True, exist_ok=True)
cfg_path.mkdir(parents=True, exist_ok=True)

return cfg_dir
return cfg_path

def _make_cfg_file(self, **kwargs):
"""Create new config file and return its path."""
Expand Down Expand Up @@ -312,7 +314,7 @@ def time_as_isostr(self) -> str:
return self.time_as_datetime.strftime(ISO_TIMEFMT)

@property
def start_time_as_datetime(self) -> datetime:
def start_time_as_datetime(self) -> datetime.datetime:
"""Start time of the model as a datetime object."""
return num2date(
self._bmi.get_start_time(),
Expand All @@ -321,7 +323,7 @@ def start_time_as_datetime(self) -> datetime:
)

@property
def end_time_as_datetime(self) -> datetime:
def end_time_as_datetime(self) -> datetime.datetime:
"""End time of the model as a datetime object'."""
return num2date(
self._bmi.get_end_time(),
Expand All @@ -330,7 +332,7 @@ def end_time_as_datetime(self) -> datetime:
)

@property
def time_as_datetime(self) -> datetime:
def time_as_datetime(self) -> datetime.datetime:
"""Current time of the model as a datetime object'."""
return num2date(
self._bmi.get_current_time(),
Expand Down
33 changes: 18 additions & 15 deletions src/ewatercycle/plugins/hype/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,11 @@
from dateutil.tz import UTC
from pydantic import PrivateAttr, root_validator

from ewatercycle import CFG
from ewatercycle.base.model import ISO_TIMEFMT, ContainerizedModel
from ewatercycle.base.parameter_set import ParameterSet
from ewatercycle.container import ContainerImage
from ewatercycle.plugins.hype.forcing import HypeForcing
from ewatercycle.util import geographical_distances, get_time, to_absolute_path
from ewatercycle.util import geographical_distances, get_time

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -137,29 +136,33 @@ def _export_config(self) -> Path:

return cfg_file

def _make_cfg_dir(self, cfg_dir: Optional[Path] = None) -> Path:
if cfg_dir:
cfg_dir = to_absolute_path(cfg_dir)
else:
# Must exist before setting up default config
timestamp = datetime.datetime.now(datetime.timezone.utc).strftime(
"%Y%m%d_%H%M%S"
)
cfg_dir = to_absolute_path(f"hype_{timestamp}", parent=CFG.output_dir)
cfg_dir.mkdir(parents=True, exist_ok=True)
def _make_cfg_dir(
self,
cfg_dir: Optional[str] = None,
**kwargs,
) -> Path:
"""Make sure there is a working directory.

Args:
cfg_dir: If cfg dir is None or does not exist then create sub-directory
in CFG.output_dir
"""
cfg_path = super()._make_cfg_dir(
cfg_dir=cfg_dir, folder_prefix="hype", **kwargs
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to do this for every model now only to add the folder prefix? Would it be possible to get that from self.__class__ or self.__name__?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using self.__class__.__name__.lower() would probably be a nice way of retrieving the folder prefix. It would usually remove the need to define _make_cfg_dir

)

# copy parameter set files to cfg_dir
assert self.parameter_set
shutil.copytree(
src=self.parameter_set.directory, dst=cfg_dir, dirs_exist_ok=True
src=self.parameter_set.directory, dst=cfg_path, dirs_exist_ok=True
)

# copy forcing files to cfg_dir
if self.forcing is not None and self.forcing.directory is not None:
forcing_dir = self.forcing.directory
shutil.copytree(src=forcing_dir, dst=cfg_dir, dirs_exist_ok=True)
shutil.copytree(src=forcing_dir, dst=cfg_path, dirs_exist_ok=True)

return cfg_dir
return cfg_path

def _make_bmi_instance(self) -> bmipy.Bmi:
"""Make the bmi instance and overwrite 'get_time_units' method."""
Expand Down
33 changes: 17 additions & 16 deletions src/ewatercycle/plugins/pcrglobwb/model.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
"""eWaterCycle wrapper around PCRGlobWB BMI."""

import datetime
import logging
from os import PathLike
from pathlib import Path
from typing import Optional

from pydantic import PrivateAttr, root_validator

from ewatercycle import CFG
from ewatercycle.base.model import ContainerizedModel
from ewatercycle.base.parameter_set import ParameterSet
from ewatercycle.container import ContainerImage
Expand Down Expand Up @@ -91,18 +90,20 @@ def _update_parameters(cls, values):
)
return values

def _setup_work_dir(self, cfg_dir: Optional[str] = None):
if cfg_dir:
self.work_dir = to_absolute_path(cfg_dir)
else:
# Must exist before setting up default config
timestamp = datetime.datetime.now(datetime.timezone.utc).strftime(
"%Y%m%d_%H%M%S"
)
self.work_dir = to_absolute_path(
f"pcrglobwb_{timestamp}", parent=CFG.output_dir
)
self.work_dir.mkdir(parents=True, exist_ok=True)
def _make_cfg_dir(
self,
cfg_dir: Optional[str] = None,
**kwargs,
) -> Path:
"""Make sure there is a working directory.

Args:
cfg_dir: If cfg dir is None or does not exist then create sub-directory
in CFG.output_dir
"""
return super()._make_cfg_dir(
cfg_dir=cfg_dir, folder_prefix="pcrglobwb", **kwargs
)

def _make_cfg_file(self, **kwargs):
self._update_config(**kwargs)
Expand Down Expand Up @@ -143,9 +144,9 @@ def _update_config(self, **kwargs):
)

def _export_config(self) -> PathLike:
self._config.set("globalOptions", "outputDir", str(self.work_dir))
self._config.set("globalOptions", "outputDir", str(self._cfg_dir))
new_cfg_file = to_absolute_path(
"pcrglobwb_ewatercycle.ini", parent=self.work_dir
"pcrglobwb_ewatercycle.ini", parent=self._cfg_dir
)
with new_cfg_file.open("w") as filename:
self._config.write(filename)
Expand Down
2 changes: 1 addition & 1 deletion src/ewatercycle/plugins/wflow/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ def _make_cfg_file(self, **kwargs) -> Path:

return cfg_file

def _make_cfg_dir(self, cfg_dir: Optional[str | Path] = None) -> Path:
def _make_cfg_dir(self, cfg_dir: Optional[str] = None, **kwargs) -> Path:
"""Create working directory for parameter sets, forcing and wflow config."""
if cfg_dir:
self._work_dir = to_absolute_path(cfg_dir)
Expand Down
7 changes: 5 additions & 2 deletions tests/plugins/hype/test_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,12 @@ def model(self, parameter_set):

@pytest.fixture
def model_with_setup(self, mocked_config, model: Hype):
with patch.object(
with (
patch.object(
BmiClientApptainer, "__init__", return_value=None
) as mocked_constructor, patch("datetime.datetime") as mocked_datetime:
) as mocked_constructor,
patch("datetime.datetime") as mocked_datetime
):
mocked_datetime.now.return_value = datetime(2021, 1, 2, 3, 4, 5)
config_file, config_dir = model.setup()
return config_file, config_dir, mocked_constructor
Expand Down
Loading