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

Adds denoising_end parameter to ControlNetPipeline for SDXL #6175

Merged
merged 12 commits into from
Mar 9, 2024

Conversation

UmerHA
Copy link
Contributor

@UmerHA UmerHA commented Dec 14, 2023

What does this PR do?

Adds denoising_end parameter to ControlNetPipeline for SDXL.

Fixes #6159 Edit: Removed changes to ControlNetXS, so does not fix #6159 yet.

Before submitting

Who can review?

Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.

Core library:

@HuggingFaceDocBuilderDev

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.

UmerHA and others added 2 commits December 14, 2023 20:33
Removed copy hints, as in original SDXLControlNetPipeline, as the `make fix-copies` seems to have issues with the @Property decorator.
@sayakpaul
Copy link
Member

Thank you! Do we need to add any "# Copied from ..." statements?

@sayakpaul sayakpaul requested a review from yiyixuxu December 15, 2023 02:19
Copy link

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.

@github-actions github-actions bot added the stale Issues that haven't received updates label Jan 15, 2024
@github-actions github-actions bot closed this Jan 26, 2024
@sayakpaul sayakpaul reopened this Jan 27, 2024
@sayakpaul
Copy link
Member

@UmerHA do you want to revive this PR?

@UmerHA
Copy link
Contributor Author

UmerHA commented Jan 27, 2024

@sayakpaul Yes, I'll do it after I'm done with the new ControlNetXS PR, which should be ~next week.

Copy link

github-actions bot commented Mar 2, 2024

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.

@github-actions github-actions bot added the stale Issues that haven't received updates label Mar 2, 2024
@yiyixuxu
Copy link
Collaborator

yiyixuxu commented Mar 3, 2024

@UmerHA
Can we only apply change for Controlnet pipeline in this PR, and get it merged now?
we have #6772 going on for controlnet XS that kinda races with this PR, no?

@yiyixuxu yiyixuxu removed the stale Issues that haven't received updates label Mar 3, 2024
@UmerHA
Copy link
Contributor Author

UmerHA commented Mar 3, 2024

@yiyixuxu Sure - will get it done on Monday!

@UmerHA
Copy link
Contributor Author

UmerHA commented Mar 4, 2024

@yiyixuxu Have removed the ControlNetXS part - ready for review!

@UmerHA UmerHA changed the title Adds denoising_end parameter to ControlNetPipeline / ControlNetXSPipeline for SDXL Adds denoising_end parameter to ControlNetPipeline for SDXL Mar 4, 2024
Copy link
Collaborator

@yiyixuxu yiyixuxu left a comment

Choose a reason for hiding this comment

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

thanks!

@yiyixuxu yiyixuxu requested a review from sayakpaul March 5, 2024 02:21
@yiyixuxu
Copy link
Collaborator

yiyixuxu commented Mar 5, 2024

also cc @stevhliu here for awareness
not sure if it's worth mentioning on the doc but now we can use controlnet SDXL pipeline together with the refiner

Copy link
Member

@sayakpaul sayakpaul 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 working on this.

Should we also add a test for this as I imagine denoising_end is something the users will want to experiment with.

@yiyixuxu WDYT?

@stevhliu
Copy link
Member

stevhliu commented Mar 5, 2024

not sure if it's worth mentioning on the doc but now we can use controlnet SDXL pipeline together with the refiner

Sure, let's add a [!TIP] to this doc saying you can use StableDiffusionXLControlNetPipeline with refiner checkpoints.

@yiyixuxu
Copy link
Collaborator

yiyixuxu commented Mar 5, 2024

@UmerHA
do you want to add a test and a tip to the doc?
we can merge soon:)

@UmerHA
Copy link
Contributor Author

UmerHA commented Mar 6, 2024

@yiyixuxu - sure done!

I've copied & adapted the fast test for mixture of denoisers from SDXL. I've left the slow tests out on purpose because mixture of denoisers for ControlNet uses the same code as SDXL and SDXL already has slow tests.

Copy link
Member

@stevhliu stevhliu left a comment

Choose a reason for hiding this comment

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

A few minor nits, but otherwise LGTM!

docs/source/en/using-diffusers/controlnet.md Outdated Show resolved Hide resolved
docs/source/en/using-diffusers/controlnet.md Outdated Show resolved Hide resolved
UmerHA and others added 2 commits March 6, 2024 20:25
Co-authored-by: Steven Liu <59462357+stevhliu@users.noreply.github.com>
Co-authored-by: Steven Liu <59462357+stevhliu@users.noreply.github.com>
@yiyixuxu yiyixuxu merged commit 3f9c746 into huggingface:main Mar 9, 2024
15 checks passed
@yiyixuxu
Copy link
Collaborator

yiyixuxu commented Mar 9, 2024

thank you:)

@UmerHA UmerHA deleted the controlnet-denoising-end branch March 9, 2024 19:52
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.

StableDiffusionXLControlNetXSPipeline does not support keyword argument 'denoising_end'
5 participants