KEMBAR78
Fix the bug that `joint_attention_kwargs` is not passed to the FLUX's transformer attention processors by HorizonWind2004 · Pull Request #9517 · huggingface/diffusers · GitHub
Skip to content

Conversation

@HorizonWind2004
Copy link
Contributor

Issue link is below:

#9516

Copy link
Collaborator

@yiyixuxu yiyixuxu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the PR! I left a question

encoder_hidden_states: torch.FloatTensor,
temb: torch.FloatTensor,
image_rotary_emb=None,
joint_attention_kwargs={},
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you explain what additional argument you need to pass down to flux attention processor?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your recognition!

In our work, I am trying to integrate box and mask into the FLUX model and implement layout control (similar to what has been done in many works on SD1.4). This requires modifying the attention processor. I believe that the architecture of FLUX and other transformers can also be used to develop better layout control algorithms, so I believe these modifications will contribute to future training-free experiments on FLUX.

Copy link
Collaborator

@yiyixuxu yiyixuxu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks! let's support this :)

encoder_hidden_states: torch.FloatTensor,
temb: torch.FloatTensor,
image_rotary_emb=None,
joint_attention_kwargs={},
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
joint_attention_kwargs={},
joint_attention_kwargs=None,

norm_hidden_states, gate = self.norm(hidden_states, emb=temb)
mlp_hidden_states = self.act_mlp(self.proj_mlp(norm_hidden_states))

joint_attention_kwargs = joint_attention_kwargs if joint_attention_kwargs is not None else {}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we pass this to attn too?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes! I think it will be useful for other trial!

@yiyixuxu yiyixuxu requested a review from a-r-r-o-w September 25, 2024 23:41
fix a little bug
@HorizonWind2004
Copy link
Contributor Author

I fixed a bug and now it is okay XD.

@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.

Copy link
Contributor

@a-r-r-o-w a-r-r-o-w left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you, looks good! could you run make style to fix the failing quality tests.

i have a question though: in this comment, you mention that your work uses this feature to modify something in, or pass additional arguments to, the attention processor. it is understandable why this would be useful, but we generally do not add features that are not testable or usable without a public implementation. is your work/method utilizing this change available for testing this? if it will be available in the near future, i think it might be best to postpone merging this PR after that

@a-r-r-o-w
Copy link
Contributor

thanks! let's support this :)

oh, i just saw yiyi's comment about being okay with supporting this. in that case, please disregard my question above. let's fix the quality tests and we should be okay to merge

@yiyixuxu
Copy link
Collaborator

@a-r-r-o-w
I think, based on my undersanding, they just want to use a custom attention processor so they can do pipe.transformer.set_attn_processor(); they do not intend to add that to diffusers;
but I think if we do not allow passing that kwargs, it won't work with a custom attention processor taking additional arguments. o

@HorizonWind2004
Copy link
Contributor Author

@a-r-r-o-w I think, based on my undersanding, they just want to use a custom attention processor so they can do pipe.transformer.set_attn_processor(); they do not intend to add that to diffusers; but I think if we do not allow passing that kwargs, it won't work with a custom attention processor taking additional arguments. o

Yes, that's it! QWQ

@HorizonWind2004
Copy link
Contributor Author

@a-r-r-o-w
I sincerely apologize for my late response. I have been busy with another project and forgot to check GitHub. I will proceed with fixing the issues that caused the quality tests to fail right away. TAT

@a-r-r-o-w
Copy link
Contributor

Hi @HorizonWind2004, looks good to me! Could you run make style in the diffusers root folder and push the changes? Happy to merge after that once @yiyixuxu approves too

@HorizonWind2004
Copy link
Contributor Author

Hi @HorizonWind2004, looks good to me! Could you run make style in the diffusers root folder and push the changes? Happy to merge after that once @yiyixuxu approves too

Yes! Now it is okay.

@HorizonWind2004
Copy link
Contributor Author

@a-r-r-o-w @yiyixuxu
Now I've run make style && make quality and push it! OVO

Copy link
Contributor

@a-r-r-o-w a-r-r-o-w left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

Copy link
Collaborator

@yiyixuxu yiyixuxu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

@yiyixuxu yiyixuxu merged commit acd6d2c into huggingface:main Oct 8, 2024
15 checks passed
leisuzz pushed a commit to leisuzz/diffusers that referenced this pull request Oct 11, 2024
… transformer attention processors (huggingface#9517)

* Update transformer_flux.py
sayakpaul pushed a commit that referenced this pull request Dec 23, 2024
… transformer attention processors (#9517)

* Update transformer_flux.py
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