From 819f17cbe3a299ded763f96bf0c4dbb35730728a Mon Sep 17 00:00:00 2001 From: Nels Frazier Date: Tue, 27 Aug 2024 15:01:27 -0600 Subject: [PATCH 1/9] test: add fixture for parameter mapping --- python/ngen_cal/tests/conftest.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/python/ngen_cal/tests/conftest.py b/python/ngen_cal/tests/conftest.py index 32c06a48..8a0602eb 100644 --- a/python/ngen_cal/tests/conftest.py +++ b/python/ngen_cal/tests/conftest.py @@ -1,5 +1,7 @@ +from __future__ import annotations + import pytest -from typing import Generator, List +from typing import Generator, List, Mapping from pathlib import Path from copy import deepcopy import json @@ -11,6 +13,7 @@ from ngen.cal.calibration_cathment import CalibrationCatchment from ngen.cal.model import EvaluationOptions from ngen.cal.agent import Agent +from ngen.cal.parameter import Parameter from hypy import Nexus from .utils import * @@ -219,3 +222,12 @@ def explicit_catchments(nexus, fabric, workdir) -> Generator[ List[ CalibrationC cat = CalibrationCatchment(workdir, id, nexus, start, end, fabric, 'Q_Out', eval_options, data) catchments.append(cat) yield catchments + +@pytest.fixture +def multi_model_shared_params() -> Mapping[str, list[Parameter]]: + p1 = Parameter(name='a', min=0, max=1, init=0) + p2 = Parameter(name='a', min=0, max=1, init=0) + + params = {'A':[p1], 'B':[p2]} + + return params From a9e2f502e0aa53c95c49e065fe0a8386ce4029e8 Mon Sep 17 00:00:00 2001 From: Nels Frazier Date: Tue, 27 Aug 2024 15:05:31 -0600 Subject: [PATCH 2/9] test: add test for parameters with same name --- python/ngen_cal/tests/test_params.py | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 python/ngen_cal/tests/test_params.py diff --git a/python/ngen_cal/tests/test_params.py b/python/ngen_cal/tests/test_params.py new file mode 100644 index 00000000..5b680f6e --- /dev/null +++ b/python/ngen_cal/tests/test_params.py @@ -0,0 +1,26 @@ +from __future__ import annotations + +from ngen.cal.ngen import _params_as_df +import pandas as pd + +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from typing import Mapping + from ngen.cal.parameter import Parameter + +def test_multi_params(multi_model_shared_params: Mapping[str, list[Parameter]]): + # This is essentially the path the params go through from + # creation in model.py to update in search.py + params = _params_as_df(multi_model_shared_params) + params = pd.DataFrame(params).rename(columns={'init':'0'}) + # create new iteration from old + params['1'] = params['0'] + #update the parameters by index + params.loc[0, '1'] = 0.5 + pa = params[ params['model'] == 'A' ] + pb = params[ params['model'] == 'B' ] + + assert pa.drop('model', axis=1).equals( pb.drop('model', axis=1) ) + +# TODO test/document case where params have same name but different min/max/init values From 3c3428be34a63f73b86695a00cba2e4ee55cd413 Mon Sep 17 00:00:00 2001 From: Nels Frazier Date: Wed, 28 Aug 2024 08:13:46 -0600 Subject: [PATCH 3/9] fix: index parameters by name in ngen param dataframe --- python/ngen_cal/src/ngen/cal/ngen.py | 4 ++-- python/ngen_cal/tests/conftest.py | 4 ++-- python/ngen_cal/tests/test_params.py | 8 ++++---- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/python/ngen_cal/src/ngen/cal/ngen.py b/python/ngen_cal/src/ngen/cal/ngen.py index 9d2cd498..ebbe25e9 100644 --- a/python/ngen_cal/src/ngen/cal/ngen.py +++ b/python/ngen_cal/src/ngen/cal/ngen.py @@ -46,13 +46,13 @@ def _params_as_df(params: Mapping[str, Parameters], name: str = None): df['model'] = k df.rename(columns={'name':'param'}, inplace=True) dfs.append(df) - return pd.concat(dfs) + return pd.concat(dfs).set_index('param') else: p = params.get(name, []) df = pd.DataFrame([s.__dict__ for s in p]) df['model'] = name df.rename(columns={'name':'param'}, inplace=True) - return df + return df.set_index('param') def _map_params_to_realization(params: Mapping[str, Parameters], realization: Realization): # don't even think about calibration multiple formulations at once just yet.. diff --git a/python/ngen_cal/tests/conftest.py b/python/ngen_cal/tests/conftest.py index 8a0602eb..4f3a8331 100644 --- a/python/ngen_cal/tests/conftest.py +++ b/python/ngen_cal/tests/conftest.py @@ -227,7 +227,7 @@ def explicit_catchments(nexus, fabric, workdir) -> Generator[ List[ CalibrationC def multi_model_shared_params() -> Mapping[str, list[Parameter]]: p1 = Parameter(name='a', min=0, max=1, init=0) p2 = Parameter(name='a', min=0, max=1, init=0) - - params = {'A':[p1], 'B':[p2]} + p3 = Parameter(name='c', min=0, max=1, init=0) + params = {'A':[p1], 'B':[p2], 'C':[p3]} return params diff --git a/python/ngen_cal/tests/test_params.py b/python/ngen_cal/tests/test_params.py index 5b680f6e..ad83d3fa 100644 --- a/python/ngen_cal/tests/test_params.py +++ b/python/ngen_cal/tests/test_params.py @@ -17,10 +17,10 @@ def test_multi_params(multi_model_shared_params: Mapping[str, list[Parameter]]): # create new iteration from old params['1'] = params['0'] #update the parameters by index - params.loc[0, '1'] = 0.5 + params.loc['a', '1'] = 0.5 pa = params[ params['model'] == 'A' ] pb = params[ params['model'] == 'B' ] - + pc = params[ params['model'] == 'C' ] assert pa.drop('model', axis=1).equals( pb.drop('model', axis=1) ) - -# TODO test/document case where params have same name but different min/max/init values + # ensure unique params/alias are not modifed by selection + assert pa.loc['a', '1'] != pc.loc['c', '1'] From 933b4658e94af7a4642ef4a6f68cd76e46acda58 Mon Sep 17 00:00:00 2001 From: Nels Frazier Date: Wed, 28 Aug 2024 08:15:51 -0600 Subject: [PATCH 4/9] test: additional multi model parameter test --- python/ngen_cal/tests/conftest.py | 9 +++++++++ python/ngen_cal/tests/test_params.py | 13 +++++++++++++ 2 files changed, 22 insertions(+) diff --git a/python/ngen_cal/tests/conftest.py b/python/ngen_cal/tests/conftest.py index 4f3a8331..1cebd170 100644 --- a/python/ngen_cal/tests/conftest.py +++ b/python/ngen_cal/tests/conftest.py @@ -231,3 +231,12 @@ def multi_model_shared_params() -> Mapping[str, list[Parameter]]: params = {'A':[p1], 'B':[p2], 'C':[p3]} return params + +@pytest.fixture +def multi_model_shared_params2() -> Mapping[str, list[Parameter]]: + p1 = Parameter(name='a', min=0, max=1, init=0) + p2 = Parameter(name='a', min=0, max=1, init=0) + p3 = Parameter(name='c', min=0, max=1, init=0) + params = {'A':[p1, p3], 'B':[p2]} + + return params diff --git a/python/ngen_cal/tests/test_params.py b/python/ngen_cal/tests/test_params.py index ad83d3fa..a21767e3 100644 --- a/python/ngen_cal/tests/test_params.py +++ b/python/ngen_cal/tests/test_params.py @@ -24,3 +24,16 @@ def test_multi_params(multi_model_shared_params: Mapping[str, list[Parameter]]): assert pa.drop('model', axis=1).equals( pb.drop('model', axis=1) ) # ensure unique params/alias are not modifed by selection assert pa.loc['a', '1'] != pc.loc['c', '1'] + +def test_multi_params2(multi_model_shared_params2: Mapping[str, list[Parameter]]): + # This is essentially the path the params go through from + # creation in model.py to update in search.py + params = _params_as_df(multi_model_shared_params2) + params = pd.DataFrame(params).rename(columns={'init':'0'}) + # create new iteration from old + params['1'] = params['0'] + #update the parameters by index + params.loc['a', '1'] = 0.5 + pa = params[ params['model'] == 'A' ].drop('model', axis=1).loc['a'] + pb = params[ params['model'] == 'B' ].drop('model', axis=1).loc['a'] + assert pa.equals( pb ) From 223161be5c3c7f7f2bd5fafec299e3bce6f3a44e Mon Sep 17 00:00:00 2001 From: Nels Frazier Date: Wed, 28 Aug 2024 10:29:24 -0600 Subject: [PATCH 5/9] refactor(test): ensure index ordering isn't coincednt --- python/ngen_cal/tests/conftest.py | 5 +++-- python/ngen_cal/tests/test_params.py | 15 +++++++++------ 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/python/ngen_cal/tests/conftest.py b/python/ngen_cal/tests/conftest.py index 1cebd170..e3af5da5 100644 --- a/python/ngen_cal/tests/conftest.py +++ b/python/ngen_cal/tests/conftest.py @@ -226,9 +226,10 @@ def explicit_catchments(nexus, fabric, workdir) -> Generator[ List[ CalibrationC @pytest.fixture def multi_model_shared_params() -> Mapping[str, list[Parameter]]: p1 = Parameter(name='a', min=0, max=1, init=0) - p2 = Parameter(name='a', min=0, max=1, init=0) + p2 = Parameter(name='d', min=2, max=3, init=0) p3 = Parameter(name='c', min=0, max=1, init=0) - params = {'A':[p1], 'B':[p2], 'C':[p3]} + p4 = Parameter(name='a', min=0, max=1, init=0) + params = {'A':[p1], 'B':[p2, p4], 'C':[p3, p4]} return params diff --git a/python/ngen_cal/tests/test_params.py b/python/ngen_cal/tests/test_params.py index a21767e3..bb483c9d 100644 --- a/python/ngen_cal/tests/test_params.py +++ b/python/ngen_cal/tests/test_params.py @@ -17,13 +17,16 @@ def test_multi_params(multi_model_shared_params: Mapping[str, list[Parameter]]): # create new iteration from old params['1'] = params['0'] #update the parameters by index - params.loc['a', '1'] = 0.5 - pa = params[ params['model'] == 'A' ] - pb = params[ params['model'] == 'B' ] - pc = params[ params['model'] == 'C' ] - assert pa.drop('model', axis=1).equals( pb.drop('model', axis=1) ) + idx1 = 'a' + idx2 = 'c' + params.loc[idx1, '1'] = 0.5 + pa = params[ params['model'] == 'A' ].loc[idx1] + pb = params[ params['model'] == 'B' ].loc[idx1] + pc = params[ params['model'] == 'C' ].loc[idx2] + + assert pa.drop('model').equals( pb.drop('model') ) # ensure unique params/alias are not modifed by selection - assert pa.loc['a', '1'] != pc.loc['c', '1'] + assert pa.loc['1'] != pc.loc['1'] def test_multi_params2(multi_model_shared_params2: Mapping[str, list[Parameter]]): # This is essentially the path the params go through from From 3ed98b1a9ff747845449fd1ff6c812eedbf040da Mon Sep 17 00:00:00 2001 From: Nels Frazier Date: Wed, 28 Aug 2024 08:18:49 -0600 Subject: [PATCH 6/9] feat: add alias to Parameter model --- python/ngen_cal/src/ngen/cal/parameter.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/python/ngen_cal/src/ngen/cal/parameter.py b/python/ngen_cal/src/ngen/cal/parameter.py index 8f83dc7e..dfcab025 100644 --- a/python/ngen_cal/src/ngen/cal/parameter.py +++ b/python/ngen_cal/src/ngen/cal/parameter.py @@ -1,7 +1,7 @@ from __future__ import annotations -from pydantic import BaseModel, Field -from typing import Sequence +from pydantic import BaseModel, Field, root_validator +from typing import Sequence, Mapping, Optional class Parameter(BaseModel, allow_population_by_field_name = True): """ @@ -11,5 +11,13 @@ class Parameter(BaseModel, allow_population_by_field_name = True): min: float max: float init: float + alias: Optional[str] + + @root_validator + def _set_alias(cls, values: dict) -> dict: + alias = values.get('alias', None) + if alias is None: + values['alias'] = values['name'] + return values Parameters = Sequence[Parameter] From ebd1b6e613a85afd75e2616ab76755312092466e Mon Sep 17 00:00:00 2001 From: Nels Frazier Date: Wed, 28 Aug 2024 08:19:26 -0600 Subject: [PATCH 7/9] feat: index and permute paramters based on alias --- python/ngen_cal/src/ngen/cal/ngen.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python/ngen_cal/src/ngen/cal/ngen.py b/python/ngen_cal/src/ngen/cal/ngen.py index ebbe25e9..7979b5b4 100644 --- a/python/ngen_cal/src/ngen/cal/ngen.py +++ b/python/ngen_cal/src/ngen/cal/ngen.py @@ -46,13 +46,13 @@ def _params_as_df(params: Mapping[str, Parameters], name: str = None): df['model'] = k df.rename(columns={'name':'param'}, inplace=True) dfs.append(df) - return pd.concat(dfs).set_index('param') + return pd.concat(dfs).set_index('alias') else: p = params.get(name, []) df = pd.DataFrame([s.__dict__ for s in p]) df['model'] = name df.rename(columns={'name':'param'}, inplace=True) - return df.set_index('param') + return df.set_index('alias') def _map_params_to_realization(params: Mapping[str, Parameters], realization: Realization): # don't even think about calibration multiple formulations at once just yet.. From ba07a3824cfe636cc6a6aef10d73cf9f85473b66 Mon Sep 17 00:00:00 2001 From: Nels Frazier Date: Wed, 28 Aug 2024 08:21:27 -0600 Subject: [PATCH 8/9] test: alias permutation mechanics --- python/ngen_cal/tests/conftest.py | 10 ++++++++++ python/ngen_cal/tests/test_params.py | 18 ++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/python/ngen_cal/tests/conftest.py b/python/ngen_cal/tests/conftest.py index e3af5da5..0c786b9e 100644 --- a/python/ngen_cal/tests/conftest.py +++ b/python/ngen_cal/tests/conftest.py @@ -241,3 +241,13 @@ def multi_model_shared_params2() -> Mapping[str, list[Parameter]]: params = {'A':[p1, p3], 'B':[p2]} return params + +@pytest.fixture +def multi_model_alias_params() -> Mapping[str, list[Parameter]]: + p1 = Parameter(name='a', alias='c', min=0, max=1, init=0) + p2 = Parameter(name='b', alias='c', min=0, max=1, init=0) + p3 = Parameter(name='d', min=0, max=1, init=0) + + params = {'A':[p1,p3], 'B':[p2, p3], 'C':[p3]} + + return params diff --git a/python/ngen_cal/tests/test_params.py b/python/ngen_cal/tests/test_params.py index bb483c9d..223ce577 100644 --- a/python/ngen_cal/tests/test_params.py +++ b/python/ngen_cal/tests/test_params.py @@ -40,3 +40,21 @@ def test_multi_params2(multi_model_shared_params2: Mapping[str, list[Parameter]] pa = params[ params['model'] == 'A' ].drop('model', axis=1).loc['a'] pb = params[ params['model'] == 'B' ].drop('model', axis=1).loc['a'] assert pa.equals( pb ) + +def test_alias_params(multi_model_alias_params: Mapping[str, list[Parameter]]): + # This is essentially the path the params go through from + # creation in model.py to update in search.py + assert multi_model_alias_params['C'][0].alias == multi_model_alias_params['C'][0].name + params = _params_as_df(multi_model_alias_params) + params = pd.DataFrame(params).rename(columns={'init':'0'}) + # create new iteration from old + params['1'] = params['0'] + #update the parameters by index + + params.loc['c', '1'] = 0.5 + pa = params[ params['model'] == 'A' ] + pb = params[ params['model'] == 'B' ] + + assert pa.drop(['model', 'param'], axis=1).equals( pb.drop(['model', 'param'], axis=1) ) + +# TODO test/document case where params have same name but different min/max/init values From 98750968f2714e8e29418b27b70deac07f97ba0c Mon Sep 17 00:00:00 2001 From: Nels Frazier Date: Wed, 28 Aug 2024 10:31:59 -0600 Subject: [PATCH 9/9] fix: ensure variable list provided to search is unique --- python/ngen_cal/src/ngen/cal/calibratable.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python/ngen_cal/src/ngen/cal/calibratable.py b/python/ngen_cal/src/ngen/cal/calibratable.py index f8ad5115..aa6ebf4b 100644 --- a/python/ngen_cal/src/ngen/cal/calibratable.py +++ b/python/ngen_cal/src/ngen/cal/calibratable.py @@ -48,9 +48,9 @@ def id(self) -> str: @property def variables(self) -> Series: """ - Index series of variables + Index of unique variables to permute """ - return Series(self.df.index.values) + return Series(self.df.index.unique().values) @property def bounds(self) -> tuple[Series, Series]: