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

[feat] Bake in sdxl-vae-fp16-fix model on checkpoint conversion #4468

Merged
merged 5 commits into from
Jan 6, 2024

Conversation

lstein
Copy link
Collaborator

@lstein lstein commented Sep 5, 2023

What type of PR is this? (check all applicable)

  • Feature

Have you discussed this change with the InvokeAI team?

  • Yes

Have you updated all relevant documentation?

  • Yes
  • No -- no obvious place to put this

Description

To avoid the ser having to remember to switch to VAE fp32 whenever rendering with SDXL models, this PR automatically replaces the default fp32-only VAE with sdxl-vae-fp16-fix during the checkpoint conversion step. This will enable all SDXL .safetensors files to be run in fp16 VAE mode. However, it does not affect diffusers models downloaded from HugginFace. These will have to use a pinned VAE in models.yaml.

Related Tickets & Documents

QA Instructions, Screenshots, Recordings

  1. Remove the models/.cache directory so that safetensors files will be re-converted.
  2. Make sure that sdxl-vae-fp16-fix is installed in models/sdxl/vae
  3. Load an SDXL .safetensors model
  4. Test fp16 when the "Default" VAE is used.

Added/updated tests?

  • Yes
  • No : performed functional testing, but hard to test in an automated fashion without a full model to convert

[optional] Are there any post deployment tasks we need to perform?

@hipsterusername
Copy link
Member

Like the idea, but this seems to be pretty opaque/brittle - User would need to have sdxl/vae/sdxl-vae-fp16-fix installed and named just that way to work, right?

Also, black format check failing

@lstein
Copy link
Collaborator Author

lstein commented Sep 5, 2023

Like the idea, but this seems to be pretty opaque/brittle - User would need to have sdxl/vae/sdxl-vae-fp16-fix installed and named just that way to work, right?

Also, black format check failing

sdxl/vae/sdxl-vae-fp16-fix is installed by default at exactly that path. If it isn't present, then the swap doesn't occur.

Fixed black issues.

@keturn
Copy link
Contributor

keturn commented Sep 5, 2023

How are we to know whether the VAE in the file we're converting is the broken one? As opposed to one that's deliberately being bundled for use with that model.

@keturn
Copy link
Contributor

keturn commented Sep 5, 2023

(Also, all of this hacking around the problem could go away if “Stability” would just stop distributing the broken model with the blessing of a stable version number. Do we have the ear of anyone inside with enough pull to get them to fix their shit?)

@hipsterusername
Copy link
Member

I will try to see who I can reach out to.

@lstein
Copy link
Collaborator Author

lstein commented Sep 5, 2023

How are we to know whether the VAE in the file we're converting is the broken one? As opposed to one that's deliberately being bundled for use with that model.

I don't know. This worries me too. However, the broken VAE has been baked into all the SDXL Civitai models I've tried so far, so it is a common problem.

An alternative approach is to add the vae override to the stanza in models.yaml. This has the advantage of making the decision explicit and easily reversible.

@yhyu13
Copy link

yhyu13 commented Sep 16, 2023

@lstein

#4454 (comment)

This vae fix model is already in the InvokeAI Download model cli manual. It is a must for fp16 vae to work. Otherwise black image

@lstein
Copy link
Collaborator Author

lstein commented Sep 25, 2023

@lstein

#4454 (comment)

This vae fix model is already in the InvokeAI Download model cli manual. It is a must for fp16 vae to work. Otherwise black image

I am aware of that. This PR bakes that fixed vae into any .safetensors SDXL file that you import. After this, you can select “Default” and “fp16” and it will work.

@lstein
Copy link
Collaborator Author

lstein commented Sep 26, 2023

This is not going to be adopted, so closing it now.

@lstein lstein closed this Sep 26, 2023
@lstein lstein reopened this Jan 6, 2024
@lstein lstein marked this pull request as draft January 6, 2024 17:38
@lstein lstein marked this pull request as ready for review January 6, 2024 18:42
@lstein
Copy link
Collaborator Author

lstein commented Jan 6, 2024

This version probes the existing VAE to see if it is the broken SDXL-1.0 one. If it is, it replaces the VAE with sdxl-vae-fp16-fix. "Baked-in" VAEs are not replaced.

@hipsterusername hipsterusername merged commit ffa05a0 into main Jan 6, 2024
9 checks passed
@hipsterusername hipsterusername deleted the feat/bake-in-fp16-fix branch January 6, 2024 19:06
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