-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Adding weighted adapter as LoRAs combination gives unexpected result with StableDiffusion compared to webui #643
Comments
Hello @kovalexal, very detailed issue description, insightful and helpful, Thank you! Yes, I know the weighted adapter method isn't mathematically equivalent of merging loras one after another. I have mentioned the consecutive merging here in #280 (comment) The current implementation is inspired by https://github.com/cloneofsimo/lora/tree/master which seems to work in practice: I agree that it is incorrect mathematically but an easier way of mixing LoRAs. I believe point 2 would fit properly without much changes:
|
Hello, @pacman100, thanks for clarification! I'll dig into it if I have capacity one day. |
Hello, the merged PR #695 should address this by using point 2 you suggested of using SVD decomposition. The new rank is the max of the ranks of the LoRAs being combined. |
Also, I have been working on adding PEFT support in Khoya-ss for training and webui extensions for inference. PEFT training of DreamBooth: pacman100/peft-dreambooth (Branch: peft-dreambooth/ at smangrul/add-peft-support) Extension to use PEFT in webui: pacman100/peft-sd-webui-additional-networks (Branch: peft-sd-webui-additional-networks/ at smangrul/add-peft-support) Sample output trying it out: ![]() |
@pacman100 Wow, great, thank you, very useful addition! I've also worked on my own version of SVD decomposition for LoRAs weights, I assumed that it can be useful for somebody to also specify an output rank for combined adapter (so somebody can create adapter with similar characteristics but with loss of precision). Would you mind if I create a PR for this? |
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. |
This issue was fully addressed in #817, now we can get identical results to what we are getting in webui! |
System Info
python3.8, diffusers, transformers, accelerate, peft versions from main branches of each library (I used slightly modified version of your peft-gpu Dockerfile)
Who can help?
@pacman100 @younesbelkada
Information
Tasks
examples
folderReproduction
Hi!
I've discovered some unexpected results when combining multiple LoRA adapters together for StableDiffusion with PEFT, compared to webui results.
web UI setting:
peft setting:
Sanity check
Let's check that both checkpoints give the same results without using LoRAs.
The results are quite similar, so this test have passed.
Single LoRA
Let's check that both checkpoints give the same results when using single LoRAs (let's use Detail Tweaker aka
add-detail
).The results are also quite similar, so this test have passed.
Mixture of two LoRAs
Let's check that both checkpoints give the same results when using mixture of multiple LoRAs (let's use Detail Tweaker aka
add-detail
and 3D rendering style aka3DMM_V11
, both with weights 1.0).We can see that the results differ dramatically.
Mixture of two LoRAs - what is going on?
Let's try this approach in peft:
We can see that the results are quite similar to what we are getting in webui. So, we can definitely say that there is a problem in creating a weighted adapter for two LoRAs.
Mixture of two LoRAs - what is going on? - diving deeper
So, from my perspective, I see that there is a possible error inside method LoraModel.add_weighted_adapter.
peft/src/peft/tuners/lora.py
Lines 467 to 472 in f5352f0
A LoRA is an addition to the base weights:
So, from my perspective, a mixture of multiple LoRAs should be calculated like this:
But currently a mixture for the same rank LoRAs is calculated like this:
Mixture of multiple LoRAs - possible solutions:
I see the following possible solutions to overcome this issue:
Perform concatenation instead of a sum (use different dims for$B$ and $A$ ):
Perform some sort of decomposition (like SVD) of just LoRAs mixture$\alpha_1 B_1 A_1 x + \alpha_2 B_2 A_2 x + \ldots$ and drop least important components:
add_weighted_adapter
changes.Replace base weights with merged LoRAs, store a copy of base weights for unmerge/unmix:
@pacman100 I am not sure if this is applicable only to my case or not (maybe it works differently for text models), but I would be happy to help your team with fixing this issue.
Expected behavior
From my perspective, merge of multiple LoRAs in peft should work just like merge in webui.
The text was updated successfully, but these errors were encountered: