huggingface / huggingface/peft

save_pretrained writes an empty/truncated adapter checkpoint (silently) when another adapter name equals a base-model module path component

Open
#3,584 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

Hi PEFT team! :wave: Continuing the same read-only audit series as #3581–#3583 (@ `5d602fdb`). This one is a **silent checkpoint data-loss** bug we could reproduce end-to-end on CPU.

**Environment:** peft `0.20.1.dev0` @ `5d602fdb` · transformers `5.15.1` · torch `2.13.0` CPU · Python 3.12 · macOS arm64

### Problem

`get_peft_model_state_dict` removes *every* key that contains `.{other_adapter}.` or ends with `.{other_adapter}` before method-specific selection (`src/peft/utils/save_and_load.py:84-90`, applied at `:134-143`). The match is raw string containment over the whole key — so if another adapter's **name equals any legitimate module-path segment** of the saved adapter's tensors ("mlp", "attn", "encoder", "score", "proj" are all real module names), those tensors are dropped *before* anything can restore them. Adapter names like this are accepted by `add_adapter` (`peft_model.py:1111-1119` only rejects duplicates / warns on tuner-prefix containment).

The empty-result fallback at `save_and_load.py:139` only rescues the case where *all* keys are filtered out; partial collisions pass through with the subset missing, and nothing warns at save time or afterwards.

### Reproduction (verified)

```python
import tempfile, torch, torch.nn as nn
from peft import LoraConfig, get_peft_model
from safetensors.torch import load_file

class Net(nn.Module):
def __init__(self):
super().__init__()
self.lin0 = nn.Linear(32, 32)
self.mlp = nn.Linear(32, 32)
self.act = nn.ReLU()
def forward(self, x):
return self.mlp(self.act(self.lin0(x)))

torch.manual_seed(0)
m = get_peft_model(Net(), LoraConfig(r=4, lora_alpha=8,
target_modules=["lin0", "mlp"], lora_dropout=0.0))
# second adapter whose NAME collides with the base module "mlp"
m.add_adapter("mlp", LoraConfig(r=4, lora_alpha=8, target_modules=["lin0"], lora_dropout=0.0))

d = tempfile.mkdtemp()
m.save_pretrained(d, selected_adapters=["default"])
print(sorted(load_file(f"{d}/adapter_model.safetensors").keys()))
# ['base_model.model.lin0.lora_A.weight', 'base_model.model.lin0.lora_B.weight']
# ← every 'mlp' tensor is gone; NO warning anywhere.
```

When *all* of the adapter's keys collide (adapter targeting only the `mlp` module), `adapter_model.safetensors` is written completely **empty**, also without warning:

```python
# same setup but target_modules=["mlp"]
print(list(load_file(f"{dA}/adapter_model.safetensors").keys())) # []
```

Reloading either artifact yields an adapter at random init for the lost modules — trained weights are permanently absent from the file. Realistic trigger: multi-adapter users naming adapters semantically ("encoder", "proj", "head" …) while another adapter targets modules under those paths.

### Suggested directions

1. Filter positionally instead of by containment — only strip segments that actually occupy an adapter-name slot (e.g. right after tuner parameter prefixes like `lora_`/`ia3_`/`modules_to_save.` infix). The load side already resolves keys structurally via `_get_tuner_state_dict_key_prefixes(...)` (`tuners_utils.py:1561`), so a symmetric approach seems feasible; or
2. Keep the negative filter as a fast path but add a post-selection sanity check per tuner prefix (selection non-empty / count matches expectation) and **warn or raise** when the produced checkpoint looks truncated.

Happy to prepare a PR (with regression tests for both the partial and total collision cases) if you tell me which approach you prefer. Thanks! 🙏

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.