Skip to content

Add NemoGlobalSumYearMeanTimeseries processing task #106

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

Merged
merged 10 commits into from
Feb 18, 2025

Conversation

valentinaschueller
Copy link
Collaborator

@valentinaschueller valentinaschueller commented Feb 11, 2025

Create new timeseries processing task as requested in #105. Added a simple test case and some basic documentation.

Some open questions:

  • What is a good variable to use as an example in the documentation? @vlap do you have a suggestion?
  • Will we need to change units when computing the global sum (cf. what we did in the SI3 area-weighted sum task)? If so, how? (Again, an example variable would be helpful here)
  • Depending on specific needs for this task, one could probably combine the NEMO timeseries tasks, since they differ by very few lines (comment, CellMethods, applied aggregation operation). The user-facing API should stay the same but it might be reasonable to essentially add a more generic NEMOTimeseries task with more specific implementations for variants like global sum, global mean, annual and monthly mean. Do you think it's worth it @uwefladrich ?

Create new timeseries processing task as requested in #105.
Added a simple test case and some basic documentation.
@vlap
Copy link

vlap commented Feb 11, 2025

Hi @valentinaschueller.

Thanks for taking the time to add this feature!

What is a good variable to use as an example in the documentation? @vlap do you have a suggestion?

Any of the flux variables would do. I suggest qt_oce (net downward heat flux at ocean surface). In ECE4 it is outputted into the same file as tos/sos that is used in other diagnostics. So, this should be straighforward.

Will we need to change units when computing the global sum

The flux itself is in [W/m2], and when integrating it, you should get something on the order of ~10 PW (I think). So, just scaling is fine. Units could also be converted to Jouls per year (~10^23 J/year). Not sure what the convention is.

NEMOTimeseries task with more specific implementations for variants like global sum, global mean, annual and monthly mean.

One also very useful feature would be to be able to average and plot a timeseries for a scalar. No integration, just time averaging. Nemo's standard output includes several scalar diagnostics (eg a 5-day output scalars https://git.smhi.se/ec-earth/ecearth4/-/blob/main/scripts/runtime/templates/xios/file_def_nemo-pisces.xml.j2#L15)

@valentinaschueller
Copy link
Collaborator Author

valentinaschueller commented Feb 14, 2025

Thanks for your comments and feedback, @vlap!

Any of the flux variables would do. I suggest qt_oce (net downward heat flux at ocean surface). In ECE4 it is outputted into the same file as tos/sos that is used in other diagnostics. So, this should be straighforward.

Could you or @klauswyser point me to an example output file, e.g., on Tetralith that I could use a reference here? The test data I have available is quite outdated and does not contain this variable, unfortunately.

The flux itself is in [W/m2], and when integrating it, you should get something on the order of ~10 PW (I think). So, just scaling is fine. Units could also be converted to Jouls per year (~10^23 J/year). Not sure what the convention is.

As far as I remember, Iris will not touch the units when computing the integral, so the units will stay at [W/m2] after the aggregation (but I would need to test this). So to be accurate, I would have to (a) convert to [W] and potentially (b) convert to [1e15 W] afterwards (is this what you meant by "scaling", @vlap?)

Here is a non-exhaustive list of options how to deal with this situation in the processing task:

  • do not convert the units (easy for me, bad for the user)
  • have the user provide the units in question
  • have specific mappings for specific variables/incoming units, as in the SI3 task mentioned above.

My suggestion would be that I implement a hardcoded conversion to [1e15 W] for now for the case that the incoming unit is [W/m2] and do not do any unit conversion otherwise. This should cover all surface fluxes, as long as they use this unit.

One also very useful feature would be to be able to average and plot a timeseries for a scalar. No integration, just time averaging. Nemo's standard output includes several scalar diagnostics (eg a 5-day output scalars https://git.smhi.se/ec-earth/ecearth4/-/blob/main/scripts/runtime/templates/xios/file_def_nemo-pisces.xml.j2#L15)

I am not familiar with these output variables, unfortunately. Are these output scalars on a grid (i.e., NemoGlobalMeanYearMeanTimeseries should be able to take care of them)? Or is it a single scalar that only depends on time, not space?

@klauswyser
Copy link
Contributor

No problem. Here is the output from a recent 21-yr long run: /nobackup/rossby27/users/sm_wyser/ecearth/ece-run/E486/output

@vlap
Copy link

vlap commented Feb 14, 2025

The test data I have available is quite outdated and does not contain this variable, unfortunately.

Hope you found net heat flux into the ocean (qt_oce) in the file together with tos and sos.

My suggestion would be that I implement a hardcoded conversion to [1e15 W] for now for the case that the incoming unit is [W/m2] and do not do any unit conversion otherwise. This should cover all surface fluxes, as long as they use this unit.

To keep things simple while covering the majority of use cases, you can assume that the incoming variable has "UNIT/m2". E.g. [W/m2], [kg/m2/s] or [g/m2/s] (mass flux units). If you could just remove "/m2" from the units after integration -- that's already good enough. I mentioned "scaling" by 1e15 (converting W to PW), but disregard that. It is best to not limit applications to qt_oce.

…gation

- Add a new class NemoTimeseries that can be used for creating NEMO timeseries processing tasks
- make NemoGlobalMeanYearMeanTimeseries and NemoGlobalSumYearMeanTimeseries children of said class
- adjust entry points accordingly (user-facing behavior does not change)

Change spatial weights computation:
Iris can automatically take care of unit conversion when aggregating, as long as the spatial weights are added to the cube as a CellMeasure. (cf. https://scitools-iris.readthedocs.io/en/latest/userguide/cube_statistics.html#area-averaging).
I have thus changed the spatial_weights function to add a "cell_size" as a cell measure, containing the grid area/volume
This means I do not have to multiply by m2/m3 depending on the aggregation.
It also leads to a slight simplification of the SI3 Timeseries task.

Updated the tests, I also now check that the units are changed when summing (but not when computing the global mean)
@valentinaschueller valentinaschueller marked this pull request as ready for review February 17, 2025 10:48
@valentinaschueller
Copy link
Collaborator Author

I have found a more elegant way to do it, since Iris can actually do the unit conversion itself during aggregation if the spatial weights are added as a CellMeasure. This also affects the internals of the SI3 Timeseries processing task.

@valentinaschueller
Copy link
Collaborator Author

valentinaschueller commented Feb 17, 2025

My implementation does not seem to work for Python 3.8 since it is in Iris 3.5 that they allowed to use Iris objects as aggregation weights. Let's see if it works to require iris>=3.5.
Edit: This did work, but changes code requirements. @uwefladrich let me know what you think!

@uwefladrich
Copy link
Owner

Does it mean that the upgrade to Iris 3.5 needs Python 3.9? The actual upgrade from Iris 3.1 (current requirement, released 2021/9/17) to Iris 3.5 (2023/4/27) is probably okay.

Python 3.8 is beyond end-of-life since October last year, but what do people use on their machines? Bumping requirements to Python 3.9 may have some consequences, even though it would make sense from a support perspective. Should probably be done for the other Python-stuff in ECE as well in that case. Technical WG?

@uwefladrich
Copy link
Owner

Hm, after reading Iris docs, I am not sure if/why Python 3.8 is not enough? It says Iris is currently supported and tested against Python 3.10, 3.8 and 3.9 running on Linux for that version. But you couldn't run it with Python 3.8 + Iris 3.5 or am I misunderstanding?

@valentinaschueller
Copy link
Collaborator Author

But you couldn't run it with Python 3.8 + Iris 3.5 or am I misunderstanding?

Yes I could, I just had to explicitly require the higher Iris version!

@uwefladrich
Copy link
Owner

[...] Yes I could, I just had to explicitly require the higher Iris version!

Okay, I was confused because you said "does not seem to work for Python 3.8" above. If it is just a bump for Iris from 3.1 to 3.5, I do not see much of a problem. The only issue I could think of is if Iris would come from system packages on some platforms, instead of PyPI or conda-forge. Maybe good to give a heads-up to the TWG in any case.

Otherwise, changes look fine to me. Good work! 👍

Copy link
Owner

@uwefladrich uwefladrich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well done!

I had only updated conda_environment.yml so far.
@valentinaschueller
Copy link
Collaborator Author

Okay, I was confused because you said "does not seem to work for Python 3.8" above.

Yes, sorry for the confusion! I meant that the Python 3.8 test case did not work, at that point I had not figured out the cause of the issue.

Maybe good to give a heads-up to the TWG in any case.

Pinging @klauswyser: Note that the changes in this PR yield a higher Iris requirement (from 3.1 bumped up to 3.5).

I'll merge this tomorrow morning, unless anyone vetoes before then.

@vlap
Copy link

vlap commented Feb 17, 2025

great work, Valentina! Thanks for finding neat workarounds to improve things.

@valentinaschueller valentinaschueller merged commit 43fe1dc into master Feb 18, 2025
22 checks passed
@valentinaschueller valentinaschueller deleted the feature/nemo-global-sum-timeseries branch February 18, 2025 07:32
@vlap
Copy link

vlap commented Mar 5, 2025

Also, please, take a look here: https://dev.ec-earth.org/issues/1406

There I attempted to add a couple plots with integrated fluxes and scalar timeseries. It worked technically, but I am not sure about the units.

@valentinaschueller
Copy link
Collaborator Author

Also, please, take a look here: https://dev.ec-earth.org/issues/1406

Happy to see that the timeseries is working! I had a short look and saw two things:

For both I would assume that the responsibility lies with XIOS, since the monitoring package only tries multiplying units with area/volume and uses the standard name from the output. Or do you have a reason to believe that the monitoring tasks are introducing weird artifacts? Then we should open an issue 🙂

@vlap
Copy link

vlap commented Mar 6, 2025

Yes, I was also confused by those units. I am not sure about the responsibility of XIOS.

When global mean was applied to generate the time series, it came out OK. You can check here
https://dev.ec-earth.org/issues/1401#DIC-flux-annual-mean
and here https://dev.ec-earth.org/issues/1401#Delta-CO2-annual-mean
Also, here is what the file is like:

ncdump -h a8cb_bgc_1m_diad_T_1981-1981.nc

	float Cflx(time_counter, y, x) ;
		Cflx:long_name = "DIC flux" ;
		Cflx:units = "mol/m2/s" ;
		Cflx:online_operation = "average" ;
		Cflx:interval_operation = "2700 s" ;
		Cflx:interval_write = "1 month" ;
		Cflx:cell_methods = "time: mean (interval: 2700 s)" ;
		Cflx:_FillValue = 1.e+20f ;
		Cflx:missing_value = 1.e+20f ;
		Cflx:coordinates = "time_centered nav_lat nav_lon" ;
	float Dpco2(time_counter, y, x) ;
		Dpco2:long_name = "Delta CO2" ;
		Dpco2:units = "uatm" ;
		Dpco2:online_operation = "average" ;
		Dpco2:interval_operation = "2700 s" ;
		Dpco2:interval_write = "1 month" ;
		Dpco2:cell_methods = "time: mean (interval: 2700 s)" ;
		Dpco2:_FillValue = 1.e+20f ;
		Dpco2:missing_value = 1.e+20f ;
		Dpco2:coordinates = "time_centered nav_lat nav_lon" ;

But inegration does something strange with the units. Should I open an issue then?

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

Successfully merging this pull request may close these issues.

4 participants