diff --git a/python/ngen_cal/changelog.md b/python/ngen_cal/changelog.md index 2b9d4b72..006309c6 100644 --- a/python/ngen_cal/changelog.md +++ b/python/ngen_cal/changelog.md @@ -1,3 +1,10 @@ +# V 0.2.1 +- `ngen.cal` `Objective` enum now properly subclasses `str`. This fixes + pydantic models' json schemas that use `Objective` as a field type. See #31 + for impacts. +- `ngen.cal` calibration strategy types: `NgenExplicit`, `NgenIndependent`, and + `NgenUniform`, now have default `strategy` field values. + # V 0.2.0 - Allow ngen configuration to specify routing output file name to look for. - `Evaluatable` objects are now responsible for maintaining the `evaluation_parms` property, including `evaluation_range` diff --git a/python/ngen_cal/src/ngen/cal/_version.py b/python/ngen_cal/src/ngen/cal/_version.py index 7fd229a3..fc79d63d 100644 --- a/python/ngen_cal/src/ngen/cal/_version.py +++ b/python/ngen_cal/src/ngen/cal/_version.py @@ -1 +1 @@ -__version__ = '0.2.0' +__version__ = '0.2.1' diff --git a/python/ngen_cal/src/ngen/cal/ngen.py b/python/ngen_cal/src/ngen/cal/ngen.py index dc591c34..733216b7 100644 --- a/python/ngen_cal/src/ngen/cal/ngen.py +++ b/python/ngen_cal/src/ngen/cal/ngen.py @@ -241,7 +241,7 @@ def update_config(self, i: int, params: 'pd.DataFrame', id: str = None, path=Pat class NgenExplicit(NgenBase): - strategy: Literal[NgenStrategy.explicit] + strategy: Literal[NgenStrategy.explicit] = NgenStrategy.explicit def __init__(self, **kwargs): #Let pydantic work its magic @@ -294,7 +294,7 @@ def update_config(self, i: int, params: 'pd.DataFrame', id: str, **kwargs): class NgenIndependent(NgenBase): # TODO Error if not routing block in ngen_realization - strategy: Literal[NgenStrategy.independent] + strategy: Literal[NgenStrategy.independent] = NgenStrategy.independent params: Mapping[str, Parameters] #required in this case... def __init__(self, **kwargs): @@ -381,7 +381,7 @@ class NgenUniform(NgenBase): which is applied to each catchment in the hydrofabric being simulated. """ # TODO Error if not routing block in ngen_realization - strategy: Literal[NgenStrategy.uniform] + strategy: Literal[NgenStrategy.uniform] = NgenStrategy.uniform params: Mapping[str, Parameters] #required in this case... def __init__(self, **kwargs): @@ -455,4 +455,4 @@ def resolve_paths(self, relative_to: Optional[Path]=None): @property def best_params(self): - return self.__root__.eval_params.best_params \ No newline at end of file + return self.__root__.eval_params.best_params diff --git a/python/ngen_cal/src/ngen/cal/strategy.py b/python/ngen_cal/src/ngen/cal/strategy.py index 84253f3e..7a6897c2 100644 --- a/python/ngen_cal/src/ngen/cal/strategy.py +++ b/python/ngen_cal/src/ngen/cal/strategy.py @@ -62,4 +62,4 @@ class Sensitivity(BaseModel): NOT IMPLEMENTED """ type: Literal['sensitivity'] - pass #Not Implemented \ No newline at end of file + pass #Not Implemented diff --git a/python/ngen_cal/tests/test_ngen.py b/python/ngen_cal/tests/test_ngen.py new file mode 100644 index 00000000..ad7f826d --- /dev/null +++ b/python/ngen_cal/tests/test_ngen.py @@ -0,0 +1,19 @@ +import pytest +from ngen.cal.ngen import NgenExplicit, NgenIndependent, NgenUniform, NgenStrategy + + +def test_NgenExplicit_strategy_default_value(): + # construct object without validation. + o = NgenExplicit.construct() + assert o.strategy == NgenStrategy.explicit + + +def test_NgenIndependent_strategy_default_value(): + # construct object without validation. + o = NgenIndependent.construct() + assert o.strategy == NgenStrategy.independent + +def test_NgenUniform_strategy_default_value(): + # construct object without validation. + o = NgenUniform.construct() + assert o.strategy == NgenStrategy.uniform diff --git a/python/ngen_conf/changelog.md b/python/ngen_conf/changelog.md index 948972d7..363ee788 100644 --- a/python/ngen_conf/changelog.md +++ b/python/ngen_conf/changelog.md @@ -1,3 +1,11 @@ +# Version 0.1.9 +- Fixed field discriminator issue in `ngen.config`'s `Formulation` model `params` field. + Note, during `Formulation` object creation if the `params` field is + deserialized (e.g. `params`'s value is a dictionary), the `name` field is + required. If `name` *is not* 'bmi_multi', the `model_type_name` field is also + required. Neither are required if a concrete known formulation instance is + provided (see #32). + # Version 0.1.8 - Fix bug when building MultiBmi's `model_name` field from passed `Formulation` objects (fixes #62). diff --git a/python/ngen_conf/src/ngen/config/_version.py b/python/ngen_conf/src/ngen/config/_version.py index c3bb2961..1c98a23a 100644 --- a/python/ngen_conf/src/ngen/config/_version.py +++ b/python/ngen_conf/src/ngen/config/_version.py @@ -1 +1 @@ -__version__ = '0.1.8' +__version__ = '0.1.9' diff --git a/python/ngen_conf/src/ngen/config/cfe.py b/python/ngen_conf/src/ngen/config/cfe.py index 13efe1b8..29eea7cc 100644 --- a/python/ngen_conf/src/ngen/config/cfe.py +++ b/python/ngen_conf/src/ngen/config/cfe.py @@ -1,4 +1,4 @@ -from typing import Optional +from typing import Optional, Literal from pydantic import BaseModel, Field from .bmi_formulation import BMIC @@ -20,7 +20,7 @@ class CFE(BMIC): main_output_variable: str = 'Q_OUT' registration_function: str = "register_bmi_cfe" #NOTE aliases don't propagate to subclasses, so we have to repeat the alias - model_name: str = Field("CFE", const=True, alias="model_type_name") + model_name: Literal["CFE"] = Field("CFE", const=True, alias="model_type_name") #can set some default name map entries...will be overridden at construction #if a name_map with the same key is passed in, otherwise the name_map diff --git a/python/ngen_conf/src/ngen/config/formulation.py b/python/ngen_conf/src/ngen/config/formulation.py index 6b147586..b4a83440 100644 --- a/python/ngen_conf/src/ngen/config/formulation.py +++ b/python/ngen_conf/src/ngen/config/formulation.py @@ -1,15 +1,39 @@ -from pydantic import BaseModel, Field -from typing import TYPE_CHECKING +from pydantic import BaseModel, validator +from typing import Any, Dict, TYPE_CHECKING, Union if TYPE_CHECKING: from typing import Optional from pathlib import Path class Formulation(BaseModel, smart_union=True): - """Model of an ngen formulation + """ + Model of an ngen formulation. + + Note, during object creation if the `params` field is deserialized (e.g. `params`'s value is a + dictionary), the `name` field is required. If `name` *is not* 'bmi_multi', the `model_type_name` + field is also required. Neither are required if a concrete known formulation instance is + provided. """ #TODO make this an enum? name: str - params: "KnownFormulations" = Field(descriminator="model_name") + params: "KnownFormulations" + + @validator("params", pre=True) + def _validate_params( + cls, value: Union[Dict[str, Any], "KnownFormulations"] + ) -> Union[Dict[str, Any], "KnownFormulations"]: + if isinstance(value, BaseModel): + return value + + if isinstance(value, dict): + name = value.get("name") + if name == "bmi_multi": + return value + + # non-bmi multi subtypes must specify their `name` and `model_type_name` fields when _deserializing_. + # note: this does not apply if a subtype instance is provided + if name is None and ("model_type_name" not in value or "model_name" not in value): + raise ValueError("'name' and 'model_type_name' fields are required when deserializing _into_ a 'Formulation'.") + return value def resolve_paths(self, relative_to: 'Optional[Path]'=None): self.params.resolve_paths(relative_to) @@ -21,4 +45,4 @@ def resolve_paths(self, relative_to: 'Optional[Path]'=None): # So we defer type cheking and importing the KnownFormulations until after #MultiBMI is defined, then update_forward_refs() from .all_formulations import KnownFormulations -Formulation.update_forward_refs() \ No newline at end of file +Formulation.update_forward_refs() diff --git a/python/ngen_conf/src/ngen/config/lstm.py b/python/ngen_conf/src/ngen/config/lstm.py index 270bacd6..c41ffe0a 100644 --- a/python/ngen_conf/src/ngen/config/lstm.py +++ b/python/ngen_conf/src/ngen/config/lstm.py @@ -9,7 +9,7 @@ class LSTM(BMIPython): python_type: Union[PyObject, str] = "bmi_lstm.bmi_LSTM" main_output_variable: Literal["land_surface_water__runoff_depth"] = "land_surface_water__runoff_depth" #NOTE aliases don't propagate to subclasses, so we have to repeat the alias - model_name: str = Field("LSTM", alias="model_type_name") + model_name: Literal["LSTM"] = Field("LSTM", alias="model_type_name") _variable_names_map = { "atmosphere_water__time_integral_of_precipitation_mass_flux":"RAINRATE" diff --git a/python/ngen_conf/src/ngen/config/multi.py b/python/ngen_conf/src/ngen/config/multi.py index 64ac1d70..ee5f5fba 100644 --- a/python/ngen_conf/src/ngen/config/multi.py +++ b/python/ngen_conf/src/ngen/config/multi.py @@ -1,4 +1,5 @@ from typing import Sequence, Mapping, Any, Optional, TYPE_CHECKING +from typing_extensions import Literal if TYPE_CHECKING: from pathlib import Path @@ -22,7 +23,7 @@ class MultiBMI(BMIParams, smart_union=True): # NOTE this is derived from the list of modules main_output_variable: Optional[str] #NOTE aliases don't propagate to subclasses, so we have to repeat the alias - model_name: Optional[str] = Field(alias="model_type_name") + model_name: str = Field(None, alias="model_type_name") #override const since these shouldn't be used for multi bmi, but are currently #required to exist as keys for ngen @@ -39,8 +40,9 @@ def resolve_paths(self, relative_to: Optional['Path']=None): def build_model_name(cls, values: Mapping[str, Any]): """Construct the model name, if none provided. - If no model name is provided, the multiBMI model_type_name - is constructed by joining each module's name using `_` + If no model name is provided, the multiBMI model_type_name is constructed by joining + each module's name using `_`. If no module's are provided, model name is the empty + string ''. Args: values (Mapping[str, Any]): Attributes to assgign to the class, including all defaults @@ -48,7 +50,7 @@ def build_model_name(cls, values: Mapping[str, Any]): Returns: Mapping[str, Any]: Attributes to assign to the class, with a (possibly) modifed `model_name` """ - name = values.get('model_name') + name = values.get('model_name') or values.get('model_type_name') modules = values.get('modules') if not name and modules: names = [] @@ -61,6 +63,8 @@ def build_model_name(cls, values: Mapping[str, Any]): except KeyError: names.append(m['params']['model_type_name']) values['model_name'] = '_'.join( names ) + elif not name: + values['model_name'] = '' return values @root_validator(pre=True) @@ -79,7 +83,14 @@ def pick_main_output(cls, values: Mapping[str, Any]) -> Mapping[str, Any]: var = values.get('main_output_variable') modules = values.get('modules') if not var and modules: - values['main_output_variable'] = modules[-1]['params']['main_output_variable'] + last = modules[-1] + + # cannot treat Formulation type like dictionary + from ngen.config.formulation import Formulation + if isinstance(last, Formulation): + values['main_output_variable'] = last.params.main_output_variable + else: + values['main_output_variable'] = last['params']['main_output_variable'] return values #NOTE To avoid circular import and support recrusive modules diff --git a/python/ngen_conf/src/ngen/config/noahowp.py b/python/ngen_conf/src/ngen/config/noahowp.py index aa23494e..d197cb58 100644 --- a/python/ngen_conf/src/ngen/config/noahowp.py +++ b/python/ngen_conf/src/ngen/config/noahowp.py @@ -1,3 +1,4 @@ +from typing import Literal from pydantic import BaseModel, Field from .bmi_formulation import BMIFortran @@ -17,7 +18,7 @@ class NoahOWP(BMIFortran): model_params: NoahOWPParams = None main_output_variable: str = 'QINSUR' #NOTE aliases don't propagate to subclasses, so we have to repeat the alias - model_name: str = Field("NoahOWP", const=True, alias="model_type_name") + model_name: Literal["NoahOWP"] = Field("NoahOWP", const=True, alias="model_type_name") _variable_names_map = { "PRCPNONC": "atmosphere_water__liquid_equivalent_precipitation_rate", diff --git a/python/ngen_conf/src/ngen/config/pet.py b/python/ngen_conf/src/ngen/config/pet.py index 3e15a099..b9481764 100644 --- a/python/ngen_conf/src/ngen/config/pet.py +++ b/python/ngen_conf/src/ngen/config/pet.py @@ -8,7 +8,7 @@ class PET(BMIC): #should all be reasonable defaults for pET main_output_variable: Literal["water_potential_evaporation_flux"] = "water_potential_evaporation_flux" #NOTE aliases don't propagate to subclasses, so we have to repeat the alias - model_name: str = Field("PET", alias="model_type_name") + model_name: Literal["PET"] = Field("PET", alias="model_type_name") # source: https://github.com/NOAA-OWP/evapotranspiration/blob/0a66999db9695bccf4c1e35d904aa86f04e6cacf/src/bmi_pet.c#L1215 registration_function: str = "register_bmi_pet" diff --git a/python/ngen_conf/src/ngen/config/sloth.py b/python/ngen_conf/src/ngen/config/sloth.py index ccfbb99d..cc9cad1b 100644 --- a/python/ngen_conf/src/ngen/config/sloth.py +++ b/python/ngen_conf/src/ngen/config/sloth.py @@ -1,4 +1,4 @@ -from typing import Optional, Mapping +from typing import Literal, Optional, Mapping from pydantic import BaseModel, Field from .bmi_formulation import BMICxx @@ -9,4 +9,4 @@ class SLOTH(BMICxx): model_params: Optional[Mapping[str, str]] #Is this a better represntation of SLOTH params??? just generic mappings? registration_function: str = "none" #FIXME this isn't required for CXX bmi in ngen? #NOTE aliases don't propagate to subclasses, so we have to repeat the alias - model_name: str = Field("SLOTH", const=True, alias="model_type_name") + model_name: Literal["SLOTH"] = Field("SLOTH", const=True, alias="model_type_name") diff --git a/python/ngen_conf/src/ngen/config/topmod.py b/python/ngen_conf/src/ngen/config/topmod.py index 0266e505..614dfe57 100644 --- a/python/ngen_conf/src/ngen/config/topmod.py +++ b/python/ngen_conf/src/ngen/config/topmod.py @@ -1,4 +1,4 @@ -from typing import Optional +from typing import Optional, Literal from pydantic import BaseModel, Field from .bmi_formulation import BMIC @@ -19,7 +19,7 @@ class Topmod(BMIC): main_output_variable: str = 'Qout' registration_function: str = "register_bmi_topmodel" #NOTE aliases don't propagate to subclasses, so we have to repeat the alias - model_name: str = Field("TOPMODEL", const=True, alias="model_type_name") + model_name: Literal["TOPMODEL"] = Field("TOPMODEL", const=True, alias="model_type_name") #can set some default name map entries...will be overridden at construction #if a name_map with the same key is passed in, otherwise the name_map diff --git a/python/ngen_conf/tests/conftest.py b/python/ngen_conf/tests/conftest.py index 16e40539..249882ff 100644 --- a/python/ngen_conf/tests/conftest.py +++ b/python/ngen_conf/tests/conftest.py @@ -5,6 +5,8 @@ from ngen.config.cfe import CFE from ngen.config.sloth import SLOTH from ngen.config.noahowp import NoahOWP +from ngen.config.topmod import Topmod +from ngen.config.lstm import LSTM from ngen.config.multi import MultiBMI #set the workdir relative to this test config @@ -42,7 +44,10 @@ def routing(): @pytest.fixture def cfe_params(): path = _workdir.joinpath("data/CFE/") - data = {'config_prefix':path, + data = { + 'model_type_name': 'CFE', + 'name': 'bmi_c', + 'config_prefix':path, # 'config': "{{id}}_config.txt", 'config': "config.txt", 'library_prefix':path, @@ -53,7 +58,10 @@ def cfe_params(): @pytest.fixture def sloth_params(): path = _workdir.joinpath("data/sloth/") - data = {'config_prefix':path, + data = { + 'model_type_name': 'SLOTH', + 'name': 'bmi_c++', + 'config_prefix':path, # 'config': "{{id}}_config.txt", 'config': "config.txt", 'library_prefix':path, @@ -64,7 +72,10 @@ def sloth_params(): @pytest.fixture def topmod_params(): path = _workdir.joinpath("data/CFE/") - data = {'config_prefix':path, + data = { + 'model_type_name': 'TOPMODEL', + 'name': 'bmi_c', + 'config_prefix':path, # 'config': "{{id}}_config.txt", 'config': "config.txt", 'library_prefix':path, @@ -76,13 +87,26 @@ def topmod_params(): def noahowp_params(): path = _workdir.joinpath("data/NOAH/") libpath = _workdir.joinpath("data/CFE") - data = {'config_prefix':path, + data = { + 'model_type_name': 'NoahOWP', + 'name': 'bmi_fortran', + 'config_prefix':path, 'config': "{{id}}.input", 'library_prefix': libpath, 'library': 'libfakecfe.so' } return data +@pytest.fixture +def lstm_params(): + path = _workdir.joinpath("data/CFE/") + data = { + 'model_type_name': 'LSTM', + 'name': 'bmi_python', + 'config_prefix':path, + 'config': "{{id}}_config.txt"} + return data + @pytest.fixture def cfe(cfe_params): return CFE(**cfe_params) @@ -91,29 +115,33 @@ def cfe(cfe_params): def sloth(sloth_params): return SLOTH(**sloth_params) +@pytest.fixture +def topmod(topmod_params): + return Topmod(**topmod_params) + @pytest.fixture def noahowp(noahowp_params): return NoahOWP(**noahowp_params) @pytest.fixture -def multi(cfe, noahowp, forcing): +def lstm(lstm_params): + return LSTM(**lstm_params) + +@pytest.fixture +def multi(cfe, noahowp): cfe.allow_exceed_end_time=True noahowp.allow_exceed_end_time=True f1 = Formulation(name=noahowp.name, params=noahowp) f2 = Formulation(name=cfe.name, params=cfe) - return MultiBMI(modules=[f1.dict(), f2.dict()], allow_exceed_end_time=True) + return MultiBMI(modules=[f1, f2], allow_exceed_end_time=True) -@pytest.fixture -def lstm_params(): - path = _workdir.joinpath("data/CFE/") - data = {'config_prefix':path, - 'config': "{{id}}_config.txt"} - return data @pytest.fixture def multi_params(cfe, noahowp): - data = {"modules":[Formulation(name=noahowp.name, params=noahowp).dict(), - Formulation(name=cfe.name, params=cfe).dict()] + data = { + "name": "bmi_multi", + "modules":[Formulation(name=noahowp.name, params=noahowp).dict(by_alias=True), + Formulation(name=cfe.name, params=cfe).dict(by_alias=True)] } return data diff --git a/python/ngen_conf/tests/data/test_config.json b/python/ngen_conf/tests/data/test_config.json index 8d7555e4..87f55bcb 100644 --- a/python/ngen_conf/tests/data/test_config.json +++ b/python/ngen_conf/tests/data/test_config.json @@ -6,7 +6,7 @@ "name": "bmi_multi", "params": { "name": "bmi_multi", - "model_type_name": "NoahOWP_CFE", + "model_type_name": "NoahOWP_CFE", "main_output_variable": "Q_OUT", "init_config": "", "allow_exceed_end_time": false, diff --git a/python/ngen_conf/tests/test_formulation.py b/python/ngen_conf/tests/test_formulation.py new file mode 100644 index 00000000..89a39e6d --- /dev/null +++ b/python/ngen_conf/tests/test_formulation.py @@ -0,0 +1,57 @@ +import pytest +from pydantic import BaseModel +from typing import Type +from ngen.config.formulation import Formulation + +from ngen.config.cfe import CFE +from ngen.config.sloth import SLOTH +from ngen.config.topmod import Topmod +from ngen.config.noahowp import NoahOWP +from ngen.config.lstm import LSTM +from ngen.config.multi import MultiBMI + +fixture_expected_type = ( + ("cfe_params", CFE), + ("sloth_params", SLOTH), + ("topmod_params", Topmod), + ("noahowp_params", NoahOWP), + ("lstm_params", LSTM), + ("multi_params", MultiBMI), +) + + +@pytest.mark.parametrize("fixture_name, expected_type", fixture_expected_type) +def test_correct_forumulation_subtype_is_deserialized( + fixture_name: str, expected_type: Type[BaseModel], request: pytest.FixtureRequest +): + params = request.getfixturevalue(fixture_name) + input_data = {"name": "test_formulation", "params": params} + + m = Formulation(**input_data) + assert isinstance( + m.params, expected_type + ), f"expected: {expected_type.__name__!r}, got {type(m.params).__name__!r}" + + +initialized_fixture_expected_type = ( + ("cfe", CFE), + ("sloth", SLOTH), + ("topmod", Topmod), + ("noahowp", NoahOWP), + ("lstm", LSTM), + ("multi", MultiBMI), +) + + +@pytest.mark.parametrize("fixture_name, expected_type", initialized_fixture_expected_type) +def test_correct_forumulation_subtype_is_initialized( + fixture_name: str, expected_type: Type[BaseModel], request: pytest.FixtureRequest +): + params = request.getfixturevalue(fixture_name) + input_data = {"name": "test_formulation", "params": params} + + m = Formulation(**input_data) + + assert isinstance( + m.params, expected_type + ), f"expected: {expected_type.__name__!r}, got {type(m.params).__name__!r}" diff --git a/python/ngen_conf/tests/test_multi.py b/python/ngen_conf/tests/test_multi.py index 0d179942..8938443e 100644 --- a/python/ngen_conf/tests/test_multi.py +++ b/python/ngen_conf/tests/test_multi.py @@ -13,8 +13,8 @@ def test_name_map(multi_params): assert _t == 'land_surface_wind__x_component_of_velocity' def test_name_map_override(multi_params): - multi_params['modules'][-1]['params']['name_map'].update( {"atmosphere_water__liquid_equivalent_precipitation_rate":"RAINRATE"} ) - multi_params['modules'][0]['params']['name_map'].update( {"UU":"WIND_U"} ) + multi_params['modules'][-1]["params"]["variables_names_map"].update( {"atmosphere_water__liquid_equivalent_precipitation_rate":"RAINRATE"} ) + multi_params['modules'][0]["params"]["variables_names_map"].update( {"UU":"WIND_U"} ) multi = MultiBMI(**multi_params) _t = multi.modules[-1].params.name_map["atmosphere_water__liquid_equivalent_precipitation_rate"] assert _t == "RAINRATE" @@ -30,4 +30,3 @@ def test_multi_formulation(multi_params): multi_formulation = Formulation( name=multi.name, params=multi ) _multi = multi_formulation.params assert _multi.name == 'bmi_multi' - assert _multi.model_name == 'NoahOWP_CFE'