-
Notifications
You must be signed in to change notification settings - Fork 6.6k
add a from_pipe method to DiffusionPipeline
#7241
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
|
cc @vladmandic here |
|
|
||
| # based on https://github.com/guoyww/AnimateDiff/blob/895f3220c06318ea0760131ec70408b466c49333/animatediff/models/unet.py#L459 | ||
| config = unet.config | ||
| config = dict(unet.config) |
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.
@DN6
currently, we will modify the original 2d unet's config - even though we do not use it here, we create a new unet motion model instead
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
| ``` | ||
| """ | ||
|
|
||
| if hasattr(pipeline, "_all_hooks") and len(pipeline._all_hooks) > 0: |
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.
this is the part we make sure if the pipeline has previous called enable_model_cpu_offload, it will still work properly with from_pipe
|
thanks yiyi!
|
|
thanks for the feedback! @vladmandic
I have not run into any issues using
I'm not so sure about this because:
with this being said, I think it won't be hard to implement and I'm open to it if you all think it's more intuitive and convenient to let the new pipelines inherit settings. cc @pcuenca here too, let me know what you think! |
|
cc @DN6 @sayakpaul for a final review |
|
thanks @yiyixuxu re: pipeline settings inheritance - IMO it would be more convenient and expected since its a pipeline switch using loaded model components (all or some), but its not a deal breaker - |
Co-authored-by: Sayak Paul <[email protected]> Co-authored-by: Dhruv Nair <[email protected]>
.../pipelines/stable_diffusion_attend_and_excite/pipeline_stable_diffusion_attend_and_excite.py
Show resolved
Hide resolved
| if name in expected_modules and name not in passed_class_obj: | ||
| # for model components, we will not switch over if the class does not matches the type hint in the new pipeline's signature | ||
| if ( | ||
| not isinstance(component, torch.nn.Module) |
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.
Maybe change this to see if it subclasses ModelMixin here?
DN6
left a comment
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 👍🏽
* add from_pipe --------- Co-authored-by: yiyixuxu <yixu310@gmail,com> Co-authored-by: Sayak Paul <[email protected]> Co-authored-by: Dhruv Nair <[email protected]>
* add from_pipe --------- Co-authored-by: yiyixuxu <yixu310@gmail,com> Co-authored-by: Sayak Paul <[email protected]> Co-authored-by: Dhruv Nair <[email protected]>
motivated by #6531
Create a stable diffusion pipeline with
from_pretrainedtest2: SD -> SAG
test3: run SD again
test4: run
pipe_sdafterpipe_sag.unload_ip_adapter()test5: SD -> AnimateDiff
test6: SD -> LPW
test7: run SD again