-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
add sentencepiece as a soft dependency #9065
Conversation
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. |
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.
Thanks. Just a single comment.
@@ -12,6 +12,7 @@ | |||
is_note_seq_available, | |||
is_onnx_available, | |||
is_scipy_available, | |||
is_sentencepiece_available, |
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.
Actually sentencepiece
is needed by all the pipelines that need T5. Should we add them here as well?
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.
technically, they can use these pipelines without t5, no? i.e. use pre-computed text embeddings
EulerDiscreteScheduler, | ||
KolorsPipeline, | ||
UNet2DConditionModel, | ||
) | ||
from diffusers.pipelines.kolors import ChatGLMModel, ChatGLMTokenizer |
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 think we already install sentencepiece
for tests. So should be good:
Line 126 in b1f43d7
"sentencepiece>=0.1.91,!=0.1.92", |
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.
yeah, that was the main reason I didn't catch this
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.
really nice! thanks.
* add sentencepiece as soft dependency for kolors * up --------- Co-authored-by: Sayak Paul <[email protected]>
fix #9034