fix variable metadata typos #249
Conversation
|
Thanks for taking care of this! |
|
When you make a 🤦 the best thing to do is fix it! |
some more adjoint fixes
|
I just went through and made some changes directly. I think it's good once @mjlosch confirms a couple of variables as noted |
xmitgcm/variables.py
Outdated
| 'ADJclimsst': dict(dims=['j', 'i'], attrs=dict( | ||
| standard_name="ADJclimsst", | ||
| long_name='dJ/dclimsst: Sensitivity to restoring surface temperature', | ||
| units='dJ/K')), | ||
| 'ADJclimsss': dict(dims=['j', 'i'], attrs=dict( | ||
| standard_name="ADJclimsss", | ||
| long_name='dJ/dclimsss: Sensitivity to restoring surface salinity', | ||
| units='dJ/pss)')), | ||
| units='dJ/psu)')), |
There was a problem hiding this comment.
Here I disagree, although my solution is not optimal either. Salinity has no units. In particular "psu" (practical salinity unit) is something that should not be used. pss = practical salinity scale is a compromise but also not very good, see e.g. https://www.myroms.org/forum/viewtopic.php?t=294
If we want to use absolute salinity, the "unit" is g/kg, which again is 1/1000 with no units. In https://github.com/MITgcm/MITgcm/blob/master/model/inc/DYNVARS.h we have "ppt" (parts per thousand), which is again not a unit but just a ratio as it should be. To be consistent, we can change this to dJ/ppt if you like.
There was a problem hiding this comment.
Thanks for another look at all of this @mjlosch! I don't have a strong opinion about what the "units" (or not!) of salinity are labeled as. I put psu because at the end of the day it is what the MITgcm diagnostics assigns to the variable SALT (and anything involving it like advection of salt, etc), at least for the setups I am using. I see what you're saying about DYNVARS.h. My vote is for consistency, and so if we go for e.g. ppt here, I would be in favor of opening this discussion in the MITgcm as well to change it there too.
Unless I'm missing something here... Let me know what you think.
There was a problem hiding this comment.
We should probably go for g/kg, and then open an issue on MITgcm, where the comment need to be changed in DYNVARS.h and the units in diagnostics_main_init.F
@rabernat please decide on the "unit"/non-unit. We can always change it, once the MITgcm code is settled.
There was a problem hiding this comment.
@timothyas @rabernat
Yesterday, I talked to @jm-c, @christophernhill, @jrscott, @jahn and we agreed to replace the "units" of salinity by "g/kg" in the MITgcm code, so I suggest to do this here as well anticipating this change ("in vorauseilendem Gehorsam"). I can do that, if you agree.
|
I'm having a hard time trusting my eyes these days, but this looks good to me 👍 |
|
Thanks everybody. I really appreciate the work. |
* fix variable metadata typos * Update variables.py * Update variables.py some more adjoint fixes * Update variables.py * fix two more instances of sloppiness * replace "psu" by "g/kg" Co-authored-by: Timothy Smith <[email protected]> Co-authored-by: Martin Losch <[email protected]>
Picks up where #247 left off.