Closed
Conversation
Previously, when doing boxed dispatch (e.g. custom ops), the dispatcher manually removed the VariableTensorId flag before dispatching because custom ops don't have variable kernels. This is one of the blockers that prevented us from using the boxed dispatch mechanism for ops from native_functions.yaml because they define variable kernels and need them to be called for autograd. This PR changes that. The dispatcher doesn't remove the VariableTensorId flag anymore. Instead, to make custom ops work, we implement a variable fallback kernel that is called whenever no other variable kernel was found. Differential Revision: [D18542342](https://our.internmc.facebook.com/intern/diff/D18542342/) [ghstack-poisoned]
This was referenced Nov 15, 2019
Closed
Closed
Previously, when doing boxed dispatch (e.g. custom ops), the dispatcher manually removed the VariableTensorId flag before dispatching because custom ops don't have variable kernels. This is one of the blockers that prevented us from using the boxed dispatch mechanism for ops from native_functions.yaml because they define variable kernels and need them to be called for autograd. This PR changes that. The dispatcher doesn't remove the VariableTensorId flag anymore. Instead, to make custom ops work, we implement a variable fallback kernel that is called whenever no other variable kernel was found. See https://fb.quip.com/czCUA1NzMeEt for the detailed plan. Differential Revision: [D18542342](https://our.internmc.facebook.com/intern/diff/D18542342/) [ghstack-poisoned]
smessmer
added a commit
that referenced
this pull request
Nov 16, 2019
Pull Request resolved: #29934 Previously, when doing boxed dispatch (e.g. custom ops), the dispatcher manually removed the VariableTensorId flag before dispatching because custom ops don't have variable kernels. This is one of the blockers that prevented us from using the boxed dispatch mechanism for ops from native_functions.yaml because they define variable kernels and need them to be called for autograd. This PR changes that. The dispatcher doesn't remove the VariableTensorId flag anymore. Instead, to make custom ops work, we implement a variable fallback kernel that is called whenever no other variable kernel was found. ghstack-source-id: 94056388 Differential Revision: [D18542342](https://our.internmc.facebook.com/intern/diff/D18542342/)
ezyang
reviewed
Nov 18, 2019
|
|
||
| void variable_fallback_kernel(const OperatorHandle& op, Stack* stack) { | ||
| at::AutoNonVariableTypeMode _var_guard(true); | ||
| Dispatcher::singleton().callBoxed(op, stack); |
Contributor
There was a problem hiding this comment.
OK, this isn't really what I've been advocating for, but I'll let it slide for now.
Contributor
Author
There was a problem hiding this comment.
it's not? IIRC, we said we're setting the thread local flag and re-dispatching. That's what I'm doing here. What would you do instead?
Contributor
There was a problem hiding this comment.
We also need to setup grad_fn on the outputs to error if you try to backprop through them! Otherwise you'll just pop out requires_grad=False outputs which will silently fail to backprop if someone uses them.
Contributor
Author
There was a problem hiding this comment.
Ah, that's what you mean. Yes, we should do that. This doesn't regress behavior though, it's just as broken as it was before. There were some issues with adding the grad_fn if I remember correctly which meant we can't do it right now, but I'll look at it in a separate PR.
Contributor
There was a problem hiding this comment.
Yep, that's why I approved.
ezyang
approved these changes
Nov 18, 2019
Previously, when doing boxed dispatch (e.g. custom ops), the dispatcher manually removed the VariableTensorId flag before dispatching because custom ops don't have variable kernels. This is one of the blockers that prevented us from using the boxed dispatch mechanism for ops from native_functions.yaml because they define variable kernels and need them to be called for autograd. This PR changes that. The dispatcher doesn't remove the VariableTensorId flag anymore. Instead, to make custom ops work, we implement a variable fallback kernel that is called whenever no other variable kernel was found. See https://fb.quip.com/czCUA1NzMeEt for the detailed plan. Differential Revision: [D18542342](https://our.internmc.facebook.com/intern/diff/D18542342/) [ghstack-poisoned]
Previously, when doing boxed dispatch (e.g. custom ops), the dispatcher manually removed the VariableTensorId flag before dispatching because custom ops don't have variable kernels. This is one of the blockers that prevented us from using the boxed dispatch mechanism for ops from native_functions.yaml because they define variable kernels and need them to be called for autograd. This PR changes that. The dispatcher doesn't remove the VariableTensorId flag anymore. Instead, to make custom ops work, we implement a variable fallback kernel that is called whenever no other variable kernel was found. See https://fb.quip.com/czCUA1NzMeEt for the detailed plan. Differential Revision: [D18542342](https://our.internmc.facebook.com/intern/diff/D18542342/) [ghstack-poisoned]
smessmer
added a commit
that referenced
this pull request
Nov 18, 2019
Pull Request resolved: #29934 Previously, when doing boxed dispatch (e.g. custom ops), the dispatcher manually removed the VariableTensorId flag before dispatching because custom ops don't have variable kernels. This is one of the blockers that prevented us from using the boxed dispatch mechanism for ops from native_functions.yaml because they define variable kernels and need them to be called for autograd. This PR changes that. The dispatcher doesn't remove the VariableTensorId flag anymore. Instead, to make custom ops work, we implement a variable fallback kernel that is called whenever no other variable kernel was found. ghstack-source-id: 94139776 Differential Revision: [D18542342](https://our.internmc.facebook.com/intern/diff/D18542342/)
Previously, when doing boxed dispatch (e.g. custom ops), the dispatcher manually removed the VariableTensorId flag before dispatching because custom ops don't have variable kernels. This is one of the blockers that prevented us from using the boxed dispatch mechanism for ops from native_functions.yaml because they define variable kernels and need them to be called for autograd. This PR changes that. The dispatcher doesn't remove the VariableTensorId flag anymore. Instead, to make custom ops work, we implement a variable fallback kernel that is called whenever no other variable kernel was found. See https://fb.quip.com/czCUA1NzMeEt for the detailed plan. Differential Revision: [D18542342](https://our.internmc.facebook.com/intern/diff/D18542342/) [ghstack-poisoned]
This was referenced Nov 22, 2019
Previously, when doing boxed dispatch (e.g. custom ops), the dispatcher manually removed the VariableTensorId flag before dispatching because custom ops don't have variable kernels. This is one of the blockers that prevented us from using the boxed dispatch mechanism for ops from native_functions.yaml because they define variable kernels and need them to be called for autograd. This PR changes that. The dispatcher doesn't remove the VariableTensorId flag anymore. Instead, to make custom ops work, we implement a variable fallback kernel that is called whenever no other variable kernel was found. See https://fb.quip.com/czCUA1NzMeEt for the detailed plan. Differential Revision: [D18542342](https://our.internmc.facebook.com/intern/diff/D18542342/) [ghstack-poisoned]
smessmer
added a commit
that referenced
this pull request
Nov 22, 2019
Pull Request resolved: #29934 Previously, when doing boxed dispatch (e.g. custom ops), the dispatcher manually removed the VariableTensorId flag before dispatching because custom ops don't have variable kernels. This is one of the blockers that prevented us from using the boxed dispatch mechanism for ops from native_functions.yaml because they define variable kernels and need them to be called for autograd. This PR changes that. The dispatcher doesn't remove the VariableTensorId flag anymore. Instead, to make custom ops work, we implement a variable fallback kernel that is called whenever no other variable kernel was found. ghstack-source-id: 94460733 Differential Revision: [D18542342](https://our.internmc.facebook.com/intern/diff/D18542342/)
Previously, when doing boxed dispatch (e.g. custom ops), the dispatcher manually removed the VariableTensorId flag before dispatching because custom ops don't have variable kernels. This is one of the blockers that prevented us from using the boxed dispatch mechanism for ops from native_functions.yaml because they define variable kernels and need them to be called for autograd. This PR changes that. The dispatcher doesn't remove the VariableTensorId flag anymore. Instead, to make custom ops work, we implement a variable fallback kernel that is called whenever no other variable kernel was found. See https://fb.quip.com/czCUA1NzMeEt for the detailed plan. Differential Revision: [D18542342](https://our.internmc.facebook.com/intern/diff/D18542342/) [ghstack-poisoned]
Previously, when doing boxed dispatch (e.g. custom ops), the dispatcher manually removed the VariableTensorId flag before dispatching because custom ops don't have variable kernels. This is one of the blockers that prevented us from using the boxed dispatch mechanism for ops from native_functions.yaml because they define variable kernels and need them to be called for autograd. This PR changes that. The dispatcher doesn't remove the VariableTensorId flag anymore. Instead, to make custom ops work, we implement a variable fallback kernel that is called whenever no other variable kernel was found. See https://fb.quip.com/czCUA1NzMeEt for the detailed plan. Differential Revision: [D18542342](https://our.internmc.facebook.com/intern/diff/D18542342/) [ghstack-poisoned]
Previously, when doing boxed dispatch (e.g. custom ops), the dispatcher manually removed the VariableTensorId flag before dispatching because custom ops don't have variable kernels. This is one of the blockers that prevented us from using the boxed dispatch mechanism for ops from native_functions.yaml because they define variable kernels and need them to be called for autograd. This PR changes that. The dispatcher doesn't remove the VariableTensorId flag anymore. Instead, to make custom ops work, we implement a variable fallback kernel that is called whenever no other variable kernel was found. See https://fb.quip.com/czCUA1NzMeEt for the detailed plan. Differential Revision: [D18542342](https://our.internmc.facebook.com/intern/diff/D18542342/) [ghstack-poisoned]
Previously, when doing boxed dispatch (e.g. custom ops), the dispatcher manually removed the VariableTensorId flag before dispatching because custom ops don't have variable kernels. This is one of the blockers that prevented us from using the boxed dispatch mechanism for ops from native_functions.yaml because they define variable kernels and need them to be called for autograd. This PR changes that. The dispatcher doesn't remove the VariableTensorId flag anymore. Instead, to make custom ops work, we implement a variable fallback kernel that is called whenever no other variable kernel was found. See https://fb.quip.com/czCUA1NzMeEt for the detailed plan. Differential Revision: [D18542342](https://our.internmc.facebook.com/intern/diff/D18542342/) [ghstack-poisoned]
smessmer
added a commit
that referenced
this pull request
Nov 26, 2019
Pull Request resolved: #29934 Previously, when doing boxed dispatch (e.g. custom ops), the dispatcher manually removed the VariableTensorId flag before dispatching because custom ops don't have variable kernels. This is one of the blockers that prevented us from using the boxed dispatch mechanism for ops from native_functions.yaml because they define variable kernels and need them to be called for autograd. This PR changes that. The dispatcher doesn't remove the VariableTensorId flag anymore. Instead, to make custom ops work, we implement a variable fallback kernel that is called whenever no other variable kernel was found. ghstack-source-id: 94563165 Differential Revision: [D18542342](https://our.internmc.facebook.com/intern/diff/D18542342/)
Previously, when doing boxed dispatch (e.g. custom ops), the dispatcher manually removed the VariableTensorId flag before dispatching because custom ops don't have variable kernels. This is one of the blockers that prevented us from using the boxed dispatch mechanism for ops from native_functions.yaml because they define variable kernels and need them to be called for autograd. This PR changes that. The dispatcher doesn't remove the VariableTensorId flag anymore. Instead, to make custom ops work, we implement a variable fallback kernel that is called whenever no other variable kernel was found. See https://fb.quip.com/czCUA1NzMeEt for the detailed plan. Differential Revision: [D18542342](https://our.internmc.facebook.com/intern/diff/D18542342/) [ghstack-poisoned]
Previously, when doing boxed dispatch (e.g. custom ops), the dispatcher manually removed the VariableTensorId flag before dispatching because custom ops don't have variable kernels. This is one of the blockers that prevented us from using the boxed dispatch mechanism for ops from native_functions.yaml because they define variable kernels and need them to be called for autograd. This PR changes that. The dispatcher doesn't remove the VariableTensorId flag anymore. Instead, to make custom ops work, we implement a variable fallback kernel that is called whenever no other variable kernel was found. See https://fb.quip.com/czCUA1NzMeEt for the detailed plan. Differential Revision: [D18542342](https://our.internmc.facebook.com/intern/diff/D18542342/) [ghstack-poisoned]
smessmer
added a commit
that referenced
this pull request
Nov 26, 2019
Pull Request resolved: #29934 Previously, when doing boxed dispatch (e.g. custom ops), the dispatcher manually removed the VariableTensorId flag before dispatching because custom ops don't have variable kernels. This is one of the blockers that prevented us from using the boxed dispatch mechanism for ops from native_functions.yaml because they define variable kernels and need them to be called for autograd. This PR changes that. The dispatcher doesn't remove the VariableTensorId flag anymore. Instead, to make custom ops work, we implement a variable fallback kernel that is called whenever no other variable kernel was found. ghstack-source-id: 94618474 Differential Revision: [D18542342](https://our.internmc.facebook.com/intern/diff/D18542342/)
wuhuikx
pushed a commit
to wuhuikx/pytorch
that referenced
this pull request
Jan 30, 2020
Summary: Pull Request resolved: pytorch#29934 Previously, when doing boxed dispatch (e.g. custom ops), the dispatcher manually removed the VariableTensorId flag before dispatching because custom ops don't have variable kernels. This is one of the blockers that prevented us from using the boxed dispatch mechanism for ops from native_functions.yaml because they define variable kernels and need them to be called for autograd. This PR changes that. The dispatcher doesn't remove the VariableTensorId flag anymore. Instead, to make custom ops work, we implement a variable fallback kernel that is called whenever no other variable kernel was found. ghstack-source-id: 94618474 Test Plan: unit tests Differential Revision: D18542342 fbshipit-source-id: a30ae35d98f89f7ae507151f55c42cfbed54a451
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stack from ghstack:
Previously, when doing boxed dispatch (e.g. custom ops), the dispatcher manually removed the VariableTensorId flag before dispatching
because custom ops don't have variable kernels.
This is one of the blockers that prevented us from using the boxed dispatch mechanism for ops from native_functions.yaml because they define variable kernels and need them to be called for autograd.
This PR changes that. The dispatcher doesn't remove the VariableTensorId flag anymore.
Instead, to make custom ops work, we implement a variable fallback kernel that is called whenever no other variable kernel was found.
See https://fb.quip.com/czCUA1NzMeEt for the detailed plan.
Differential Revision: D18542342