BUG: __numpy_ufunc__ passes on only a single output argument

I'm trying to use __numpy_ufunc__ for the Quantity subclass in astropy (which adds and propagates units associated with the array), and while mostly it is a joy compared to the previous __array_prepare__ etc tangle, for functions with two outputs there is a problem, in that only one output is passed on among the keyword arguments. This can be seen from the following example:

import numpy as np
class trial(np.ndarray):               
    def __new__(cls, *args, **kwargs):
        return np.array(*args, **kwargs).view(cls)
    def __numpy_ufunc__(self, ufunc, method, i, inputs, **kwargs):
        print(self, ufunc, method, i, inputs, kwargs)
        inputs = [arg.view(np.ndarray) for arg in inputs]
        return ufunc(*inputs, **kwargs)

a = trial(100)
b = np.array([1.])
c = np.array([2.])
np.modf(a, b)
#100 <ufunc 'modf'> __call__ 0 (trial(100),) {'out': array([ 1.])}
# (array([ 0.]), array([ 100.]))
np.modf(a, b, c)
#100 <ufunc 'modf'> __call__ 0 (trial(100),) {'out': array([ 2.])}
# (array([ 0.]), array([ 100.]))

@pv

This looks like a pretty serious bug in __numpy_ufunc__, since it makes it impossible for modf and other multiple-output ufuncs to work correctly with any class defining a __numpy_ufunc__ method when out= is used, and causes wrong results to be returned silently. (It's not even possible to fall back on the default implementation, because the way __numpy_ufunc__ works its the job of the called method to do this, and when this bug kicks in then the arguments are already lost before it can delegate.) I'm going to tentatively mark it as a 1.9 blocker.

CC: @pv, @cowlicks

Yep, 1.9 blocker. The case if multiple output args is ignored in the implementation. How to deal with them is also unspecified in the spec.

This is probably simple to fix, however, now that we are not bound by any backward compatibility constraints.

@pv

@pv - the docstrings of both np.modf and np.frexp start with

modf(x[, out1, out2])

This would suggest one possible choice, of having out1 and/or out2 among kwargs.

Note, though, that one cannot give these as keyword arguments in the direct call either. While np.modf(a, b, c) and np.modf(a, out=b) work, np.modf(a, out=b, out2=c) or anything equivalent does not; rather, it raises

ValueError: cannot specify 'out' as both a positional and keyword argument

ps. while trying the above, I realised the test on whether something is a keyword argument is simply whether it starts with "out"; np.modf(100.5, outblabla=a) does fine.

@pv

Iirc the clearer ufunc calling convention is out=(arr1, arr2), so that's
what we'd want to normalize to for numpy_ufunc.

The outblahblah thing sounds like a bug too, but a different one - maybe
file it separately?
On 31 Dec 2013 19:45, "Marten van Kerkwijk" [email protected]
wrote:

@pv https://github.com/pv - the docstrings of both np.modf and np.frexpstart with

modf(x[, out1, out2])

This would suggest one possible choice, of having out1 and/or out2 among
kwargs.

Note, though, that one cannot give these as keyword arguments in the
direct call either. While np.modf(a, b, c) and np.modf(a, out=b) work, np.modf(a,
out=b, out2=c) or anything equivalent does not; rather, it raises

ValueError: cannot specify 'out' as both a positional and keyword argument

ps. while trying the above, I realised the test on whether something is a
keyword argument is simply whether it starts with "out"; np.modf(100.5,
outblabla=a) does fine.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/4160#issuecomment-31417092
.

@njsmith

So in the PyUFunc_CheckOverride function we implicitly assume there is only one output here:

    /* If we have more args than nin, the last one must be `out`.*/
    if (nargs > nin) {
        obj = PyTuple_GET_ITEM(args, nargs - 1);
        PyDict_SetItemString(normal_kwds, "out", obj);
    }

I think what we actually want to do is grab all the positional arguments after the ninth one and pass them like out = (arr1, arr2, ...) (as @njsmith suggests) in the kwargs dict.

I'll try this out and get back to y'all.

edit: override was supposed to be kwargs

@njsmith

Here is how I propose we handle output variables:

one positional output variable x is passed in the kwargs dict as out : x
multiple positional output variables x0, x1, ... are passed as a tuple in the kwargs dict as out : (x0, x1, ...)
keyword output variables like out = x and out = (x0, x1, ...) are passed unchanged to the kwargs dict like out : x and out : (x0, x1, ...) respectively.
combinations of positional and keyword output variables are not supported.

Note that this does not change the proposed interface. Does this sound okay? cc @njsmith @pv

See pull 4171.

Fixed in gh-4171

Navigation Menu

Repository navigation

BUG: __numpy_ufunc__ passes on only a single output argument #4160

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

BUG: __numpy_ufunc__ passes on only a single output argument #4160

Description

Activity

njsmith commented on Dec 31, 2013

pv commented on Jan 1, 2014

mhvk commented on Jan 1, 2014

njsmith commented on Jan 1, 2014

cowlicks commented on Jan 2, 2014

cowlicks commented on Jan 4, 2014

cowlicks commented on Jan 4, 2014

pv commented on Jan 8, 2014

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions