huggingface / huggingface/peft
bnb LoRA merge with lora_bias=True drops scaling factor on bias
- Dominant language
- Python
- Stars
- 21.7k
- Forks
- 2.5k
- Avg merge
- 4d 16h
- Merged PRs (30d)
- 60
Description
### Summary
Merging a LoRA adapter with `lora_bias=True` into a bitsandbytes 8-bit layer silently drops the `* scaling` factor on the bias — while the identical merge into a regular `nn.Linear` applies it (`lora/layer.py:978`). At `lora_alpha=32, r=4` (scaling 8.0) the merged bias is off by 3.35 max-abs. Unmerge omits the factor too, so a merge→unmerge round-trip restores the base and hides the corruption; only the *merged* artifact (the thing actually deployed) is wrong. Note: the audit's companion claim about the `safe_merge` guard missing `.all()` is already fixed (present since #1009) and is not part of this issue.
### Minimal reproduction (CPU; dequant stubbed, bias formula untouched)
```python
import torch, bitsandbytes as bnb
from torch import nn
import peft.tuners.lora.bnb as bnb_mod
from peft import LoraConfig, get_peft_model
bnb_mod.dequantize_bnb_weight = lambda w, state=None: w.data.float() # int8 dequant needs CUDA; bias path is real
class BnbNet(nn.Module):
def __init__(self):
super().__init__()
self.lin = bnb.nn.Linear8bitLt(16, 16, bias=True)
self.is_loaded_in_8bit = True
torch.manual_seed(0)
m = get_peft_model(BnbNet(), LoraConfig(r=4, lora_alpha=32, target_modules=["lin"],
lora_bias=True, init_lora_weights=False))
layer = m.base_model.model.lin # Linear8bitLt tuner, scaling 8.0
expected = layer.get_base_layer().bias.data.clone() + layer.lora_B["default"].bias * 8.0
m.merge_adapter()
merged = layer.get_base_layer().bias.data.clone()
print(torch.allclose(merged.float(), expected.float(), atol=1e-3)) # False; max-abs error 3.35
```
### Root cause
`src/peft/tuners/lora/bnb.py`: merge does `base.bias.data + lora_B.bias` (:127) and unmerge does `bias.data -= lora_B.bias` (:171) — neither multiplies by `self.scaling[adapter]`, unlike the non-bnb merge/unmerge (`lora/layer.py:978` / unmerge counterpart). The weight path is unaffected (`get_delta_weight` carries scaling). Earlier partial backport (#2489, `37f8dc34`) fixed `layer.py` only and never reached `bnb.py`.
### Expected behavior
bnb merge/unmerge apply `* self.scaling[adapter]` to the bias exactly like the base implementation, so merged 8-bit checkpoints match merged fp checkpoints.
### Proposed fix
Two one-line changes (`* self.scaling[active_adapter]` on :127 and :171), plus a CPU-runnable merge test with scaling != 1 (the existing GPU test's tolerance/setup never catches this since merge→unmerge round-trips clean). Happy to PR after a nod.
Environment: Python 3.12, torch 2.13.0+cpu, bitsandbytes 0.50.2, transformers 5.15.1, peft @ `ab2db1e0`.
Contributor guide
Assessment
This issue has not been assessed yet.