-
Notifications
You must be signed in to change notification settings - Fork 523
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
One-line change to correctly dispatch to cpu function for inference #2881
Conversation
✅ Deploy Preview for pytorch-fbgemm-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
This pull request was exported from Phabricator. Differential Revision: D48574563 |
This pull request was exported from Phabricator. Differential Revision: D48574563 |
1b3573e
to
56d5696
Compare
This pull request was exported from Phabricator. Differential Revision: D48574563 |
…ytorch#2881) Summary: Pull Request resolved: pytorch#2881 # context * the fundamental issue is that a "dispatch_to_cpu" function is calling the "autograd" function * a side issue is the poor naming: `permute_pooled_embs_auto_grad` sounds like an "autograd version" but actually as an operator the real backend (CPU, GPU, META, AUTOGRAD, etc.) is determined by the dispatcher. * this defact exists from day-1 since this operator was developed (D31271923). * the impact is that in the inference flow where the autograd is not needed, although the dispatcher does call the CPU version of this operator, the actual function calls the autograd function, which is not desired. # how to do it properly: * For a user-faced operator, we first need to give it a good name, like `permute_multi_embedding`. * then we need to implement functions for the necessary backends, commonly 4: CPU, CUDA, META, and AUTOGRAD * it's better to name the function as {operator_name}_{backend}, and **very important to link them correctly**, here is a good example: D48720379 * in training, the dispatcher will **ALWAYS** pickup the autograd version, then [calls cuda/cpu/meta version from the autograd function](https://fburl.com/code/2d1acuop) based on device or other context. * in inference, the dispatcher will ignore the autograd, and directly calls the cpu/cuda/meta version Reviewed By: dstaay-fb, sryap Differential Revision: D48574563
56d5696
to
dbd0569
Compare
…ytorch#2881) Summary: Pull Request resolved: pytorch#2881 # context * the fundamental issue is that a "dispatch_to_cpu" function is calling the "autograd" function * a side issue is the poor naming: `permute_pooled_embs_auto_grad` sounds like an "autograd version" but actually as an operator the real backend (CPU, GPU, META, AUTOGRAD, etc.) is determined by the dispatcher. * this defact exists from day-1 since this operator was developed (D31271923). * the impact is that in the inference flow where the autograd is not needed, although the dispatcher does call the CPU version of this operator, the actual function calls the autograd function, which is not desired. # how to do it properly: * For a user-faced operator, we first need to give it a good name, like `permute_multi_embedding`. * then we need to implement functions for the necessary backends, commonly 4: CPU, CUDA, META, and AUTOGRAD * it's better to name the function as {operator_name}_{backend}, and **very important to link them correctly**, here is a good example: D48720379 * in training, the dispatcher will **ALWAYS** pickup the autograd version, then [calls cuda/cpu/meta version from the autograd function](https://fburl.com/code/2d1acuop) based on device or other context. * in inference, the dispatcher will ignore the autograd, and directly calls the cpu/cuda/meta version Reviewed By: dstaay-fb, sryap Differential Revision: D48574563
This pull request was exported from Phabricator. Differential Revision: D48574563 |
dbd0569
to
44ad142
Compare
This pull request has been merged in 500c5fa. |
Summary: # context
Reviewed By: dstaay-fb
Differential Revision: D48574563