Skip to content

feat: add act_fn param to OutValueFunctionBlock #3994

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 27 commits into from
Jul 19, 2023
Merged

feat: add act_fn param to OutValueFunctionBlock #3994

merged 27 commits into from
Jul 19, 2023

Conversation

SauravMaheshkar
Copy link
Contributor

What does this PR do?

(Hopefully) fixes #1287

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.

Request for Review: @natolambert

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Jul 7, 2023

The documentation is not available anymore as the PR was closed or merged.

@natolambert
Copy link
Contributor

Let's add some folks from the Diffusers team, I'm not actively working on this anymore @patrickvonplaten

@patrickvonplaten
Copy link
Contributor

I don't fully understand what this fixes. Could you maybe elaborate here @SauravMaheshkar ? :-)

@SauravMaheshkar
Copy link
Contributor Author

SauravMaheshkar commented Jul 17, 2023

I don't fully understand what this fixes. Could you maybe elaborate here @SauravMaheshkar ? :-)

Hey @patrickvonplaten thanks for getting back. This doesn't fix an issue but I rather wanted to raise the discussion about how to fix #1287. A very naive fix could be to simply change the activation to swish in prepare_init_args_and_inputs_for_common ( in tests/models/test_models_unet_1d.py) to solve the problem (and updating the OutValueFunctionBlock to not have mish)?

But if not what would you suggest is a way to fix it ? I'm afraid I can't understand #1287 (comment)

@patrickvonplaten
Copy link
Contributor

TBH, I also don't fully understand the discussion of #1287 - I'm ok with merging this PR as is though if it helps for your use case :-)

@SauravMaheshkar
Copy link
Contributor Author

SauravMaheshkar commented Jul 18, 2023

Well this PR changes the activation function to be swish instead of mish so we can drop the decorators (@unittest.skipIf(torch_device == "mps", "mish op not supported in MPS")). @patrickvonplaten I made changes in this PR. LMK if they make sense.

@patrickvonplaten
Copy link
Contributor

Perfect!

@patrickvonplaten patrickvonplaten merged commit 470f51c into huggingface:main Jul 19, 2023
@SauravMaheshkar SauravMaheshkar deleted the saurav/unet1d-mish-fix branch July 19, 2023 12:53
orpatashnik pushed a commit to orpatashnik/diffusers that referenced this pull request Aug 1, 2023
* feat: add act_fn param to OutValueFunctionBlock

* feat: update unet1d tests to not use mish

* feat: add `mish` as the default activation function

Co-authored-by: Patrick von Platen <[email protected]>

* feat: drop mish tests from unet1d

---------

Co-authored-by: Patrick von Platen <[email protected]>
orpatashnik pushed a commit to orpatashnik/diffusers that referenced this pull request Aug 1, 2023
* feat: add act_fn param to OutValueFunctionBlock

* feat: update unet1d tests to not use mish

* feat: add `mish` as the default activation function

Co-authored-by: Patrick von Platen <[email protected]>

* feat: drop mish tests from unet1d

---------

Co-authored-by: Patrick von Platen <[email protected]>
orpatashnik pushed a commit to orpatashnik/diffusers that referenced this pull request Aug 1, 2023
* feat: add act_fn param to OutValueFunctionBlock

* feat: update unet1d tests to not use mish

* feat: add `mish` as the default activation function

Co-authored-by: Patrick von Platen <[email protected]>

* feat: drop mish tests from unet1d

---------

Co-authored-by: Patrick von Platen <[email protected]>
yoonseokjin pushed a commit to yoonseokjin/diffusers that referenced this pull request Dec 25, 2023
* feat: add act_fn param to OutValueFunctionBlock

* feat: update unet1d tests to not use mish

* feat: add `mish` as the default activation function

Co-authored-by: Patrick von Platen <[email protected]>

* feat: drop mish tests from unet1d

---------

Co-authored-by: Patrick von Platen <[email protected]>
AmericanPresidentJimmyCarter pushed a commit to AmericanPresidentJimmyCarter/diffusers that referenced this pull request Apr 26, 2024
* feat: add act_fn param to OutValueFunctionBlock

* feat: update unet1d tests to not use mish

* feat: add `mish` as the default activation function

Co-authored-by: Patrick von Platen <[email protected]>

* feat: drop mish tests from unet1d

---------

Co-authored-by: Patrick von Platen <[email protected]>
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.

Update UNet1D Tests to not use Mish
4 participants