Skip to content

Commit

Permalink
Merge pull request #166 from rasbt/update-lora-init
Browse files Browse the repository at this point in the history
Update lora init
  • Loading branch information
rasbt committed May 20, 2024
2 parents a8a2801 + f3a2e93 commit 451a629
Show file tree
Hide file tree
Showing 3 changed files with 186 additions and 103 deletions.

3 comments on commit 451a629

@d-kleine
Copy link
Contributor

@d-kleine d-kleine commented on 451a629 May 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rasbt Maybe you could also add a check for rank=0 like here:
https://github.com/microsoft/LoRA/blob/4c0333854cb905966f8cc4e9a74068c1e507c7b7/loralib/layers.py#L46C1-L53C32

I think it also can be useful to add a check for alpha=0, because - no matter of the rank - this would nullify the update, resulting in no effective adaptation of the model.

Might be also relevant for Appendix E

@rasbt
Copy link
Owner Author

@rasbt rasbt commented on 451a629 May 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@d-kleine Thanks for the suggestion, but I'd say it doesn't really need additional code to check for that.
If you set the rank to 0, there will already be a PyTorch warning: UserWarning: Initializing zero-element tensors is a no-op. And if you set alpha to 0, that's kind of similar to setting the learning rate to 0, which PyTorch also allows you to do, even though it's not training the model then. But thanks for suggesting!

@d-kleine
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, makes sense - thank you! 👍🏻

Please sign in to comment.