Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add support for SD 3.5 IP-Adapters #9966

Closed
vladmandic opened this issue Nov 19, 2024 · 9 comments · Fixed by #10718
Closed

Add support for SD 3.5 IP-Adapters #9966

vladmandic opened this issue Nov 19, 2024 · 9 comments · Fixed by #10718

Comments

@vladmandic
Copy link
Contributor

First IP-Adapter for SD 3.5 just released at https://huggingface.co/InstantX/SD3.5-Large-IP-Adapter
with code for the modified pipeline available in the same location.

Ask is to integrate support for SD 3.5 IP-Adapter into standard t2i/i2i/inpaint pipelines

@yiyixuxu @sayakpaul @DN6 @asomoza

@sayakpaul
Copy link
Member

CC @fabiorigano as well. Since the code is already out I think it might be nice to open this to the community and we help along the way. Maybe we give it a week and if it's not picked up we integrate ourselves. @asomoza meanwhile, maybe you could run some tests as well?

@yiyixuxu
Copy link
Collaborator

cc @haofanwang too

@guiyrt
Copy link
Contributor

guiyrt commented Nov 21, 2024

I started working on it and have a first draft without modifying the original code, but takes a while to test because SD3.5 is too big for my GPU.

For this PR, is the goal to have the pipeline with no changes as a community pipeline or as an optimized pipeline in src/pipelines? I noticed that it's not inheriting IPAdapterMixin loader, for example. This is my first contribution, still learning about repo structure :)

Edit: I know issues labeled as "New pipeline/model" are regarded as most difficult in the contributor guidelines, but as there is already working code, this should be easier to pick-up (hopefully) 🤞

@guiyrt
Copy link
Contributor

guiyrt commented Jan 10, 2025

Is was expanding the integration into SD3 ControlNet Inpainting pipeline, but I don't think we can test it. I can only find the inpainting controlnet for SD3-Medium, and we only have IP-Adapter for SD3.5-Large. Do you know of any IP-Adapter for SD3-Medium or Inpainting controlnet for SD3.5-Large?

I also noticed that the HF PR with updated keys and safetensors is not merged yet https://huggingface.co/InstantX/SD3.5-Large-IP-Adapter/discussions/10, which means that for the diffusers integration to work, we need to provide the commit-id of the proposed change, otherwise we get mismatched keys when loading the IP-Adapter (as someone noticed here). So, we either kindly ask for someone of the InstantX team to approve the PR, or we need to add runtime conversion for the expected keys.

@hlky
Copy link
Member

hlky commented Jan 13, 2025

@guiyrt We had runtime conversion in the SD3.5 IPAdapter PR, no? If not we should add that but feel free to upload Diffusers format on the Hub called something like InstantX-SD3.5-Large-IP-Adapter-diffusers and we can update the examples in the mean time, you can also use revision="refs/pr/10" rather than the commit id.

@guiyrt
Copy link
Contributor

guiyrt commented Jan 13, 2025

We had at first, but then we removed it as it would be simpler to just update the keys on InstantX/SD3.5-Large-IP-Adapter to match with the diffusers integration, as it's the only IP-Adapter (on the Hub at least).

I have to create a new model? I searched for a way to fork InstantX/SD3.5-Large-IP-Adapter, but it's not that straightforward, right?

@hlky
Copy link
Member

hlky commented Jan 13, 2025

You would clone InstantX/SD3.5-Large-IP-Adapter, replace safetensors then commit everything to a new Hub repo. Let's just change the examples to use revision="refs/pr/10" until we've added runtime conversion, also could you change the Hub PR to add Diffusers version with a suffix like _diffusers rather than replacing the existing version to maintain compatibility with InstantX's own implementation and other implementations, then we'll have both options.

@yiyixuxu
Copy link
Collaborator

cc @haofanwang here too - do you guys want to host diffusers weights?

@guiyrt
Copy link
Contributor

guiyrt commented Jan 14, 2025

I updated the Hub PR with diffusers checkpoints with suffix "_diffusers", if there's intent to have them there.
I also created guiyrt/InstantX-SD3.5-Large-IP-Adapter-diffusers, which contains only diffusers checkpoints (I also added a bit more info about GPU memory constraints on README).

If you prefer to add runtime conversion now, I can handle it and open a PR. Otherwise, I can update the docs and point it to guiyrt/InstantX-SD3.5-Large-IP-Adapter-diffusers or add revision="refs/pr/10", if the plan is to host diffusers weights in InstantX/SD3.5-Large-IP-Adapter soon.

Based on the decision from InstantX, we should also update the README there, as it currently says it is not integrated into diffusers in Inference.

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants