Add API for listing functions overridable by __torch_function__ #33791

ngoldbaum · 2020-02-25T23:31:23Z

This adds private API functions that developers of types that implement __torch_function__ can use to ensure full coverage of the subset of the PyTorch API that can be overrided.

I've refactored some of the code in the tests into a new torch._overrides.get_overridable_functions function. I've also changed TENSOR_LIKE_TORCH_OVERRIDES into torch._overrides.get_testing_overrides and IGNORED_TORCH_FUNCTIONS into torch._overrides.get_ignored_functions. Making these two static global variables in the tests into functions should allow rewriting their implementation to construct their return values instead of just statically defining the return value as is done here. Currently that is blocked on not being able to inspect function signatures of compiled kernels in PyTorch (see #28233). See the docs I've added for usage examples of these new functions. I also refactored the existing override tests to make use of these new functions, which should be a good forcing function to make sure they're kept up-to-date.

Finally, while working on this I discovered that TestTorchFunctionOverrides.test_mean and TestTorchFunctionOverrides.test_mm weren't ever being run because they were getting clobbered by the other dynamically generated override tests. I fixed that by renaming the tests and then fixing the actual test code. I've verified that all the subclassing semantics is correct and that the updated test answers are correct. I'm happy to put the fixes to the existing tests in as a separate pull request if that would be easier to review.

ping @cpuhrsch since the feature request originally came from them.

ezyang · 2020-02-27T04:39:12Z

@cpuhrsch I'm deferring this code review to you. @ngoldbaum let me know if there's anything specific you want me to look at.

docs/source/notes/extending.rst

test/test_overrides.py

cpuhrsch · 2020-02-27T18:02:22Z

docs/source/notes/extending.rst

+functions, however in practice this is not enough to write tests for all of
+these functions without laboriously and manually copying the signature of each
+function for each test. To ease this process, the
+``torch._overrides.get_testing_overrides`` function returns a dictionary mapping


I see in the test below you exercise the torch.mean function, but there are two overloads for torch.mean

torch.mean(input) → Tensor

and

torch.mean(input, dim, keepdim=False, out=None) → Tensor

How is the second one tests?

Further we have our own argparser, which means the Python signature of a given function might not accurately capture all capabilities.

How is this dealt with?

mean is a C++ kernel, so its handled via the C++ argument parsing, e.g.

pytorch/torch/csrc/utils/python_arg_parser.cpp

Lines 247 to 251 in b10761d

if (THPVariable_Check(obj)) {

if (check_has_torch_function(obj)) {

append_overloaded_arg(overloaded_args, obj);

}

return true;

.

I don't think there's a direct test for both signatures in the override tests. In principle we could add it but I'm not sure if it would be worth the extra boilerplate.

cpuhrsch

This looks great! I added a few nits and a general question, but afterwards I think this can be approved and landed even if just treated as a code refactor / abstraction.

ngoldbaum · 2020-02-27T19:00:43Z

Fixed the nits, thank you for the copyediting :)

dr-ci · 2020-02-27T19:31:15Z

💊 CircleCI build failures summary and remediations

As of commit c5f7ea3:

None of the build failures appear to be your fault.

2/2 broken upstream at merge base 095de1e since Feb 27
Please rebase on the viable/strict branch (expand for instructions)

If your commit is newer than viable/strict, you can try basing on an older, stable commit:
```
git fetch origin viable/strict
git rebase --onto viable/strict $(git merge-base origin/master HEAD)
```
If your commit is older than viable/strict:
```
git fetch origin viable/strict
git rebase viable/strict
```
Check out the recency history of this "viable master" tracking branch.

Detailed failure analysis

One may explore the probable reasons each build failed interactively on the Dr. CI website.

🚧 2 upstream failures recognized by patterns:

These builds matched patterns, but were probably caused by upstream breakages:

pytorch_linux_xenial_py3_6_gcc5_4_ge_config_legacy_test from Feb 27
pytorch_macos_10_13_py3_test from Feb 27

This comment was automatically generated by Dr. CI (expand for details).

Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions on the GitHub issue tracker.

This comment has been revised 1 time.

cpuhrsch

Looks good to me! I'll adopt this within pytorch/nestedtensor as soon as it lands for testing.

facebook-github-bot

@cpuhrsch has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

facebook-github-bot · 2020-03-04T05:12:59Z

@cpuhrsch merged this pull request in ad2825a.

mrshenli · 2020-03-04T17:44:40Z

Looks like pytorch_linux_xenial_py3_5_test starts to become flaky after this test.

Mar 03 23:09:46 ======================================================================
Mar 03 23:09:46 ERROR: test_sigmoid (__main__.TestTorchFunctionOverride)
Mar 03 23:09:46 ----------------------------------------------------------------------
Mar 03 23:09:46 Traceback (most recent call last):
Mar 03 23:09:46   File "test_overrides.py", line 478, in test
Mar 03 23:09:46     self.assertEqual(func(*func_args), -1)
Mar 03 23:09:46   File "/opt/python/3.5/lib/python3.5/site-packages/torch/nn/functional.py", line 1569, in sigmoid
Mar 03 23:09:46     return input.sigmoid()
Mar 03 23:09:46 AttributeError: 'TensorLike' object has no attribute 'sigmoid'
Mar 03 23:09:46 
Mar 03 23:09:46 ======================================================================
Mar 03 23:09:46 ERROR: test_tanh (__main__.TestTorchFunctionOverride)
Mar 03 23:09:46 ----------------------------------------------------------------------
Mar 03 23:09:46 Traceback (most recent call last):
Mar 03 23:09:46   File "test_overrides.py", line 478, in test
Mar 03 23:09:46     self.assertEqual(func(*func_args), -1)
Mar 03 23:09:46   File "/opt/python/3.5/lib/python3.5/site-packages/torch/nn/functional.py", line 1558, in tanh
Mar 03 23:09:46     return input.tanh()
Mar 03 23:09:46 AttributeError: 'TensorLike' object has no attribute 'tanh'

https://app.circleci.com/jobs/github/pytorch/pytorch/4678835
https://app.circleci.com/jobs/github/pytorch/pytorch/4678098

ngoldbaum · 2020-03-04T17:49:29Z

@mrshenli the fix for that is to add __torch_function__ handling for those two functions. Would you like me to put in a fix?

mrshenli · 2020-03-04T17:50:58Z

Hey @ngoldbaum, thanks a lot for prompt reply.

@mrshenli the fix for that is to add torch_function handling for those two functions. Would you like me to put in a fix?

That will be great! Please send the fix and I will stamp. Thanks!!

mrshenli · 2020-03-04T17:53:10Z

@ngoldbaum BTW, do you know why it is flaky instead of fail consistently. Below is a passing run:

https://app.circleci.com/jobs/github/pytorch/pytorch/4685311

ngoldbaum · 2020-03-04T17:54:10Z

I'm not sure but if I had to guess it's because of dict order randomization on Python3.5 and something else is clobbering those entries. I'm going to see if I can figure out exactly why right now...

mrshenli · 2020-03-04T17:58:40Z

Hey @ngoldbaum, since we need to wait for 5-6 hours for the CI to pass before we can land the fix, do you mind if I revert this PR for now, and you can send in the resubmit together with the fix?

ngoldbaum · 2020-03-04T18:00:17Z

Go ahead, sorry for the trouble!

mrshenli · 2020-03-04T18:00:27Z

Thanks! No worries :)

…rch#33791) Summary: Fixes pytorch#33182 This adds private API functions that developers of types that implement `__torch_function__` can use to ensure full coverage of the subset of the PyTorch API that can be overrided. I've refactored some of the code in the tests into a new `torch._overrides.get_overridable_functions` function. I've also changed `TENSOR_LIKE_TORCH_OVERRIDES` into `torch._overrides.get_testing_overrides` and `IGNORED_TORCH_FUNCTIONS` into `torch._overrides.get_ignored_functions`. Making these two static global variables in the tests into functions should allow rewriting their implementation to construct their return values instead of just statically defining the return value as is done here. Currently that is blocked on not being able to inspect function signatures of compiled kernels in PyTorch (see pytorch#28233). See the docs I've added for usage examples of these new functions. I also refactored the existing override tests to make use of these new functions, which should be a good forcing function to make sure they're kept up-to-date. Finally, while working on this I discovered that `TestTorchFunctionOverrides.test_mean` and `TestTorchFunctionOverrides.test_mm` weren't ever being run because they were getting clobbered by the other dynamically generated override tests. I fixed that by renaming the tests and then fixing the actual test code. I've verified that all the subclassing semantics is correct and that the updated test answers are correct. I'm happy to put the fixes to the existing tests in as a separate pull request if that would be easier to review. ping cpuhrsch since the feature request originally came from them. Pull Request resolved: pytorch#33791 Differential Revision: D20195053 Pulled By: cpuhrsch fbshipit-source-id: 1585f4e405f5223932b410eae03a288dc8eb627e

…n__" (#34240) Summary: This is a redo of #33791, which was reverted because it introduced a flaky test. The test was flaky and only flaky on Python3.5 because of dict order randomization. I've fixed the issue with tests clobbering each other in b539fec and removed the override tests for `torch.nn.functional.tanh` and `torch.nn.functional.sigmoid`, which are deprecated and shouldn't be overridable in e0d7402. I also verified that no more test clobbering is happening. Pull Request resolved: #34240 Differential Revision: D20252442 Pulled By: cpuhrsch fbshipit-source-id: 069568e342a41c90e1dc76cbf85ba4aed47f24be

pytorchbot added the open source label Feb 25, 2020

ngoldbaum force-pushed the override-listing branch from dbae475 to 73ada4a Compare February 26, 2020 00:05

ngoldbaum requested a review from ezyang February 26, 2020 00:08

mruberry added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Feb 26, 2020

ezyang requested a review from cpuhrsch February 27, 2020 04:38

cpuhrsch reviewed Feb 27, 2020

View reviewed changes

docs/source/notes/extending.rst Outdated Show resolved Hide resolved

cpuhrsch reviewed Feb 27, 2020

View reviewed changes

test/test_overrides.py Outdated Show resolved Hide resolved

cpuhrsch reviewed Feb 27, 2020

View reviewed changes

cpuhrsch requested changes Feb 27, 2020

View reviewed changes

ngoldbaum added 4 commits February 27, 2020 13:00

fix test_mean and test_mm being clobbered in TestTorchFunctionOverride

6b73625

add private API to list overridable functions and dummy overrides

d87ec24

add narrative documentation for override listing functions

22fb4a6

fix review nits

c5f7ea3

ngoldbaum force-pushed the override-listing branch from 73ada4a to c5f7ea3 Compare February 27, 2020 19:00

cpuhrsch approved these changes Feb 28, 2020

View reviewed changes

facebook-github-bot reviewed Mar 2, 2020

View reviewed changes

pytorch deleted a comment from ezyang Mar 2, 2020

facebook-github-bot closed this in ad2825a Mar 3, 2020

facebook-github-bot added the merged label Mar 4, 2020

ngoldbaum mentioned this pull request Mar 4, 2020

Redo of "Add API for listing functions overridable by __torch_function__" #34240

Closed

mruberry added the Merged label Oct 28, 2020

coreylammie mentioned this pull request Jul 12, 2022

torch.overrides.get_testing_overrides does not function as intended for native tensor methods/operations #81323

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add API for listing functions overridable by __torch_function__ #33791

Add API for listing functions overridable by __torch_function__ #33791

ngoldbaum commented Feb 25, 2020

ezyang commented Feb 27, 2020

cpuhrsch Feb 27, 2020

ngoldbaum Feb 27, 2020

cpuhrsch left a comment

ngoldbaum commented Feb 27, 2020

dr-ci bot commented Feb 27, 2020 •

edited

cpuhrsch left a comment

facebook-github-bot left a comment

facebook-github-bot commented Mar 4, 2020

mrshenli commented Mar 4, 2020

ngoldbaum commented Mar 4, 2020

mrshenli commented Mar 4, 2020

mrshenli commented Mar 4, 2020

ngoldbaum commented Mar 4, 2020

mrshenli commented Mar 4, 2020

ngoldbaum commented Mar 4, 2020

mrshenli commented Mar 4, 2020

	if (THPVariable_Check(obj)) {
	if (check_has_torch_function(obj)) {
	append_overloaded_arg(overloaded_args, obj);
	}
	return true;

Add API for listing functions overridable by __torch_function__ #33791

Add API for listing functions overridable by __torch_function__ #33791

Conversation

ngoldbaum commented Feb 25, 2020

ezyang commented Feb 27, 2020

cpuhrsch Feb 27, 2020

Choose a reason for hiding this comment

ngoldbaum Feb 27, 2020

Choose a reason for hiding this comment

cpuhrsch left a comment

Choose a reason for hiding this comment

ngoldbaum commented Feb 27, 2020

dr-ci bot commented Feb 27, 2020 • edited

💊 CircleCI build failures summary and remediations

Detailed failure analysis

🚧 2 upstream failures recognized by patterns:

cpuhrsch left a comment

Choose a reason for hiding this comment

facebook-github-bot left a comment

Choose a reason for hiding this comment

facebook-github-bot commented Mar 4, 2020

mrshenli commented Mar 4, 2020

ngoldbaum commented Mar 4, 2020

mrshenli commented Mar 4, 2020

mrshenli commented Mar 4, 2020

ngoldbaum commented Mar 4, 2020

mrshenli commented Mar 4, 2020

ngoldbaum commented Mar 4, 2020

mrshenli commented Mar 4, 2020

dr-ci bot commented Feb 27, 2020 •

edited