Repository navigation
MAINT: post __array_ufunc__ merger to-do list #8892
Description
EDITED to update with items from @eric-wieser and @shoyer. Feel free to edit to add other items.
A reminder of clean-up after #8247 that could most usefully be done:
-
Currently, arguments in ufuncs are normalized in
get_ufunc_arguments(inufunc_object.c), well after a check for an override is done; the latter code separately normalized arguments inoverride.c. Obviously, it would be better if only one of those was done (to the extend possible). As part of this, the type of exception raised should also be made consistent with python, i.e.,TypeErrorfor too many/too few arguments, things occurring both as positional and keyword arguments, etc.) -
BUG:
__array_ufunc__on arguments likeaxiswould also override the ufunc (TST: __array_ufunc__ cannot be overridden on axis, etc. #9107). -
np.dotandnp.matmulare no longer overridden, but it would be good to overridenp.matmulin particular (as it implements a binop). This would need (1) turnnp.matmulinto a generalized ufunc (or have it use a new gufunc, after prepending/appending of axes as appropriate); (2) use the same generalized ufunc where feasible insidenp.dot(andndarray.dot), so that the most common cases can be overridden (for other cases, we might want to error on the presence of__array_ufunc__). -
(@eric-wieser) Set the
__name__field of the generatedNDArrayOperatorsMixinmethods (MAINT: Set the __name__ of generated methods #9012) -
(@shoyer) Need to switch ufunc overrides to always raise
TypeErrorif any arguments set__array_ufunc__ = None(Ufunc overrides should always raise TypeError if any arguments set __array_ufunc__ = None #9011) -
(@shoyer) Choose the recommended way of implementing binary ops (calling the ufunc directly, or calling
__array_ufunc__), and clarify guidance in the NEP (DOC: update binary-op / ufunc interactions and recommendations in ufunc overrides NEP #9027) -
(@shoyer) Backwards compatible mixin like NDArrayOperatorsMixin as a separate project (Backwards compatible mixin like NDArrayOperatorsMixin as a separate project #9016).
-
(@shoyer) BUG: no way to override matmul/@ if
__array_ufunc__is set (BUG: no way to override matmul/@ if __array_ufunc__ is set #9028) -
(@mhvk) BUG: no way to override
__eq__for structuredndarray(BUG: ndarray.__eq__ does not respect overrides for void #9153) -
ENH/MAINT: let
ndarray.__array_ufunc__be the overall gateway to applying ufuncs onndarray; that way, this function can just participate in the different trials of objects to see if they can handle the ufunc. As part of that, should they just turn subclasses into regular arrays (since those have had a chance anyway? see discussion in Misleading __array_ufunc__ error #9079) -
(@shoyer) Document the table of ndarray arithmetic special methods and corresponding ufuncs in the proper docs (e.g.,
reference/array.classes.rst, not the NEP).
Activity
[-]MAINT: ufunc argument normalization should be unified (post __array_ufunc__)[/-][+]MAINT: post __array_ufunc__ merger to-do list[/+]mattip commentedon Apr 10, 2018
issue #9028 is marked for 1.15. Are there any other of these tasks that should also be in 1.15?
mhvk commentedon Apr 10, 2018
@mattip - the list is fairly complete, and all should at some point be solved... But I think the
__matmul__one is by far the most annoying/urgent..mattip commentedon Dec 12, 2018
updated - matmul is now a ufunc
seberg commentedon Dec 13, 2019
I am not sure about this, is it considered a bug that
__array_ufunc__currently will pass on additional keywords (not supported by numpy) happily, or is that intentional?mhvk commentedon Dec 13, 2019
I'd think it is a bug and (too implicitly) part of the first item above.
seberg commentedon Dec 13, 2019
Looking at faster argparsing, I was starting to clean that up a bit (hopeully actually making it cleaner). That would "fix" that, but was not sure if I would have to add a cludge for it. Unfortunately it will be one hell of a merge conflict with my dtypes branch, but overall maybe it will make things easier anyway, since that needs a lot of cleanup as well.
mhvk commentedon Dec 13, 2019
This is something I'm still interested in 😄 - possibly it can be made easier by aiming at the new fast call interface in python 3.8 (still private, but good for experiments; https://docs.python.org/3/whatsnew/3.8.html#vectorcall-a-fast-calling-protocol-for-cpython
p.s. My abandoned speed-up attempts are in #11372. Possibly individual commits are still useful. It definitely was a big mistake to try to do many things at once, though...
seberg commentedon Dec 13, 2019
@mhvk yes, I am doing that, although my code right now has
#ifdefto support both. If python is much faster on its side, it may help, otherwise I am not so sure. kwargs are pretty rare for usual ufunc calls (but very common forreduce). So for usual ufunc calls, my hope is currently mainly deleting some code duplication.Anyway, halfway there, the review/some cleanup will be a pain though.
seberg commentedon Dec 13, 2019
@mhvk geee, I did not realize that bunch of code/changes from you were lurking around. Will have a look at it, since you seemed to have gotten something out here, probably there are some thing I can copy over easily.
seberg commentedon Dec 16, 2019
@mhvk I realize that I need to clean it up a bit, but would you be willing to take some look at the ufunc side of trying to get
METH_FASTCALLat some point? (andvectorcall, but I am not sure it is really all that much worth it for now.ufunc.__call__is often called without keywords I think, and it has a pretty high overhead in any case).It is a pretty huge diff though, so I can understand if its too much :/
An initial diff is here (I need to clean it out, and maybe squash it). Some of the things I did:
objectinitiallyfull_args(tuples of input and output arguments)full_argstuples inside the overrides, so that it does not need to do any parsing/dict manipulation (unless one of the inputs/outputs supports__array_ufunc__).This also moves the argument parsing to the initial
ufunc_generic_callbasically, and what you did also, moves the split betwen gufuncs up to there. (I am not 100% positive I already wrote the shim for the oldPyUfunc_GenericFunction, which is an annoying C-API, that nobody uses.https://github.com/seberg/numpy/compare/2d5b87e9f4837b70c068e832bd347b6b454e3bde..24ecc53f4d7a704d3ad0473d04550ba4539dabe6
mhvk commentedon Dec 29, 2019
@seberg - I had an initial look and very much like the simplification in the processing order! I see you circumvented the annoyance I had of some functions being in the API which really are not handy any more (but likely are never used...).
One note on keyword arguments: if they were parsed as quickly as positional, it would actually become useful to pass
outto ufuncs, which is much more readable than positional.seberg commentedon Jan 2, 2020
Thanks for having a look. I will split out the things into 3 (or 4) PRs, it hangs a bit on us being OK to duplicate some of the work that C-Python also semi-privdes with argument parsing.
Using kwargs are still quite a bit slower, but the speed difference is much smaller than before. For module level functions though, nothing much changes:
__array_function__just adds too much overhead and cannot leverageMETH_FASTCALL.seberg commentedon Mar 23, 2021
Marked first two as done, since gh-15271 covers them.
seberg commentedon May 20, 2022
Going to close this, I don't think keep this open serves much purpose anymore:
__eq__for voids, should not likely be solved by making void comparisons a ufunc (non-trivial, but I still think that is probably the right way)__array_ufunc__entry-point may be interesting, but I am not sure and keeping it around here won't make it happen.mhvk commentedon May 20, 2022
Yes, makes sense to close - the one that is still a buglet has its own issue anyway.