Allow Flux2 to use MagCache - #14351
Conversation
| elif diff > 0: | ||
| residual = out_hidden - in_hidden[:, diff:] # Fallback to matching tail | ||
| else: | ||
| residual = out_hidden - in_hidden # Fallback to matching tail | ||
| residual = out_hidden[:, -diff:] - in_hidden # Fallback to matching tail |
There was a problem hiding this comment.
This branch of the if statement appears to have the intent to slice out the text encoding if it remains within the hidden state, but it was never been implemented (if you look on main, there is an if-else that does the exact same thing). On flux 2, this will error out because out_hidden ends up as size 4608 while in_hidden ends up as 4096 due to the single transformer block returning a fused hidden state (there is a single transformer block kwarg to break it into encoder_hidden_state and hidden_state, but I believe it would break other things and possibly the pipeline and it's cleaner to just deal with it here).
There was a problem hiding this comment.
Let's add a comment about it and also, a test to https://github.com/huggingface/diffusers/blob/main/tests/hooks/test_mag_cache.py.
Cc: @AlanPonnachan could you comment on this?
There was a problem hiding this comment.
yes, flux2 has this exception. It receives un-fused tensors, concatenates them internally, and returns a single fused tensor without splitting it back (https://github.com/huggingface/diffusers/blob/main/src/diffusers/models/transformers/transformer_flux2.py#L847-L873).
This pr solution will work for where text tokens are preprended (which is true for flux2). But if a model returns a fused tensor but appends the text tokens, this math will need adjusting??
yes, please add a comment and test case.
There was a problem hiding this comment.
returns a fused tensor but appends the text tokens
Does the happen to be a model in diffusers a model that does this? That would be helpful as reference
|
Hi @Ednaordinary, thanks for the PR! It does not appear to link an issue it fixes. If this PR addresses an existing issue, please add a closing keyword (e.g. |
|
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. |
|
failing tests seem unrelated (timeouts) |
|
Hi @Ednaordinary, a follow-up on the reminder above: this PR still does not link an issue it fixes. Please note that PRs without a linked issue are likely to be automatically closed 10 days after this notice. Adding a closing keyword (e.g. |
|
I guess I can open an issue (is this the recommended course of action? If so, the bot should probably be more specific about that instead of implying it) |
sayakpaul
left a comment
There was a problem hiding this comment.
Just a small comment and very sorry about the delay.
What does this PR do?
MagCache on Flux2 used to error out, this should fix it by adding flux2 to hooks (maybe allowing it to be used by other caches? I haven't checked) and fixes a case in MagCache that a code comment signaled intent to handle but never did.
Tested on 3090 ti
Test script
Steps v
this cat has many limbs
Before submitting
self-reviewskill on the diff?documentation guidelines, and
here are tips on formatting docstrings.
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.
@AlanPonnachan @sayakpaul