Skip to content

Conversation

@patrickvonplaten
Copy link
Contributor

@patrickvonplaten patrickvonplaten commented Oct 7, 2022

This is needed to allow to use the "device_map" for models that have nn.Parameter at the top level, such as the safety checker of diffusers: https://github.com/huggingface/diffusers/pull/772/files#r990269773

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Oct 7, 2022

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

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.

What's a reproducer? I'm not sure how you get to a parameter here since we only apply the children method.

@patrickvonplaten
Copy link
Contributor Author

patrickvonplaten commented Oct 7, 2022

What's a reproducer? I'm not sure how you get to a parameter here since we only apply the children method.

Haha that was fast. Here a repro:

from diffusers.pipelines.stable_diffusion import StableDiffusionSafetyChecker

StableDiffusionSafetyChecker._no_split_modules = ["CLIPEncoderLayer"]
pipe = StableDiffusionSafetyChecker.from_pretrained('CompVis/stable-diffusion-safety-checker', device_map='auto')

(sorry, it's 1GB of download)

@sgugger
Copy link
Collaborator

sgugger commented Oct 7, 2022

Which branch should I check? On main I just get an error telling me device_map auto is not supported yet

@patrickvonplaten
Copy link
Contributor Author

Ah sorry just updated it above, can you try:

from diffusers.pipelines.stable_diffusion import StableDiffusionSafetyChecker

StableDiffusionSafetyChecker._no_split_modules = ["CLIPEncoderLayer"]
pipe = StableDiffusionSafetyChecker.from_pretrained('CompVis/stable-diffusion-safety-checker', device_map='auto')

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.

Thanks for the reproducer! The function is indeed used with some parameters inside accelerate, so this is needed.

Just suggested another test.

@patrickvonplaten
Copy link
Contributor Author

Awesome! Can I merge after one ✔️ here or wait for another reviewer?

@sgugger
Copy link
Collaborator

sgugger commented Oct 7, 2022

Nope, you can go ahead and merge!

@patrickvonplaten patrickvonplaten merged commit b047761 into main Oct 10, 2022
@patrickvonplaten patrickvonplaten deleted the parameters_do_not_have_kids branch October 10, 2022 13:13
sgugger added a commit that referenced this pull request Oct 17, 2022
* [Device map] nn.Parameter don't have children

* Update src/accelerate/utils/modeling.py

Co-authored-by: Sylvain Gugger <[email protected]>

Co-authored-by: Sylvain Gugger <[email protected]>
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.

4 participants