Skip to content

fix: enable unet_3d_condition to support time_cond_proj_dim #7364

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

Merged
merged 4 commits into from
Mar 21, 2024

Conversation

yhZhai
Copy link
Contributor

@yhZhai yhZhai commented Mar 18, 2024

What does this PR do?

This PR adds an argument time_cond_proj_dim to UNet3DConditionModel (the same as UNet2DConditionModel), and thus it enables the time_embedding module to create a cond_proj module.

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.

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

@sayakpaul
Copy link
Member

Thanks for the PR. Is this for a use case? Just curious.

@sayakpaul sayakpaul requested a review from yiyixuxu March 18, 2024 10:06
@yhZhai
Copy link
Contributor Author

yhZhai commented Mar 18, 2024

Thanks for the PR. Is this for a use case? Just curious.

Hi, I encountered this problem when trying to replace the UNet2DConditionModel with UNet3DConditionModel in consistency model training example. In the original code, time_cond_proj_dim is passed to the 2D UNet, and is used to create a new linear layer here. In this way, the 2D consistency model can takes the guidance scale as a conditional input.

This PR enables the consistency model training example to support 3D Unet.

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.

I am okay with the change. Thanks!

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.

thanks!

@yiyixuxu yiyixuxu requested a review from DN6 March 18, 2024 16:47
@sayakpaul
Copy link
Member

@yhZhai please ping us once the CI run is complete.

@yhZhai
Copy link
Contributor Author

yhZhai commented Mar 20, 2024

@sayakpaul @yiyixuxu @DN6 Hi all, the CI run has completed. Could you please take a look? Thank you!

@sayakpaul
Copy link
Member

Sorry this needs to be run again because previously, the CI was broken (for an internal mistake). Should hopefully be fixed 🤞

@yhZhai
Copy link
Contributor Author

yhZhai commented Mar 20, 2024

@sayakpaul Hi, could you please take a look at the CI test result? Thanks!

@sayakpaul sayakpaul merged commit 405a1fa into huggingface:main Mar 21, 2024
@sayakpaul
Copy link
Member

Thank you for your contributions!

AbhinavGopal pushed a commit to AbhinavGopal/diffusers that referenced this pull request Mar 27, 2024
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.

5 participants