Skip to content
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

[BugFix] Fix chat API continuous usage stats #9357

Merged
merged 1 commit into from
Oct 15, 2024

Conversation

njhill
Copy link
Member

@njhill njhill commented Oct 15, 2024

  • completion_tokens in returned continuous usage stats was not cumulative as it should be.
  • completion_tokens in the final usage stats should be the total for all choices.
  • Includes some related code simplification/de-duplication.

This fixes an omission in the changes I made recently in the delta-streaming optimization #7381.

- completion_tokens in returned continuous usage stats was not cumulative as it should be.
- completion_tokens in the final usage stats should be the total for all choices.
- Includes some related code simplification/deduplication.
@njhill njhill force-pushed the fix-chat-stream-usage branch from fcb0d5e to 558e3a3 Compare October 15, 2024 03:33
@njhill njhill marked this pull request as ready for review October 15, 2024 03:34
@vllm-project vllm-project deleted a comment from github-actions bot Oct 15, 2024
@njhill njhill added the ready ONLY add when PR is ready to merge/full CI is needed label Oct 15, 2024
Copy link
Member

@DarkLight1337 DarkLight1337 left a comment

Choose a reason for hiding this comment

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

Entrypoints tests pass, so this should be good to go!

@simon-mo simon-mo enabled auto-merge (squash) October 15, 2024 06:19
@simon-mo simon-mo disabled auto-merge October 15, 2024 06:19
@simon-mo simon-mo merged commit e9d517f into vllm-project:main Oct 15, 2024
60 of 66 checks passed
@njhill njhill deleted the fix-chat-stream-usage branch October 15, 2024 14:39
njhill added a commit to njhill/vllm that referenced this pull request Oct 17, 2024
Similar to what was done for the chat API in vllm-project#9357. Ensure that the final chunk with usage data contains aggregate counts across all choices.

Also simplify some of the prompt-handling logic in the API implementation.
charlifu pushed a commit to charlifu/vllm that referenced this pull request Oct 23, 2024
vrdn-23 pushed a commit to vrdn-23/vllm that referenced this pull request Oct 23, 2024
Signed-off-by: Vinay Damodaran <vrdn@hey.com>
Alvant pushed a commit to compressa-ai/vllm that referenced this pull request Oct 26, 2024
garg-amit pushed a commit to garg-amit/vllm that referenced this pull request Oct 28, 2024
Signed-off-by: Amit Garg <mitgarg17495@gmail.com>
FerdinandZhong pushed a commit to FerdinandZhong/vllm that referenced this pull request Oct 29, 2024
Signed-off-by: qishuai <ferdinandzhong@gmail.com>
sumitd2 pushed a commit to sumitd2/vllm that referenced this pull request Nov 14, 2024
Signed-off-by: Sumit Dubey <sumit.dubey2@ibm.com>
KuntaiDu pushed a commit to KuntaiDu/vllm that referenced this pull request Nov 20, 2024
mfournioux pushed a commit to mfournioux/vllm that referenced this pull request Nov 20, 2024
Signed-off-by: Maxime Fournioux <55544262+mfournioux@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready ONLY add when PR is ready to merge/full CI is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants