ngen-config Update model_param spec

[hellkite500]

https://github.com/NOAA-OWP/ngen-cal/blob/3a170b81632050381f14c374b1eaeac06da0f607/python/ngen_conf/src/ngen/config/bmi_formulation.py#L45C5-L45C17

ngen now supports a model param mapping to link model params to hydrofabric feature attributes.

The config structure can be seen here in this test code.

The config schema should be updated to reflect this change and properly validate/generate configs that may have this structure.

[aaraney]

Thanks for pinging me, @hellkite500. From a quick glance it looks like model_params can now be one either None (unspecified), Mapping[str, str] (current specified form), or a Mapping[str, Mapping[str, str]]. So as a type hint that would look like:

    model_params: Optional[Mapping[str, Union[str, Mapping[str, str]]]]

Is it valid for the inner associated value types to be either str or Mapping[str, str] in the same outer mapping type? Or are the two variant exclusive? For example is this allowed?

{
    "model_params": {
        "str_kv": "str_v",
        "map_kv": {
            "map_k": "map_v"
        }
    }
}

Or are the following two examples only allowed?

{
    "model_params": {
        "str_kv": "str_v"
    }
}

{
    "model_params": {
        "map_kv": {
            "map_k": "map_v"
        }
    }
}

Edit: So as type hints the differences are Optional[Union[Mapping[str, str], Mapping[str, Mapping[str, str]]]] vs Optional[Mapping[str, Union[str, Mapping[str, str]]]].

[hellkite500]

It can be mixed. Though there is a current constraint that if it is a Mapping it should be in the form of

"model_params": {
    "key": {
        "source" : "source_value",
        "from" : "from_value"
    }
}

So we should probably define a custom linking type which has strict source and from keys, and the type would become
Optional[ Mapping[ str, Union[ str, LinkType] ] ]
I'm also not convinced that str is the only type...I'm pretty sure it can be numeric or list as well, just to complicate things a bit haha.

[aaraney]

Thanks for the clarification!

I'm also not convinced that str is the only type...I'm pretty sure it can be numeric or list as well, just to complicate things a bit haha.

If you are talking about the keys, only str's are allowed as keys in json.

[hellkite500]

I'm refering to the values, e.g.

"model_params": {
    "param1": 4.2,
    "param2": [ 4, 2 ]
}

[aaraney]

Ah, so sequences are also supported?

[aaraney]

I would assume sequences of numbers or str's?

[hellkite500]

I'm not sure str is actually supported at all, I'll have to check the backend NGEN code haha

[hellkite500]

https://github.com/NOAA-OWP/ngen/blob/1dcd25596596f5a2c2aef172fc53e12fecd101f9/include/realizations/catchment/Bmi_Module_Formulation.hpp#L574

Found it, not sure why "str" was here in the first place, unless there was some type conversion happening somewhere else in ngen...still looking a bit

[aaraney]

@hellkite500, thanks for digging into that and also pulling links. Im sure either we or someone else will appreciate the paper trail in the future.

Looking at the NextGen link you sent, it looks like the appropriate type hint should be:

Numeric = Union[int, float]
MODEL_PARAMS_T = Optional[Mapping[str, Union[Numeric, Sequence[Numeric], Mapping[str, LinkType]]]]

[hellkite500]

Sounds about right

[aaraney]

@hellkite500, just opened #75 to fix this.