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

u² is saved under variable name u (possibly same for v): confusion for cosima-cookbook database #234

Closed
navidcy opened this issue Feb 8, 2021 · 13 comments

Comments

@navidcy
Copy link
Contributor

navidcy commented Feb 8, 2021

I noticed that is saved as a netcdf file with a variable name (inside the netcdf) u.
This creates issues when loading variables via the cosima-cookbook, e.g.,

u = cosima_cookbook.querying.getvar(experiment, session,'u', session, frequency = '1 monthly', ...)

will load both u and or a random combination of the two!

See, e.g., output in

/g/data/e14/rmh561/access-om2/archive/025deg_jra55_ryf_nostress/output000/ocean/ocean-3d-u-1-monthly-mean-ym_1900_01.nc

and

/g/data/e14/rmh561/access-om2/archive/025deg_jra55_ryf_nostress/output000/ocean/ocean-3d-u-1-monthly-pow02-ym_1900_01.nc

Could we fix/change the netcdf variable to be usquare or something else other than u?

(Yes, the problem can be alleviated by providing an ncfile=... argument to cosima_cookbook.querying.getvar() but that's a hack. Also this is not what the data explorer would suggest you to do so nobody would initially think that such thing is necessary.)

cc @dhruvbhagtani2105, @aidanheerdegen, @angus-g, @aekiss, @rmholmes, @AndyHoggANU

@AndyHoggANU
Copy link
Contributor

@aekiss - this is a good point. Is there any way you can run a quick check on all the standard configs to ensure that there are no other variables being saved with a common name? We should then change the default names for these two files in the control directories -- I guess that needs a change in the logic of diag_table_source.yaml?

@aekiss
Copy link
Contributor

aekiss commented Feb 10, 2021

This issue would affect any variable that is output with multiple reduction_methods in diag_table_source.yaml, e.g. we have a few other variables that use reduction methods of min, max, snap.
I guess we could set output_name = field_name+'_'+reduction_method when reduction_method is not mean, so in this case the variable name would be u_pow02.

In the case of reduction_method = pow## the units should also be corrected - though this is really a MOM bug and should be fixed there.

@aekiss
Copy link
Contributor

aekiss commented Feb 10, 2021

new MOM issue re. units: mom-ocean/MOM5#335

@navidcy
Copy link
Contributor Author

navidcy commented Feb 10, 2021

@aekiss, I see your point eg regarding u snap and u monthly mean. These are two different reduction methods. But u^2 is not a reduction method. It's a different quantity. You can still have u^2 snap and u^2 monthly mean etc. Am I wrong?

An issue that arises is that cookbook's data explorer doesn't suggest the user to prescribe also the appropriate ncfile argument and therefore confusion is generated and propagated to the following generations. (cc @aidanheerdegen). Is there at least some way for dataexplorer to inform the naïve user that u might also mean u^2 or u^4 and what not? This would be perhaps a way around this issue.

2 cents deposited

@AndyHoggANU
Copy link
Contributor

Would adding reduction_method have implications for existing variables that we regularly use? If not, then this would seem a sensible way to go ...

@aekiss
Copy link
Contributor

aekiss commented Feb 10, 2021

Yes u^2 is a different physical quantity but it's specified as a pow02 reduction method in diag_table, and calculated as a mean of u^2 over the sampling period (e.g. monthly or daily, which is specified separately via output_freq and output_freq_units in diag_table_source.yaml). But snap is also a reduction method, so you can have daily or monthly u^2 but you can't have u^2 snap.

If the cookbook was to differentiate these it would need to use the variable's cell_methods attribute, which is "time: mean_pow(02)" for u^2. This would save confusion for those accessing existing data so seems preferable to only fixing the setup for future runs.

@aekiss
Copy link
Contributor

aekiss commented Feb 10, 2021

If a reduction method selector in the cookbook defaulted to cell_methods="time: mean" this would work as expected for existing data.

@aekiss
Copy link
Contributor

aekiss commented Feb 10, 2021

On further investigation it seems that min and max are appended to the variable name by MOM, so there's no ambiguity there.
But snap (ie cell_methods="time: point") is not differentiated from mean.

@aekiss
Copy link
Contributor

aekiss commented Feb 10, 2021

This is an existing cookbook issue - see COSIMA/cosima-cookbook#212 and COSIMA/cosima-cookbook#214

@navidcy
Copy link
Contributor Author

navidcy commented Feb 10, 2021

I didn't know about the methods keyword arg in getvar!

Also didn't notice that this was a duplicate of another issue. Sorry 😔

@aekiss
Copy link
Contributor

aekiss commented Feb 11, 2021

No worries, I'm glad you raised it as I'd forgotten about that issue.
AFAIK there's no methods keyword yet?

@navidcy
Copy link
Contributor Author

navidcy commented Feb 11, 2021

AFAIK there's no methods keyword yet?

Oh I see, that was a suggestion of yours... :)

@navidcy
Copy link
Contributor Author

navidcy commented Jul 30, 2021

Closed by COSIMA/cosima-cookbook#255

@navidcy navidcy closed this as completed Jul 30, 2021
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