Skip to content

Conversation

@Yukun-Huang
Copy link
Contributor

What does this PR do?

Fixes #4752 potential type mismatch errors in SDXL pipelines.

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.

@sayakpaul

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.

@HuggingFaceDocBuilderDev

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

@patrickvonplaten
Copy link
Contributor

I still can't reproduce the bug sadly - could you add a reproduceable here @hyk1996 🙏

@Yukun-Huang
Copy link
Contributor Author

I still can't reproduce the bug sadly - could you add a reproduceable here @hyk1996 🙏

@patrickvonplaten
I reproduce the issue on a Colab Notebook:
https://colab.research.google.com/gist/hyk1996/82ab6af1a9ad3d0b82b1f599f877c96a/scratchpad.ipynb
and it should be reproducible with torch < 2.0 and without xformers.

@patrickvonplaten
Copy link
Contributor

Interesting! I can reproduce the problem

@patrickvonplaten
Copy link
Contributor

Looking into it

@patrickvonplaten
Copy link
Contributor

Hey @hyk1996,

I now understand what was meant - thanks a lot for bearing with us here!
The reason this bug was not caught is because almost everybody uses either xformers or PyTorch 2.0 when dealing with SDXL in which case the bug doesn't happen. Nevertheless this is a very important bug fix - great job!

I changed a couple of things in the PR to make sure everything now stays consistent between calls:

  • The vae should actually not be touched when only the latents are returned
  • The vae should be "back casted" to fp16 in it has been force upcasted before. This is important to ensure that the dtype not automagically changes which can be very confusing.

Are the changes as applied to your PR ok for you?

Also cc @sayakpaul for a final review maybe?

Thanks a bunch for fiinding this very important bug @hyk1996

@Yukun-Huang
Copy link
Contributor Author

Yukun-Huang commented Aug 28, 2023

@patrickvonplaten
The changes are ok for me, glad to be of help :-)

@patrickvonplaten patrickvonplaten merged commit 85b3f08 into huggingface:main Aug 31, 2023
yoonseokjin pushed a commit to yoonseokjin/diffusers that referenced this pull request Dec 25, 2023
* Fix potential type conversion errors in SDXL pipelines

* make sure vae stays in fp16

---------

Co-authored-by: Patrick von Platen <patrick.v.platen@gmail.com>
AmericanPresidentJimmyCarter pushed a commit to AmericanPresidentJimmyCarter/diffusers that referenced this pull request Apr 26, 2024
* Fix potential type conversion errors in SDXL pipelines

* make sure vae stays in fp16

---------

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.

Input type (c10::Half) and bias type (float) mismatch in pipeline_stable_diffusion_xl.py

4 participants