Skip to content
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

FSDP enhancements and fixes #1753

Merged
merged 4 commits into from
Jul 21, 2023

Conversation

pacman100
Copy link
Contributor

@pacman100 pacman100 commented Jul 21, 2023

What does this PR do?

  1. If the model is already an instance of FSDP, bypass the warning and additional FSDP preparation logic. This is to enable PR fsdp fixes and enhancements transformers#24980 which Fixes New Version Usage Issue transformers#24724
  2. allow usage of _no_split_modules to simplify UX when using FSDP. Fixes Set FSDP transformer_layer_cls_to_wrap to model._no_split_modules ? transformers#24568 via fsdp fixes and enhancements transformers#24980
  3. extract_from_parallel how unwraps FSDP models too but these will lead to FSDP issues if there are modules as part of the global FSDP unit as the parameters of those modules won't be gathered if one isn't calling global FSDP wrapper's forward call. This is wrt issue Validation Errors When Training ControlNet with FSDP diffusers#4037

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Jul 21, 2023

The documentation is not available anymore as the PR was closed or merged.

@pacman100 pacman100 changed the title if the model is already an FSDP instance, remove the warning and prep overhead if the model is already an FSDP instance, remove the warning and prepare overhead Jul 21, 2023
@pacman100 pacman100 changed the title if the model is already an FSDP instance, remove the warning and prepare overhead If the model is already an instance of FSDP, bypass the warning and additional FSDP preparation logic Jul 21, 2023
@pacman100 pacman100 changed the title If the model is already an instance of FSDP, bypass the warning and additional FSDP preparation logic FSDP enhancements and fixes Jul 21, 2023
@pacman100 pacman100 marked this pull request as ready for review July 21, 2023 09:42
@pacman100 pacman100 requested a review from sgugger July 21, 2023 09:42
Copy link
Collaborator

@sgugger sgugger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me, thanks!

@pacman100 pacman100 merged commit a2d8f54 into huggingface:main Jul 21, 2023
winglian pushed a commit to OpenAccess-AI-Collective/accelerate that referenced this pull request Jul 25, 2023
* if the model is already an FSDP instance, remove the warning and prep overhead

* allow usage of `_no_split_modules` to simplify UX when using FSDP

* Update other.py

* fixes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New Version Usage Issue Set FSDP transformer_layer_cls_to_wrap to model._no_split_modules ?
3 participants