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

Enable unit conversion for model_params #640

Open
mattw-nws opened this issue Sep 15, 2023 · 0 comments
Open

Enable unit conversion for model_params #640

mattw-nws opened this issue Sep 15, 2023 · 0 comments

Comments

@mattw-nws
Copy link
Contributor

Enable unit conversion for model_params, especially when the parameter values come from some "source" instead of explicit values in the realization config.

Current behavior

In #588 the ability was added to retrieve values from the hydrofabric and pass them to modules as model_params, e.g. area_sqkm. Similarly, #639 will add the ability to communicate simulation_time parameters such as simulation_timestep, which will be in seconds. With either of these values, it is conceivable that a receiving model may want to be parameterized with these quantities in different units. However, ngen does not currently call GetVarUnits or attempt any unit conversion when setting model_params. There is actually currently no path to record/pass unit metadata to the part of the code that calls SetValue to set the paramters.

Expected behavior

  1. Add support for a "units" key to the object-style definition of parameters that was added in Allow passing feature properties to BMI models through model_params #588, similar to the following:
{
  "model_params": {
    "area_sqkm": {
      "units": "m3",
      "source": "hydrofabric",
      "from": "area"
    },
    "some_static_param": 3.0,
    "second_static_param": {
      "value": 3.0,
      "units": "kg"
    },
    "length_km": {
      "units": "km",
      "source": "hydrofabric"
    }
  }
}

In the case of a hydrofabric-sourced value, units are not necessarily known and should be specifiable via this mechanism.

Note that the above second_static_param introduces a new value-specified param format that allows for specified units. This is bonus and could be dropped from this issue, but probably comes nearly free (see below) and adds value. The non-object format used above with some_static_param should be preserved.

  1. Modify (Bmi_Module_Formulation::set_initial_bmi_parameters)[https://github.com/NOAA-OWP/ngen/blame/944f8fe45448f11ee0d11255e0a69c45336824d8/include/realizations/catchment/Bmi_Module_Formulation.hpp#L614] to accept a property tree instead of a value directly, and change the code that was added in Allow passing feature properties to BMI models through model_params #588 to "wrap" value-only params (like some_static_param above) with an object-style representation, such that the "units" key-value-pair can be passed through to the function. That way set_initial_bmi_parameters can perform unit conversion between any units in the config and any returned from the module's GetVarUnits function. If either is unspecified, no unit conversion need be attempted.

Steps to replicate behavior (include URLs)

Screenshots

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant