-
Notifications
You must be signed in to change notification settings - Fork 24.6k
[FSDP] Generalize fsdp_modules() #73553
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
Conversation
Generalize fsdp_modules(): 1. Make fsdp_modules() a static method so that it can be called even when users have no reference to the FSDP module -- it can be wrapped inside an non-FSDP module. 2. Add root_only option to returns only the root FSDP module(s). Differential Revision: [D34537408](https://our.internmc.facebook.com/intern/diff/D34537408/) [ghstack-poisoned]
🔗 Helpful links
💊 CI failures summary and remediationsAs of commit 39a6c15 (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).Please report bugs/suggestions to the (internal) Dr. CI Users group. |
CI Flow Status⚛️ CI FlowRuleset - Version:
|
Generalize fsdp_modules(): 1. Make fsdp_modules() a static method so that it can be called even when users have no reference to the FSDP module -- it can be wrapped inside an non-FSDP module. 2. Add root_only option to returns only the root FSDP module(s). Differential Revision: [D34537408](https://our.internmc.facebook.com/intern/diff/D34537408/) ghstack-source-id: 150164370 Pull Request resolved: #73553
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.
Lgtm but could we have some unit test with this change?
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.
codes look good to me, thanks for adding this!
Generalize fsdp_modules(): 1. Make fsdp_modules() a static method so that it can be called even when users have no reference to the FSDP module -- it can be wrapped inside an non-FSDP module. 2. Add root_only option to returns only the root FSDP module(s). Differential Revision: [D34537408](https://our.internmc.facebook.com/intern/diff/D34537408/) [ghstack-poisoned]
Generalize fsdp_modules(): 1. Make fsdp_modules() a static method so that it can be called even when users have no reference to the FSDP module -- it can be wrapped inside an non-FSDP module. 2. Add root_only option to returns only the root FSDP module(s). Differential Revision: [D34537408](https://our.internmc.facebook.com/intern/diff/D34537408/) [ghstack-poisoned]
Pull Request resolved: #73553 Generalize fsdp_modules(): 1. Make fsdp_modules() a static method so that it can be called even when users have no reference to the FSDP module -- it can be wrapped inside an non-FSDP module. 2. Add root_only option to returns only the root FSDP module(s). ghstack-source-id: 150392207 Differential Revision: [D34537408](https://our.internmc.facebook.com/intern/diff/D34537408/)
Generalize fsdp_modules(): 1. Make fsdp_modules() a static method so that it can be called even when users have no reference to the FSDP module -- it can be wrapped inside an non-FSDP module. 2. Add root_only option to returns only the root FSDP module(s). Differential Revision: [D34537408](https://our.internmc.facebook.com/intern/diff/D34537408/) [ghstack-poisoned]
Pull Request resolved: #73553 Generalize fsdp_modules(): 1. Make fsdp_modules() a static method so that it can be called even when users have no reference to the FSDP module -- it can be wrapped inside an non-FSDP module. 2. Add root_only option to returns only the root FSDP module(s). ghstack-source-id: 150442403 Differential Revision: [D34537408](https://our.internmc.facebook.com/intern/diff/D34537408/)
Generalize fsdp_modules(): 1. Make fsdp_modules() a static method so that it can be called even when users have no reference to the FSDP module -- it can be wrapped inside an non-FSDP module. 2. Add root_only option to returns only the root FSDP module(s). Differential Revision: [D34537408](https://our.internmc.facebook.com/intern/diff/D34537408/) [ghstack-poisoned]
Pull Request resolved: #73553 Generalize fsdp_modules(): 1. Make fsdp_modules() a static method so that it can be called even when users have no reference to the FSDP module -- it can be wrapped inside an non-FSDP module. 2. Add root_only option to returns only the root FSDP module(s). ghstack-source-id: 150690955 Differential Revision: [D34537408](https://our.internmc.facebook.com/intern/diff/D34537408/)
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.
Thanks for adding the tests!
Summary: Pull Request resolved: #73553 Generalize fsdp_modules(): 1. Make fsdp_modules() a static method so that it can be called even when users have no reference to the FSDP module -- it can be wrapped inside an non-FSDP module. 2. Add root_only option to returns only the root FSDP module(s). ghstack-source-id: 150690955 Test Plan: CI Reviewed By: rohan-varma Differential Revision: D34537408 fbshipit-source-id: 1bf057704954e8177d0a28f2b3d0dae4da882249
Hey @fegin. |
Summary: Pull Request resolved: pytorch/pytorch#73553 Generalize fsdp_modules(): 1. Make fsdp_modules() a static method so that it can be called even when users have no reference to the FSDP module -- it can be wrapped inside an non-FSDP module. 2. Add root_only option to returns only the root FSDP module(s). ghstack-source-id: 150690955 Test Plan: CI Reviewed By: rohan-varma Differential Revision: D34537408 fbshipit-source-id: 1bf057704954e8177d0a28f2b3d0dae4da882249 (cherry picked from commit 437aad7a3b612ca275213c32c2fa6ac37f2ed3ab)
Summary: Pull Request resolved: pytorch/pytorch#73553 Generalize fsdp_modules(): 1. Make fsdp_modules() a static method so that it can be called even when users have no reference to the FSDP module -- it can be wrapped inside an non-FSDP module. 2. Add root_only option to returns only the root FSDP module(s). ghstack-source-id: 150690955 Test Plan: CI Reviewed By: rohan-varma Differential Revision: D34537408 fbshipit-source-id: 1bf057704954e8177d0a28f2b3d0dae4da882249 (cherry picked from commit 437aad7a3b612ca275213c32c2fa6ac37f2ed3ab)
Stack from ghstack (oldest at bottom):
Generalize fsdp_modules():
Differential Revision: D34537408