fix for #7365, prevent pipelines from overriding provided prompt embeds

[bghira]

What does this PR do?

Fixes #7365

Before submitting

• This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
• Did you read the contributor guideline?
• Did you read our philosophy doc (important for complex PRs)?
• Was this discussed/approved via a GitHub issue or the forum? Please add a link to it if that's the case.
• Did you make sure to update the documentation with your changes? Here are the
  documentation guidelines, and
  here are tips on formatting docstrings.
• Did you write any new necessary tests?

Who can review?

@sayakpaul @yiyixuxu

[HuggingFaceDocBuilderDev]

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.

[yiyixuxu]

why are all the tests failing?

[yiyixuxu]

can we look into the remaining failing tests?

[bghira]

wow, i hadn't expected all of those, or really looked into them until now. i'm not sure why this simple check made those tests fail. i am wondering if there's something i need to check first before assigning the prompt embeds 🤔 like whether we have any? should i try and run the unit tests locally again?

[bghira]

@yiyixuxu i was trying again today and make test doesn't work here as i guess i don't have the right version of pytest installed, and now running pip install diffusers[dev] but i think there must be a way to run this in a Docker container?

edit: after pip install diffusers[dev] it now works to run make test

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.

Please note that issues that do not follow the contributing guidelines are likely to be ignored.

[sayakpaul]

Thanks, @bghira! Apologies for the delay on my part. Would it be possible to add a test for this to https://github.com/huggingface/diffusers/blob/main/tests/pipelines/stable_diffusion_xl/test_stable_diffusion_xl.py?

[bghira]

cc @a-r-r-o-w for test

[sayakpaul]

Yeah let's ensure to add at least one test before merging.

[bghira]

was having trouble getting the test suite running when i pushed this and my new system doesn't have it working yet either - but a regression test would be great to keep it workin

[sayakpaul]

Seems like the core tests are failing with the changes.

[a-r-r-o-w]

i see, will take a look at tests

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.

Please note that issues that do not follow the contributing guidelines are likely to be ignored.

[bghira]

nice try bot, but it's not stale

[a-r-r-o-w]

Nice try indeed, bot. Sorry about the delay, but should be good to merge now.

The previous fix did not really work as expected unless one always passed pooled_prompt_embeds, which is what caused all the test errors. The prompt_embeds[0] should only be assigned when encoding prompt_2 via text_encoder_2. The ndim check suffices atm but open to suggestions

[a-r-r-o-w]

@yiyixuxu Our make fix-copies does not really work for things in examples/ folder. I wonder if that's by choice or simply overlooked. I think it wouldn't be too difficult to support (I got it working but it creates too many changes in the existing community pipelines so could maybe take up in another PR, but important to note that every change made to the # Copied from stuff in community folder was Find & Replace. Using the update fix-copies implementation simply breaks a lot of pipelines)

[a-r-r-o-w]

Gentle ping @yiyixuxu for a final review

[yiyixuxu]

ohh so this is for when users pass a pre-generated pooled_prompt_embeds but not prompt_embeds? can you explain why they need to do that? (they need to run the text encoder to get the prompt_embeds and will get the pooled_prompt_embeds anyway)

[a-r-r-o-w]

So, when someone wants to use their own provided pooled_prompt_embeds, but use a different prompt (they don't pass prompt_embeds here), we will encode the prompt and overwrite the value that they passed. This PR only assigns the value if a custom pooled_prompt_embeds was not passed because you should be able to use different prompts for different text encoder, and by that reasoning different precomputed embeddings too (@linoytsaban did some nice threads/blogs testing this with flux).

[yiyixuxu]

so is the custom pooled_prompt_embeds not specific to the prompt? - because if it is processed based on the text_encoder output, I would imagine the user also would have prompt_embeds at hands and can pass it along with the pooled_prompt_embeds so that we don't need to run text encoder again

either way it is ok to merge because I don't think it will cause any problem, just try to understand the use case

[a-r-r-o-w]

I think this is a really old PR at this point and changes may have been made over time such that it was relevant then but may not be now, so I'm not 100% sure either - I saw it was stale so either we help move it to completion or we can close if not needed. Maybe @bghira can elaborate further on his use case.

But, I also don't see a way of getting the pooled_prompt_embeds just from having access to prompt_embeds like you mention. When we get the last_hidden_state from the text encoders, it is a ndim=3 tensor of shape [1, 77, 768], and we concatenate the embeddings across channel dim. But pooled embeddings are ndim=2 tensors of shape [1, 768]. Am I missing something trivial here?

[a-r-r-o-w]

And yes, pooled_prompt_embeds can be custom here, completely different from what prompt is about, if passed, or prompt_embeds is about.

See these X threads by Linoy (although this is in the context of Flux):

• https://x.com/linoy_tsaban/status/1831700970105094593
• https://x.com/linoy_tsaban/status/1830528436194193792
• https://x.com/linoy_tsaban/status/1829201687543734368
• https://x.com/linoy_tsaban/status/1825834935288213834