Skip to content

[UniDiffuser Tests] Fix some tests #3609

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 2 commits into from
May 30, 2023
Merged

Conversation

sayakpaul
Copy link
Member

@sayakpaul sayakpaul commented May 30, 2023

W.r.t the UniDiffusers failing here.

This PR fixes some UniDiffuser tests.

I first reproduced the environment in which our test suite is run following https://github.com/huggingface/diffusers/blob/main/.github/workflows/push_tests.yml. It's as simple as:

  • First, run the Docker containing (diffusers/diffusers-pytorch-cuda).
  • Clone diffusers from the PR branch.
  • Install the dependencies.

Then I ran:

RUN_SLOW=1 python -m pytest -n 1 --max-worker-restart=0 --dist=loadfile tests/pipelines/unidiffuser/test_unidiffuser.py

On an A100, with the changes introduced in this PR all the tests from test_unidiffuser.py passed including the SLOW ones.

But on a V100, it failed with:

FAILED tests/pipelines/unidiffuser/test_unidiffuser.py::UniDiffuserPipelineSlowTests::test_unidiffuser_default_joint_v1_fp16 - AssertionError: assert 'A living room' == 'a living room'

@patrickvonplaten any thoughts on how this should be handled? Since our SLOW tests are run on a V100 always, I think it's okay to switch "'a living room'" to ensure the test passes but on an A100 this would fail complaining:

FAILED tests/pipelines/unidiffuser/test_unidiffuser.py::UniDiffuserPipelineSlowTests::test_unidiffuser_default_joint_v1_fp16 - AssertionError: assert 'a living room' == 'A living room'

(Just the opposite of the failure noted earlier)

Cc: @dg845

@sayakpaul sayakpaul marked this pull request as ready for review May 30, 2023 10:56
@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented May 30, 2023

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

@patrickvonplaten
Copy link
Contributor

cool!

@patrickvonplaten patrickvonplaten merged commit 0612f48 into main May 30, 2023
@patrickvonplaten patrickvonplaten deleted the fix/unidiffusers-tests branch May 30, 2023 11:07
@sayakpaul
Copy link
Member Author

@patrickvonplaten I think there's still an issue with one SLOW test when run on V100 as stated initially. Maybe you missed it?

@patrickvonplaten
Copy link
Contributor

Ah sorry yeah, we should try to make the code work for V100 since our tests run on a V100

@sayakpaul
Copy link
Member Author

Ah sorry yeah, we should try to make the code work for V100 since our tests run on a V100

Will make the necessary changes 👍

AmericanPresidentJimmyCarter pushed a commit to AmericanPresidentJimmyCarter/diffusers that referenced this pull request Apr 26, 2024
* fix: unidiffuser test failures.

* living room.
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.

3 participants