Resolve warning about S_REF units #425
Conversation
The units of S_REF were first registered in "PSU" and then later by benchmark in "ppt" which led to a WARNING. Although there is no uniformity to the units of S_REF throughout the user code (currently PSU, ppt, and 1e-3) are all used, benchmark is the only one that leads to a warning in the current suite and changing all to be consistent would 1) unnecessarily update the doc files, and 2) not be correct for all models. Bottom line, I'm punting on the best way to handle units of salinity. This commit resolves the only WARNING we currently get in the MOM6-examples suite.
Codecov Report
@@ Coverage Diff @@
## dev/gfdl #425 +/- ##
============================================
- Coverage 38.05% 38.05% -0.01%
============================================
Files 269 269
Lines 77417 77417
Branches 14300 14300
============================================
- Hits 29462 29461 -1
Misses 42613 42613
- Partials 5342 5343 +1
... and 1 file with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
I think that this may be the most parsimonious solution, but it is going in the wrong direction. "ppt" and "1e-3" are generally synonymous (although some might point out that "ppt" could also mean "1e-12" for parts per trillion), but "PSU" is only used with one specific definition of salinity, based on the measured conductivity of water, and is only useable with a subset of our equations of state. My preference would be to move the generic functions (like the ones that set the vertical coordinate) toward all using "ppt". |
|
Any idea how much work that would be? I found about 7 instances of Although in the wrong direction, maybe this could be taken as fixing one problem while leaving the other problem in more or less the same state? |
|
My count in the code is 27 instances of PSU as a part of a These are not large numbers, and they are easy enough to find. There are, however, more than 700 instances where salinity units are documented in comments via a construct like '[S ~> ppt]', and the only instances where we use '[S ~> PSU]' is in some of the equation of state code where the salinity must be in PSU for the expressions to be physically correct. My proposal would be to use 'PSU' only when we actually mean PSU and not parts per thousand. As for the others, I would suggest we try to standardize the parameter units with ppt or 1e-3, although 'ppt-1' might be better than '1e3' or '(1e-3)-1' when salinity units appear in the denominator. |
|
I concur with all the points raised by @Hallberg-NOAA . |
|
Gaea regression: https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines/20375 ✔️ |
The units of S_REF were first registered in "PSU" and then later by benchmark in "ppt" which led to a WARNING. Although there is no uniformity to the units of S_REF throughout the user code (currently PSU, ppt, and 1e-3, are all used), benchmark is the only one that leads to a warning in the current suite and changing all to be consistent would 1) unnecessarily update the doc files, and 2) not be correct for all models. Bottom line, I'm punting on the best way to handle units of salinity. This commit resolves the only WARNING we currently get in the MOM6-examples suite.