huggingface / peft

🤗 PEFT: State-of-the-art Parameter-Efficient Fine-Tuning.
https://huggingface.co/docs/peft
Apache License 2.0
16.46k stars 1.62k forks source link

PEFT Config checking update request #2112

Closed lemingshen closed 1 month ago

lemingshen commented 1 month ago

Feature request

Position: In peft/tuners/lora/model.py --> LoRaModel._check_new_adapter_config() The source code suggests a TODO: image

Please complete this checking function ASAP.

Motivation

When I tried to load multiple LoRA adapters into one base model, a ValueError was raised: image

After tracing the source code, I found that the adapter config checking function was not completed. So, please complete it.

Your contribution

NA

BenjaminBossan commented 1 month ago

Just to make sure: You are loading multiple LoRA adapters and you have more than one adapter with bias != "none". However, you think the error should not be raised in your case because these LoRA adapters only target modules that don't have a bias. Is that right?

If that's the case, you could edit the adapter_config.json to change the bias argument to "none" and the adapter should load fine.

The reason why I'm asking like this is because adding this check is unfortunately not trivial, which is why there is a TODO there instead of it already being implemented.

lemingshen commented 1 month ago

Just to make sure: You are loading multiple LoRA adapters and you have more than one adapter with bias != "none". However, you think the error should not be raised in your case because these LoRA adapters only target modules that don't have a bias. Is that right?

If that's the case, you could edit the adapter_config.json to change the bias argument to "none" and the adapter should load fine.

The reason why I'm asking like this is because adding this check is unfortunately not trivial, which is why there is a TODO there instead of it already being implemented.

Thanks for the reply. Well, actually, I trained multiple LoRA adapters separately and tried to combine them with one shared base model. I am just confused; why do we not support multiple adapters with different biases? I manually disabled the "raise ValueError" part in the source code and I could still successfully execute the code. Therefore, I'm confused why we need this check. Could you please provide some references if possible? I would be really appreciated.

BenjaminBossan commented 1 month ago

The issue is that if bias != "none", it means that we not only train the LoRA weights, but we also train the bias term of the original layer. So if you train the model with lora0 and add the bias, then you train a separate lora1 also with bias, it means that between the two the biases have diverged. Thus we cannot use both LoRAs together.

As to why this option exists, I'm not sure, as I was not working on PEFT when it was implemented. Honestly, I would highly recommend not to use it at all exactly to avoid this issue.

If you ignore the error and it still works, the reason is probably that the bias was not so important overall and the results are thus still good. However, we cannot assume that this is always so, therefore, we need to have this check.

lemingshen commented 1 month ago

Got it. Thank you so much for the detailed explanations. 👍