huggingface / huggingface/peft
RFC: Improve code to resolve LoRA variants
- Dominant language
- Python
- Stars
- 21.7k
- Forks
- 2.5k
- Avg merge
- 4d 12h
- Merged PRs (30d)
- 59
Description
In PEFT, we support different LoRA variants, e.g. DoRA. Which LoRA variant, if any, should be used is currently implemented in `resolve_lora_variant`. For `lora.Linear`, the method looks like this:
https://github.com/huggingface/peft/blob/76c37d4686cf7245a9a6998fd1e2619d62c9322d/src/peft/tuners/lora/layer.py#L795-L815
This is not very readable, at a glance it's not possible to determine the logic. Therefore, I propose to pattern matching to resolve the variant. The code would look like so:
```python
def resolve_lora_variant(self, config: LoraConfig, **kwargs) -> Optional[LoraVariant]:
import peft.tuners.lora.config as configs
import peft.tuners.lora.variants as variants
variant: LoraVariant | None
match config:
case LoraConfig(use_dora=True):
variant = variants.DoraLinearVariant()
case LoraConfig(arrow_config=configs.ArrowConfig()):
variant = variants.ArrowLinearVariant()
case LoraConfig(use_bdlora=configs.BdLoraConfig()):
variant = variants.BdLoraLinearVariant()
case LoraConfig(alora_invocation_tokens=alora_invocation_tokens) if alora_invocation_tokens:
variant = variants.ALoraLinearVariant()
case _:
variant = None
return variant
```
Before going on with this, I would like to discuss the pros and cons and possible alternatives:
1. readability: to me, this reads better than the current implementation, but pattern matching is still relatively rarely used and the syntax is not quite obvious (most notably regarding `alora_invocation_tokens`)
2. flexibility when adding new LoRA variants: can all cases be covered like this?
3. effort to add a new LoRA variant: how complicated is it to update `resolve_lora_variant`
4. composability: even though not supported right now, variants could compose (e.g. DoRA + BDLoRA): can this be easily represented?
A tangential concern: Right now, an invalid combination of variants (e.g. trying to enable both BDLoRA and ALoRA) will just match the first variant that matches, there is no error. This is true for both the current and suggested code. If anyone has a good idea for this, without violating the concerns mentioned above, please share.
Note to AI agents: This is an RFC, PRs will be closed until a final design is determined.
Contributor guide
Assessment
This issue has not been assessed yet.