Skip to content

Conversation

@cpelley
Copy link

@cpelley cpelley commented Nov 6, 2014

Vectorised and cached bounds checking.
15% speedup going from 30second (21600, 43200) source to N96 (145, 192)

@rhattersley
Copy link
Member

Please could you share your findings on the impact of this change - thanks.

@cpelley
Copy link
Author

cpelley commented Nov 6, 2014

In my profiling, looking at primarily a 30second (21600, 43200) source regridded to N96 (145, 192) or N216 (325, 432). The three most expensive contributors to the regrid are _cropped_bounds (PR to come), _within_bounds and ma_average calls. This accounts for ~30%, ~25% for the _cropped_bounds and _within_bounds (these figures are explicitly for regrid - I have factored out the touching of data by doing this beforehand).

Though results will depend on the src and target grids, going from the 30second to the N96 with this vectorisation and caching yields a 15% speedup.
(simply caching the _cropped_bounds call for the src points then further leads to a combined doubling of speed for regrid). Since the regrid is for orthogonal 1D coordinates, this type of optimisation is fitting.

This will make a dramatic difference to systems in place we hope to replace which perform a similar caching.

@rhattersley
Copy link
Member

Thanks @cpelley - that sets the scene nicely.

@cpelley
Copy link
Author

cpelley commented Nov 6, 2014

No problem, cheers @rhattersley

Copy link
Member

Choose a reason for hiding this comment

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

You can probably drop this comment as you've added it later.

@cpelley
Copy link
Author

cpelley commented Nov 18, 2014

Thank you for your interest @esc24, I have made the suggested changes.

@cpelley
Copy link
Author

cpelley commented Nov 18, 2014

oh dear:

$ conda update conda

Fetching package metadata: ..

Solving package specifications:

Error: maximum recursion depth exceeded in cmp

The command "conda update conda" failed and exited with 1 during .

Your build has been stopped.

Looks like a conda bug? interesting how this doesn't appear to be happening on PRs pointing to master though. Does anyone know about this or should I continue to investigate?

@rhattersley
Copy link
Member

Does anyone know about this or should I continue to investigate?

It's OK - see #1441. You just need a rebase.

@cpelley
Copy link
Author

cpelley commented Nov 21, 2014

I have cherry-picked your commit. Should be all good now thanks @rhattersley

@cpelley
Copy link
Author

cpelley commented Nov 24, 2014

Is there anything I can do to get help these PRs along? If there is any further information required, please let me know.
Currently we are working from a checkout of master with changes from v1.7.x merged into it then cherry-picking these two PRs (#1431 and #1432) in order to be able to work with our data. I fear our setup is now getting rather complex :)

esc24 added a commit that referenced this pull request Nov 24, 2014
@esc24 esc24 merged commit 91724b6 into SciTools:v1.7.x Nov 24, 2014
@cpelley
Copy link
Author

cpelley commented Nov 24, 2014

Thanks @esc24 thats great!

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