huggingface / huggingface/peft

Merging a subset of the active adapters silently disables the remaining active adapters in forward

Open
#3,756 2 comments 0 reactions 1 assignee Claimed by @BenjaminBossan View on GitHub
Dominant language
Python
Stars
21.7k
Forks
2.5k
Avg merge
4d 12h
Merged PRs (30d)
59

Description

### System Info

- peft: main (`fd570a5f`), 0.21.0
- transformers: 5.17.0
- torch: 2.14.0
- accelerate: 1.15.0
- Python: 3.11
- Platform: Linux, CPU only

### Who can help?

@BenjaminBossan

### Reproduction

```python
import torch
from torch import nn
from peft import LoraConfig, get_peft_model

class MLP(nn.Module):
def __init__(self):
super().__init__()
self.lin = nn.Linear(8, 8)

def forward(self, x):
return self.lin(x)

torch.manual_seed(0)
x = torch.randn(2, 8)
config = LoraConfig(target_modules=["lin"], r=4, init_lora_weights=False)

model = get_peft_model(MLP(), config, adapter_name="A").eval()
model.add_adapter("B", config)
model.base_model.set_adapter(["A", "B"]) # both adapters active

with torch.no_grad():
out_both = model(x)
model.merge_adapter(adapter_names=["A"]) # merge only A; B stays active and unmerged
out_after_partial_merge = model(x)
model.unmerge_adapter()
out_after_unmerge = model(x)

print("active adapters:", model.active_adapters)
print("same output after merging A only:", torch.allclose(out_both, out_after_partial_merge))
print("same output after unmerging again:", torch.allclose(out_both, out_after_unmerge))
print("max abs diff:", (out_both - out_after_partial_merge).abs().max().item())

# The partially merged model behaves like adapter A alone:
model.set_adapter("A")
with torch.no_grad():
out_only_A = model(x)
print("partially merged output == adapter A alone:", torch.allclose(out_only_A, out_after_partial_merge))
```

Output on `main`:

```
active adapters: ['A', 'B']
same output after merging A only: False
same output after unmerging again: True
max abs diff: 0.5326088070869446
partially merged output == adapter A alone: True
```

`B` is still reported as active (`model.active_adapters == ['A', 'B']`, `get_layer_status()` shows `active_adapters=['A', 'B']`, `merged_adapters=['A']`) but contributes nothing to the output. `unmerge_adapter()` restores the combined output, so the adapter weights are intact; it is purely the forward path. No warning is emitted.

### Cause

Every tuner layer's `forward` short-circuits once anything is merged, e.g. in `lora/layer.py`:

```python
elif self.merged:
result = self.base_layer(x, *args, **kwargs)
else:
for active_adapter in self.active_adapters:
...
```

so the loop over `self.active_adapters` is skipped entirely. `check_adapters_to_merge` only removes already-merged adapters from the request; nothing checks that the active adapters left unmerged will still be applied. The same pattern exists in all tuners (LoRA, IA³, LoHa, LoKr, OFT, ...). The mirror case, merging an *inactive* adapter while a different one is active, drops the active one for the same reason.

### Expected behavior

Either the unmerged active adapters are still applied on top of the merged weights, or `merge_adapter` refuses / warns when the merge would leave an active adapter unapplied, the same policy `_check_forward_args` already uses for `adapter_names` with merged layers ("Cannot pass `adapter_names` when there are merged adapters"). Silently changing the output while still reporting the adapter as active is the part to avoid.

I'm happy to implement whichever you prefer: the guard rail in `check_adapters_to_merge` plus a test in `tests/test_custom_models.py` next to `test_multiple_active_adapters_merge_and_unmerge` is small; applying unmerged active adapters in `forward` is bigger and I'd scope it to LoRA first.

AI assistance (Claude) was used to investigate and draft this; I have reproduced it myself.

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.