Not all stop sequences are created equal #534
Closed
Conversation
This file contains 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
zhuohan123
requested changes
Jul 25, 2023
There was a problem hiding this comment.
Thank you for your contribution! Can you fix the formatting error with format.sh? In addition, does this change lead to an O(N) string comparison at every iteration, which leads to O(N^2) general complexity? Will this affect the performance of long sequences? Can we somehow just compare stop_str with the newly generated token, instead of the whole sequence?
|
I believe #1724 is a dupe of this one. Would like to see this merged though. |
|
@zacharyblank is this PR still necessary? If yes, do you still plan to get it merged? |
|
#3672 is a more complete fix for this. |
|
I'll close this in favour of yours @njhill |
pi314ever
pushed a commit
to pi314ever/vllm
that referenced
this pull request
Nov 23, 2024
…on (vllm-project#534) This PR is a follow-up to [https://github.com/HabanaAI/vllm-hpu-extension/pull/40](https://github.com/HabanaAI/vllm-hpu-extension/pull/40). It removes all bucketing logic that was moved to vllm-hpu-extension
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes some stop sequences not being matched. When generating and decoding tokens, sometimes a single token will generate the stop sequence plus additional characters. This caused
if seq.output_text.endswith(stop_str):not to behave as expected.For example. If a stop sequence is defined as
",and the model generates","as a single token, as is the case withEleutherAI/gpt-neox-20b, then the stop sequence will not be detected and generation will not stop.This is a small PR that, instead of checking only the end of the generated sequence, checks the entire sequence for the stop sequence