8000 Support `__rlshift__` and `__rrshift__` · Issue #58121 · pytorch/pytorch · GitHub
[go: up one dir, main page]

Skip to content

Support __rlshift__ and __rrshift__ #58121

8000
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
asi1024 opened this issue May 12, 2021 · 7 comments
Closed

Support __rlshift__ and __rrshift__ #58121

asi1024 opened this issue May 12, 2021 · 7 comments
Labels
feature A request for a proper, new feature. module: python array api Issues related to the Python Array API triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module

Comments

@asi1024
Copy link
Contributor
asi1024 commented May 12, 2021

🚀 Feature

Support torch.Tensor.{__rlshift__, __rrshift__}.
(cc: @mruberry, @rgommers, @emcastillo and @kmaehashi)

Motivation and Pitch

To enhance the compatibility with NumPy’s interface (c.f. #38349).

>>> 1 << numpy.array([1, 2, 3])
array([2, 4, 8])
>>> 1 << torch.tensor([1, 2, 3])
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: unsupported operand type(s) for <<: 'int' and 'Tensor'

References

@mruberry mruberry added feature A request for a proper, new feature. module: python array api Issues related to the Python Array API triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module labels May 12, 2021
@mruberry
Copy link
Collaborator

Thanks @asi1024. I think we would accept PRs for each of these functions.

@asi1024
Copy link
Contributor Author
asi1024 commented Jun 3, 2021

@mruberry I'm planning to support also function interface, but NumPy has a different interface from what Python array API specifies. Which policy of interface should we follow? Or should we support both interfaces?

Python Array API: bitwise_left_shift
NumPy API reference: numpy.left_shift

@mruberry
Copy link
Collaborator
mruberry commented Jun 3, 2021

Hey @asi1024! Great question. We want to implement the Python Array API, but it's OK to have extra keyword arguments, like an out= parameter, that make the function more consistent with other PyTorch functions.

@asi1024
Copy link
Contributor Author
asi1024 commented Jun 3, 2021

My concern is about the difference in function names. NumPy supports left_shift ufunc, but Python array API specifies bitwise_left_shift 😢

@mruberry
Copy link
Collaborator
mruberry commented Jun 4, 2021

Let's ask @rgommers if the difference is intentional. In Python it's also "left shift," so I'm a little surprised the Python Array API differs from it. Maybe an oversight?

We can also alias bitwise_left_shift to left_shift in the future.

@rgommers
Copy link
Collaborator
rgommers commented Jun 7, 2021

Yes, this is on purpose, see comments in: data-apis/array-api#54. NumPy was a bit inconsistent here, prepending bitwise_ to the four other bit-wise functions but not left/right-shift. TensorFlow was more consistent, and the stdlib operator used yet again different names than NumPy, so the consensus was to go with a consistent bitwise_ prepended set of names here.

@mruberry
Copy link
Collaborator
mruberry commented Jun 7, 2021

Note that PyTorch already has some shift functionality:

- func: __lshift__.Scalar(Tensor self, Scalar other) -> Tensor

but it doesn't appear to be documented.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature A request for a proper, new feature. module: python array api Issues related to the Python Array API triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants
0