Skip to content

Conversation

aseyboldt
Copy link
Member

Disabling the generation of cpython wrappers should speed up compilation quite a bit.
Most of our intermediate functions are never called from python, so we don't need numba to generate wrapper code.

Note, that calling a njit function with no_cpython_wrapper directly from python (so not within a different njit function) will not work, and usually segfault.

The compilation time should further improve, if something like numba/numba#9566 lands in numba.

Copy link
Member

@ricardoV94 ricardoV94 left a comment

Choose a reason for hiding this comment

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

Sweet

@aseyboldt aseyboldt force-pushed the no-cpython-wrapper2 branch from c21e76c to 10366ad Compare May 13, 2024 17:04
Copy link

codecov bot commented May 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.83%. Comparing base (db73461) to head (945dc09).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #765   +/-   ##
=======================================
  Coverage   80.83%   80.83%           
=======================================
  Files         162      162           
  Lines       46860    46864    +4     
  Branches    11465    11465           
=======================================
+ Hits        37877    37883    +6     
+ Misses       6732     6730    -2     
  Partials     2251     2251           
Files Coverage Δ
pytensor/link/numba/dispatch/basic.py 85.74% <100.00%> (+0.06%) ⬆️
pytensor/link/numba/dispatch/elemwise.py 88.72% <100.00%> (ø)
pytensor/link/numba/linker.py 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

@aseyboldt aseyboldt merged commit 3ed2c49 into pymc-devs:main May 13, 2024
@aseyboldt aseyboldt deleted the no-cpython-wrapper2 branch May 13, 2024 19:32
@ricardoV94
Copy link
Member

How much speedup did you observe with this?

@aseyboldt
Copy link
Member Author

aseyboldt commented May 13, 2024 via email

@ricardoV94
Copy link
Member

ricardoV94 commented May 13, 2024

Compilation took a bit more than half as long with the change applied
for two models I tried. Would be curious what other models do though.

And do you expect further speed up if the numba changes get merged or this achieves mostly the same?

@aseyboldt
Copy link
Member Author

And do you expect further speed up if the numba changes get merged or this achieves mostly the same?

The model I've been using as a baseline went from 20s to 13s with this PR, and then to 8s with the numba PR. But I don't know if that is representative. (All measured hopefully without any caches).

(somehow I managed to edit a comment instead of replying, sorry)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants