diffusers icon indicating copy to clipboard operation
diffusers copied to clipboard
Published 1 month ago • verified publisher icon huggingface

Make IPAdapter compatible to `torch.compile`

Open rootonchair opened this issue 1 year ago • 9 comments

What does this PR do?

Fixes #7985

Before submitting

  • [ ] This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • [ ] Did you read the contributor guideline?
  • [ ] Did you read our philosophy doc (important for complex PRs)?
  • [ ] Was this discussed/approved via a GitHub issue or the forum? Please add a link to it if that's the case.
  • [ ] Did you make sure to update the documentation with your changes? Here are the documentation guidelines, and here are tips on formatting docstrings.
  • [ ] Did you write any new necessary tests?

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.

rootonchair avatar May 20 '24 14:05 rootonchair

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.

HuggingFaceDocBuilderDev avatar May 20 '24 15:05 HuggingFaceDocBuilderDev

cc @fabiorigano too

yiyixuxu avatar May 20 '24 15:05 yiyixuxu

hi @yiyixuxu thanks for adding me here IPAdapterPlusImageProjection was added some time ago, I am not sure why it has been implemented in this way

I think it would be nice to have a forward function that uses the https://github.com/huggingface/diffusers/blob/d6ca1209875b536937dfaa5293db732940ce0e0b/src/diffusers/models/embeddings.py#L892 that we introduced with the Face ID PR #7186

fabiorigano avatar May 20 '24 16:05 fabiorigano

thanks @fabiorigano! cc @rootonchair, does face id currently work with torch.compile?

yiyixuxu avatar May 20 '24 16:05 yiyixuxu

@yiyixuxu yes it does. As @fabiorigano suggested, we could utilize IPAdapterPlusImageProjectionBlock however weight loading also need to be updated

rootonchair avatar May 21 '24 02:05 rootonchair

This works for me. Thank you, @rootonchair!

Perhaps the use of IPAdapterPlusImageProjectionBlock could live in a different PR? @yiyixuxu WDYT?

sayakpaul avatar May 21 '24 02:05 sayakpaul

I can open a new PR to use IPAdapterPlusImageProjectionBlock, if @rootonchair agrees

fabiorigano avatar May 21 '24 07:05 fabiorigano

That would be great, thank you!

sayakpaul avatar May 21 '24 07:05 sayakpaul

Sure @fabiorigano, thank you

rootonchair avatar May 21 '24 07:05 rootonchair

@rootonchair does https://github.com/huggingface/diffusers/pull/7994 work for you?

sayakpaul avatar May 29 '24 04:05 sayakpaul

@sayakpaul yes it does. Fantastic works

rootonchair avatar May 29 '24 14:05 rootonchair

Fantastic. Would you mind closing the PR then? I am sorry about the inconvenience here.

sayakpaul avatar May 29 '24 14:05 sayakpaul

Ah, no problem. I assume closing #7985 too?

rootonchair avatar May 29 '24 14:05 rootonchair