-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Base Model Revision #1567
Comments
Thanks for this suggestion, it sounds good. A PR would be great, but no need to rush it. |
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. |
@mnoukhov are you still interested in providing a PR? |
I looked into it and found that there's already actually a Line 235 in 56773b9
It's just not being used when calling Line 104 in 56773b9
I've opened the PR #1658 but in order to do unit tests, I need a model with a base model with revision. If you don't mind this not being on |
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. |
not stale |
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. |
resolved by #1658 |
Feature request
Currently, the
adapter_config
only stores thebase_model_name_or_path
but not therevision
. This means that if aPeftModel
is trained on top of a model with arevision
then loadingPeftModel.from_pretrained
will load themain
revision of the base model, not the correct revision.We should add a
base_model_revision
to theadapter_config
so that we can correctly load the base modelMotivation
This will cause a bug if using
PeftModel.from_pretrained
when the base model has a revision.Your contribution
I can try to submit a PR if I have time in 2-3 weeks
The text was updated successfully, but these errors were encountered: