`LoraConfig` multiple properties should be unified

[Qubitium]

@BenjaminBossan I am trying to add dynamic Lora support to both vLLM and SGLang as LoraConfig already supports this dynamic control via the following variables:

• rank_pattern: regex matching of which different r/rank values are applied
• exclude_modules: regex: which modules are not excluded from lora completedly
• alpha_pattern: regex matching of alpha override. extactly the same as rank_pattern but different property.

Nothing wrong with them individually but together, they become unncessary detached and has negative impact on code cost but also on dynamic control efficiency.

GPTQModel uses a single dynamic: Diction[str, Dict[]] where the str is a regex with +: (positive prefix, optional), -: negative prefix (Optional).

The dict value is the property override in string: value format.

Example as applied to PEFT (Proposal):

# implicit +: prefix if not used
# prefixs are stripped before the regex is performed
"mlp\.down_proj": { "r": 128 } # implicit positive
"+:mlp\.down_proj": { "r": 256 } # explicit positive
"-:mlp\.gate_proj": {} # negative

This simple control allows 3 states.

• Positive match == override any property values in base config (LoraConfig).
• Negative match == skip this modele for Lora (no LoraConfig at all)
• No match == There is no module matched so Base LoraConfig is used.

This single control replaces all existing PEFT control with same functionally while allowing ALL properties to be dynamically overriden (if necessary) without any additional apis/LoraConfig vars. As it exists, you need to add code and logic to every LoraConfig property that participates in dynamic override/control.

Basically I want Peft LoraConfig to the clean standard for vLLM and SGLang when it comes to dynamic control. Having a unified dynamic override system makes everyone's life so much easier and at the same time eliminate the issue that we have to write code each time a new LoraConfig property comes into pace.

Let me know what you think. I am willing to spend time working on it. You can also reach me at qubitium@modelcloud.ai and on X: qubitium. I really would love to chat with you for like 15 minutes to ping-pong this idea with you.

CC: @SunMarc @MekkCyber

[BenjaminBossan]

Hi @Qubitium, thanks for starting this discussion.

It is indeed true that the current system to control which layers are targeted can be a bit complicated, since it has grown over time and wasn't designed top down. In reality, it is even more complex than what you sketched out, since we also have the layers_to_transform and layers_pattern arguments, but I think those are not widely used.

I'm not completely sure what you mean by "dynamic", but I think I roughly understand the system you propose. For the adoption in PEFT, here are my considerations:

1. Every change we make should be backwards compatible.
2. Ideally, let's avoid adding one more way to configure the patterns, as it would increase complexity.
3. I could, however, envision the possibility to have a conversion function that takes a config as you described and converts it to a valid PEFT config.
4. I'm not a big fan of the + and - syntax. I'd like to stick with regex, as that can already encode positive and negative matches, instead of requiring users to learn a new concept.
5. There is currently a PR with some changes to rank_pattern / alpha_pattern (ENH Allow rank/alpha keys to be "fully qualified" #2382) but I converted it to a draft PR for now to put it on hold.

Let me know what you think. I am willing to spend time working on it. You can also reach me at qubitium@modelcloud.ai and on X: qubitium. I really would love to chat with you for like 15 minutes to ping-pong this idea with you.

Are you in any of the HF collab Slack channels?

pinging @githubnemo to have your opinion on this.

[Qubitium]

# implicit +: prefix if not used
# prefixs are stripped before the regex is performed
"mlp\.down_proj": { "r": 128 } # implicit positive
"+:mlp\.down_proj": { "r": 256 } # explicit positive
"-:mlp\.gate_proj": {} # negative

Every change we make should be backwards compatible.

I think full backward compat is doable.

Ideally, let's avoid adding one more way to configure the patterns, as it would increase complexity.

Yes. But current system is not sustainable so maybe one-off cost is good to save tons of code. When I mean tons of code I also means 3rd party vLLM and SGLang that would use the new system and no need for the old system.

Backward compat can be achieced where both new and old params are encoded inside config. Libs like vllm and sglang has little concern for backward compat so they would never touch the old params thus code debt is very low.

I could, however, envision the possibility to have a conversion function that takes a config as you described and converts it to a valid PEFT config.

I had this exact system in mind, except in reverse while thinking how I should parse all thse individual vars for vllm/sglang. =)

If new format, use new format, if old, still use new format but migrate/convert old format into new format so that internally Peft always use the agreed upon unified format. This make code alot easier and have backward compat dedicated to a conversion logic method that merge all the various vars into one filter/override/dynamic system however you name it.

I'm not a big fan of the + and - syntax. I'd like to stick with regex, as that can already encode positive and negative matches, instead of requiring users to learn a new concept.

I think we are talking about different representation of negative. The -: prefix in my sample means if matched then module should be skipped == negtive == skip module logic. The matching are using regex. The prefix above is actually a labeler to give the regex 2 extra properties: Postive Match (override mode), Match but Skip (Negtive). Can python re api do these? I am fairly new to python.

Are you in any of the HF collab Slack channels?

Not in any HF slack yet. Do you have public Slack workspace links? Thanks.

[githubnemo]

Hey :)

I like the initiative to consolidate regex semantics for targeting layers by name. Ideally there is one utility function that does it.

Regarding your proposed example, as far as I can see positive and negative matching is already possible with python regex, no need for a prefix. Your example with the exclude (?!) operator:

import re

pattern = r"mlp\.(?!gate_proj)down_proj"

assert re.match(pattern, "mlp.down_proj")
assert not re.match(pattern, "mlp.gate_proj")

Such patterns can be extended as is typical, e.g. (?!gate_proj|mix_proj) to exclude more types.

Therefore, I'm not sure if we need something more than regex and just need to focus on consolidating functionality in code and making sure that full regex compat. is given (e.g., if a user supplies ^foo, as it currently stands there would be no match since the pattern is prepended with \.).

[Qubitium]

Hey :)

HI @githubnemo ! Thought you were a github bot based on the handle. =)

pattern = r"mlp.(?!gate_proj)down_proj"

assert re.match(pattern, "mlp.down_proj")
assert not re.match(pattern, "mlp.gate_proj")

As my lastest reply to @BenjaminBossan shows, my wording of Negative matching vs Postivie matching is regrettable.

-: prefix means this regex (prefix removed), if matched, this module is excluded from LoraConfig as in this module has zero LoraConfig == module is skipped for lora completely. Without this, we can't handle exclusion.

I don't think internal regex postiv/negative match means the same thing? You have positive/negative interal string matching but that doesnt present what the actual regex string stands for.

We need three states:

• Match for Override == +: (implicit default)
• Match for Module Skip/Exclusion == -:
• No match == do nothing == the regex of above 2 didn't match any module name.

[githubnemo]

HI @githubnemo ! Thought you were a github bot based on the handle. =)

beep boop.

-: prefix means this regex (prefix removed), if matched, this module is excluded from LoraConfig as in this module has zero LoraConfig == module is skipped for lora completely. Without this, we can't handle exclusion.

I don't think internal regex postiv/negative match means the same thing? You have positive/negative interal string matching but that doesnt present what the actual regex string stands for.

We need three states:

* `Match for Override` == `+:` (implicit default)

* `Match for Module Skip/Exclusion` == `-:`

* No match == do nothing == the regex of above 2 didn't match any module name.

I fail to see the need, sorry. Why would we need a LoRA config that states that the module is explicitly excluded when could ignore the layer completely instead?

The only way for this to be necessary is when we assume that there is more than one regex, e.g. - to stick with your proposed syntax - "+:proj_.*" and "-:proj_out". But then you could simply write "(!?proj_out)proj_.*" and achieve the same in one pattern. So what's the benefit?

[Qubitium]

We need three states:

* `Match for Override` == `+:` (implicit default)

* `Match for Module Skip/Exclusion` == `-:`

* No match == do nothing == the regex of above 2 didn't match any module name.

I fail to see the need, sorry. Why would we need a LoRA config that states that the module is explicitly excluded when could ignore the layer completely instead?

I still think we are talking about two different things here. Let me write out an example:

Right now we have a base LoraConfig. We want to apply this base config to all, but override some configs for layer 2 up_proj modules, and exclude down_proj in in exaclty layer 4.

So in my mind I would write this:

[
  "+:layer.1.up_proj": { "r": 256 }, # <- if match, we do override
  "layer.1.up_proj": { "r": 256 }, # <- same as above
  "-:layer.3.down_proj": None,  # <- if match, we exclude module (no LoraConfig applied)  
]

This replaces `exclude_modules` and `rank_pattern` which already exists in LoraConfig config dataclass. 

I don't understand how you can represent the above context with 1 regex? How does it give you two distinct ctx states? It only eval to true/false right? Basically the -/+ is not about negative/positive matching but what ACTION to take when match happens. Is the ACTION = override, or ACTION = exclude. It has nothing to do with the matching regex internally.

My question is how can we apply override { "r": 256 } and do module exclusion in 1 regex using above example?

So another way of expression this is follows:

[
  "override:layer.1.up_proj": { "r": 256 }, # <- if match, we do override
  "layer.1.up_proj": { "r": 256 }, # <- same as above
  "exclude:layer.3.down_proj": None,  # <- if match, we exclude module (no LoraConfig applied)  
]

Right now LoraConfig has exclude_modules to exclude modules, and rank_pattern to override ranks per module. Both use regex. Both can be unified is my goal here that can also override any property inside LoraConfig's and exclude modules entirely (future proofing).

[Qubitium]

I fail to see the need, sorry. Why would we need a LoRA config that states that the module is explicitly excluded when could ignore the layer completely instead?

LoraConfig.exclude_modules PR:
#2102

[githubnemo]

Thank you for the examples, I think I understand your angle a bit better now, especially the semantics of the -: prefix. To further the mutual understanding, here's an example for a LoraConfig how I understand the feature you propose:

LoraConfig(
    ...,
    module_overrides={
      # exclude all projection layers
      r'.*_proj': None,
      
      # except for down and gate projections
      r'down_proj': {
        'rank': 10, 'init': 'eva',
      },
      
      r'gate_proj': {
        'rank': 20, 'alpha': 5,
      },
    },
)

Note that I'm using None to mark a module to be excluded. This would alleviate the need for the prefix, no?

[Qubitium]

LoraConfig(
    ...,
    module_overrides={
      # exclude all projection layers
      r'.*_proj': None,
      
      # except for down and gate projections
      r'down_proj': {
        'rank': 10, 'init': 'eva',
      },
      
      r'gate_proj': {
        'rank': 20, 'alpha': 5,
      },
    },

Yes! This is the idea.

Note that I'm using None to mark a module to be excluded. This would alleviate the need for the prefix, no?

This works too.

I still like the explicit declaration of -: preifx vs using value of None to dictate regex Action but its minor nitpick in the whole scheme of things.

Onward with more thoughts on this.

One rules execution:

For the sake of calarity, perhaps there should only be ONE rule matches per module allowed?

Only 1 rule can match a module. First rule match exits matching module matching loop. Meaning, there should not be 2 rules that matches a module.

I am trying to think of a case where a person would write 2 rules for the same module in the same config. Would it be useful? Does it make it more clear or more confusing?

So perhaps avoid, not allow, users to exclude all modules and then at the same time allow modules. This is ambiguous. First rule match wins solves this. Make users generate non-conflicting concise rules.

[BenjaminBossan]

Just a general comment: Let's keep in mind that not all those configs are necessarily user-written. E.g. in diffusers, these configs are often created by code. The need for this arises because there are different LoRA trainers out there that don't use PEFT. They often only store the state_dict without meta data. To load those checkpoints into diffusers (which uses PEFT for LoRA handling), a LoraConfig is created based on the base model and the state dict.

[githubnemo]

@Qubitium

I think this change would be quite complex, even if it is just for LoRA for the moment. This in itself is not an issue but I think there's little basis for why it is worthwhile. Can you elaborate a bit more why this change is necessary and, especially, what use-cases are now easier to implement (or were near impossible before)?

I could see this as a refactor when there is a sensible feature request for, say, LoRA to support init methods depending on the layer type. But as it stands I fear that cost is far greater than use right now.

[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.