-
Notifications
You must be signed in to change notification settings - Fork 571
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
[Inference] Support stop
parameter in text-generation
instead of stop_sequences
#2473
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.
Makes sense! cc @gante @ArthurZucker maybe at some point we'll want to align stop_sequences
to stop
if that's what's used everywhere else
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!
@@ -1655,7 +1655,8 @@ def text_generation( # type: ignore | |||
repetition_penalty: Optional[float] = None, | |||
return_full_text: Optional[bool] = False, # Manual default value | |||
seed: Optional[int] = None, | |||
stop_sequences: Optional[List[str]] = None, # Same as `stop` | |||
stop: Optional[List[str]] = None, | |||
stop_sequences: Optional[List[str]] = None, # Deprecated, use `stop` instead |
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.
Note that transformers
now uses stop_strings
but as you mention, transformers
will be deployed with TGI so not big deal
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 this will be an issue with IE deployment if not using TGI right?
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.
Yes and no, it's harder than that as you found out (private gdoc)
Thanks both for the reviews! |
Fix #2471 cc @sadra-barikbin.
This PR deprecates
stop_sequences
in favor of thestop
parameter for the text_generation task.Context: in both TGI and the
text_generation
specs thestop
parameter is provide stop tokens to the model. However historicallytransformers
was using thestop_sequences
parameter which had been propagated to Inference API andInferenceClient
. Since we are now TGI-first (i.e. eventransformers
models are served with TGI), let's just exposestop
.