Making ops c10-full: Generator arguments by smessmer · Pull Request #49013 · pytorch/pytorch

smessmer · 2020-12-08T12:22:18Z

Stack from ghstack:

Remove use_c10_dispatcher: full lines #49259 Remove use_c10_dispatcher: full lines
Remove generated_unboxing_wrappers and setManuallyBoxedKernel #49251 Remove generated_unboxing_wrappers and setManuallyBoxedKernel
Remove .impl_UNBOXED() and functionalities associated with it #49220 Remove .impl_UNBOXED() and functionalities associated with it
Remove codegen logic to support non-c10-full ops #49164 Remove codegen logic to support non-c10-full ops
Making ops c10-full: list of optional tensors #49138 [wip] Making ops c10-full: list of optional tensors
Making ops c10-full: ConstQuantizerPtr #49150 Making ops c10-full: ConstQuantizerPtr
Making ops c10-full: Storage arguments #49146 Making ops c10-full: Storage arguments
Making ops c10-full: optional lists #49088 Making ops c10-full: optional lists
Making ops c10 full: optional out arguments #49083 Making ops c10 full: optional out arguments
Making ops c10-full: Generator arguments #49013 Making ops c10-full: Generator arguments
Making ops c10 full: out overloads with default arguments #49012 Making ops c10 full: out overloads with default arguments
Making ops c10-full: Dimname arguments #49008 Making ops c10-full: Dimname arguments
Making ops c10-full: ops blocked by manual registrations #49007 Making ops c10-full: ops blocked by manual registrations
Making ops c10 full: ops returning multiple out arguments #49006 Making ops c10 full: ops returning multiple out arguments
Make out ops c10-full (with hacky-wrapper) #48912 Make out ops c10-full (with hacky-wrapper)

I don't know why this works. I know, this is never a good way to start a PR description :P
I know that Generator is a dispatch relevant argument when called from an unboxed API and is ignored
for dispatch purposes when called from a boxed API. This should break something, but maybe we don't
have test cases for that.

We likely need to align the unboxed and boxed dispatch behavior before landing this.
The best solution would be to make Generator not dispatch relevant in unboxing. But that might be a bigger change.
An acceptable solution could be to make Generator dispatch relevant in boxing, but that needs perf measurements.

This PR needs further discussion.

Differential Revision: D25394998

I don't know why this works. I know, this is never a good way to start a PR description :P I know that Generator is a dispatch relevant argument when called from an unboxed API and is ignored for dispatch purposes when called from a boxed API. This should break something, but maybe we don't have test cases for that. We likely need to align the unboxed and boxed dispatch behavior before landing this. The best solution would be to make Generator not dispatch relevant in unboxing. But that might be a bigger change. An acceptable solution could be to make Generator dispatch relevant in boxing, but that needs perf measurements. This PR needs further discussion. Differential Revision: [D25394998](https://our.internmc.facebook.com/intern/diff/D25394998/) [ghstack-poisoned]

I don't know why this works. I know, this is never a good way to start a PR description :P I know that Generator is a dispatch relevant argument when called from an unboxed API and is ignored for dispatch purposes when called from a boxed API. This should break something, but maybe we don't have test cases for that. We likely need to align the unboxed and boxed dispatch behavior before landing this. The best solution would be to make Generator not dispatch relevant in unboxing. But that might be a bigger change. An acceptable solution could be to make Generator dispatch relevant in boxing, but that needs perf measurements. This PR needs further discussion. Differential Revision: [D25394998](https://our.internmc.facebook.com/intern/diff/D25394998/) ghstack-source-id: 118081571 Pull Request resolved: #49013

dr-ci · 2020-12-08T12:56:30Z

💊 CI failures summary and remediations

As of commit ba167fd (more details on the Dr. CI page):

1/1 failures introduced in this PR

🕵️ 1 new failure recognized by patterns

The following CI failures do not appear to be due to upstream breakages:

pytorch_xla_linux_bionic_py3_6_clang9_build (1/1)

Step: "Build" (full log | diagnosis details | 🔁 rerun)

Dec 15 18:45:44 Failed to generate ATEN bindings: ['/var/lib/jenkins/workspace/xla/scripts/generate_code.sh']

Dec 15 18:45:43 AtenXlaType function missed override: std::tuple<Tensor&, Tensor&> min_out(Tensor& min, Tensor& min_indices, const Tensor& self, int64_t dim, bool keepdim); // min_out(Tensor,Tensor,Tensor,int64_t,bool)->std::tuple<Tensor,Tensor>
Dec 15 18:45:43 Traceback (most recent call last):
Dec 15 18:45:43   File "/var/lib/jenkins/workspace/xla/scripts/gen.py", line 1172, in <module>
Dec 15 18:45:43     generate(args)
Dec 15 18:45:43   File "/var/lib/jenkins/workspace/xla/scripts/gen.py", line 1142, in generate
Dec 15 18:45:43     assert check_overrides(overrides, overridden)
Dec 15 18:45:43 AssertionError
Dec 15 18:45:44 Building torch_xla version: 1.6
Dec 15 18:45:44 XLA Commit ID: fe89172b2bd1c9a1c104bd2cbe565f30e6c8e328
Dec 15 18:45:44 PyTorch Commit ID: b1e59d59dc19e5978a7c08a0cd57cd1409d1302c
Dec 15 18:45:44 Failed to generate ATEN bindings: ['/var/lib/jenkins/workspace/xla/scripts/generate_code.sh']
Dec 15 18:45:44 =================== sccache compilation log ===================
Dec 15 18:45:44 + cleanup
Dec 15 18:45:44 + retcode=1
Dec 15 18:45:44 + set +x
Dec 15 18:45:44 =========== If your build fails, please take a look at the log above for possible reasons ===========
Dec 15 18:45:44 Compile requests                    4562
Dec 15 18:45:44 Compile requests executed           4268
Dec 15 18:45:44 Cache hits                          3141
Dec 15 18:45:44 Cache hits (C/C++)                  3141
Dec 15 18:45:44 Cache misses                        1109

XLA failure

Job pytorch_xla_linux_bionic_py3_6_clang9_build is failing. Please create an issue with title prefixed by `[PT_BREAK]` in `pytorch/xla` and link to to this PR. If you have questions, please reach out to @ailzhang / @dlibenzi / @JackCaoG.

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 to the (internal) Dr. CI Users group.

This comment has been revised 115 times.

I don't know why this works. I know, this is never a good way to start a PR description :P I know that Generator is a dispatch relevant argument when called from an unboxed API and is ignored for dispatch purposes when called from a boxed API. This should break something, but maybe we don't have test cases for that. We likely need to align the unboxed and boxed dispatch behavior before landing this. The best solution would be to make Generator not dispatch relevant in unboxing. But that might be a bigger change. An acceptable solution could be to make Generator dispatch relevant in boxing, but that needs perf measurements. This PR needs further discussion. Differential Revision: [D25394998](https://our.internmc.facebook.com/intern/diff/D25394998/) [ghstack-poisoned]

Pull Request resolved: #49013 I don't know why this works. I know, this is never a good way to start a PR description :P I know that Generator is a dispatch relevant argument when called from an unboxed API and is ignored for dispatch purposes when called from a boxed API. This should break something, but maybe we don't have test cases for that. We likely need to align the unboxed and boxed dispatch behavior before landing this. The best solution would be to make Generator not dispatch relevant in unboxing. But that might be a bigger change. An acceptable solution could be to make Generator dispatch relevant in boxing, but that needs perf measurements. This PR needs further discussion. ghstack-source-id: 118097566 Differential Revision: [D25394998](https://our.internmc.facebook.com/intern/diff/D25394998/)

I don't know why this works. I know, this is never a good way to start a PR description :P I know that Generator is a dispatch relevant argument when called from an unboxed API and is ignored for dispatch purposes when called from a boxed API. This should break something, but maybe we don't have test cases for that. We likely need to align the unboxed and boxed dispatch behavior before landing this. The best solution would be to make Generator not dispatch relevant in unboxing. But that might be a bigger change. An acceptable solution could be to make Generator dispatch relevant in boxing, but that needs perf measurements. This PR needs further discussion. Differential Revision: [D25394998](https://our.internmc.facebook.com/intern/diff/D25394998/) [ghstack-poisoned]

Pull Request resolved: #49013 I don't know why this works. I know, this is never a good way to start a PR description :P I know that Generator is a dispatch relevant argument when called from an unboxed API and is ignored for dispatch purposes when called from a boxed API. This should break something, but maybe we don't have test cases for that. We likely need to align the unboxed and boxed dispatch behavior before landing this. The best solution would be to make Generator not dispatch relevant in unboxing. But that might be a bigger change. An acceptable solution could be to make Generator dispatch relevant in boxing, but that needs perf measurements. This PR needs further discussion. ghstack-source-id: 118116629 Differential Revision: [D25394998](https://our.internmc.facebook.com/intern/diff/D25394998/)

bhosmer

I don't think it's plausible to stop dispatching on Generator for unboxed, is it? That would necessitate a redesign of the current RNG approach AIUI.

Agree that we should teach DispatchKeyExtractor about Generator and run some perf tests. But I don't imagine Generator scanning per se will affect perf much - I'd think the change would be dominated by the move from generated unboxing wrappers to the boxed protocol, no? (And as such, should we have some comparison numbers further up the stack for other ops that have made the same transition?)

bhosmer · 2020-12-09T06:48:26Z

onnx errors look real btw

I don't know why this works. I know, this is never a good way to start a PR description :P I know that Generator is a dispatch relevant argument when called from an unboxed API and is ignored for dispatch purposes when called from a boxed API. This should break something, but maybe we don't have test cases for that. We likely need to align the unboxed and boxed dispatch behavior before landing this. The best solution would be to make Generator not dispatch relevant in unboxing. But that might be a bigger change. An acceptable solution could be to make Generator dispatch relevant in boxing, but that needs perf measurements. This PR needs further discussion. Differential Revision: [D25394998](https://our.internmc.facebook.com/intern/diff/D25394998/) [ghstack-poisoned]

Pull Request resolved: #49013 I don't know why this works. I know, this is never a good way to start a PR description :P I know that Generator is a dispatch relevant argument when called from an unboxed API and is ignored for dispatch purposes when called from a boxed API. This should break something, but maybe we don't have test cases for that. We likely need to align the unboxed and boxed dispatch behavior before landing this. The best solution would be to make Generator not dispatch relevant in unboxing. But that might be a bigger change. An acceptable solution could be to make Generator dispatch relevant in boxing, but that needs perf measurements. This PR needs further discussion. ghstack-source-id: 118177107 Differential Revision: [D25394998](https://our.internmc.facebook.com/intern/diff/D25394998/)

I don't know why this works. I know, this is never a good way to start a PR description :P I know that Generator is a dispatch relevant argument when called from an unboxed API and is ignored for dispatch purposes when called from a boxed API. This should break something, but maybe we don't have test cases for that. We likely need to align the unboxed and boxed dispatch behavior before landing this. The best solution would be to make Generator not dispatch relevant in unboxing. But that might be a bigger change. An acceptable solution could be to make Generator dispatch relevant in boxing, but that needs perf measurements. This PR needs further discussion. Differential Revision: [D25394998](https://our.internmc.facebook.com/intern/diff/D25394998/) [ghstack-poisoned]

smessmer · 2020-12-09T14:52:15Z

I found an old issue about this: #36733
This issue is long standing and this PR shouldn't change anything, it doesn't work before and it doesn't work after. I think we can go ahead and land this.

I don't know why this works. I know, this is never a good way to start a PR description :P I know that Generator is a dispatch relevant argument when called from an unboxed API and is ignored for dispatch purposes when called from a boxed API. This should break something, but maybe we don't have test cases for that. We likely need to align the unboxed and boxed dispatch behavior before landing this. The best solution would be to make Generator not dispatch relevant in unboxing. But that might be a bigger change. An acceptable solution could be to make Generator dispatch relevant in boxing, but that needs perf measurements. This PR needs further discussion. Differential Revision: [D25394998](https://our.internmc.facebook.com/intern/diff/D25394998/) [ghstack-poisoned]

Summary: Pull Request resolved: #49013 I don't know why this works. I know, this is never a good way to start a PR description :P I know that Generator is a dispatch relevant argument when called from an unboxed API and is ignored for dispatch purposes when called from a boxed API. This should break something, but maybe we don't have test cases for that. We likely need to align the unboxed and boxed dispatch behavior before landing this. The best solution would be to make Generator not dispatch relevant in unboxing. But that might be a bigger change. An acceptable solution could be to make Generator dispatch relevant in boxing, but that needs perf measurements. This PR needs further discussion. ghstack-source-id: 118619230 (Note: this ignores all push blocking failures!) Test Plan: waitforsandcastle Reviewed By: bhosmer Differential Revision: D25394998 fbshipit-source-id: f695c659ee6e3738f74cdf0af1a514ac0c30ebff

facebook-github-bot · 2020-12-15T19:23:20Z

This pull request has been merged in ba3bcab.

Summary: Pull Request resolved: pytorch#49013 I don't know why this works. I know, this is never a good way to start a PR description :P I know that Generator is a dispatch relevant argument when called from an unboxed API and is ignored for dispatch purposes when called from a boxed API. This should break something, but maybe we don't have test cases for that. We likely need to align the unboxed and boxed dispatch behavior before landing this. The best solution would be to make Generator not dispatch relevant in unboxing. But that might be a bigger change. An acceptable solution could be to make Generator dispatch relevant in boxing, but that needs perf measurements. This PR needs further discussion. ghstack-source-id: 118619230 (Note: this ignores all push blocking failures!) Test Plan: waitforsandcastle Reviewed By: bhosmer Differential Revision: D25394998 fbshipit-source-id: f695c659ee6e3738f74cdf0af1a514ac0c30ebff

Summary: Pull Request resolved: pytorch/pytorch#49013 I don't know why this works. I know, this is never a good way to start a PR description :P I know that Generator is a dispatch relevant argument when called from an unboxed API and is ignored for dispatch purposes when called from a boxed API. This should break something, but maybe we don't have test cases for that. We likely need to align the unboxed and boxed dispatch behavior before landing this. The best solution would be to make Generator not dispatch relevant in unboxing. But that might be a bigger change. An acceptable solution could be to make Generator dispatch relevant in boxing, but that needs perf measurements. This PR needs further discussion. ghstack-source-id: 118619230 (Note: this ignores all push blocking failures!) Reviewed By: bhosmer Differential Revision: D25394998 fbshipit-source-id: f695c659ee6e3738f74cdf0af1a514ac0c30ebff

facebook-github-bot added the cla signed label Dec 8, 2020

smessmer mentioned this pull request Dec 8, 2020

Gaps for making template-unboxing work for all operators #32366

Closed

24 tasks

smessmer requested review from bhosmer and ezyang December 8, 2020 13:44

ezyang approved these changes Dec 8, 2020

View reviewed changes

bhosmer reviewed Dec 9, 2020

View reviewed changes

smessmer mentioned this pull request Dec 9, 2020

remove hacky_wrapper from BackendSelect #49079

Closed

smessmer mentioned this pull request Dec 9, 2020

Making ops c10 full: optional out arguments #49083

Closed

smessmer mentioned this pull request Dec 9, 2020

Making ops c10-full: optional lists #49088

Closed

smessmer mentioned this pull request Dec 10, 2020

Making ops c10-full: list of optional tensors #49138

Closed

This was referenced Dec 10, 2020

Making ops c10-full: Storage arguments #49146

Closed

Making ops c10-full: ConstQuantizerPtr #49150

Closed

smessmer added 2 commits December 9, 2020 23:17

smessmer mentioned this pull request Dec 10, 2020

Remove codegen logic to support non-c10-full ops #49164

Closed

bhosmer approved these changes Dec 11, 2020

View reviewed changes

smessmer added 3 commits December 10, 2020 21:53

smessmer mentioned this pull request Dec 11, 2020

Remove .impl_UNBOXED() and functionalities associated with it #49220

Closed

smessmer added 5 commits December 10, 2020 23:51

smessmer mentioned this pull request Dec 11, 2020

Remove generated_unboxing_wrappers and setManuallyBoxedKernel #49251

Closed

ailzhang mentioned this pull request Dec 11, 2020

Handle out funcs with c10_full. pytorch/xla#2683

Merged

smessmer mentioned this pull request Dec 11, 2020

Remove use_c10_dispatcher: full lines #49259

Closed

smessmer added 2 commits December 12, 2020 02:23

facebook-github-bot closed this in meta-pytorch/csprng@ba3bcab Dec 15, 2020

facebook-github-bot added the Merged label Dec 15, 2020

facebook-github-bot deleted the gh/smessmer/277/head branch December 19, 2020 15:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Making ops c10-full: Generator arguments#49013

Making ops c10-full: Generator arguments#49013
smessmer wants to merge 22 commits intogh/smessmer/277/basefrom
gh/smessmer/277/head

smessmer commented Dec 8, 2020 •

edited

Loading

Uh oh!

dr-ci bot commented Dec 8, 2020 •

edited by facebook-github-bot

Loading

Uh oh!

bhosmer left a comment

Uh oh!

bhosmer commented Dec 9, 2020

Uh oh!

smessmer commented Dec 9, 2020

Uh oh!

facebook-github-bot commented Dec 15, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

smessmer commented Dec 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dr-ci bot commented Dec 8, 2020 • edited by facebook-github-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💊 CI failures summary and remediations

🕵️ 1 new failure recognized by patterns

pytorch_xla_linux_bionic_py3_6_clang9_build (1/1)

XLA failure

Job pytorch_xla_linux_bionic_py3_6_clang9_build is failing. Please create an issue with title prefixed by [PT_BREAK] in pytorch/xla and link to to this PR. If you have questions, please reach out to @ailzhang / @dlibenzi / @JackCaoG.

Uh oh!

bhosmer left a comment

Choose a reason for hiding this comment

Uh oh!

bhosmer commented Dec 9, 2020

Uh oh!

smessmer commented Dec 9, 2020

Uh oh!

facebook-github-bot commented Dec 15, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

smessmer commented Dec 8, 2020 •

edited

Loading

dr-ci bot commented Dec 8, 2020 •

edited by facebook-github-bot

Loading

Job pytorch_xla_linux_bionic_py3_6_clang9_build is failing. Please create an issue with title prefixed by `[PT_BREAK]` in `pytorch/xla` and link to to this PR. If you have questions, please reach out to @ailzhang / @dlibenzi / @JackCaoG.