Skip to content

Fix the penultimate token sometimes being lost with SSE streaming#1031

Merged
LostRuins merged 4 commits intoLostRuins:concedo_experimentalfrom
pi6am:fix/penultimate-sse
Jul 29, 2024
Merged

Fix the penultimate token sometimes being lost with SSE streaming#1031
LostRuins merged 4 commits intoLostRuins:concedo_experimentalfrom
pi6am:fix/penultimate-sse

Conversation

@pi6am
Copy link
Copy Markdown

@pi6am pi6am commented Jul 29, 2024

The token immediately before an eot token was lost when SSE streaming was enabled if that token was contained entirely within a stop sequence. As an example of when this could happen, consider this prompt:

Type the phrase 'pleas' once.

In a Llama 3-derived model, 'pleas' tokenizes as 'ple' 'as'. The token 'as' is contained within this instruct mode stop sequence: <|eot_id|><|start_header_id|>assistant<|end_header_id|> due to the word 'assistant'. Since string_contains_sequence_substring returns True for 'as', this token is added to tokenReserve instead of being streamed immediately. If the '<|eot_id|>' token was generated next, the text in tokenReserve would be discarded.

pi6am added 4 commits July 9, 2024 00:10
The token immediately before an eot token was lost when SSE streaming
was enabled if that token was contained entirely within a stop sequence.
As an example of when this could happen, consider this prompt:
  Type the phrase 'pleas' once.
In a Llama 3-derived model, 'pleas' tokenizes as 'ple' 'as'. The token
'as' is contained within this instruct mode stop sequence:
  <|eot_id|><|start_header_id|>assistant<|end_header_id|>
due to the word 'assistant'. Since `string_contains_sequence_substring`
returns True for 'as', this token is added to `tokenReserve` instead of
being streamed immediately. If the '<|eot_id|>' token was generated
next, the text in `tokenReserve` would be discarded.
@LostRuins LostRuins added bug Something isn't working needs review needs review labels Jul 29, 2024
Copy link
Copy Markdown
Owner

@LostRuins LostRuins left a comment

Choose a reason for hiding this comment

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

thanks

@LostRuins LostRuins changed the base branch from concedo to concedo_experimental July 29, 2024 12:16
@LostRuins LostRuins merged commit 26f1df5 into LostRuins:concedo_experimental Jul 29, 2024
@pi6am pi6am deleted the fix/penultimate-sse branch July 29, 2024 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working needs review needs review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants