[LoRA] make set_adapters() method more robust. by sayakpaul · Pull Request #9535 · huggingface/diffusers (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation10 Commits7 Checks18 Files changed
Conversation
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
[ Show hidden characters]({{ revealButtonHref }})
What does this PR do?
First caught in #9525 (comment) by @asomoza. But then it turned out to be #9525 (comment).
This PR:
- Makes our
set_adapters()method more robust by catching errors on adapter names that are wrong (and adds a test for it). - Makes our existing test on
set_adapters()more robust.
@cshowley do you wanna give this a try?
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.
Failing tests are completely unrelated.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding this check, I agree it makes sense to prevent typos etc.
I have some comments to potentially improve the code, but no blockers.
| check_if_lora_correctly_set(pipe.text_encoder_2), "Lora not correctly set in text encoder 2" |
|---|
| ) |
| with self.assertRaises(ValueError) as err_context: |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could use self.assertRaisesRegex to directly check the error message.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We just prefer to do a bit more explicitly across the codebase (which is what I have followed here).
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the quick adjustments.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks!
Co-authored-by: YiYi Xu yixu310@gmail.com
This was referenced
Oct 7, 2024
leisuzz pushed a commit to leisuzz/diffusers that referenced this pull request
make set_adapters() method more robust.
remove patch
better and concise code.
Update src/diffusers/loaders/lora_base.py
Co-authored-by: YiYi Xu yixu310@gmail.com
Co-authored-by: YiYi Xu yixu310@gmail.com
sayakpaul added a commit that referenced this pull request
make set_adapters() method more robust.
remove patch
better and concise code.
Update src/diffusers/loaders/lora_base.py
Co-authored-by: YiYi Xu yixu310@gmail.com
Co-authored-by: YiYi Xu yixu310@gmail.com