Skip to content

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.

4 participants