-
Notifications
You must be signed in to change notification settings - Fork 1.3k
finetune_lora upgrades #2086
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: main
Are you sure you want to change the base?
finetune_lora upgrades #2086
Conversation
for more information, see https://pre-commit.ci
…; fixed test_cli and other errors
for more information, see https://pre-commit.ci
…s/litgpt into finetune_lora_upgrade divergent
UPDATE: tested it in a 4xA100 environment, works well for multi-gpu setup |
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.
LGTM, just confused about the legacy staff
Essentially it's the exact lora implementation before this PR. The changes are quite breaking so my rationale is it may be apt to still give users the option to run the previous version of lora.py, hence the name |
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.
Looks great! I think we should add some tests to new functions though, left some comments
Integrated some improvements to finetune_lora based on @lantiga's reference implementation.
Most of the optimizations are there, but I did not include the model registry feature because litgpt does not currently support litmodels and adding litmodels support is a PR in itself IMO.
Still WIP as I need to run some experiments (and potentially having to modify some test cases)