-
Notifications
You must be signed in to change notification settings - Fork 18
Add some marine variables #131
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
base: main
Are you sure you want to change the base?
Conversation
|
Tagging @travissluka, @shlyaeva, @Dooruk |
|
There's a failing check on this PR that should be resolved by commit 2cfaeb5, but the test didn't get re-triggered. |
|
Note that I'm aware of the apparent inconsistency between adding the names |
gold2718
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a question about units but this otherwise seems fine.
standard_names.xml
Outdated
| </standard_name> | ||
| <standard_name name="sea_water_salinity" | ||
| description="The salinity of sea water"> | ||
| <type units="ppt m">real</type> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am familiar with ppt for seawater salinity but not ppt m. What is this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know. (I'm a software engineer, not a scientist, so I'm probably not the best person to create this PR.) It could well be incorrect. I just copied the units in use for the existing sea_water_salinity_in_diurnal_thermocline variable, figuring they would have the same units. But, since I don't understand the diurnal_thermocline part, maybe that's incorrect. I'm happy to correct it to just ppt if needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After a quick google search, it seems like the units for sea_water_salinity should either be ppt (parts per thousand), psu (practical salinity units, which seems to be the same numerical value as ppt), or possibly even just percent. Sorry for not looking into this matter more carefully originally. Can someone with ocean expertise advise on which units should be used here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
short answer: use PSU, and state "practical salinity of sea water" in the description
long answer:
Technically PSU is not a unit, so i've seen places where the unit is "1", but who cares.
Also, "salinity" is ambiguous, there are 2 main types of salinity 1) "practical salinity" which is what ocean models historically used to use, and is a measure of conductivity and 2) "absolute salinity" which is the more recent standard that models are now starting to use, and has units of g/kg
edit: and, fyi, ppt is close to psu, but technically not the same
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should stick with PSU which is the current standard for measurements. It may be confusing because in Gibbs Seawater (GSW) toolbox, absolute salinity (AS) is used but there is a conversion from pracitcal salinity (PS) to AS.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @travissluka. Since the goal of ESM naming is to avoid ambiguity, should the name of this variable be changed to sea_water_practical_salinity instead of sea_water_salinity? It is possibly only a matter of time until the more recent kind of salinity is added to the ESM naming, so we might as well prepare so we don't have to change names later. Or it is even simple enough to add both names now.
|
Tagging @twsearle |
| <standard_name name="sea_water_salinity" | ||
| description="The practical salinity of sea water"> | ||
| <type units="PSU">real</type> | ||
| </standard_name> | ||
| <standard_name name="sea_water_absolute_salinity" | ||
| description="The absolute salinity of sea water"> | ||
| <type units="g kg-1">real</type> | ||
| </standard_name> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I took a look at the CF conventions, and added their names for these two quantities. It looks to me like CF uses ppt, not PSU for the units of sea_water_salinity. But I stuck with PSU here since that is what @Dooruk and @travissluka recommended.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I looked again, closer, and CF naming has three names:
sea_water_salinitywith units of1e-3(which I think means the same asppt?)sea_water_practical_salinitywith units of1(which Travis says is the same asPSU)sea_water_absolute_salinitywith units ofg kg-1
Do we want to duplicate all three names in ESM? I need scientific guidance here.
Description
We need some marine variables added to the ESM standard since we wish to use them in generic JEDI code for use with SOCA. I have attempted to follow the conventions used for the corresponding
airvariables.Issues
Variables needed for https://github.com/JCSDA-internal/vader/pull/208