Skip to content

Add PIA Model/Pipeline #6698

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.

Sign up for GitHub

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

Jump to bottom
Merged
patrickvonplaten merged 18 commits into from
Jan 31, 2024
Merged

Add PIA Model/Pipeline #6698

patrickvonplaten merged 18 commits into from
Jan 31, 2024

Conversation

@DN6
Copy link
Collaborator

@DN6 DN6 commented Jan 24, 2024 •
edited
Loading

What does this PR do?

Adds the Personalized Image Animator (PIA) Model/Pipeline to diffusers

TODO:

  • Update docs with official PIA Motion Adapter checkpoint links (waiting on authors to respond on which org this checkpoint should live in)

Fixes # (issue)

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.

@DN6 DN6 requested a review from patrickvonplaten January 24, 2024 15:34
@DN6 DN6 mentioned this pull request Jan 24, 2024
Closed
a-r-r-o-w
a-r-r-o-w approved these changes Jan 24, 2024
View reviewed changes
Copy link
Contributor

@a-r-r-o-w a-r-r-o-w left a comment

Choose a reason for hiding this comment

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

Thank you so much for working on this @DN6 ❤️ The results look amazing from my testing. In comparison to the original repo, though, there seems to be more artifacts in videos in terms of motion or abrupt movements, not sure why. Applying freeinit seems to work great though.

PR closes #6494.

yiyixuxu
yiyixuxu reviewed Jan 25, 2024
View reviewed changes
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.

super cool:)
I left some comments and questions.

for a future PR maybe we can think about how to refactor free_init. I really don't like the free_init as a pipeline feature - It makes the pipelines hard to read, and also inconsistent with our design. I think it has a similar nature to pipelines such as attend-and-excite that are modifying the denosing loop in some way in order to enhance the generations, so essentially it should be a different pipeline. Would love to hear your thoughts and ideas:)

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Jan 25, 2024

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.

@DN6
Copy link
Collaborator Author

DN6 commented Jan 25, 2024

Thank you so much for working on this @DN6 ❤️ The results look amazing from my testing. In comparison to the original repo, though, there seems to be more artifacts in videos in terms of motion or abrupt movements, not sure why. Applying freeinit seems to work great though.

@a-r-r-o-w Could you share a snippet I can use to test with?

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

a-r-r-o-w commented Jan 26, 2024 •
edited
Loading

@a-r-r-o-w Could you share a snippet I can use to test with?

For comparing PIA, I've been using this colab by camenduru. I believe the problem is not with the diffusers implementation but just usual quality problems with AnimateDiff from time to time.

To get similar results from both, passing generator=torch.Generator().manual_seed(seed) does not seem to work. Doing this before generation, however, is giving same/similar results:

torch.manual_seed(seed)
# np.random.seed(seed) # probably not needed

I suspect the reason is that the original codebase (which camenduru's colab uses) has statements like torch.randn_like(latents) that do not make use of a generator, and directly call manual_seed in app.py. After experimenting a bit more, it seems like I just had a lucky streak with animatediff when using the original codebase when I first tested?!

patrickvonplaten
patrickvonplaten approved these changes Jan 26, 2024
View reviewed changes
Copy link
Contributor

@patrickvonplaten patrickvonplaten left a comment

Choose a reason for hiding this comment

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

Left mostly nits

patrickvonplaten
patrickvonplaten reviewed Jan 31, 2024
View reviewed changes
patrickvonplaten
patrickvonplaten reviewed Jan 31, 2024
View reviewed changes
patrickvonplaten
patrickvonplaten reviewed Jan 31, 2024
View reviewed changes
src/diffusers/pipelines/pia/pipeline_pia.py
Comment on lines +1214 to +1225
if self.free_init_enabled:
latents = self._free_init_loop(
height=height,
width=width,
num_frames=num_frames,
batch_size=batch_size,
num_videos_per_prompt=num_videos_per_prompt,
denoise_args=denoise_args,
device=device,
)
else:
latents = self._denoise_loop(**denoise_args)
Copy link
Contributor

@patrickvonplaten patrickvonplaten Jan 31, 2024

Choose a reason for hiding this comment

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

Let's not forgot to clean up the design once the PR is merged (cc @yiyixuxu )

patrickvonplaten
patrickvonplaten reviewed Jan 31, 2024
View reviewed changes
patrickvonplaten
patrickvonplaten reviewed Jan 31, 2024
View reviewed changes
patrickvonplaten
patrickvonplaten reviewed Jan 31, 2024
View reviewed changes
patrickvonplaten
patrickvonplaten reviewed Jan 31, 2024
View reviewed changes
patrickvonplaten
patrickvonplaten reviewed Jan 31, 2024
View reviewed changes
Copy link
Contributor

@patrickvonplaten patrickvonplaten left a comment

Choose a reason for hiding this comment

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

Still a couple things to improve here

@patrickvonplaten
Copy link
Contributor

patrickvonplaten commented Jan 31, 2024

Ok to merge! Failing test is unrelated

@patrickvonplaten patrickvonplaten merged commit 56bea6b into main Jan 31, 2024
dg845 pushed a commit to dg845/diffusers that referenced this pull request Feb 2, 2024
@DN6 @dg845
Add PIA Model/Pipeline (huggingface#6698)
23a08d7 
* update

* update

* updaet

* add tests and docs

* clean up

* add to toctree

* fix copies

* pr review feedback

* fix copies

* fix tests

* update docs

* update

* update

* update docs

* update

* update

* update

* update
AmericanPresidentJimmyCarter pushed a commit to AmericanPresidentJimmyCarter/diffusers that referenced this pull request Apr 26, 2024
@DN6
Add PIA Model/Pipeline (huggingface#6698)
96f6aee 
* update

* update

* updaet

* add tests and docs

* clean up

* add to toctree

* fix copies

* pr review feedback

* fix copies

* fix tests

* update docs

* update

* update

* update docs

* update

* update

* update

* update
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yiyixuxu yiyixuxu yiyixuxu left review comments

+2 more reviewers

@patrickvonplaten patrickvonplaten patrickvonplaten approved these changes

@a-r-r-o-w a-r-r-o-w a-r-r-o-w approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@DN6 @HuggingFaceDocBuilderDev @a-r-r-o-w @patrickvonplaten @yiyixuxu