Skip to content

Conversation

dg845
Copy link
Contributor

@dg845 dg845 commented Aug 30, 2023

What does this PR do?

This PR fixes the get_dummy_inputs method for StableDiffusionInpaintPipelineFastTests. Currently, get_dummy_inputs creates a black image and black mask_image, which means that we are technically not repainting the image at all, which doesn't seem like the right behavior.

This PR is a spin-off of #4536, where the above issue was noticed (see #4536 (comment)).

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.

@patrickvonplaten
@yiyixuxu

…oduce a random image and a white mask_image.
@HuggingFaceDocBuilderDev

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint.

@patrickvonplaten
Copy link
Contributor

Hey @dg845, thanks a lot for the PR - the change makes a lot of sense!

Could you maybe try to update the failing tests? Think then we can merge

@dg845
Copy link
Contributor Author

dg845 commented Sep 1, 2023

The tests which are currently failing are:

  • test_stable_diffusion_inpaint.py::StableDiffusionInpaintPipelineFastTests::test_stable_diffusion_inpaint
  • test_stable_diffusion_inpaint.py::StableDiffusionSimpleInpaintPipelineFastTests::test_stable_diffusion_inpaint

which makes sense, given that the input image and mask have changed. @patrickvonplaten, do you know what reference implementation(s) were used to get the expected slices for StableDiffusionInpaintPipeline testing? Based on the slow tests using the runwayml/stable-diffusion-inpainting checkpoint for the dedicated inpainting model and runwayml/stable-diffusion-v1-5 for the simple text-to-image model, I assume the reference implementation for both was runwayml/stable-diffusion.

(Looking through the code, the only fast tests that use an expected_slice are the two tests listed above. I guess we aren't forced to use the same reference implementation to get the new expected_slices, but it seems natural to me to try to keep it consistent with the previous expected_slices, and with the reference implementation used to get the expected_slices for the slow tests.)

@patrickvonplaten
Copy link
Contributor

Hey @dg845,

Could you maybe just update the expected_slice to match the results you're getting in this PR? Those fast test are "dummy" integration tests

@dg845
Copy link
Contributor Author

dg845 commented Sep 3, 2023

I have updated the expected_slices for the *.test_stable_diffusion_inpaint tests to match the pipeline outputs (e.g. the output of sd_pipe(**inputs).images).

@patrickvonplaten
Copy link
Contributor

Test failure is unrelated - merging!

@patrickvonplaten patrickvonplaten merged commit c73e609 into huggingface:main Sep 4, 2023
@dg845 dg845 deleted the sd-inpaint-tests-fix-get-inputs branch September 4, 2023 11:55
AmericanPresidentJimmyCarter pushed a commit to AmericanPresidentJimmyCarter/diffusers that referenced this pull request Apr 26, 2024
…4845)

* Change StableDiffusionInpaintPipelineFastTests.get_dummy_inputs to produce a random image and a white mask_image.

* Add dummy expected slices for the test_stable_diffusion_inpaint tests.

* Remove print statement

---------

Co-authored-by: Patrick von Platen <patrick.v.platen@gmail.com>
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