Make lora_alpha a float - #10004
Open
sliedes wants to merge 1 commit into
Open
Conversation
There's nothing fundamentally integer or continuous about `lora_alpha`. While artificially restricting to integers may not have a huge impact in typical (human) usage, needlessly discretized parameters can be tricky for hyperparameter search using cleverer-than-grid-search methods (e.g. optuna, Ax). Now, the `peft` dependency admittedly does have type hints expecting `int`. It seems to deal with floats fine. `peft` upstream documents in their CONTRIBUTING.md a dislike for typing-only fixes. I have nevertheless raised this point, offering to create a PR that changes the types, here: huggingface/peft#3615
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
NOTE: This is something I've been using locally, that I think makes sense for swift and that works for me; but I also think you might reasonably want to see if PEFT upstream chimes in in the issue I link below.
PR type
PR information
There's nothing fundamentally integer or continuous about
lora_alpha. While artificially restricting to integers may not have a huge impact in typical (human) usage, needlessly discretized parameters can be tricky for hyperparameter search using cleverer-than-grid-search methods (e.g. optuna, Ax).Now, the
peftdependency admittedly does have type hints expectingint. It seems to deal with floats fine.peftupstream documents in their CONTRIBUTING.md a dislike for typing-only fixes. I have nevertheless raised this point, offering to create a PR that changes the types, here: huggingface/peft#3615