-
Notifications
You must be signed in to change notification settings - Fork 6k
[Feat] Enable State Dict For Textual Inversion Loader #3439
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
[Feat] Enable State Dict For Textual Inversion Loader #3439
Conversation
The documentation is not available anymore as the PR was closed or merged. |
Hey @ghunkins, What exactly is the use case to allow for this? Would be great to better understand why we need it :-) |
This addition simplifies the loading of a textual inversion if you already have the textual inversion in memory. Currently, you have to save the textual inversion to disk and then pass in the filename. This addition removes that intermediary step. Use cases this simplifies:
Additionally, this also gives this loader feature parity with the LoraLoaderMixin where this is already enabled! |
Tried to re-run the tests to get CI to pass, but I am guessing #3449 is the fix for that. Requesting review before this gets lost, thanks @patrickvonplaten @patil-suraj @williamberman! 🙏 |
Hey @ghunkins, Generally, this PR works for me! Could you maybe also add a quick test to this test function: diffusers/tests/pipelines/test_pipelines.py Line 494 in 6dd3871
|
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.
Changes look good for me. Like @patrickvonplaten mentioned, would be nice to add some tests to ensure that the changes are robust.
I'd also like a review from @pdoane since they helped us with batch loading of text inversion parameters (#3193).
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 good!
@ghunkins let me know if you need more clarification on how to add a test :-) |
@patrickvonplaten Great call, tests are added! Let me know if this works. |
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.
What a comprehensive coverage of tests! Love'em, just love'em!
Thanks so much!
Great, thanks @sayakpaul! So much easier when great tests are already there to build off of and benchmark against 🚀 Let me know if there's anything else I can do to get this across the finish line. Looks like only contributors with |
Very cool PR, good job! @ghunkins |
* enable state dict for textual inversion loader * Empty-Commit | restart CI * Empty-Commit | restart CI * Empty-Commit | restart CI * Empty-Commit | restart CI * add tests * fix tests * fix tests * fix tests --------- Co-authored-by: Patrick von Platen <[email protected]>
* enable state dict for textual inversion loader * Empty-Commit | restart CI * Empty-Commit | restart CI * Empty-Commit | restart CI * Empty-Commit | restart CI * add tests * fix tests * fix tests * fix tests --------- Co-authored-by: Patrick von Platen <[email protected]>
Enable passing a pre-loaded state dict via the
TextualInversionLoaderMixin
.Benchmarked the implementation of
LoraLoaderMixin
.