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

Use UDUNITS names #6

Open
mdpiper opened this issue Sep 22, 2020 · 3 comments
Open

Use UDUNITS names #6

mdpiper opened this issue Sep 22, 2020 · 3 comments

Comments

@mdpiper
Copy link
Member

mdpiper commented Sep 22, 2020

In the BMI documentation, we recommend using UDUNITS names and abbreviations for units returned by get_time_units and get_var_units. The UDUNITS library is explicitly used in the bmi-tester and pymt. Several of the unit names used in the PRMSSurface BMI don't comply with UDUNITS. This isn't a problem when working with the components themselves, but it will be a problem when coupling with other models.

@mdpiper
Copy link
Member Author

mdpiper commented Sep 22, 2020

Although the problem manifests itself here (the bmi-tester fails on invalid unit names), it's the BMI code in nhm-usgs/bmi-prms6-surface that needs to be modified.

@rmcd-mscb
Copy link
Contributor

Hey Mark (@mdpiper),
Regarding units, I took another look at the i/o variables identified in the surface bmi and another look at UDUNITS, and without some basic re-engineering of prms, and a default "unitless" unit available in UDUNITS, we'll have to live with this for now. I'm curious, if bmi-tester has a flag that can set unit tests to a warning level rather than a pass/fail, like for standard names? However, there are also i/o variables that are conditional, dependent on how the model is parameterized, and those fail also, as currently set up, so even if the units had a warning, the test as currently set up would still fail.

It's not satisfying to have an incomplete integration with pymt and the bmi-tester. I guess in this experiment, these failing's of the PRMS BMIs to not fully meet these requirements fall into the lessons learned bin.

@mcflugen
Copy link

@rmcd-mscb I had a look at get_var_units in nhm-usgs/bmi-prms6-surface and I'm not sure it would take much re-engineering to fix this units issue. The model can use whatever units it likes, it just needs to report them using the UDUNITS standard, so I think it just means changing the strings returned by get_var_units.

Some suggestions (guesses):

  • decimal-fraction: "1" (or "")
  • fraction-inches: inches
  • temp_units: degree_f
  • fraction / day: 1 / day
  • calories per degrees celcius > 0: thermochemical_calorie / degree_c (or maybe just thermochemical_calorie?)
  • degree-day/temp_units: degree_f s / degree_f
  • degree-day: degree_f s
  • decimal degrees: degree_east, degree_north
  • per degrees Fahrenheit: 1 / degree_f
  • -: dimensionless units are represented either as "" or "1"

I assume the use of temp_units means that the actual temperature units are conditional, based maybe on an input file. If so, the reported units should be dynamically set so that, depending on the model set-up, get_var_units may return e.g. degree_c or degree_f.

Similarly, if i/o some variables are conditional, then then get_input_var_names and get_output_var_names should only return the variables that are available for that parameterization. This would mean that the bmi-tester should only call get_input_var_names and get_output_var_names after a parameterization has been set (after initialize, I assume?).

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

3 participants