Skip to content
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

Clean up flaky behaviour on Slow CUDA Pytorch Push Tests #4759

Merged
merged 2 commits into from
Aug 24, 2023

Conversation

DN6
Copy link
Collaborator

@DN6 DN6 commented Aug 24, 2023

Certain Mixin Classes and Tests use the sum of the absolute differences between two an expected and output tensor to check if the models/pipelines are behaving as expected. e.g

max_diff = (image - new_image).abs().sum()
assert max_diff < 1e-5

Using sum leads to more flaky testing behaviour due since small differences over many individual values in the tensors accumulate. Proposing to move this to use the maximum absolute difference between tensors instead.
e.g

max_diff = (image - new_image).abs().max()
assert max_diff < 1e-5

Additionally, the PipelineTesterMixin has a some tests: test_from_save_pretrained, test_from_save_pretrained_variant, test_save_load_float16 etc that tests against precision tolerances but do not currently allow the pipelines inheriting from them to pass in custom precision values, which is leading to a number of failures. This PR adds an expected_max_diff argument to each of these tests to control the precision level.

Fixes # (issue)

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.

@DN6 DN6 changed the title use max diff to compare model outputs Clean up flaky behaviour on Slow CUDA Pytorch Push Tests Aug 24, 2023
@DN6 DN6 requested a review from patrickvonplaten August 24, 2023 11:44
Copy link
Contributor

@patrickvonplaten patrickvonplaten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool!

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Aug 24, 2023

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

@patrickvonplaten
Copy link
Contributor

Failing tests are unrelated - feel free to merge! Failing tests will be fixed in: #4761

@DN6 DN6 merged commit 4f05058 into main Aug 24, 2023
@DN6 DN6 mentioned this pull request Aug 28, 2023
6 tasks
@kashif kashif deleted the test-cleanup-use-max-to-compare branch September 13, 2023 13:41
AmericanPresidentJimmyCarter pushed a commit to AmericanPresidentJimmyCarter/diffusers that referenced this pull request Apr 26, 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.

3 participants