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

Accelerate xclim import #1135

Closed
1 task done
aulemahal opened this issue Jul 21, 2022 · 2 comments · Fixed by #1167
Closed
1 task done

Accelerate xclim import #1135

aulemahal opened this issue Jul 21, 2022 · 2 comments · Fixed by #1167
Assignees
Milestone

Comments

@aulemahal
Copy link
Collaborator

Generic Issue

  • xclim version: all
  • Python version: any
  • Operating System: any

Description

Importing xclim.sdba is currently quite slow. I think this is due to the numba utilities being pre-compiled because we give explicit input types.

What I Did

In perfimport.py:

import xclim as xc
from xclim import sdba


if __name__ == '__main__':
    print('yeah')

and then:

python -X importtime perfimport.py 2> import.log
tuna import.log

Using tuna.

What I Received

image

I believe the other long import, "xclim/indices/_conversion.py", may be due to the UTCI function, which also uses numba.

Code of Conduct

  • I agree to follow this project's Code of Conduct
@aulemahal aulemahal added this to the v0.39 milestone Jul 21, 2022
@aulemahal aulemahal self-assigned this Jul 21, 2022
@Zeitsperre
Copy link
Collaborator

Do you mean to say that this can be solved by relaxing static typing in SDBA? Is this a "feature" of numba?

@aulemahal
Copy link
Collaborator Author

Yep, this is what I read from the doc! https://numba.readthedocs.io/en/stable/user/jit.html#eager-compilation

IIRC, my goal with the static typing was avoid dtype promotions (float32 to 64). With this "eager" compilation, the tests would fail if the functions received mixed inputs, which I would then try to explicitly avoid. It may be thus be unnecessary to have them "in production". Also, a dtype promotion is not a tragic bug...

@aulemahal aulemahal modified the milestones: v0.39, v0.38 Aug 30, 2022
aulemahal added a commit that referenced this issue May 26, 2023
<!--Please ensure the PR fulfills the following requirements! -->
<!-- If this is your first PR, make sure to add your details to the
AUTHORS.rst! -->
### Pull Request Checklist:
- [ ] This PR addresses an already opened issue (for bug fixes /
features)
    - This PR fixes an issue raised verbally by `xscen` developers.
- [ ] Tests for the changes have been added (for bug fixes / features)
- [ ] (If applicable) Documentation has been added / updated (for bug
fixes / features)
- [x] CHANGES.rst has been updated (with summary of main changes)
- [x] Link to issue (:issue:`number`) and pull request (:pull:`number`)
has been added

### What kind of change does this PR introduce?

PR #1135 removed all signatures where possible, so that `@jit`-decorated
function would be compiled at run time (you know "just in time"...).
However, `@guvectorize` -decorated functions _must_ have signatures and
thus must be compiled at compile time, which means upon import.

This PR adds `cache=True` to these function, so numba compiles them once
and than saves the compilation to a `__pycache__` folder if possible.
The first import will be slow as before, but subsequent imports should
be much faster.
I'm sure there are edgecases where the user of xclim doesn't have write
access to the source folder, which might mean this caching will be
unavailable. But for most users, this should be a nice performance
boost.

On my machine I went from 13s to 2.5s.

This PR also forces `nopython=True` to these `@guvectorize` functions.
Numba 0.59 will change the default value of this parameter and it was
raising warning complaining about our lack of precision.

### Does this PR introduce a breaking change?
It shouldn't. But I do not know all the ramifications of caching these
functions...

### Other information:
@RondeauG, fyi.
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 a pull request may close this issue.

2 participants