-
-
Notifications
You must be signed in to change notification settings - Fork 12k
Improve small reduction performance #4186
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
Conversation
|
This does appear to be bug with static PyObject *
ufunc_reduce(PyUFuncObject *ufunc, PyObject *args, PyObject *kwds)
{
int errval;
PyObject *override = NULL;
errval = PyUFunc_CheckOverride(ufunc, "reduce", args, kwds, &override,
ufunc->nin);
if (errval) {
return NULL;
}
else if (override) {
return override;
}
return PyUFunc_GenericReduction(ufunc, args, kwds, UFUNC_REDUCE);
} |
|
Redo Travis, IIRC there have been some |
|
If there have been fixes, I think it may need to be rebased to run the new tests? |
|
Well, it still fails... |
|
Restarted Travis tests. |
|
Could use a rebase, Yeah. |
|
OK, checked a rebased version and Looks fixable. |
|
What is the correct way to get the number of arguments for ufunc reductions? I'm looking for something like If there isn't currently a way to do this. Is there a precise definition of |
|
In So I think we'll have to add a special case for each reduction method so its arguments get parsed correctly. |
|
reduce and accumulate both exist only if nin==2, and they always take outer I guess acts like call reduceat IIRC will always take 2 array arguments, and only exists for On Tue, Apr 1, 2014 at 10:58 PM, Blake Griffith [email protected]:
Nathaniel J. Smith |
PyArg_ParseTupleAndKeywords is pretty slow for keywords as it needs to create python strings first. Using positional arguments avoids this and gains 15-20% performance for small reductions.
|
this can now go in, I think its still useful for 1.9 to fit the theme of faster small array operations |
|
LGTM, thanks Julian. |
Improve small reduction performance
See commit messages for some details.
__numpy_ufunc__string attribute lookup on pure python types which are very expensivenp.sumetc. Non keyword arguments parse a lot faster with the regular python function. This is an alternative to ENH: speed up ufunc reduce argument parsing #4174 which improves keyword parsing in general but has higher complexity. The keyword arguments one of the major reasonsnp.sumis so much slower thannp.add.reduce(4us vs 2.5 us).The last point fails a
__numpy_ufunc__test which I think is a bug in the logic of the override.The override assumes all non input and output arguments are passed in via keyword arguments and then uses the ufunc number of inputs to consider all following arguments as output.
The last commit breaks both assumptions,
outis not provided via a keyword and the reduction only has one input and not two as the non reduction types.The effect is that you get a tuple
[dtype, out, keepdims]as the out argument of the ufunc (axis is missing as nin is not treated right for reduce).This looks to me like a bug, but I'm not very familiar with the override, please advise.