Skip to content
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

VariableKernel calls into scattered C++ api #44158

Closed
wants to merge 15 commits into from

Conversation

smessmer
Copy link
Contributor

@smessmer smessmer commented Sep 3, 2020

Stack from ghstack:

Previously, the C++ API only supported calling ops with a gathered TensorOptions object. So even if the VariableKernel took scattered arguments,
it had to re-gather them to call into the C++ API. But a diff stacked below this one introduced a scattered API for the C++ frontend.

This reaps the benefits and makes sure that if the Variable kernel gets scattered arguments (i.e. it's a c10-full op), then it passes those on without regathering

codegen diff: P143616493

Differential Revision: D23512538

Previously, the C++ API only supported calling ops with a gathered TensorOptions object. So even if the VariableKernel took scattered arguments,
it had to re-gather them to call into the C++ API. But a diff stacked below this one introduced a scattered API for the C++ frontend.

This reaps the benefits and makes sure that if the Variable kernel gets scattered arguments (i.e. it's a c10-full op), then it passes those on without regathering

Differential Revision: [D23512538](https://our.internmc.facebook.com/intern/diff/D23512538/)

[ghstack-poisoned]
smessmer added a commit that referenced this pull request Sep 3, 2020
Previously, the C++ API only supported calling ops with a gathered TensorOptions object. So even if the VariableKernel took scattered arguments,
it had to re-gather them to call into the C++ API. But a diff stacked below this one introduced a scattered API for the C++ frontend.

This reaps the benefits and makes sure that if the Variable kernel gets scattered arguments (i.e. it's a c10-full op), then it passes those on without regathering

Differential Revision: [D23512538](https://our.internmc.facebook.com/intern/diff/D23512538/)

ghstack-source-id: 111389736
Pull Request resolved: #44158
@dr-ci
Copy link

dr-ci bot commented Sep 3, 2020

💊 CI failures summary and remediations

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


Commit 2cee265 was recently pushed. Waiting for builds...


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

See how this bot performed.

This comment has been revised 88 times.

Previously, the C++ API only supported calling ops with a gathered TensorOptions object. So even if the VariableKernel took scattered arguments,
it had to re-gather them to call into the C++ API. But a diff stacked below this one introduced a scattered API for the C++ frontend.

This reaps the benefits and makes sure that if the Variable kernel gets scattered arguments (i.e. it's a c10-full op), then it passes those on without regathering

Differential Revision: [D23512538](https://our.internmc.facebook.com/intern/diff/D23512538/)

[ghstack-poisoned]
smessmer added a commit that referenced this pull request Sep 17, 2020
Pull Request resolved: #44158

Previously, the C++ API only supported calling ops with a gathered TensorOptions object. So even if the VariableKernel took scattered arguments,
it had to re-gather them to call into the C++ API. But a diff stacked below this one introduced a scattered API for the C++ frontend.

This reaps the benefits and makes sure that if the Variable kernel gets scattered arguments (i.e. it's a c10-full op), then it passes those on without regathering
ghstack-source-id: 112287026

Differential Revision: [D23512538](https://our.internmc.facebook.com/intern/diff/D23512538/)
Previously, the C++ API only supported calling ops with a gathered TensorOptions object. So even if the VariableKernel took scattered arguments,
it had to re-gather them to call into the C++ API. But a diff stacked below this one introduced a scattered API for the C++ frontend.

This reaps the benefits and makes sure that if the Variable kernel gets scattered arguments (i.e. it's a c10-full op), then it passes those on without regathering

Differential Revision: [D23512538](https://our.internmc.facebook.com/intern/diff/D23512538/)

[ghstack-poisoned]
smessmer added a commit that referenced this pull request Sep 22, 2020
Pull Request resolved: #44158

Previously, the C++ API only supported calling ops with a gathered TensorOptions object. So even if the VariableKernel took scattered arguments,
it had to re-gather them to call into the C++ API. But a diff stacked below this one introduced a scattered API for the C++ frontend.

This reaps the benefits and makes sure that if the Variable kernel gets scattered arguments (i.e. it's a c10-full op), then it passes those on without regathering
ghstack-source-id: 112590388

Differential Revision: [D23512538](https://our.internmc.facebook.com/intern/diff/D23512538/)
Previously, the C++ API only supported calling ops with a gathered TensorOptions object. So even if the VariableKernel took scattered arguments,
it had to re-gather them to call into the C++ API. But a diff stacked below this one introduced a scattered API for the C++ frontend.

This reaps the benefits and makes sure that if the Variable kernel gets scattered arguments (i.e. it's a c10-full op), then it passes those on without regathering

Differential Revision: [D23512538](https://our.internmc.facebook.com/intern/diff/D23512538/)

[ghstack-poisoned]
smessmer added a commit that referenced this pull request Sep 22, 2020
Pull Request resolved: #44158

Previously, the C++ API only supported calling ops with a gathered TensorOptions object. So even if the VariableKernel took scattered arguments,
it had to re-gather them to call into the C++ API. But a diff stacked below this one introduced a scattered API for the C++ frontend.

This reaps the benefits and makes sure that if the Variable kernel gets scattered arguments (i.e. it's a c10-full op), then it passes those on without regathering
ghstack-source-id: 112608372

Differential Revision: [D23512538](https://our.internmc.facebook.com/intern/diff/D23512538/)
Previously, the C++ API only supported calling ops with a gathered TensorOptions object. So even if the VariableKernel took scattered arguments,
it had to re-gather them to call into the C++ API. But a diff stacked below this one introduced a scattered API for the C++ frontend.

This reaps the benefits and makes sure that if the Variable kernel gets scattered arguments (i.e. it's a c10-full op), then it passes those on without regathering

Differential Revision: [D23512538](https://our.internmc.facebook.com/intern/diff/D23512538/)

[ghstack-poisoned]
Previously, the C++ API only supported calling ops with a gathered TensorOptions object. So even if the VariableKernel took scattered arguments,
it had to re-gather them to call into the C++ API. But a diff stacked below this one introduced a scattered API for the C++ frontend.

This reaps the benefits and makes sure that if the Variable kernel gets scattered arguments (i.e. it's a c10-full op), then it passes those on without regathering

Differential Revision: [D23512538](https://our.internmc.facebook.com/intern/diff/D23512538/)

[ghstack-poisoned]
smessmer added a commit that referenced this pull request Sep 24, 2020
Pull Request resolved: #44158

Previously, the C++ API only supported calling ops with a gathered TensorOptions object. So even if the VariableKernel took scattered arguments,
it had to re-gather them to call into the C++ API. But a diff stacked below this one introduced a scattered API for the C++ frontend.

This reaps the benefits and makes sure that if the Variable kernel gets scattered arguments (i.e. it's a c10-full op), then it passes those on without regathering
ghstack-source-id: 112810178

Differential Revision: [D23512538](https://our.internmc.facebook.com/intern/diff/D23512538/)
Previously, the C++ API only supported calling ops with a gathered TensorOptions object. So even if the VariableKernel took scattered arguments,
it had to re-gather them to call into the C++ API. But a diff stacked below this one introduced a scattered API for the C++ frontend.

This reaps the benefits and makes sure that if the Variable kernel gets scattered arguments (i.e. it's a c10-full op), then it passes those on without regathering

Differential Revision: [D23512538](https://our.internmc.facebook.com/intern/diff/D23512538/)

[ghstack-poisoned]
smessmer added a commit that referenced this pull request Sep 24, 2020
Pull Request resolved: #44158

Previously, the C++ API only supported calling ops with a gathered TensorOptions object. So even if the VariableKernel took scattered arguments,
it had to re-gather them to call into the C++ API. But a diff stacked below this one introduced a scattered API for the C++ frontend.

This reaps the benefits and makes sure that if the Variable kernel gets scattered arguments (i.e. it's a c10-full op), then it passes those on without regathering
ghstack-source-id: 112825787

Differential Revision: [D23512538](https://our.internmc.facebook.com/intern/diff/D23512538/)
Copy link
Contributor

@ezyang ezyang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If possible, if you can post before and after codegen, that would be v helpful. Thanks!

Previously, the C++ API only supported calling ops with a gathered TensorOptions object. So even if the VariableKernel took scattered arguments,
it had to re-gather them to call into the C++ API. But a diff stacked below this one introduced a scattered API for the C++ frontend.

This reaps the benefits and makes sure that if the Variable kernel gets scattered arguments (i.e. it's a c10-full op), then it passes those on without regathering

Differential Revision: [D23512538](https://our.internmc.facebook.com/intern/diff/D23512538/)

[ghstack-poisoned]
Previously, the C++ API only supported calling ops with a gathered TensorOptions object. So even if the VariableKernel took scattered arguments,
it had to re-gather them to call into the C++ API. But a diff stacked below this one introduced a scattered API for the C++ frontend.

This reaps the benefits and makes sure that if the Variable kernel gets scattered arguments (i.e. it's a c10-full op), then it passes those on without regathering

Differential Revision: [D23512538](https://our.internmc.facebook.com/intern/diff/D23512538/)

[ghstack-poisoned]
@smessmer
Copy link
Contributor Author

If possible, if you can post before and after codegen, that would be v helpful. Thanks!

@ezyang I added a link to the codegen diff in the description

Previously, the C++ API only supported calling ops with a gathered TensorOptions object. So even if the VariableKernel took scattered arguments,
it had to re-gather them to call into the C++ API. But a diff stacked below this one introduced a scattered API for the C++ frontend.

This reaps the benefits and makes sure that if the Variable kernel gets scattered arguments (i.e. it's a c10-full op), then it passes those on without regathering

codegen diff: P143616493

Differential Revision: [D23512538](https://our.internmc.facebook.com/intern/diff/D23512538/)

[ghstack-poisoned]
smessmer added a commit that referenced this pull request Sep 25, 2020
Pull Request resolved: #44158

Previously, the C++ API only supported calling ops with a gathered TensorOptions object. So even if the VariableKernel took scattered arguments,
it had to re-gather them to call into the C++ API. But a diff stacked below this one introduced a scattered API for the C++ frontend.

This reaps the benefits and makes sure that if the Variable kernel gets scattered arguments (i.e. it's a c10-full op), then it passes those on without regathering
ghstack-source-id: 112920037

Differential Revision: [D23512538](https://our.internmc.facebook.com/intern/diff/D23512538/)
Previously, the C++ API only supported calling ops with a gathered TensorOptions object. So even if the VariableKernel took scattered arguments,
it had to re-gather them to call into the C++ API. But a diff stacked below this one introduced a scattered API for the C++ frontend.

This reaps the benefits and makes sure that if the Variable kernel gets scattered arguments (i.e. it's a c10-full op), then it passes those on without regathering

codegen diff: P143616493

Differential Revision: [D23512538](https://our.internmc.facebook.com/intern/diff/D23512538/)

[ghstack-poisoned]
smessmer added a commit that referenced this pull request Sep 29, 2020
Pull Request resolved: #44158

Previously, the C++ API only supported calling ops with a gathered TensorOptions object. So even if the VariableKernel took scattered arguments,
it had to re-gather them to call into the C++ API. But a diff stacked below this one introduced a scattered API for the C++ frontend.

This reaps the benefits and makes sure that if the Variable kernel gets scattered arguments (i.e. it's a c10-full op), then it passes those on without regathering
ghstack-source-id: 113120133

Differential Revision: [D23512538](https://our.internmc.facebook.com/intern/diff/D23512538/)
Previously, the C++ API only supported calling ops with a gathered TensorOptions object. So even if the VariableKernel took scattered arguments,
it had to re-gather them to call into the C++ API. But a diff stacked below this one introduced a scattered API for the C++ frontend.

This reaps the benefits and makes sure that if the Variable kernel gets scattered arguments (i.e. it's a c10-full op), then it passes those on without regathering

codegen diff: P143616493

Differential Revision: [D23512538](https://our.internmc.facebook.com/intern/diff/D23512538/)

[ghstack-poisoned]
Previously, the C++ API only supported calling ops with a gathered TensorOptions object. So even if the VariableKernel took scattered arguments,
it had to re-gather them to call into the C++ API. But a diff stacked below this one introduced a scattered API for the C++ frontend.

This reaps the benefits and makes sure that if the Variable kernel gets scattered arguments (i.e. it's a c10-full op), then it passes those on without regathering

codegen diff: P143616493

Differential Revision: [D23512538](https://our.internmc.facebook.com/intern/diff/D23512538/)

[ghstack-poisoned]
smessmer added a commit that referenced this pull request Sep 30, 2020
Pull Request resolved: #44158

Previously, the C++ API only supported calling ops with a gathered TensorOptions object. So even if the VariableKernel took scattered arguments,
it had to re-gather them to call into the C++ API. But a diff stacked below this one introduced a scattered API for the C++ frontend.

This reaps the benefits and makes sure that if the Variable kernel gets scattered arguments (i.e. it's a c10-full op), then it passes those on without regathering
ghstack-source-id: 113235799

Differential Revision: [D23512538](https://our.internmc.facebook.com/intern/diff/D23512538/)
Previously, the C++ API only supported calling ops with a gathered TensorOptions object. So even if the VariableKernel took scattered arguments,
it had to re-gather them to call into the C++ API. But a diff stacked below this one introduced a scattered API for the C++ frontend.

This reaps the benefits and makes sure that if the Variable kernel gets scattered arguments (i.e. it's a c10-full op), then it passes those on without regathering

codegen diff: P143616493

Differential Revision: [D23512538](https://our.internmc.facebook.com/intern/diff/D23512538/)

[ghstack-poisoned]
smessmer added a commit that referenced this pull request Oct 1, 2020
Pull Request resolved: #44158

Previously, the C++ API only supported calling ops with a gathered TensorOptions object. So even if the VariableKernel took scattered arguments,
it had to re-gather them to call into the C++ API. But a diff stacked below this one introduced a scattered API for the C++ frontend.

This reaps the benefits and makes sure that if the Variable kernel gets scattered arguments (i.e. it's a c10-full op), then it passes those on without regathering
ghstack-source-id: 113341222

Differential Revision: [D23512538](https://our.internmc.facebook.com/intern/diff/D23512538/)
Previously, the C++ API only supported calling ops with a gathered TensorOptions object. So even if the VariableKernel took scattered arguments,
it had to re-gather them to call into the C++ API. But a diff stacked below this one introduced a scattered API for the C++ frontend.

This reaps the benefits and makes sure that if the Variable kernel gets scattered arguments (i.e. it's a c10-full op), then it passes those on without regathering

codegen diff: P143616493

Differential Revision: [D23512538](https://our.internmc.facebook.com/intern/diff/D23512538/)

[ghstack-poisoned]
smessmer added a commit that referenced this pull request Oct 1, 2020
Pull Request resolved: #44158

Previously, the C++ API only supported calling ops with a gathered TensorOptions object. So even if the VariableKernel took scattered arguments,
it had to re-gather them to call into the C++ API. But a diff stacked below this one introduced a scattered API for the C++ frontend.

This reaps the benefits and makes sure that if the Variable kernel gets scattered arguments (i.e. it's a c10-full op), then it passes those on without regathering
ghstack-source-id: 113355690

Differential Revision: [D23512538](https://our.internmc.facebook.com/intern/diff/D23512538/)
@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 82cc86b.

@facebook-github-bot facebook-github-bot deleted the gh/smessmer/256/head branch October 5, 2020 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants