Skip to content

Comments

[core / Lora] Fix lora scale on text encoders#5087

Closed
younesbelkada wants to merge 1 commit intohuggingface:mainfrom
younesbelkada:fix-scale-lora
Closed

[core / Lora] Fix lora scale on text encoders#5087
younesbelkada wants to merge 1 commit intohuggingface:mainfrom
younesbelkada:fix-scale-lora

Conversation

@younesbelkada
Copy link
Contributor

What does this PR do?

Found a potential bug in adjust_lora_scale_text_encoder

xxxPipeline calls under the hood this method: https://github.com/huggingface/diffusers/blob/main/src/diffusers/models/lora.py#L28-L39 to adjust the lora scale on the text encoder. That method seems to force assign the lora_scale of the LoRA layers to a new one. On the other hand, when calling fuse_lora it seems that we multiply the the lora_scale: https://github.com/huggingface/diffusers/blob/main/src/diffusers/models/lora.py#L130
Maybe the reason this has not been flagged by the CI is that self.lora_layer.network_alpha / self.lora_layer.rank was somehow equal to 1.

cc @patrickvonplaten @sayakpaul

@patrickvonplaten
Copy link
Contributor

Actually I think this is expected since one method is just used for the forward pass (since we can't pass forward to transformers) and the other one is for fusing. @sayakpaul wdyt?

@sayakpaul
Copy link
Member

@patrickvonplaten exactly.

For fusing the LoRA parameters for text encoders, lora_scale is supposed to be passed to fuse_lora(). After the LoRA parameters have been fused, passing any scale parameters via cross_attention_kwargs via the pipeline call becomes a no-op.

So, not sure, if this is the best way to tackle this actually.

@github-actions
Copy link
Contributor

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.

@github-actions github-actions bot added the stale Issues that haven't received updates label Oct 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stale Issues that haven't received updates

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants