Skip to content

Conversation

antoine-scenario
Copy link
Contributor

@antoine-scenario antoine-scenario commented Dec 22, 2023

Add IP-Adapter to StableDiffusionXLControlNetImg2ImgPipeline as it was missing so far.

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.

@antoine-scenario antoine-scenario force-pushed the main branch 3 times, most recently from 2c11c02 to eb48d76 Compare December 22, 2023 13:19
@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.

@charchit7
Copy link
Contributor

@antoine-scenario run make quality

@antoine-scenario
Copy link
Contributor Author

@antoine-scenario run make quality

done 👍

@patrickvonplaten
Copy link
Contributor

@yiyixuxu could you take a look here?

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.

looking great! thank you :)

@yiyixuxu
Copy link
Collaborator

yiyixuxu commented Dec 28, 2023

can we fix the test? you need to add the optional image_encoder component (None) here

Update src/diffusers/pipelines/controlnet/pipeline_controlnet_sd_xl_img2img.py

Co-authored-by: YiYi Xu <yixu310@gmail.com>

fix tests
@antoine-scenario
Copy link
Contributor Author

can we fix the test? you need to add the optional image_encoder component (None) here

@yiyixuxu I did the fix you suggested but the test still fails 😕

@yiyixuxu
Copy link
Collaborator

yiyixuxu commented Jan 2, 2024

ok can we find out what's the difference? I think you can add some print lines here to find out (i.e. print out pipe.components.keys()) and init_components.keys())

self.assertTrue(set(pipe.components.keys()) == set(init_components.keys()))

@antoine-scenario
Copy link
Contributor Author

antoine-scenario commented Jan 3, 2024

ok can we find out what's the difference? I think you can add some print lines here to find out (i.e. print out pipe.components.keys()) and init_components.keys())

self.assertTrue(set(pipe.components.keys()) == set(init_components.keys()))

do you have a command to run the test locally ? I followed the instructions from the doc and tried pytest tests/pipelines/test_pipelines_common.py but everything is being skipped.

Otherwise in the CI the print doesn't show.

@yiyixuxu
Copy link
Collaborator

yiyixuxu commented Jan 3, 2024

@antoine-scenario
oh you should run this test file pytest tests/pipelines/controlnet/test_controlnet_sdxl_img2img.py

@antoine-scenario
Copy link
Contributor Author

@antoine-scenario oh you should run this test file pytest tests/pipelines/controlnet/test_controlnet_sdxl_img2img.py

oh yes you're right ! everything seems to be fine by now 😇

@yiyixuxu
Copy link
Collaborator

yiyixuxu commented Jan 3, 2024

cc @sayakpaul for a final review

@yiyixuxu yiyixuxu requested a review from sayakpaul January 3, 2024 19:07
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.

Very nice.

Brainstorming, not related to the PR: I wonder if we should work on a tutorial showing the different use cases of IP Adapters in light of various pipelines for which it's supported.

IP Adapters are getting increasingly popular, so I definitely think it's a good idea. @stevhliu WDYT?

@charchit7
Copy link
Contributor

Very nice.

Brainstorming, not related to the PR: I wonder if we should work on a tutorial showing the different use cases of IP Adapters in light of various pipelines for which it's supported.

IP Adapters are getting increasingly popular, so I definitely think it's a good idea. @stevhliu WDYT?

Yes, IP Adapters quality is really good.

@stevhliu
Copy link
Member

stevhliu commented Jan 4, 2024

IP Adapters are getting increasingly popular, so I definitely think it's a good idea. @stevhliu WDYT?

We currently have a section for IP-Adapters here where we show off how to use it with various pipelines and for different tasks as well, so I don't necessarily think we need a new doc for it. But I think we can spin this section out into a standalone doc to increase visibility and keep adding to it. 🙂

@yiyixuxu yiyixuxu merged commit 3e8b632 into huggingface:main Jan 10, 2024
AmericanPresidentJimmyCarter pushed a commit to AmericanPresidentJimmyCarter/diffusers that referenced this pull request Apr 26, 2024
…face#6293)

* add IP-Adapter to StableDiffusionXLControlNetImg2ImgPipeline

Update src/diffusers/pipelines/controlnet/pipeline_controlnet_sd_xl_img2img.py

Co-authored-by: YiYi Xu <yixu310@gmail.com>

fix tests

* fix failing test

---------

Co-authored-by: Patrick von Platen <patrick.v.platen@gmail.com>
Co-authored-by: Sayak Paul <spsayakpaul@gmail.com>
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.

7 participants