-
Notifications
You must be signed in to change notification settings - Fork 300
Speed up operations that use the Coord.cells method for time coordinates
#4969
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
Conversation
|
I ran a few more experiments, and this implementation is faster if you generate more than roughly 100 cells. |
trexfeathers
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @bouweandela, looks like you've found some useful shortcuts here ❤
The minor slowdowns for the smaller cases are IMO acceptable, since these are known times for known sizes. Meanwhile the benefits of speed-up at scale get larger as the scale increases.
- I can replicate all the timing differences you report.
- I can confirm that the worse memory efficiency of a
Generatoris of little consequence - even10000-lengthCoords produceGenerators/Iterators<100B. - Functional test coverage provides assurance that this hasn't broken anything. No direct tests for
Coord.cells()(probably should be), but both_CoordConstraint.extract()andCoord.intersect()useCoord.cells()and these have plenty of coverage. - Our rudimentary benchmark suite means further coverage that will catch unexpected slowdowns for a variety of user cases (this runs nightly on any changes to
mainfrom the previous day).
Please can you add a What's New entry? Then I will merge.
|
Thanks! I added a what's new entry. |
Co-authored-by: Martin Yeo <[email protected]>
|
I've just been manually re-running some benchmarks, congratulations! |
🚀 Pull Request
Description
This speeds up functions that make use of the
Coord.cellsmethod to generate many cells describing a time coordinate. This affects for exampleCube.extract,Cube.subset, andCoord.intersection.Here is a script that demonstrates this:
Before:
After:
Note that the changes in this pull request do slow down the case where only a few cells are actually generated:
Before:
After:
but you can still use
Coord.cellif you need just a few cells.Closes #4957.
Consult Iris pull request check list