8000 BUG cannot specify second output to ufunc as keyword argument · Issue #4752 · numpy/numpy · GitHub
[go: up one dir, main page]

Skip to content
8000

BUG cannot specify second output to ufunc as keyword argument #4752

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

Closed
mhvk opened this issue May 28, 2014 · 12 comments · Fixed by #5621
Closed

BUG cannot specify second output to ufunc as keyword argument #4752

mhvk opened this issue May 28, 2014 · 12 comments · Fixed by #5621

Comments

@mhvk
Copy link
Contributor
mhvk commented May 28, 2014

Currently, it does not seem possible to pass on an array to catch the second output of a ufunc via a keyword parameter:

a1 = np.array([1.])
a2 = np.array([1.])
np.modf(1.333, out=a1, out2=a2)
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-4-9ca83f6d80bd> in <module>()
----> 1 np.modf(1.333, out=a1, out2=a2)

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

One can do np.modf(1.333, out=a1), which works as expected. If one gives instead np.modf(1.333, out2=a2), the first part is assigned to a2. (Note that the documentation suggests to use out1, out2, but any characters after out simply seem to be skipped.)

It would seem good to have the option of giving these as keyword arguments. One solution might be to have it be consistent with the usage in __numpy_ufunc__, i.e., one would give np.modf(values, out=(a1, a2)).

@jaimefrio
Copy link
Member

Currently the only way to specify multiple output arguments is as positional arguments. Using your example:

>>> np.modf(1.3333, a1, a2)
(array([ 0.3333]), array([ 1.]))
>>> a1
array([ 0.3333])
>>> a2
array([ 1.])

To add a different behavior (such as the tuple of arrays passed as a kwarg with out=) would require discussion on the list.

@njsmith
Copy link
Member
njsmith commented Oct 17, 2014

Yeah, we should discuss it on the list, but everyone will be in favor :-)
out=(out1, out2) just makes sense.

On Fri, Oct 17, 2014 at 2:42 AM, Jaime notifications@github.com wrote:

Currently the only way to specify multiple output arguments is as
positional arguments. Using your example:

np.modf(1.3333, a1, a2)
(array([ 0.3333]), array([ 1.]))
a1
array([ 0.3333])
a2
array([ 1.])

To add a different behavior (such as the tuple of arrays passed as a kwarg
with out=) would require discussion on the list.


Reply to this email directly or view it on GitHub
#4752 (comment).

Nathaniel J. Smith
Postdoctoral researcher - Informatics - University of Edinburgh
http://vorpus.org

@jaimefrio
Copy link
Member

I'm sure someone will not like the color of the bike shed... Just pinged the list, lets see if anyone cares. We should take advantage of me having the code of ufunc_object.c still fresh in memory.

@mhvk
Copy link
Contributor Author
mhvk commented Feb 27, 2015

@jaimefrio - this has been quite a while, but maybe you are still well-versed enough in ufunc_object.c to make the dual-output case work? I ilke how your suggestion of ufunc(<inputs>, out=tuple(outputs)) jives with what one gets passed on with __numpy_ufunc__.

@jaimefrio
Copy link
Member

It is certainly no longer in the L1 cache, but I will try to find time to take a look again this weekend. If I can make sense of it I'll try pinging the list again: if I remember correctly, there wasn't a single response from the list last time. :-(

@njsmith
Copy link
Member
njsmith commented Feb 27, 2015

I think this is one of those cases where silence can be read as consent.
It's not exactly a hot-button controversial change.
On Feb 27, 2015 1:46 PM, "Jaime" notifications@github.com wrote:

It is certainly no longer in the L1 cache, but I will try to find time to
take a look again this weekend. If I can make sense of it I'll try pinging
the list again: if I remember correctly, there wasn't a single response
from the list last time. :-(


Reply to this email directly or view it on GitHub
#4752 (comment).

@charris
Copy link
Member
charris commented Feb 27, 2015

Seems reasonable to me also.

@jaimefrio
Copy link
Member

I have it working, but am getting this error when I run the tests:

ERROR: test_out_subok (test_umath.TestOut)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/jaimefrio/open_source/numpy/build/testenv/lib/python2.7/site-packages/numpy/core/tests/test_umath.py", line 65, in test_out_subok
    r1, r2 = np.frexp(d, out=o1, subok=b)
TypeError: 'out' must be an array or tuple of arrays

The way I have it set up, it only accepts a single array to out (as opposed to a tuple) if it only has one return. I guess I'll have to modify it so that you can set the first output array regardless of how many returns the ufunc has. Question is, do we want to keep this behavior forever? Or should we deprecate it?

@njsmith
Copy link
Member
njsmith commented Mar 1, 2015

Should probably check with the list, but I doubt there are many people who
will be affected either way.

There is a functionality benefit to allowing some out= arguments to be
unspecified; someone might want to supply some and let the ufunc allocate
the others.

The current mechanism for this is ugly though, because you can provide the
first out= arg while allocating the second, but not vice-versa.

For the new api, maybe we should require that if a tuple is given, the
tuple must have nout entries, but any subset of them can be None, meaning
that entry should be allocated?

In the mean time, I guess we should still allow out=arr to be equivalent to
out=(arr, None, None, ...) for backwards compatibility. In my mind this is
closely linked to how we allow stuff like frexp(a, b, c) using positional
arguments, and there's a ton of code out there relying on positional output
arguments.

Possibly we should deprecate both positional output arguments and the use
of out=arr for multi-output ufuncs, with no plan to actually remove it, but
just to officially nudge people to the cleaner and more readable kwarg api.
That could be a separate discussion though...
On Mar 1, 2015 2:01 PM, "Jaime" notifications@github.com wrote:

I have it working, but am getting this error when I run the tests:

ERROR: test_out_subok (test_umath.TestOut)

Traceback (most recent call last):
File "/Users/jaimefrio/open_source/numpy/build/testenv/lib/python2.7/site-packages/numpy/core/tests/test_umath.py", line 65, in test_out_subok
r1, r2 = np.frexp(d, out=o1, subok=b)
TypeError: 'out' must be an array or tuple of arrays

The way I have it set up, it only accepts a single array to out (as
opposed to a tuple) if it only has one return. I guess I'll have to modify
it so that you can set the first output array regardless of how many
returns the ufunc has. Question is, do we want to keep this behavior
forever? Or should we deprecate it?


Reply to this email directly or view it on GitHub
#4752 (comment).

@mhvk
Copy link
Contributor Author
mhvk commented Mar 2, 2015

Agreed with @njsmith - converting to out=(array, None) would seem most logical. I'm not even sure it is worth deprecating -- most important might be to ensure the documentation is clear.

@jaimefrio
Copy link
Member

I'll make a PR even if it's half cooked only, and we can have the discussion over the code, which will probably explain itself better than me. My current implementation already does what @njsmith proposes: tuple of nout elements, but None is a perfectly valid entry of that tuple.

@mhvk
Copy link
Contributor Author
mhvk commented Mar 8, 2015

Thanks, @jaimefrio!

683A

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants
0