huggingface / huggingface/peft

PeftMixedModel.disable_adapter does not preserve nested or pre-disabled state

Open Beginner friendly
#3,507 2 comments 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
21.7k
Forks
2.5k
Avg merge
4d 12h
Merged PRs (30d)
59

Description

### System Info

- PEFT: `0.20.1.dev0` at commit `3af2ad701aecdf5c50bf9e539341dfdb42dd8bfe`
- The relevant `src/peft/mixed_model.py` implementation is unchanged from tag `v0.20.0`
- Python: `3.12.13`
- PyTorch: `2.13.0`
- Transformers: `5.14.1`
- Platform: macOS 26.5.1 arm64, CPU

### Who can help?

This concerns `PeftMixedModel.disable_adapter` and its context-manager state restoration.

### Reproduction

`PeftMixedModel.disable_adapter()` always enables adapter layers when it exits. As a result, an inner context re-enables adapters while an outer context is still active:

```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(4, 4, bias=False)

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

x = torch.arange(4, dtype=torch.float32).reshape(1, 4)
model = get_peft_model(
MLP(),
LoraConfig(target_modules=["lin"], init_lora_weights=False),
mixed=True,
)
model.eval()

with torch.no_grad():
adapter_output = model(x)

with model.disable_adapter():
with torch.no_grad():
base_output = model(x)
with model.disable_adapter():
inner_output = model(x)
after_inner_output = model(x)

print("adapter differs from base:", not torch.allclose(adapter_output, base_output))
print("inner is base:", torch.allclose(inner_output, base_output))
print("after inner is base:", torch.allclose(after_inner_output, base_output))
print("after inner is adapter:", torch.allclose(after_inner_output, adapter_output))
```

Actual output:

```text
adapter differs from base: True
inner is base: True
after inner is base: False
after inner is adapter: True
```

The same unconditional restoration changes a model that was already disabled before entering the context:

```python
model.base_model.disable_adapter_layers()
before = {
module.disable_adapters
for module in model.modules()
if hasattr(module, "disable_adapters")
}

with model.disable_adapter():
pass

after = {
module.disable_adapters
for module in model.modules()
if hasattr(module, "disable_adapters")
}
print("disabled flags before:", before)
print("disabled flags after:", after)
```

Actual output:

```text
disabled flags before: {True}
disabled flags after: {False}
```

The equivalent controls with a regular `PeftModel` preserve the disabled state in both cases.

The current mixed-model test suite passes, so this state-restoration behavior is not covered:

```text
python -m pytest tests/test_mixed.py -q
34 passed
```

### Expected behavior

Exiting `PeftMixedModel.disable_adapter()` should restore the adapter-enabled state that existed when that specific context was entered:

- after an inner context exits, adapters should remain disabled until the outer context exits;
- if adapters were already disabled before entering, they should remain disabled afterward;
- the existing behavior for the normal initially-enabled case should remain unchanged.

The root cause appears to be the unconditional `enable_adapter_layers()` in the `finally` block:

```python
@contextmanager
def disable_adapter(self):
try:
self.base_model.disable_adapter_layers()
yield
finally:
self.base_model.enable_adapter_layers()
```

Regular `PeftModel.disable_adapter()` already snapshots the model status and only re-enables layers when they were not fully disabled on entry.

I searched open and closed issues and PRs for mixed-model, nested-context, and state-restoration terms and found no existing report. I also checked the open PRs that modify `mixed_model.py`: #3504 concerns deletion of merged adapters, while #3493 contains type-annotation changes only. Issue #3447 is also distinct: it concerns `requires_grad` changes when re-enabling an inference-mode adapter, not restoring an already-disabled or nested context state.

Would a focused PR aligning `PeftMixedModel.disable_adapter()` with the regular model's state-restoration behavior be welcome? The proposed scope would be the context manager plus CPU regression tests in `tests/test_mixed.py`.

AI assistance was used to audit this path and prepare the report. I ran both failing reproductions, the regular `PeftModel` controls, and the complete `tests/test_mixed.py` suite against the checkout above.

Contributor guide

Open the contributing guide

Research direction

Start by reading src/peft/mixed_model.py and running python -m pytest tests/test_mixed.py -q. Add focused regression coverage for nested and pre-disabled contexts, and confirm the existing initially-enabled behavior still passes with adapters restored only to their entry state.

Written by the indexing model from the issue text.

Assessment

Tech stack
python, pytorch
Domain
machine-learning, testing
Issue type
Bug
Difficulty
2/5
Estimated time
Half a day
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
84/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.