[Reland] [DDP] Implement a hook which performs FunctionalSGD step. #62177
Conversation
Reland of #61678 Fix CI failure by gating including torchvision model on whether torchvision is available or not. Differential Revision: [D29904101](https://our.internmc.facebook.com/intern/diff/D29904101/) [ghstack-poisoned]
rohan-varma
requested review from
cbalioglu,
H-Huang,
mingzhe09088,
mrshenli,
pritamdamania87,
wayi1 and
zhaojuanmao
as code owners
🔗 Helpful links
💊 CI failures summary and remediationsAs of commit 8e88c47 (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. |
facebook-github-bot
added
oncall: distributed
Reland of #61678 Fix CI failure by gating including torchvision model on whether torchvision is available or not. Differential Revision: [D29904101](https://our.internmc.facebook.com/intern/diff/D29904101/) ghstack-source-id: 134282165 Pull Request resolved: #62177
| @@ -22,7 +22,8 @@ def __init__( | |||
| momentum: float = 0.0, | |||
| dampening: float = 0.0, | |||
| weight_decay: float = 0.0, | |||
| nesterov: bool = False | |||
| nesterov: bool = False, | |||
| allow_empty_param_list: bool = False | |||
There was a problem hiding this comment.
Curious, should we be concerned with TorchScript forward compatibility issues due to changing the number of function arguments? If not, is that because _FunctionalSGD is private, so its API is permitted to change?
I also want to mention that I agree with the motivation of this allow_empty_param_list argument. I similarly found the need for it when working on overlapping DDP with ZeRO.
There was a problem hiding this comment.
Regarding FC, indeed a TS interpreter built without this diff would not recognize the new argument, but I don't think this is being used in any such scenarios and _FunctionalSGD is also private.
|
This pull request has been merged in 6dc2c07. |
Stack from ghstack:
Reland of #61678
Fix CI failure by gating including torchvision model on whether torchvision is available or not.
Differential Revision: D29904101