Skip to content
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

[LoRA] pop the LoRA scale so that it doesn't get propagated to the weeds #7338

Merged
merged 7 commits into from
Mar 19, 2024

Conversation

sayakpaul
Copy link
Member

What does this PR do?

The changes should be self-explanatory. But let me know in case of any explanation.

I think we should maybe do a patch release for this.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@sayakpaul sayakpaul requested a review from yiyixuxu March 15, 2024 08:48
Comment on lines -1091 to -1100
cross_attention_kwargs (`dict`, *optional*):
A kwargs dictionary that if specified is passed along to the [`AttnProcessor`].
added_cond_kwargs: (`dict`, *optional*):
A kwargs dictionary containin additional embeddings that if specified are added to the embeddings that
are passed along to the UNet blocks.
down_block_additional_residuals (`tuple` of `torch.Tensor`, *optional*):
additional residuals to be added to UNet long skip connections from down blocks to up blocks for
example from ControlNet side model(s)
mid_block_additional_residual (`torch.Tensor`, *optional*):
additional residual to be added to UNet mid block output, for example from ControlNet side model
Copy link
Member Author

Choose a reason for hiding this comment

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

Duplicates.

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

@sayakpaul sayakpaul changed the title [LoRA] [LoRA] pop the LoRA scale so that it doesn't get propagated to the weeds Mar 15, 2024
# we're popping the `scale` instead of getting it because otherwise `scale` will be propagated
# to the internal blocks and will raise deprecation warnings. this will be confusing for our users.
if cross_attention_kwargs is not None and "scale" in cross_attention_kwargs:
lora_scale = cross_attention_kwargs.pop("scale", 1.0)
Copy link
Member

Choose a reason for hiding this comment

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

If "scale" in cross_attention_kwargs, we don't need a default for pop, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah that is correct.

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!

@sayakpaul
Copy link
Member Author

@yiyixuxu do you want me to do a patch release for this as this way the users won't get confused.

@yiyixuxu
Copy link
Collaborator

@sayakpaul sure

do you want me to do a patch release for this as this way the users won't get confused.

@sayakpaul
Copy link
Member Author

Cool, I will take care of it.

sayakpaul and others added 3 commits March 19, 2024 08:42
Co-authored-by: YiYi Xu <yixu310@gmail.com>
@sayakpaul sayakpaul merged commit ce9825b into main Mar 19, 2024
17 checks passed
@sayakpaul sayakpaul deleted the pop-scale branch March 19, 2024 03:42
sayakpaul added a commit that referenced this pull request Mar 19, 2024
…eds (#7338)

* pop scale from the top-level unet instead of getting it.

* improve readability.

* Apply suggestions from code review

Co-authored-by: YiYi Xu <yixu310@gmail.com>

* fix a little bit.

---------

Co-authored-by: YiYi Xu <yixu310@gmail.com>
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.

None yet

4 participants