Model with LoRA adapter errors when saving with `safe_serialization=True`

mliu-aqtc commented 3 months ago

System Info

transformers version: 4.41.2
Platform: Linux-5.11.0-41-generic-x86_64-with-glibc2.31
Python version: 3.11.9
Huggingface_hub version: 0.23.3
Safetensors version: 0.4.3
Accelerate version: 0.31.0
PyTorch version (GPU?): 2.3.0.post301 (True)
peft version: 0.11.1

Who can help?

No response

Information

[ ] The official example scripts
[ ] My own modified scripts

Tasks

[ ] An officially supported task in the examples folder
[ ] My own task or dataset (give details below)

Reproduction

To repro, download a model, add a lora adapter that sets bias to "all" and includes some modules to save, and then try to save it.

from transformers import BertForSequenceClassification
from peft import LoraConfig

model = BertForSequenceClassification.from_pretrained('yiyanghkust/finbert-tone')
config = LoraConfig(
    r=256,
    lora_alpha=16,
    target_modules=["query", "value"],
    bias="all",  # Setting to "lora_only" does not error
    modules_to_save=["classifier"],
)
model.add_adapter(config)

model.save_pretrained('output_dir', safe_serialization=True)

UPDATE: Using get_peft_model(...) instead of add_adapter(...) does not error even when setting bias to "all".

Error:

RuntimeError: The weights trying to be saved contained shared tensors [{'base_model.model.classifier.modules_to_save.bias', 'base_model.model.classifier.bias'}] that are mismatching the transformers base configuration. Try saving using `safe_serialization=False` or remove this tensor sharing.

Expected behavior

The model is expected to save properly without error. Maybe the issue is that the library is doing some optimization that introduces some shared tensor?

BenjaminBossan commented 3 months ago

I could not replicate the issue using bert-base-uncased:

from transformers import BertForSequenceClassification
from peft import LoraConfig

model = BertForSequenceClassification.from_pretrained('bert-base-uncased')
config = LoraConfig(
    r=256,
    lora_alpha=16,
    target_modules=["query", "value"],
    bias="all",  # Setting to "lora_only" does not error
    modules_to_save=["classifier"],
)
model.add_adapter(config)
model.save_pretrained('/tmp/peft/issue-1876', safe_serialization=True)

Could the error be related to the specific model you're using (I didn't test it as it's a pickle file).

Using get_peft_model(...) instead of add_adapter(...) does not error even when setting bias to "all".

The idea is to use get_peft_model to initialize a PeftModel instance, which has some more advanced features. If you call add_adapter on the BertForSequenceClassification, the model stays a BertForSequenceClassification instance with the LoRA adapters injected in the appropriate place. Depending on the type of work you're doing, this could be sufficient but I would generally recommend to use get_peft_model.

mliu-aqtc commented 3 months ago

I would generally recommend to use get_peft_model.

I see, thanks for the tip. I ended up using get_peft_model which worked. However, I did also run into a separate error with the peft model in combination with transformers.Trainer, which I found a workaround for here: https://discuss.huggingface.co/t/eval-with-trainer-not-running-with-peft-lora-model/53286. I'll submit a separate issue for that instead.

In general though, what is the best practice on when to use get_peft_model and when to use add_adapter? I could not find much material on this and assumed they were interchangeable.

BenjaminBossan commented 3 months ago

In general though, what is the best practice on when to use get_peft_model and when to use add_adapter? I could not find much material on this and assumed they were interchangeable.

Yes, we should probably work on improving the docs a bit in this regard.

In general, I would recommend to use add_adapter if you want to stay as close as possible to transformers and can live without some of the more advanced PEFT features. For example, say you want to only load a single adapter and run inference with it.

I would use get_peft_model if I need, or may need in the future, the full breadth of the PEFT features. Generally, I would also prefer it for training. And finally, add_adapter does not support all the different PEFT methods that we have implemented, so if you think you might switch to another one in the future, you're better off going with get_peft_model.

github-actions[bot] commented 2 months ago

This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.

huggingface / peft