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

Animatediff Controlnet Community Pipeline IP Adapter Fix #7413

Merged

Conversation

AbhinavGopal
Copy link
Contributor

@AbhinavGopal AbhinavGopal commented Mar 21, 2024

What does this PR do?

The animatediff controlnet community pipeline had a small bug that supplied an incorrect parameter to encode_image. I pushed the fix for it.

Before submitting

Who can review?

@sayakpaul @yiyixuxu @DN6 @a-r-r-o-w

@sayakpaul
Copy link
Member

Could you also tag the author who contributed the pipeline for a review?

@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.

@a-r-r-o-w
Copy link
Contributor

a-r-r-o-w commented Mar 21, 2024

This is bizarre, I'm not sure how it even passed locally in the other PR! The issue comes from encode_image not being the latest implementation. It seems like # Copied from has no effect for community pipelines.

The changes in this PR are incorrect btw. For both AnimateDiff controlnet/img2vid, could you update the encode_image function to this:

def encode_image(self, image, device, num_images_per_prompt, output_hidden_states=None):

Edit: I see the issue that caused this. In my original PR, I did update to latest encode_image function. However, I was working on multiple things at the time and overlapped branches quite stupidly. When I tested, it worked as expected because it was the overlapping branch instead of the base that added latest IP Adapter support 🤦 Apologies for breaking the implementation, I'll be more careful next time

@a-r-r-o-w
Copy link
Contributor

@sayakpaul @yiyixuxu From a quick look at the utils/check_copies.py script, it does not seem like the community folder falls in the "Copied From" checks. Could you verify if that's the case, and if yes, may I open a PR adding community folder to it? I believe some things might break and need fixing due to overlaps with the latest implementations.

@sayakpaul
Copy link
Member

Yeah we don't don't copy checks on community pipelines.

@AbhinavGopal AbhinavGopal force-pushed the animatediff-controlnet-ipadapter-bugfix branch from a1beedb to de9adb9 Compare March 21, 2024 05:44
@AbhinavGopal AbhinavGopal reopened this Mar 21, 2024
@AbhinavGopal
Copy link
Contributor Author

Fixed as requested above. It's a bit weird though, as this optional variable is never used in the encode_image function.

@a-r-r-o-w
Copy link
Contributor

a-r-r-o-w commented Mar 21, 2024

Fixed as requested above. It's a bit weird though, as this optional variable is never used in the encode_image function.

Please copy the entire function body of encode_image from the latest SD pipeline implementation. Please also do it for animatediff img2vid in this PR if possible as both pipelines suffer from common issue.

@AbhinavGopal
Copy link
Contributor Author

Gotcha! And the error doesn't seem to be there for the img2video pipeline

@a-r-r-o-w
Copy link
Contributor

Gotcha! And the error doesn't seem to be there for the img2video pipeline

Oh I see, I'm looking at my local branches that were not synced and comparing against here. Looks good to go now! Thanks

Would be great if you could run the example code and verify if everything's working again

@AbhinavGopal
Copy link
Contributor Author

Seems to work!

@a-r-r-o-w
Copy link
Contributor

cc @DN6

@yiyixuxu yiyixuxu merged commit d1e3f48 into huggingface:main Apr 20, 2024
8 checks passed
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.

None yet

5 participants