Return NotImplemented from tensor arithmetic operators by peterbell10 · Pull Request #26507 · pytorch/pytorch

peterbell10 · 2019-09-19T22:25:29Z

This effectively rewrites all infix arithmetic operators op as:

def __op__(self, other):
    try:
        return self.op(other)
    except TypeError:
        return NotImplemented

Where TypeError is raised from the argument parser when other is not a Tensor. This should be okay, so long as TypeError isn't raised by the function for any other reasons. I couldn't find any examples where this was an issue.

ezyang

Nifty!

ezyang · 2019-09-20T17:27:23Z

@pytorchbot rebase this please

ezyang · 2019-09-20T17:27:30Z

Waiting on CI

pytorchbot · 2019-09-20T17:27:30Z

Sorry, I can't merge this because there are conflicts. To merge this yourself, run the commands below:

git fetch origin master
git fetch git@github.com:Quansight/pytorch.git peterbell10/operator-NotImplemented
git checkout FETCH_HEAD
git merge origin/master
git push git@github.com:Quansight/pytorch.git HEAD:peterbell10/operator-NotImplemented

(To learn more about this bot, see Bot commands.)

ezyang · 2019-09-20T21:14:11Z

@pytorchbot rebase this please

pytorchbot · 2019-09-20T21:14:18Z

Sorry, I can't merge this because there are conflicts. To merge this yourself, run the commands below:

git fetch origin master
git fetch git@github.com:Quansight/pytorch.git peterbell10/operator-NotImplemented
git checkout FETCH_HEAD
git merge origin/master
git push git@github.com:Quansight/pytorch.git HEAD:peterbell10/operator-NotImplemented

(To learn more about this bot, see Bot commands.)

ezyang · 2019-09-20T21:14:41Z

Gosh, this PR conflicts a lot for some reason.

ezyang · 2019-09-20T21:15:53Z

Oh, you didn't grant maintainers rights to update the PR, so the bot can't update it. Could you please rebase again to clear up the errors?

peterbell10 · 2019-09-20T21:29:28Z

Oh, you didn't grant maintainers rights to update the PR, so the bot can't update it.

That's odd, "Allow edits from maintainers." is shown a selected on my screen. Could it be an issue with submitting from the Quansight repo?

cc @rgommers

rgommers · 2019-09-21T04:02:03Z

That's odd, "Allow edits from maintainers." is shown a selected on my screen. Could it be an issue with submitting from the Quansight repo?

Weirdly enough, I think it is. Just tried on PR 25629 and that failed as well.

Our repository settings are a bit unusual, I'll look into it.

Tried adding pytorchbot as a collaborator to fix it, but that requires it to accept an invitation:

I'll try to look for a more structural fix, because if it is repository settings then it will happen on other repos as well.

ezyang · 2019-09-23T13:39:30Z

I had pytorchbot accept the invitation.

ezyang · 2019-09-23T13:39:43Z

@pytorchbot rebase this please

pytorchbot · 2019-09-23T13:39:50Z

Sorry, I can't merge this because there are conflicts. To merge this yourself, run the commands below:

git fetch origin master
git fetch git@github.com:Quansight/pytorch.git peterbell10/operator-NotImplemented
git checkout FETCH_HEAD
git merge origin/master
git push git@github.com:Quansight/pytorch.git HEAD:peterbell10/operator-NotImplemented

(To learn more about this bot, see Bot commands.)

ezyang · 2019-09-23T13:40:17Z

Didn't work :/

rgommers · 2019-09-23T14:02:45Z

Hmm, then I have no idea. Will have to do some trial and error on a dummy repo. @peterbell10 for next PRs I suggest to open them from your own fork until we figure this out.

facebook-github-bot

@ezyang is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

facebook-github-bot · 2019-09-30T19:05:50Z

@ezyang merged this pull request in 585a597.

gchanan · 2019-10-01T18:19:47Z

should we be returning NotImplemented for the inplace functions? That doesn't seem correct.

Also, this doesn't work fork for functions that are written in native_functions.yaml and aren't explicitly bound.

ezyang · 2019-10-02T15:13:54Z

The docs specify that it is valid for inplace functions too:

Special value which should be returned by the binary special methods (e.g. eq(), lt(), add(), rsub(), etc.) to indicate that the operation is not implemented with respect to the other type; may be returned by the in-place binary special methods (e.g. imul(), iand(), etc.) for the same purpose. Its truth value is true.

Rereviewing this diff, I regret not asking for tests though...

Summary: Fixes #26333 Fixes the operators missed in #26507 and includes a test for all operators. Pull Request resolved: #27423 Differential Revision: D17835390 Pulled By: ezyang fbshipit-source-id: 7a1351c7ccc8ad11454dbaa00d3701dcee4f06a8

Summary: Fixes pytorch#26333 This effectively rewrites all infix arithmetic operators *op* as: ```python def __op__(self, other): try: return self.op(other) except TypeError: return NotImplemented ``` Where `TypeError` is raised from the argument parser when `other` is not a `Tensor`. This should be okay, so long as `TypeError` isn't raised by the function for any other reasons. I couldn't find any examples where this was an issue. Pull Request resolved: pytorch#26507 Differential Revision: D17669097 Pulled By: ezyang fbshipit-source-id: 2c1a1087057c9298915d713d3fea7d682d014c72

peterbell10 added the open source label Sep 19, 2019

peterbell10 requested a review from ezyang September 19, 2019 22:25

pytorchbot added module: internals Related to internal abstractions in c10 and ATen module: pybind Related to our Python bindings / interactions with other Python libraries labels Sep 19, 2019

peterbell10 force-pushed the peterbell10/operator-NotImplemented branch from 4304869 to 720f0ab Compare September 20, 2019 12:39

ezyang approved these changes Sep 20, 2019

View reviewed changes

peterbell10 force-pushed the peterbell10/operator-NotImplemented branch from 720f0ab to 2024e0c Compare September 20, 2019 18:30

peterbell10 force-pushed the peterbell10/operator-NotImplemented branch from 2024e0c to ec97fe6 Compare September 20, 2019 21:29

Return NotImplemented from tensor arithmetic operators

7268b60

peterbell10 force-pushed the peterbell10/operator-NotImplemented branch from ec97fe6 to 7268b60 Compare September 23, 2019 15:16

facebook-github-bot reviewed Sep 30, 2019

View reviewed changes

facebook-github-bot closed this in 585a597 Sep 30, 2019

facebook-github-bot added the merged label Sep 30, 2019

peterbell10 deleted the peterbell10/operator-NotImplemented branch September 30, 2019 19:45

gchanan mentioned this pull request Oct 1, 2019

torch.Tensor reverse operators (__rmul__) should return NotImplemented for unsupported types #26333

Closed

peterbell10 mentioned this pull request Oct 5, 2019

Return NotImplemented from all binary math ops #27423

Closed

mruberry added the Merged label Oct 28, 2020

albanD mentioned this pull request Mar 9, 2022

User raised TypeError from __torch_dispatch__ gets turned into generic "unsupported operand type" message #73930

Open

Conversation

peterbell10 commented Sep 19, 2019

Uh oh!

ezyang left a comment

Choose a reason for hiding this comment

Uh oh!

ezyang commented Sep 20, 2019

Uh oh!

ezyang commented Sep 20, 2019

Uh oh!

pytorchbot commented Sep 20, 2019

Uh oh!

ezyang commented Sep 20, 2019

Uh oh!

pytorchbot commented Sep 20, 2019

Uh oh!

ezyang commented Sep 20, 2019

Uh oh!

ezyang commented Sep 20, 2019

Uh oh!

peterbell10 commented Sep 20, 2019

Uh oh!

rgommers commented Sep 21, 2019

Uh oh!

ezyang commented Sep 23, 2019

Uh oh!

ezyang commented Sep 23, 2019

Uh oh!

pytorchbot commented Sep 23, 2019

Uh oh!

ezyang commented Sep 23, 2019

Uh oh!

rgommers commented Sep 23, 2019

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Sep 30, 2019

Uh oh!

gchanan commented Oct 1, 2019

Uh oh!

ezyang commented Oct 2, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants