Skip to content

VariableKernel calls into scattered C++ api#44158

Closed
smessmer wants to merge 15 commits intogh/smessmer/256/basefrom
gh/smessmer/256/head
Closed

VariableKernel calls into scattered C++ api#44158
smessmer wants to merge 15 commits intogh/smessmer/256/basefrom
gh/smessmer/256/head

Conversation

@smessmer
Copy link
Copy Markdown
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
@smessmer smessmer requested review from bhosmer and ezyang September 3, 2020 22:32
@dr-ci
Copy link
Copy Markdown

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]
@smessmer smessmer requested a review from ezyang September 24, 2020 15:17
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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.

4 participants