huggingface / huggingface/peft
PeftModel.disable_adapter is not reentrant for prompt-specific adapters
- 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 `607895c9907936b296598d2a6520a9e5a74f82a5`
- Transformers: `5.15.0`
- PyTorch: `2.13.0+cpu`
- Python: `3.13.5`
- Platform: Windows 11, CPU
### Problem
`PeftModel.disable_adapter()` does not preserve nested disabled state in its prompt-specific branches:
1. With Adaption Prompt, entering a nested context raises and the outer cleanup masks the original exception with `KeyError: 'default'`.
2. With prompt learning, forwarding remains disabled after the inner context exits, but `_adapters_disabled` is reset to `False`, so `has_active_enabled_adapter` incorrectly reports `True` while the outer context is still active.
The regular tuner branch already snapshots the model status before disabling. This report concerns the two earlier branches in `PeftModel.disable_adapter()`, not `PeftMixedModel` from #3507.
### Reproduction
This is CPU-only and constructs the models from local configs; it does not download a checkpoint.
```python
from transformers import GPT2Config, GPT2LMHeadModel
from peft import AdaptionPromptConfig, PromptTuningConfig, TaskType, get_peft_model
config = GPT2Config(
n_layer=2,
n_head=2,
n_embd=16,
n_positions=32,
n_ctx=32,
vocab_size=64,
bos_token_id=1,
eos_token_id=2,
)
adaption = get_peft_model(
GPT2LMHeadModel(config),
AdaptionPromptConfig(adapter_layers=1, adapter_len=2, task_type=TaskType.CAUSAL_LM),
)
with adaption.disable_adapter():
with adaption.disable_adapter():
pass
prompt = get_peft_model(
GPT2LMHeadModel(config),
PromptTuningConfig(task_type=TaskType.CAUSAL_LM, num_virtual_tokens=2),
)
with prompt.disable_adapter():
assert not prompt.has_active_enabled_adapter
with prompt.disable_adapter():
assert not prompt.has_active_enabled_adapter
assert not prompt.has_active_enabled_adapter
```
The Adaption Prompt case exits with:
```text
KeyError: 'default'
```
If that block is removed so the prompt-learning case can run, its last assertion fails because `has_active_enabled_adapter` becomes `True` after the inner context exits.
### Root cause
The prompt-learning and Adaption Prompt branches unconditionally set `_adapters_disabled = False` in `finally`. The Adaption Prompt branch also unconditionally calls `disable_adapter_layers()` on entry and `enable_adapter_layers()` on exit.
After the outer Adaption Prompt context removes and caches the `AdaptedAttention` wrapper, the inner call tries to remove it again. `_remove_adapted_attentions()` now sees the original attention module and accesses `attn.model`, which raises. During unwinding, the nested enable/disable operations consume the same cache twice, and the final exception becomes `KeyError: 'default'`.
For prompt learning, method restoration remains nested correctly because each context saves its current callables, but the boolean status is not restored to its entry value.
### Expected behavior
- Nested `disable_adapter()` contexts should not raise.
- Adapters should remain disabled until the outermost active context exits.
- `has_active_enabled_adapter` should agree with that state throughout.
- A context entered while adapters are already disabled should leave them disabled on exit.
Would a focused PR be welcome that snapshots the entry state for these two branches, avoids repeating the Adaption Prompt layer transition when already disabled, and adds CPU regression tests for both cases? I will wait for maintainer approval before starting the patch.
### Baseline verification
The existing Adaption Prompt suite passes and does not cover nested contexts:
```text
python -m pytest tests/test_adaption_prompt.py -q
33 passed, 3 xfailed
```
### AI assistance disclosure
AI assistance was used for the audit and write-up. I ran the reproductions and existing test suite against the checkout above, reviewed the relevant state transitions, and will review and be able to explain every changed line if a PR is approved.
Contributor guide
Research direction
Start at PeftModel.disable_adapter() and its prompt-specific branches, then read tests/test_adaption_prompt.py and run the existing baseline command. Add focused CPU regression coverage for nested Adaption Prompt and prompt-learning contexts, including contexts entered while already disabled. Done means nested contexts exit without errors and has_active_enabled_adapter remains false until the outer context exits.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python, pytorch
- Domain
- machine-learning, testing-qa
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 68/100