-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
[utils
] add merge_lora utility function
#227
[utils
] add merge_lora utility function
#227
Conversation
This PR is now ready for review! all tests are passing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @younesbelkada for adding this feature which resolves many issues related to merging the lora params back into the base model without any wrappers on it.✨
Left a couple of comments and suggestions
@@ -233,6 +238,34 @@ def enable_adapter_layers(self): | |||
def disable_adapter_layers(self): | |||
self._set_adapter_layers(enabled=False) | |||
|
|||
def merge_and_unload(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would not work when the model is loaded in 8bit. An assertion error for the same would be helpful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the pointer! added a check for that
Co-authored-by: Sourab Mangrulkar <13534540+pacman100@users.noreply.github.com>
…into add-merge-lora
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Final comments/suggestions
Co-authored-by: Sourab Mangrulkar <13534540+pacman100@users.noreply.github.com>
What does this PR do?
This PR introduces a new utility function
merge_lora
to conveniently merge lora layers into the base model, so that users can use this model as a standalonetransformers
modelAdded also some tests (that are currently failing)
cc @pacman100