huggingface / huggingface/peft
PeftMixedModel.disable_adapter does not preserve nested or pre-disabled state
- 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
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