-
Notifications
You must be signed in to change notification settings - Fork 70
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
Add partial SDXL model #61
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Only a minor comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! The only major adjustment is the NotImplementedError
for the pretrained
arg. I also had a question around which VAE we should prefer if we can run it on bf16.
For the future, there should be a future PR that adds the other modeling features (in addition to the conditioning PR), so we can run a pre-trained SDXL. Sorry for going back-and-forth on whether to fully implement SDXL
This PR adds
diffusion.models.models.stable_diffusion_xl
, which incorporates a few ideas from SDXL. Namely:Currently we're using the
madebyollin/sdxl-vae-fp16-fix
VAE checkpoint rather than the officialstabilityai/stable-diffusion-xl-base-1.0
VAE checkpoint. The stabilityai checkpoint leads to NaNs when training with fp16. More on that issue here.