Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge pull request #166 from rasbt/update-lora-init
Update lora init
- Loading branch information
Update lora init
451a629There 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.
@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
451a629There 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.
@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!451a629There 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.
Alright, makes sense - thank you! 👍🏻