-
Notifications
You must be signed in to change notification settings - Fork 23k
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
[DDP Communication Hook] Simplify the implementation of parseHookResult of PythonCommHook #62389
Conversation
…lt of PythonCommHook Simplify the implementation of `parseHookResult` of `PythonCommHook`, by not directly accepting the output of allreduce, which is a tensor list. Address the comment on #62074 (comment) Additionally, formatter is also applied to `OptimizerHookState` and `hook_then_optimizer`. Differential Revision: [D29982485](https://our.internmc.facebook.com/intern/diff/D29982485/) [ghstack-poisoned]
🔗 Helpful links
💊 CI failures summary and remediationsAs of commit cb923fe (more details on the Dr. CI page): 💚 💚 Looks good so far! There are no failures yet. 💚 💚 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. |
…lt of PythonCommHook Simplify the implementation of `parseHookResult` of `PythonCommHook`, by not directly accepting the output of allreduce, which is a tensor list. Address the comment on #62074 (comment) Additionally, formatter is also applied to `OptimizerHookState` and `hook_then_optimizer`. Differential Revision: [D29982485](https://our.internmc.facebook.com/intern/diff/D29982485/) ghstack-source-id: 134599912 Pull Request resolved: #62389
…rseHookResult of PythonCommHook" Simplify the implementation of `parseHookResult` of `PythonCommHook`, by not directly accepting the output of allreduce, which is a tensor list. Address the comment on #62074 (comment) Additionally, formatter is also applied to `OptimizerHookState` and `hook_then_optimizer`. Differential Revision: [D29982485](https://our.internmc.facebook.com/intern/diff/D29982485/) [ghstack-poisoned]
…rseHookResult of PythonCommHook" Simplify the implementation of `parseHookResult` of `PythonCommHook`, by not directly accepting the output of allreduce, which is a tensor list. Address the comment on #62074 (comment) Additionally, formatter is also applied to `OptimizerHookState` and `hook_then_optimizer`. Differential Revision: [D29982485](https://our.internmc.facebook.com/intern/diff/D29982485/) [ghstack-poisoned]
…rseHookResult of PythonCommHook" Simplify the implementation of `parseHookResult` of `PythonCommHook`, by not directly accepting the output of allreduce, which is a tensor list. Address the comment on #62074 (comment) Additionally, formatter is also applied to `OptimizerHookState` and `hook_then_optimizer`. Differential Revision: [D29982485](https://our.internmc.facebook.com/intern/diff/D29982485/) [ghstack-poisoned]
…rseHookResult of PythonCommHook" Simplify the implementation of `parseHookResult` of `PythonCommHook`, by not directly accepting the output of allreduce, which is a tensor list. Address the comment on #62074 (comment) Additionally, formatter is also applied to `OptimizerHookState` and `hook_then_optimizer`. Differential Revision: [D29982485](https://our.internmc.facebook.com/intern/diff/D29982485/) [ghstack-poisoned]
…rseHookResult of PythonCommHook" Simplify the implementation of `parseHookResult` of `PythonCommHook`, by not directly accepting the output of allreduce, which is a tensor list. Address the comment on #62074 (comment) Additionally, formatter is also applied to `OptimizerHookState` and `hook_then_optimizer`. Differential Revision: [D29982485](https://our.internmc.facebook.com/intern/diff/D29982485/) [ghstack-poisoned]
…lt of PythonCommHook Pull Request resolved: #62389 Simplify the implementation of `parseHookResult` of `PythonCommHook`, by not directly accepting the output of allreduce, which is a tensor list. Address the comment on #62074 (comment) Additionally, formatter is also applied to `OptimizerHookState` and `hook_then_optimizer`. ghstack-source-id: 134603664 Differential Revision: [D29982485](https://our.internmc.facebook.com/intern/diff/D29982485/)
…rseHookResult of PythonCommHook" Simplify the implementation of `parseHookResult` of `PythonCommHook`, by not directly accepting the output of allreduce, which is a tensor list. Address the comment on #62074 (comment) Additionally, formatter is also applied to `OptimizerHookState` and `hook_then_optimizer`. Differential Revision: [D29982485](https://our.internmc.facebook.com/intern/diff/D29982485/) [ghstack-poisoned]
…lt of PythonCommHook Pull Request resolved: #62389 Simplify the implementation of `parseHookResult` of `PythonCommHook`, by not directly accepting the output of allreduce, which is a tensor list. Address the comment on #62074 (comment) Additionally, formatter is also applied to `OptimizerHookState` and `hook_then_optimizer`. ghstack-source-id: 134626246 Differential Revision: [D29982485](https://our.internmc.facebook.com/intern/diff/D29982485/)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have we updated all the docstrings to indicate the appropriate return value is now Future[Tensor]
?
Good point! Will do in the next PR. |
This pull request has been merged in acba9b3. |
Stack from ghstack:
Simplify the implementation of
parseHookResult
ofPythonCommHook
, by not directly accepting the output of allreduce, which is a tensor list.Address the comment on #62074 (comment)
Additionally, formatter is also applied to
OptimizerHookState
andhook_then_optimizer
.Differential Revision: D29982485