-
Notifications
You must be signed in to change notification settings - Fork 6.4k
Fix Chroma attention padding order and update docs to use lodestones/Chroma1-HD
#12508
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
base: main
Are you sure you want to change the base?
Conversation
Removed unnecessary blank lines for cleaner code.
|
Need to run a final test before it's ready for review, since I was testing on an older diffusers version. |
|
Okay all good. Unsure who to tag, but IIRC @sayakpaul merged Chroma impl originally. |
| Chroma is a text to image generation model based on Flux. | ||
|
|
||
| Original model checkpoints for Chroma can be found [here](https://huggingface.co/lodestones/Chroma). | ||
| Original model checkpoints for Chroma can be found [here](https://huggingface.co/lodestones/Chroma1-HD). |
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.
This is intentional. As if there's any other official compatible checkpoint released under https://huggingface.co/lodestones/, the users will likely notice it.
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.
https://huggingface.co/lodestones/Chroma is actually a deprecated repo, rather than a hub for all Chroma models. I don't think there's currently a 'hub' repo. Did you mean to imply that it should link to https://huggingface.co/lodestones rather than https://huggingface.co/lodestones/Chroma or https://huggingface.co/lodestones/Chroma1-HD?
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.
I made some changes - unsure if this fixes your concerns.
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.
Thanks a lot for fixing this!
|
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. |
|
@bot /style |
|
Style bot fixed some files and pushed the changes. |
What does this PR do?
Chroma inference is currently incorrect, since the padding token should be added for the transformer forward pass, not for the T5 encoder forward pass. The T5 embedding step should use the regular attention mask.
This change fixes that to align with official code and ComfyUI.
I've also snuck in an update to use the final checkpoint in the docs/comments: https://huggingface.co/lodestones/Chroma1-HD
Top is before fix, bottom is after fix. I used
lodestones/Chroma1-Base, since it's what I had on hand. Doesn't seem to be a huge difference, except for first column. Might have a stronger effect for shorter prompts, but I didn't test.