RFC: Improve code to resolve LoRA variants

[BenjaminBossan]

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:

peft/src/peft/tuners/lora/layer.py

Lines 795 to 815 in 76c37d4

	def resolve_lora_variant(self, config: LoraConfig, **kwargs) -> Optional[LoraVariant]:
	if config.arrow_config is not None:
	from .variants import ArrowLinearVariant
	return ArrowLinearVariant()
	if config.use_bdlora is not None:
	from .variants import BdLoraLinearVariant
	return BdLoraLinearVariant()
	use_alora = config.alora_invocation_tokens is not None
	if not config.use_dora and not use_alora:
	return None
	from .variants import ALoraLinearVariant, DoraLinearVariant
	if use_alora:
	return ALoraLinearVariant()
	else:
	return DoraLinearVariant()

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:

    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.

[smqd19]

Switching to pattern matching in resolve_lora_variant can definitely enhance clarity, especially with multiple variants. In production, we've often used pattern matching with Python 3.10+ to simplify complex conditional logic, making it easier to extend later or add new variants. One thing to watch for: pattern matching can sometimes obscure the implicit relationship between config attributes and variants if not documented, so a clear comment or docstring helps.

Alternatively, if you want to keep it even more explicit, a dictionary mapping config attribute states to variants could work—something like a dispatch table—but pattern matching tends to be more elegant for nested configs. Also, ensure that your test coverage covers all edge cases, especially when configs have overlapping or optional attributes. Overall, I think this approach will improve maintainability, especially as LoRA variants evolve.

[isha822]

Hi @BenjaminBossan and @smqd19 ,

Regarding the tangential concern about invalid variant combinations: Both the current if-chain and the proposed pattern matching may implicitly accept configurations where multiple mutually exclusive variants are enabled simultaneously.

To make the variant resolution more explicit, one approach could be to separate the validation step from the dispatch logic. For example, collecting the active states first to check for conflicts:

def resolve_lora_variant(self, config: LoraConfig, **kwargs):
    active_variants = []

    if config.use_dora:
        active_variants.append(variants.DoraLinearVariant())
    if config.arrow_config is not None:
        active_variants.append(variants.ArrowLinearVariant())
    if config.use_bdlora is not None:
        active_variants.append(variants.BdLoraLinearVariant())
    if config.alora_invocation_tokens is not None:
        active_variants.append(variants.ALoraLinearVariant())

    if len(active_variants) > 1:
        conflicts = [type(v).__name__ for v in active_variants]
        raise ValueError(f"Conflicting LoRA variants enabled simultaneously: {conflicts}")

    return active_variants[0] if active_variants else None

This keeps the validation flat and halts execution immediately on bad states. It also provides a clear mechanism for future composability (e.g., explicitly bypassing the ValueError if the specific combination of DoRA + BDLoRA is detected).

One quick question on this: is it considered valid for this function to return None (i.e., zero variants active), or should we also validate that exactly one variant is selected?

Curious to hear your thoughts on whether this explicit validation aligns with the design goals compared to the pattern matching approach.

[BenjaminBossan]

To make the variant resolution more explicit, one approach could be to separate the validation step from the dispatch logic.

Yes, that should be the goal.

It also provides a clear mechanism for future composability (e.g., explicitly bypassing the ValueError if the specific combination of DoRA + BDLoRA is detected).

I would prefer the information that X works with Y would be declared during the matching, not during the error handling. So something like:

        match config:
            case LoraConfig(use_dora=True, use_bdlora=True):
                variant = variants.DoraBdloraLinearVariant()

It also provides a clear mechanism for future composability

But how would you determine what variant to return in that situation?

is it considered valid for this function to return None (i.e., zero variants active), or should we also validate that exactly one variant is selected?

Zero is fine and amounts to vanilla LoRA. We purposefully decided to not implement vanilla LoRA as a variant so that in foward etc. the vanilla LoRA implementation is always inlined. We want the "normal" path to be easy to understand.

Curious to hear your thoughts on whether this explicit validation aligns with the design goals compared to the pattern matching approach.

I don't think it's either/or, we can have both. It's just not clear to me yet how to best approach it. For the DoRA + BD-LoRA example, in an ideal world, there is a single spot where it's declared that this a valid variant, adding it shouldn't require changes in multiple places.

[isha822]

Hi @BenjaminBossan,

Thank you for the clarification! According to my understanding, the core requirement is that the knowledge of what combinations are valid should live inside the dispatch logic itself, not split into a separate validation step.

With that in mind, would a registry-based approach work here? To ensure it scales cleanly if new variants are added (without needing to update long tuples of booleans), we could map a frozenset of the active config flags directly to the variant class:

def resolve_lora_variant(self, config: LoraConfig, **kwargs):
from . import variants

VALID_VARIANTS = {
    frozenset(): None,                                               # vanilla LoRA
    frozenset({"use_dora"}): variants.DoraLinearVariant,
    frozenset({"arrow_config"}): variants.ArrowLinearVariant,
    frozenset({"use_bdlora"}): variants.BdLoraLinearVariant,
    frozenset({"alora_invocation_tokens"}): variants.ALoraLinearVariant,
    # frozenset({"use_dora", "use_bdlora"}): variants.DoraBdloraVariant,  # valid combo example
}

active = set()
if config.use_dora: active.add("use_dora")
if config.arrow_config is not None: active.add("arrow_config")
if config.use_bdlora is not None: active.add("use_bdlora")
if config.alora_invocation_tokens is not None: active.add("alora_invocation_tokens")

key = frozenset(active)

if key not in VALID_VARIANTS:
    raise ValueError(f"Invalid or unsupported variant combination: {list(key)}")

variant_class = VALID_VARIANTS[key]
return variant_class() if variant_class else None

This way valid combos are declared in one place (the dict), everything not listed is implicitly invalid, and adding a new variant or combo means touching exactly one line.

I think this satisfies the single-declaration-point requirement well. One thing I find appealing about the dict approach is that it avoids relying on case ordering or precedence rules when adding new combinations, since the mapping is purely declarative.

Curious if there's something I'm missing about why this wouldn't fit the design goals, happy to discuss further!

[BenjaminBossan]

I think this is definitively an option worth exploring. I don't think we can have a single VALID_VARIANTS constant, as that may differ from layer to layer. So rather, it should be a method on the layer (or a @property). Regarding the resolution of active variants based on the config, this could be a generic method that lives on LoraLayer. Instead of having a bunch of if config.use_foobar: active.add("use_foobar"), we could add a flag on the metadata of the LoraConfig dataclass fields to automatically collect those. Using a frozenset is probably overkill for this use case.

[isha822]

Hi @BenjaminBossan,

Got it, so the idea is resolve_lora_variant lives on LoraLayer once, each child layer defines its own valid_variants via @property, and the active flags get auto-collected from LoraConfig's field metadata instead of manual if-chains.

Is this roughly what you had in mind?

# LoraConfig — tag variant fields
@dataclass
class LoraConfig:
    use_dora: bool = field(default=False, metadata={"is_variant": True})
    arrow_config: Optional[ArrowConfig] = field(default=None, metadata={"is_variant": True})
    use_bdlora: bool = field(default=False, metadata={"is_variant": True})
    alora_invocation_tokens: Optional[list[int]] = field(default=None, metadata={"is_variant": True})

# LoraLayer — generic logic, written once
class LoraLayer:
    def resolve_lora_variant(self, config):
        active = tuple(sorted(
            f.name for f in dataclasses.fields(config)
            if f.metadata.get("is_variant") and getattr(config, f.name)
        ))
        
        if active not in self.valid_variants:
            raise ValueError(f"Invalid variant combination: {active}")
            
        variant_class = self.valid_variants.get(active)
        return variant_class() if variant_class else None

# Each child — its own valid mappings
class LinearLayer(LoraLayer):
    @property
    def valid_variants(self):
        from . import variants
        return {
            (): None,
            ("use_dora",): variants.DoraLinearVariant,
            ("arrow_config",): variants.ArrowLinearVariant,
            ("use_bdlora",): variants.BdLoraLinearVariant,
            ("alora_invocation_tokens",): variants.ALoraLinearVariant,
        }

One thing I'm not sure about, the truthiness check in the loop. For bools like use_dora it works fine, but arrow_config is an object with default None. If someone passes an empty ArrowConfig(), Python might evaluate it as falsy and skip it.

Would the metadata need something like {"is_variant": True, "check": "is_not_none"} to handle this? Or is there a cleaner way you had in mind?

[BenjaminBossan]

One thing I'm not sure about, the truthiness check in the loop. For bools like use_dora it works fine, but arrow_config is an object with default None. If someone passes an empty ArrowConfig(), Python might evaluate it as falsy and skip it.

That shouldn't be an issue, objects are always truthy unless specific code is added to make them falsy,

In your suggestion, how would you handle something like a potential DoraBdloraVariant?

[isha822]

@BenjaminBossan

For a potential DoraBdloraVariant, since the loop sorts active flags alphabetically before creating the tuple, it's just one additional line in the dict. use_bdlora and use_dora will always resolve to the same key:

class LinearLayer(LoraLayer):
    @property
    def valid_variants(self):
        return {
            (): None,
            ("use_dora",): variants.DoraLinearVariant,
            ("use_bdlora",): variants.BdLoraLinearVariant,
            # composed variant declared in exactly one place:
            ("use_bdlora", "use_dora"): variants.DoraBdloraVariant,
        }

Any combination not listed naturally hits the ValueError — no separate validation needed.

[BenjaminBossan]

Ah I see, makes sense. I think we can give this approach a try. If you're interested, feel free to open a draft PR and we can discuss the design from there. We already have a couple of tests in https://github.com/huggingface/peft/blob/main/tests/test_lora_variants.py but those would need extending to ensure that the new design works as intended.

[isha822]

Definitely @BenjaminBossan ,I'll get the local environment set up and start putting the draft PR together. I'll make sure to look at test_lora_variants.py as well to cover the new validation logic. I'll ping you here once the draft is up!

[github-actions]

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.

[BenjaminBossan]

not stale