- 
                Notifications
    You must be signed in to change notification settings 
- Fork 6.5k
fixed bug in defining embed dim for UNet1D #12111
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
fixed bug in defining embed dim for UNet1D #12111
Conversation
| I should note that although I tried to follow the contributing doc, there were parts that were hard for me to replicate / understand. Will happily go back and fix this before review if asked. For example, I ran  Which is unrelated to my modification. I think this means I set it up wrong? | 
| 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. | 
| if time_embedding_type == "fourier": | ||
| self.time_proj = GaussianFourierProjection( | ||
| embedding_size=8, set_W_to_weight=False, log=False, flip_sin_to_cos=flip_sin_to_cos | ||
| embedding_size=block_out_channels[0], set_W_to_weight=False, log=False, flip_sin_to_cos=flip_sin_to_cos | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it might be better to introduce an optional time_embedding_dim argument to the class init and configure this way
| time_embed_dim = time_embedding_dim or block_out_channels[0] * 2 | 
…myAgrawal/diffusers into unet-1d-time-embed-debugging
| @DN6 I updated to follow the Unet2d interface more closely, as you recommended. | 
| @DN6 Requesting re-review | 
Co-authored-by: Dhruv Nair <[email protected]>
| Thank you @SammyAgrawal 👍🏽 | 

What does this PR do?
Fixes #12110
Hi! First time contributor to diffusers, apologies for anything I'm doing wrong in the PR. I noticed what I believe is a very simple and small bug in creating 1D UNets. A value is hardcoded that should not be. I noticed this while trying to debug why my UNet kept giving me shape errors. I just transitioned from using a 2d model to 1d for my personal use case, and this was causing me a headache! I am basically solving my own "Good First Issue" (linked above) noticed during my usage. I believe the Issue itself explains the problem and solution pretty straightforwardly.
Before submitting
documentation guidelines, and
here are tips on formatting docstrings.
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.
Tagging @sayakpaul @yiyixuxu @DN6