-
Notifications
You must be signed in to change notification settings - Fork 4
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
BUG: fix nout=2 ufuncs #109
Conversation
torch_np/_ufuncs.py
Outdated
out1, out2 = out | ||
if out1.shape != out2.shape or out1.dtype != out2.dtype: | ||
raise ValueError("out1, out2 must be compatible") | ||
if num_outs == 2: | ||
if out1 is not None or out2 is not None: | ||
raise TypeError( | ||
"cannot specify 'out' as both a positional and keyword argument" | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't thin this logic is correct. If num_outs == 2
and out = (None, None)
, here we are setting setting out1
and out2
to None
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, thanks for spotting this! Fixed in a855a37. The relevant test needs to stay xfailed for a while, since we have hardcoded the name "out" for postprocessing.
The current state is that out=tuple
works, and there is an xfailed test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice. Note that before, since we were not using normalize
, we were not casting x1 and x2 here. I guess that that happened to work because this function is a composite of other functions, but yeah...
Let's get this in then --- am adding Indeed, not calling normalize was when composing normalize-decorated functions. Which is indeed brittle and hopefully not happening anymore with the final version of the wrapper/impl split. |
A minimal fix for a bug in handling of
out=(ndarray, ndarray)
.