-
Notifications
You must be signed in to change notification settings - Fork 6.6k
All in one Stable Diffusion Pipeline #821
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
|
The documentation is not available anymore as the PR was closed or merged. |
|
Also cc @osanseviero @apolinario |
keturn
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.
usage looks straightforward enough 👍
|
|
||
| @property | ||
| def components(self) -> Dict[str, Any]: | ||
| return {k: v for k, v in vars(self).items() if not k.startswith("_")} |
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 part spooks me a little bit, just because vars(self) is so broad. I'd be more comfortable with something more explicit, maybe drawing from ConfigMixin.config?
I guess the thing to consider when thinking about the risk is: What does it look like when it fails? i.e. what will StableDiffusionPipeline.__init__ do when we pass it some attribute we picked up from here that it doesn't expect?
Does it fail with a useful message? Does it fail with a mysterious message? Does it silently ignore it?
Is there anything in place to make sure the signatures of all StableDiffusion*Pipeline are consistent and remain that way?
e.g. They may all be DiffusionPipelines, but not all DiffusionPipelines have a feature_extractor, etc.
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.
Understood! Ideally we should make a solid function like this right for the DiffusionPipeline class .
I'll try to make the function a bit safer (with better error messages)
| callback: Optional[Callable[[int, int, torch.FloatTensor], None]] = None, | ||
| callback_steps: Optional[int] = 1, | ||
| ): | ||
| # For more information on how this function works, please see: https://huggingface.co/docs/diffusers/api/pipelines/stable_diffusion#diffusers.StableDiffusionImg2ImgPipeline |
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.
Move this link to the docstring?
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.
Like add docstring for the whole function and then state that it's copied ? Note that this won't be in our official docs as it's not in src/diffusers/pipelines
…add_all_in_one_sd_pipeline
* uP * correct * make style * small change
* uP * correct * make style * small change
Once this is merged one can:
when using
diffusers >= 0.4.0