-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Expanded init fields in StableDiffusionPipeline cause incompatibilities with many/most inherited pipelines #6969
Comments
Cc: @yiyixuxu |
Hi @vladmandic I don't consider this a breaking change for this reason. However would very much like to make the community pipelines work correctly again. We can ask the community to help refactor these PRs to use cc @sayakpaul @DN6 here I think we should start to check the base classes for community pipeline PRs and don't merging in unless they use |
What you suggested is the best case here as it's the simplest. |
Is this something can be done in SD.Next for a temporary solution while we trying to update all the community pipelines to use
|
thanks - i agree with action plan. |
i confirm that monkey-patching works as as stop-gap solution from diffusers.pipelines import pipeline_utils
def hijack_register_modules(self, **kwargs):
for name, module in kwargs.items():
if module is None or isinstance(module, (tuple, list)) and module[0] is None:
register_dict = {name: (None, None)}
elif isinstance(module, bool):
pass
else:
library, class_name = pipeline_utils._fetch_class_library_tuple(module)
register_dict = {name: (library, class_name)}
self.register_to_config(**register_dict)
setattr(self, name, module)
pipeline_utils.DiffusionPipeline.register_modules = hijack_register_modules |
This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread. Please note that issues that do not follow the contributing guidelines are likely to be ignored. |
I think this is not stale. @yiyixuxu can you confirm? |
This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread. Please note that issues that do not follow the contributing guidelines are likely to be ignored. |
ping. |
This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread. Please note that issues that do not follow the contributing guidelines are likely to be ignored. |
ping. |
@vladmandic do yo maybe have a list of the community pipelines that are you interested in fixing? Probably will be better if we open an issue with a specific list than a generic one. Regional prompting is the main one right? For some time I wanted to take a look into it and maybe do a new one with the same for controlnet. |
i'd need to go over the list of community pipelines to see which ones are most useful. |
yeah, I'll probably take that one, I mean the SDXL one, I don't use SD 1.5 anymore but I can still fix it as a bonus while making the SDXL one. |
This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread. Please note that issues that do not follow the contributing guidelines are likely to be ignored. |
This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread. Please note that issues that do not follow the contributing guidelines are likely to be ignored. |
ping to remove stale |
added the wip so it doesn't get stale. Also added the contributions tags so if people want to contribute we would really appreciate it. This is a good first issue for new comers. |
Describe the bug
class
StableDiffusionPipeline
has its init section as:and here
image_encoder
was recently introduced thus changing the class signaturebut most community pipelines do not include init field for recently included
image_encoder
and thus order or params is wrongfor example,
examples/community/regional_prompting_stable_diffusion.py
has this in its init:which means
bool
value fromrequires_safety_checker
is going to be passed asimage_encoder
and pipeline will fail during initialization like this:this is a conceptual problem with changing master class signature while all inherited classes pass args list as simple list.
i don't see a simple solution as going back is bad and going forward requires updates to a lot of pipelines.
but at the very basic, at least add error handling to
_fetch_class_library_tuple
so invalid type does not cause entire solution to crash.Reproduction
load nearly any community pipeline or any pipeline inherited from StableDiffusionPipeline, but not updated to use new signature in its
super().__init__
callLogs
No response
System Info
diffusers==0.26.3
Who can help?
@yiyixuxu @DN6 @sayakpaul @patrickvonplaten
The text was updated successfully, but these errors were encountered: