huggingface / huggingface/peft

bnb LoRA merge with lora_bias=True drops scaling factor on bias

Open
#3,741 2 comments 0 reactions 1 assignee Claimed by @BenjaminBossan View on GitHub
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.