Skip to content

Conversation

yunfeng-scale
Copy link
Contributor

@yunfeng-scale yunfeng-scale commented Dec 8, 2023

Fixes #1973

tested by running server locally

@simon-mo
Copy link
Collaborator

this looks good! what's the variable name in TGI? How about placing it near the stop parameters and name it like include_stop_in_output or others you would like.

@yunfeng-scale
Copy link
Contributor Author

this looks good! what's the variable name in TGI? How about placing it near the stop parameters and name it like include_stop_in_output or others you would like.

thanks for the review. TGI does not have this variable. it's default behavior is to include stop string in output.

@yunfeng-scale
Copy link
Contributor Author

@WoosukKwon @zhuohan123 would you take a look?

Copy link
Collaborator

@WoosukKwon WoosukKwon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @yunfeng-scale, thanks for submitting the PR! I think this makes sense. A friend of mine actually requested this 😂

Could you resolve the merge conflict and see my comment on the variable name? Thanks.

Copy link
Collaborator

@WoosukKwon WoosukKwon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for the contribution!

@WoosukKwon WoosukKwon merged commit c06170c into vllm-project:main Dec 15, 2023
xjpang pushed a commit to xjpang/vllm that referenced this pull request Dec 18, 2023
hongxiayang pushed a commit to hongxiayang/vllm that referenced this pull request Feb 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Flag to include stop string in output
3 participants