Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[fix] revision of the adapter model can now be specified. #3079

Merged
merged 3 commits into from
Nov 27, 2024

Conversation

pesuchin
Copy link
Contributor

Resolved: #3061

Solution

I modified the _load_peft_model method so that when the config is PeftConfig, it handles loading both the base model and the adapter model. Additionally, when loading the base model, I temporarily save and remove the revision from model_args before loading the base model.

This ensures that the base model is loaded using the default revision instead of the adapter model's revision, resolving the error.

Verification

The following three codes were tested and worked correctly.

  • verification1:
from sentence_transformers.models.Transformer import Transformer

args = {
    "token": <your token>,
    "trust_remote_code": False,
    "revision": <your repository revision>,
    "local_files_only": False
}

transformer = Transformer(
    model_name_or_path=<your model_path>,
    cache_dir=None,
    backend="torch",
    max_seq_length=512,
    do_lower_case=True,
    model_args=args,
    tokenizer_args=args,
    config_args=args
)
  • verification2:
from sentence_transformers import SentenceTransformer
model = SentenceTransformer("sentence-transformers-testing/stsb-bert-tiny-safetensors")
model.encode("Hello, World!")
  • verification3:
from sentence_transformers import SentenceTransformer

kwargs = {
    "revision": <your revision>,
}
model = SentenceTransformer(<your model repository path>,
                            use_auth_token=<your token>,
                            model_kwargs=kwargs,
                            config_kwargs=kwargs,
                            tokenizer_kwargs=kwargs)
model.encode("Hello, World!")

@tomaarsen
Copy link
Collaborator

Hello!

I see a minor issue here that I'd like to see resolved:

  • The T5 and MT5 models can also have PEFT Adapters, but no longer with this PR.

I had a crack at this, and I think the easiest solution is actually to have the _load_config return whether the model is a PEFT Model or not. That simplifies everything else a lot. Then we can load the base model like before, but beforehand we update the model_args and then afterwards we load the PEFT adapter on top.

I also added a simple test with the https://huggingface.co/sentence-transformers-testing/stsb-bert-tiny-lora model.

I'm curious to hear what you think!

  • Tom Aarsen

@tomaarsen
Copy link
Collaborator

Apologies for expanding the PR a bit more via 7ee9fa8. In https://github.com/UKPLab/sentence-transformers/pull/3079/files you can select only the first 2 commits of this PR by shift clicking the first 2
image

That's the easiest way to understand my changes in ec366b5

  • Tom Aarsen

@pesuchin
Copy link
Contributor Author

@tomaarsen
Hello!
Thank you for your suggested revisions.
I think it’s a great idea.
The implementation has become much simpler.
Thank you for adding a simple test as well.

@tomaarsen tomaarsen merged commit a542b0a into UKPLab:master Nov 27, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error when trying to load Adapter model of revision in repository on HF
2 participants