TEOS-10 Accessor Improvement Suggestions

[DocOtak]

Hi @gmaze

I was poking though the xarray accessor code and noticed some things in the teos10() method:

• There is a pressure to depth conversion before calculating conservative temperature. The last parameter of the CT_from_t function is pressure.
• If the "standard_name" attribute is meant to be CF compliant (the GDAC argo data themselves claim to be CF compliant), the value of it needs to come from the CF Standard Name table, the buoyancy frequency and Potential Vorticity variable do not have a standard name, but there is a proposal process for getting names added to the list if we want to.
• I think it might be a good idea to match the "variable names" in the GSW libraries (including case):
  • SA
  • CT
  • SIG0 -> sigma0
  • N2 -> Nsquared
  • PV (maybe IPV? I don't know if this is the same thing, the TEOS-10 manual talks a lot of about isopycnal potential vorticity)
  • PTEMP -> pt
• It should be documented that the Nsquared has been shifted back to the CT pressure levels rather than midpoints.
• The PV value is not directly from the gsw toolbox, I'm not too familiar with these calculations, but maybe should also be noted in the docstring.

The first point about pressure and depth in CT is an actual bug.

I can prepare a PR to address some/all of these.

One other comment, the recent gsw 3.4 release has support for directly operating on xarray objects, including Datasets/arrays that are backed by some sort of chunked storage (e.g. dask), I think the calls to .values would need to be removed if support for this was desired.

[ocefpaf]

The first point about pressure and depth in CT is an actual bug.

Yep. That is definitely not desirable. It should be pressure and not depth.

I can prepare a PR to address some/all of these.

If you want to tackle that I'd rather have a PR for the bug and another one for the variable names. However, I will leave the variable name discussion to @gmaze b/c, while that is nice to have, it may break people's workflow. Maybe leave that for a major release?

One other comment, the recent gsw 3.4 release has support for directly operating on xarray objects, including Datasets/arrays that are backed by some sort of chunked storage (e.g. dask), I think the calls to .values would need to be removed if support for this was desired.

Indeed. Taking advantage of that will be nice.

[DocOtak]

@ocefpaf Any thoughts on the "standard_name" attribute stuff? Variable names are low priority, but the values being set in the "standard_name" attribute are not from the CF standard name table.

[ocefpaf]

@ocefpaf Any thoughts on the "standard_name" attribute stuff? Variable names are low priority, but the values being set in the "standard_name" attribute are not from the CF standard name table.

I have to confess that I've ignored CF errors for so long that I'm skeptical on any precise implementation of it. I usually just "outsource" to "good enough" compliance checkers and other parsers.

With that said, yes, having correct standard_name is better. Don't listen to the old bitter man from the paragraph above.

[gmaze]

Thanks @DocOtak , using CF standard name is a nice improvement !
Ask me for a review on #74 when you think it's ready

[gmaze]

PV

As of now, we compute Planetary PV as a simple:

pv = f * n2 / gsw.grav(lat, pres)

but, this is pragmatic and not fully compliant with TEOS-10.
In the TEOS-10 user manual, page 54 (section 3.20), we read:
"The deficiencies of fN2 as a form of planetary potential vorticity have not been widely appreciated"

So, should we use this instead ? :

pv = f * n2 * gsw.IPV_vs_fNsquared_ratio(sa, ct, pres)

in this case, we would need to move back results on the initial pressure levels, like we do for N2.
Also the following attributes could be added:

PV.attrs['long_name'] = 'Planetary Potential Vorticity'
PV.attrs['comment'] = 'Based on Isopycnal Potential Vorticity, see Eqn. (3.20.17) of IOC et al. (2010)'

Variable names

The variable naming issue is not straightforward, basically, should we use in argopy the Argo vocabulary or the GSW one ?

But since Argo is not verbose wrt the following variables, I think we can use the GSW naming combined with the Argo habit of capitalizing variable names:

• SIG0 -> SIGMA0
• N2 -> NSQUARED
• PV -> PV (I don't see "IPV" widely used, I would stick with "PV" and provide comment in attributes to explain)
• PTEMP -> PT

but before implementing this, we would need to raise warning for future deprecation of the old names

[gmaze]

Ok, so #74 addresses:

• using proper CF standard name
• rename undocumented CF standard name into long name attributes
• improve docstrings
• document changes in the whats-new doc section
• update unit testing (not necessary here)

To be addressed on another PR:

• Use a more TEOS compliant PV computation formulae
• Prepare for new variable names to be computed by the argop.teos10 accessor method, to follow the TEOS10 conventions, implement this in 2 steps:
  1. 0.1.7 release will raise warnings about the upcoming deprecation of 0.1.6 names
  2. 0.1.8 release will implement new names and deprecate old ones

Raise a thumb if you agree

[github-actions]

This issue was marked as staled automatically because it has not seen any activity in 90 days

[github-actions]

This issue was marked as staled automatically because it has not seen any activity in 90 days

[github-actions]

This issue was closed automatically because it has not seen any activity in 365 days

[gmaze]

Re-opening, because this is on the wish list, although we don't have ressources to work on it

[github-actions]

This issue was marked as staled automatically because it has not seen any activity in 90 days