Repository navigation
BUG: no way to override matmul/@ if __array_ufunc__ is set #9028
Description
With NumPy master, it is currently impossible to override ndarray @ other if other also implements __array_ufunc__.
Consider:
import numpy as np
np.__version__ # '1.13.0.dev0+9ef5891'
class OptOut:
__array_ufunc__ = None
def __rmatmul__(self, other):
return 'rmatmul'
class OtherArray:
def __array_ufunc__(self, *args, **kwargs):
return 'array_ufunc'
def __rmatmul__(self, other):
return 'rmatmul'
array = np.arange(3)
opt_out = OptOut()
other_array = OtherArray()
# OptOut works as expected:
array * opt_out # TypeError: unsupported operand type(s) for *: 'numpy.ndarray' and 'OptOut'
array @ opt_out # 'rmatmul'
# But OtherArray does not:
array * other_array # 'array_ufunc'
array @ other_array # TypeError: Object arrays are not currently supportedThis is problematic. We definitely need a way to allow for overriding @ from the second argument, whether than means breaking the rules on __array_ufunc__ (allowing matmul to be passed even though it isn't a ufunc) or breaking the rules for __matmul__ (to call __rmatmul__ or allow __rmatmul__ to be called by Python, even when __array_ufunc__ is implemented on the other argument).
Activity
shoyer commentedon Apr 30, 2017
Probably the simplest thing to do is to make
np.matmuluse__array_ufunc__again, even though it isn't a real ufunc. It's probably less surprising than for@not be overridable if you use__array_ufunc__.njsmith commentedon Apr 30, 2017
The whole reason binops care about
__array_ufunc__is that they dispatch to ufuncs, so saying that binops that don't dispatch to ufuncs don't pay attention to__array_ufunc__seems pretty defensible. OTOH I worry that if we do that then it'll become some big breaking change if/whenmatmulstarts dispatching to a ufunc...shoyer commentedon Apr 30, 2017
Agreed, this is also totally reasonable.
__matmul__could continue the current state of affairs, deferring based on__array_priority__.Yes, this would be my concern as well. We only get one chance to introduce
__array_ufunc__for the first time.charris commentedon Apr 30, 2017
The
@operator currently forwards on__array_priority__and__array_ufunc__ = None.charris commentedon Apr 30, 2017
I think the opt-out is dual use, it basically tells arrays to "leave me alone". If arrays weren't such greedy bastards and only ate other arrays, we wouldn't need this solution.
mhvk commentedon Apr 30, 2017
Arguably easiest would be to get
matmulbecome agufunc-- it didn't seem terribly difficult, but might be less efficient.eric-wieser commentedon Apr 30, 2017
Alternatively, could we just create some
ufunc_likeclass, that exposesnoutandnin? That should cover enough cases that we can just passnp.matmulinto__array_ufunc__, even if it's not really a ufunc.mhvk commentedon Apr 30, 2017
Note that the basic matrix multiply
gufuncis already written: https://github.com/numpy/numpy/blob/master/numpy/core/src/umath/umath_tests.c.src#L143What would seem to be needed is:
@seberg, @eric-wieser - since you last looked at that file -- is this possible?
shoyer commentedon Apr 30, 2017
I think it would be great to have matmul be a
gufunc, if/when gufuncs support multiple dispatch. But I'm skeptical about dispatching to multiple gufuncs fornp.matmul.If you write
ndarray @ other, how can ndarray even know which ofmatmat_multiply,matvec_multiplyorvecmat_multiplyto pass toother.__numpy_ufunc__, without inspecting potentially opaque details aboutother? For example, ifotheris anxarray.Dataset(i.e., a dict of labeled arrays),other.ndimis not even defined.charris commentedon Apr 30, 2017
I don't think the call needs to go directly to the gufunc, there can be preparation before and after the call, the override should still work as long as there is only one call to the gufunc. And the return can be reshaped. Multiple calls would get dicey.
charris commentedon Apr 30, 2017
BTW, I don't think this needs to be in 1.13, we already have too much stuff.
mhvk commentedon Apr 30, 2017
But it would be good to solve the issue here one way or another, of not being able to override
__matmul__. My own sense would still be that separatematmat,matvecandvecmatgufuncs might still be the easiest route; they'd be useful even apart from helping solvematmul.charris commentedon Apr 30, 2017
Plus vecvec. In general, one would like to reshape (..., i, j) dot (j,) to (n,j) dot (j,) for efficiency in multiplication, and then unreshape, same with various other combinations, plus BLAS has contiguity requirements, so some preliminary work is always required. Timewise, there doesn't seem to be much difference between the special cases and reshaping and having just one function may make things easier for the user who wants to override.
charris commentedon Apr 30, 2017
Note that
@doesn't allow stacked vectors, so the functions for vec matrix cominations are still desirable, but they can be implemented using matmat, so the override should still work.14 remaining items
mhvk commentedon Apr 18, 2018
@hameerabbasi - I should perhaps have been clear that there is no reason not to implement
vecvecindependently ofmatmul. The real question here, though, is how to deal with@-- the ideal solution would quite obviously be that, like all other operators, it can dispatch directly to a ufunc.njsmith commentedon Apr 18, 2018
An option that falls short of full arbitrary multi-signature dispatch, would be to add support to gufuncs specifically for the way matmul does signatures. For example, syntax like
(n?,k),(k,m?)->(n?,m?)could mean that n and m are optional dimensions; if missing in the input, they're treated as 1, and then dropped from the output. This would allow us to express matmul, and alsonp.linalg.solve.mhvk commentedon Apr 18, 2018
Yes, that sounds like a good solution! I do somewhat worry that this (like, e.g., #5015), will need some tweaking to the ufunc structure and thus break C-API. Right now all we pass on from each separate element/character of the signature is a simple index
core_dim_ixsinto the full set of core indices (there's also the number of core dimensions for each operandcore_num_dims, an offset in the listcore_offsetand total number of elements in the signature,core_num_dim_ix).But perhaps the right way to proceed is to try and add a set of flags or so for every element (anything that's expandable...). This would allow #5015 as well.
njsmith commentedon Apr 18, 2018
We're going to have to rewrite a lot of ufunc internals anyway to progress on both the duck array and the user-defined dtypes work. A few years ago I did a lot of checking and found that there was no code on GitHub or in Debian that touched the internals of the ufunc struct, except numba and dynd. Dynd is defunct now, and we can coordinate with numba. (Probably should double check that there situation hasn't changed too.) So I think opaqueifying the ufunc internals is viable.
njsmith commentedon Apr 18, 2018
Sorry, between poor phrasing and autocorrect this came out kind of garbled. What I meant to say is, since my checking was a few years ago now, we should probably rerun it, in case any new users have appeared.
mattip commentedon Apr 19, 2018
If I understand how @njs 's proposal would play out, it means implementing
1and exposing app-level access as in3to the various internal ufunc alternatives. It indeed looks like #5015 is a related type of ufunc extension, and should be taken into consideration when implementing the necessary changes.@njs do you remember how you did the search? Whenever I try to search GitHub for NumPy C-API consumers, I end up getting many false-positives for repos that fork/copy NumPy code
njsmith commentedon Apr 19, 2018
Yeah, that's the problem :-/ Hence:
https://github.com/njsmith/codetrawl
https://github.com/njsmith/ufunc-abi-analysis
https://mail.python.org/pipermail/numpy-discussion/2015-September/073568.html
(BTW @njs is, sadly, someone else.)
mattip commentedon Apr 19, 2018
@njsmith Thanks for the tool
Did I understand your proposed changes correctly, implementing
1via a change to ufunc signatures?njsmith commentedon Apr 19, 2018
Yeah, it's (1) with the additional note that it's not adding generic "multi-signature" support, just a specific narrow form of multi-signature support.
charris commentedon Dec 4, 2018
Closing. matmul is now implemented as a ufunc. What is the status of
dot?