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

Add lazy 'engine' for multicube statistics #950

Closed
wants to merge 6 commits into from

Conversation

Peter9192
Copy link
Contributor

@Peter9192 Peter9192 commented Jan 18, 2021

Description

This PR adds an alternative function for computing statistics across cubes. It relies on existing Iris functionality to concatenate cubes along a new auxiliary dimension, and then calculate the statistics across that new dimension.

This function supports lazy evaluation. It is also less error prone since much of the work is passed on to Iris. It is less flexible when the input cubes are not homogeneous. Therefore, it cannot immediately replace the existing function. However, it is very well suited for the ensemble statistics function we are developing in #673. Anticipating #744 and follow-up actions in #781, we can gradually work towards this 'engine' for multi-model statistics as well.


Before you get started

Checklist

If you make backwards incompatible changes to the recipe format:

  • N/A Update ESMValTool and link the pull request(s) in the description

To help with the number pull requests:

Separate the ESMValCore internals, dealing with products, from
the core function operating on cubes.

This makes it easier to add a new preprocessor for ensemble statistics,
and also to make a new core function (lazily) compute statistics
across cubes.
This function operates directly on iris cubes. It supports
lazy evaluation, and requires less error-prone array handling.
However, it requires the input cubes to be very homogeneous, so
for backwards compatibility I kept the old function as well.

This way we can easily test and improve the new function until
we're happy, and then deprecate the old function.
@Peter9192
Copy link
Contributor Author

@stefsmeets I think it would be good if we could add some regressions tests for this function. Do you agree, and could you help me with that?

@stefsmeets
Copy link
Contributor

@stefsmeets I think it would be good if we could add some regressions tests for this function. Do you agree, and could you help me with that?

Might as well give this the same treatment as the other 'engine' :-)

Currently the tests fail - as expected. Reasons are that
'cell methods differ' or 'coordinates (time/air pressure) differ'.

We can discuss whether this should be addressed within or outside
of the multimodel/ensemble statistics funcions.
@Peter9192
Copy link
Contributor Author

The latest commit includes tests to check whether the new engine actually works for the sample data. This is not (yet) expected, and therefore the tests are implemented with the xfail mark.

The tests fail because the cell_methods and coordinates (time and air_pressure) differ. The coord differences are mostly due to inconsistent attributes, but there are also a few inconsistencies in coordinates themselves.

I've played a bit with the iris development version, and it seems that the cell_methods and attribute errors will be solved by the new lenient cube arithmetics in iris3.

With respect to the error on mismatching coordinates, I'd like to keep those, for we are planning to make the time coordinates consistent in #744 and if some of the other coordinates are unexpectedly different, then I'd like to be notified about that.

I've made an issue as to why the tests are not failing for the inconsistent plev with the 'normal' engine (#956).

@Peter9192
Copy link
Contributor Author

PS tests are failing due to numpy not present !?

@stefsmeets
Copy link
Contributor

PS tests are failing due to numpy not present !?

I had the same. I think it is related to #951. Merging master should fix it.

@Peter9192 Peter9192 changed the title Add Iris 'engine' for multicube statistics calculations Add lazy 'engine' for multicube statistics Jan 21, 2021
@valeriupredoi
Copy link
Contributor

cheers @Peter9192 !OK am seeing this fully includes #949 so let's finalize that one first before revieweing this one, I've added a couple comments there 👍

@Peter9192
Copy link
Contributor Author

Peter9192 commented Jan 28, 2021

Abandoning this PR in favour of #968

@Peter9192 Peter9192 closed this Jan 28, 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

Successfully merging this pull request may close these issues.

3 participants