Skip to content

Conversation

@Foundsheep
Copy link
Contributor

@Foundsheep Foundsheep commented Dec 10, 2024

What does this PR do?

Fixes #10139

  • UNet2DCondition model can be passed for sample_size parameter with int or Tuple[int, int] types in unet_2d_condition.py. However, StableDiffusionPipeline was not compatible with that aspect and assumed the argument would be only int type.
  • Now, StableDiffusionPipeline can expect sample_size for unet which is then to be passed as height and width to be not just a single number, but also two different numbers in a tuple or list.
  • Also modified the difference between docs and type hint to match each other.
  • Also added a test case to check if the StableDiffusionPipeline would be instantiated without an error in that case.

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.
@yiyixuxu @hlky

@HuggingFaceDocBuilderDev

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

@SahilCarterr
Copy link
Contributor

Hey @Foundsheep it would be great if you write a test for testing unet's sample_size

@Foundsheep
Copy link
Contributor Author

@SahilCarterr thanks. That makes perfect sense, and actually I made my own script to test it and removed it after the test, since the script did not seem to fit with the original code.

I'm wondering, where would the test code be placed in the repository if I should make one?

@SahilCarterr
Copy link
Contributor

SahilCarterr commented Dec 11, 2024

You can write the test here. Check out other tests for reference

Also this PR is not tagged with the corresponding Issue.

Fixes # (10139)

Remove parenthesis to tag it.
Fixes #10139
@Foundsheep

@Foundsheep
Copy link
Contributor Author

@SahilCarterr thanks. I edited the tag.

But I'm a bit confused about writing the test code. Am I supposed to add a method in the StableDiffusionPipelineFastTests class, and do others follow this procedure if they need to add a test case?

@SahilCarterr
Copy link
Contributor

Yes, add test here StableDiffusionPipelineFastTests @Foundsheep

Copy link
Member

@sayakpaul sayakpaul left a comment

Choose a reason for hiding this comment

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

Looks lit! Thank you!

Comment on lines +843 to +849
def test_pipeline_accept_tuple_type_unet_sample_size(self):
# the purpose of this test is to see whether the pipeline would accept a unet with the tuple-typed sample size
sd_repo_id = "stable-diffusion-v1-5/stable-diffusion-v1-5"
sample_size = [60, 80]
customised_unet = UNet2DConditionModel(sample_size=sample_size)
pipe = StableDiffusionPipeline.from_pretrained(sd_repo_id, unet=customised_unet)
assert pipe.unet.config.sample_size == sample_size
Copy link
Member

Choose a reason for hiding this comment

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

Could we use a smaller checkpoint?
https://huggingface.co/hf-internal-testing/tiny-sd-pipe

@hlky hlky merged commit b756ec6 into huggingface:main Dec 19, 2024
12 checks passed
Foundsheep added a commit to Foundsheep/diffusers that referenced this pull request Dec 23, 2024
sayakpaul pushed a commit that referenced this pull request Dec 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unet sample_size not fully considered in pipeline init

5 participants