-
-
Notifications
You must be signed in to change notification settings - Fork 151
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
Use objmode in scipy.special without numba-scipy #1078
base: main
Are you sure you want to change the base?
Conversation
c18a1b3
to
996c10c
Compare
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.
What is the cython_support
machinery for? At first glance, it looks like some low-level things that were previously handled by numba-scipy
are being brought into this project. We definitely don't want to bring that functionality here; it's better that we address such things in numba-scipy
itself, whenever possible.
d000ec3
to
a26b4a2
Compare
@brandonwillard I had the same thought at first, but I'm not really sure how this would fit anywhere into either numba_scipy or numba itself. The main complication we have, is that we don't just want exact matches between the special functions and their types, but we also want to allow some casting. So if we ask for a single precision function, we are also ok with a double precision implementation if that's the only thing that's available. A bit (but probably not too much) of the code here could probably still live in numba_scipy if we wanted, but then we'd still have to import it. I don't really like that either however: In general I'm not really in favor of the general approach of numba_scipy, to just register numba implementations for scipy functions, because this modifies global state. Just importing numba_scipy in a library can completely change the behavior of a different library, because suddenly the typing behavior of numba changes globally. |
e62943a
to
ceb5741
Compare
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #1078 +/- ##
==========================================
+ Coverage 74.12% 79.29% +5.16%
==========================================
Files 174 153 -21
Lines 48654 48099 -555
Branches 10353 10937 +584
==========================================
+ Hits 36064 38138 +2074
+ Misses 10300 7452 -2848
- Partials 2290 2509 +219
|
The point of From what I can tell, the code in |
I'll try if I can figure out how to put the improved cython support into a PR to numba itself, maybe that actually is the correct place for this. If this works out we might actually still push something to aesara in the meantime, numba just had a release and they don't seem to make releases all that often. |
|
ceb5741
to
ce1cdcc
Compare
52e1523
to
e8eed6c
Compare
Here are a few important guidelines and requirements to check before your PR can be merged:
pre-commit
is installed and set up.Replaces the numba-scipy related part in #1062.
This adds a new config option
numba_scipy
that indicates if the numba backend uses numba-scipy forscipy.special
functions.If it is not enabled or if numba_scipy can't be imported it will fall back to objectmode. This makes sure we do not end up with hard to understand numba typing errors if a user didn't install numba-scipy, but instead just a warning about objectmode and somewhat bad performance. Previously these functions would work in the test environment, because numba_scipy is installed there, but not on a normal user install, because numba_scipy is not a dependency of aesara.
I added a new function
generate_fallback_impl
, extracted from the fallbacknumba_functify
, so that we can call this from within specializations.I also changed
numba_funcify_Elemwise
a bit. Previously we would pass the node of the elemwise op to thenumba_funcify
calls of thescalar_op
. Now we create a new node if the op isn't a Composite (we don't need a op there, becauseComposite.fgraph
contains all we need).