-
Notifications
You must be signed in to change notification settings - Fork 89
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
fix: return ufunc as it is for user defined vectorize
d funcs
#3025
fix: return ufunc as it is for user defined vectorize
d funcs
#3025
Conversation
0ad9a14
to
9b39852
Compare
jax.numpy
while wrapping vectorize
d funcs as ufuncsvectorize
d funcs
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files
|
@Saransh-cpp this looks good to me! I anticipate problems if the custom ufunc doesn't understand JAX arrays, but I suppose for now we can try the ufunc to see what happens. In future, we might want some kind of mechanism to associate e.g. a numba ufunc with a JAX-aware overload, but that can be a later step. |
I see, this makes sense. Looking at this again, shouldn't this be on the user side? If I understand correctly, shouldn't a user ensure that their functions are compatible with JAX arrays if they are planning to use the jax backend? |
Yes I think so. As it stands, I'm not sure what that mechanism would be. It would be nice if a user can leverage someone-else's code, and patch their ufuncs to make JAX work, for example. |
Oh I see, yes that would be nice too. I can look into this if any issues pop up in the future. Thanks! |
What's the status of this one? |
The fix works! Jax and numba might still clash but that would be on the user side and not on the awkward side. But as mentioned by @agoose77, I'll look into providing users with an awkward-fix (maybe a decorator?) for their functions if we get any complaints. |
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.
Okay, great! And it's such a simple change on the Awkward side, with a test, that I'll just merge it now. Thanks!
Fixes #2603