-
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
Hotswap allow different alpha scalings and ranks #2177
Merged
BenjaminBossan
merged 29 commits into
huggingface:main
from
BenjaminBossan:hotswap-allow-different-alpha-scalings
Feb 5, 2025
Merged
Changes from all commits
Commits
Show all changes
29 commits
Select commit
Hold shift + click to select a range
355e2ab
[WIP] Allow different alphas for hotswapping
BenjaminBossan 135dfda
Only enable hotswap tests in CI for now
BenjaminBossan 9b7d979
More debugging
BenjaminBossan 3a6d4b5
Merge branch 'main' into hotswap-allow-different-alpha-scalings
BenjaminBossan 088a9d9
More debugging
BenjaminBossan 3da33ba
Take alpha from new config
BenjaminBossan 0bba669
Remove some debug code
BenjaminBossan c73dafc
Merge branch 'main' into hotswap-allow-different-alpha-scalings
BenjaminBossan 552ecac
Compiled hotswapping works w/ different scalings
BenjaminBossan 2dcd6a1
[WIP] Hotswap with different ranks
BenjaminBossan 620d43e
Make hotswapping with different ranks work
BenjaminBossan efaf2b0
More tests, docs, fixes
BenjaminBossan 25d7a77
Make style, fix
BenjaminBossan 88bd814
Merge branch 'main' into hotswap-allow-different-alpha-scalings
BenjaminBossan 2a29370
Remove obsolete test
BenjaminBossan 9b132ec
Merge branch 'main' into hotswap-allow-different-alpha-scalings
BenjaminBossan ddf1bad
Disable test skips for CI (only testing)
BenjaminBossan 3251c6c
Clean up, documentation
BenjaminBossan 9966c10
Reviewer feedback: Refactor _pad_lora_weights
BenjaminBossan 01924ad
Reviewer feedback: Add clarifying comments
BenjaminBossan 66b987e
Fix issue with bias term
BenjaminBossan d32d23f
Clarify docstring
BenjaminBossan 8b7e602
Reviewer feedback:
BenjaminBossan d0a1fc5
Reviewer feedback: add clarifying comments
BenjaminBossan 7772e2e
Reviewer feedback: address model.compile()
BenjaminBossan fe19da5
Revert testing
BenjaminBossan 5bc0dbe
Simplify tests for recompilation
BenjaminBossan a412f7a
Reviewer feedback: Better handling of torch cache
BenjaminBossan 0943519
Change fixture to clean up dynamo cache after test
BenjaminBossan File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
Oops, something went wrong.
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.
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.
Would it make sense to allow a util like
infer_max_rank_from_state_dict()
? Because it can be difficult to know for the users to knowmax_rank
from the get-go. WDYT?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.
I was thinking about how an application would use this in practice for the reason you mentioned. If we break it down like this:
If we had a
infer_max_rank_from_state_dict()
it would be for the 2nd case, which has the mentioned issues, but correct me if I'm wrong. But regardless of that, where would the state dicts come from? Since we have multiple LoRA formats in the diffusion ecosystem and diffusers knows how to deal with them, would that function not be better added to diffusers?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.
Fair enough.