Repository navigation
Fix for issue 7402: implement missing numpy ufunc interface - #7403
Conversation
|
@guilhermeleobas Many thanks for the PR! There are a few other members / properties that ufuncs have that dynamic gufuncs do not, including:
Does it make sense to add / pass through any of these too? |
gmarkall
added
4 - Waiting on author
stuartarchibald
left a comment
stuartarchibald
left a comment
There was a problem hiding this comment.
Thanks for the patch, think this needs the noted tests (in comments), else looks good.
| @property | ||
| def signature(self): | ||
| return self.ufunc.signature | ||
|
|
||
| @property | ||
| def accumulate(self): | ||
| return self.ufunc.accumulate | ||
|
|
||
| @property | ||
| def at(self): | ||
| return self.ufunc.at | ||
|
|
||
| @property | ||
| def outer(self): | ||
| return self.ufunc.outer | ||
|
|
||
| @property | ||
| def reduce(self): | ||
| return self.ufunc.reduce | ||
|
|
||
| @property | ||
| def reduceat(self): | ||
| return self.ufunc.reduceat |
There was a problem hiding this comment.
I think it'd be a good idea to add tests for these properties such that future refactors don't reintroduce this bug or similar.
gmarkall
left a comment
gmarkall
left a comment
There was a problem hiding this comment.
The tests test that dynamic GUFuncs export the attributes, but they don't test that they do the right thing. It's not obvious to me that just because they're exported, they do the right thing - what happens if they are used with a data type not encountered previously?
|
Done! I've added code that tests the exposed properties.
A new version of the gufunc will be compiled. But the attributes exposed are not related a guvectorized function being dynamic or not. |
stuartarchibald
added
4 - Waiting on reviewer
|
Hey @gmarkall, could you take a look at this PR when you have some cycles to spare? Thanks :) |
gmarkall
left a comment
gmarkall
left a comment
There was a problem hiding this comment.
Should the following work?
from numba import guvectorize
import numpy as np
@guvectorize("(),()->()")
def gufunc(x, y, res):
res[0] = x + y
a = np.array([1, 2, 3, 4])
b = a * 2
res = np.zeros_like(a)
gufunc(a, b, res)
x = np.arange(24).astype(np.float64)
y = x * 2
gufunc.reduce(x)This gives me:
Traceback (most recent call last):
File "/home/gmarkall/numbadev/issues/7403/repro3.py", line 17, in <module>
gufunc.reduce(x)
TypeError: No loop matching the specified signature and casting was found for ufunc gufunc
Also, at seems to have a problem. The following:
from numba import guvectorize
import numpy as np
@guvectorize("(n)->(n)")
def gufunc(x, res):
acc = 0
for i in range(x.shape[0]):
acc += x[i]
res[i] = acc
a = np.array([1, 2, 3, 4])
res = np.array([0, 0, 0, 0])
gufunc(a, res)
x = np.arange(24).reshape(4, 6)
y = x * 2
gufunc.at(x, [0, 0], y[0, :])gives a segfault:
$ gdb --args python repro2.py
(gdb) run
Program received signal SIGSEGV, Segmentation fault.
0x00007fffd9bed1a6 in __gufunc__._ZN8__main__10gufunc$241E5ArrayIxLi1E1A7mutable7alignedE5ArrayIxLi1E1A7mutable7alignedE ()
(gdb) bt
#0 0x00007fffd9bed1a6 in __gufunc__._ZN8__main__10gufunc$241E5ArrayIxLi1E1A7mutable7alignedE5ArrayIxLi1E1A7mutable7alignedE ()
#1 0x00007fffef4ecf3c in ufunc_at ()
from /home/gmarkall/miniconda3/envs/numbanp120/lib/python3.9/site-packages/numpy/core/_multiarray_umath.cpython-39-x86_64-linux-gnu.so
#2 0x00005555556deb88 in cfunction_call () at /tmp/build/80754af9/python-split_1617985467049/work/Objects/methodobject.c:548
#3 0x0000555555697d84 in _PyObject_MakeTpCall.localalias.3 () at /tmp/build/80754af9/python-split_1617985467049/work/Objects/call.c:191
...```
No, the above shouldn't work. Compilation is triggered by calling the function, and
I don't think this is an issue introduced in this PR. The same code crashes on Numba 0.53, prior to #5938 |
sklam
changed the title
|
I can confirm that segfault issue is a numpy problem. For any gufunc that has a non-scalar signature, it will the import numpy as np
import numpy.core.umath_tests as ut
x = np.arange(16).reshape(4, 4)
y = x * 2
print(ut.matrix_multiply.signature)
ut.matrix_multiply.at(x, (), y)will segfault. The reason is that numpy from numba import guvectorize
import numpy as np
@guvectorize("(x)->(x)")
def gufunc(x, res):
acc = 0
print(x.shape, x.strides)
res[0] = x[0]
a = np.array([1, 2, 3, 4])
res = np.array([0, 0, 0, 0])
gufunc(a, res)
x = np.arange(24).reshape(4, 6)
y = x * 2
gufunc.at(x, [0, 0], y[0, :]) # this will show 0xdeadbeef (3735928559) for shape and strides |
sklam
left a comment
sklam
left a comment
There was a problem hiding this comment.
Thanks for the patch!
This patch will fix the regression of missing ufunc interface. However, requiring an invocation to GUFunc.__call__ before a GUFunc.at can be used in the case of dynamic gufunc is not very user friendly. There needs to be a follow up PR to add automatic compiling of the required signature.
|
@gmarkall, do you agree with #7403 (review) or have other concerns? |
gmarkall
left a comment
gmarkall
left a comment
There was a problem hiding this comment.
Looks good now, though I agree with #7403 (review).
gmarkall
added
5 - Ready to merge
|
#7590 tracks the remaining task. Thanks for the patch @guilhermeleobas and reviews @gmarkall @sklam. |
stuartarchibald
added
Effort - medium
5 participants
As title. cc @gmarkall