Skip to content

fix(serve): return the requested number of chat top_logprobs - #5008

Open
MohammadHijjawi97 wants to merge 1 commit into
InternLM:mainfrom
MohammadHijjawi97:fix/chat-top-logprobs-include-sampled
Open

MohammadHijjawi97 wants to merge 1 commit into
InternLM:mainfrom
MohammadHijjawi97:fix/chat-top-logprobs-include-sampled

Conversation

@MohammadHijjawi97

Copy link
Copy Markdown

Motivation

/v1/chat/completions returns the wrong number of top_logprobs candidates.

For each output position, both engines return the model top-k logprobs, plus the sampled token when it falls outside the top-k (PyTorch compute_logprobs, TurboMind _get_logprobs_impl). _create_chat_completion_logprobs always removed the sampled token from top_logprobs, so:

The OpenAI API lists the k most likely tokens, including the sampled one when it is among them. The /generate endpoint (_create_top_logprobs) already does this.

Modification

  • _create_chat_completion_logprobs takes top_logprobs. It sorts candidates by logprob, drops the sampled token only when it is the extra row appended outside the top-k, and caps the list at top_logprobs. The sampled token's own token/bytes/logprob are unchanged.
  • Both the streaming and non-streaming chat paths pass request.top_logprobs or 0.
  • Added tests/test_lmdeploy/serve/openai/chat_completions/test_logprobs.py. It covers the sampled token inside the top-k (k=1 and k=3), the sampled token outside the top-k, and top_logprobs=0.

pytest tests/test_lmdeploy/serve/openai/chat_completions/ passes (51 tests), and pre-commit is clean on the changed files.

BC-breaking (Optional)

No API change. Responses now contain top_logprobs entries as requested; the new parameter defaults to 0.

Checklist

  1. Pre-commit or other linting tools are used to fix the potential lint issues.
  2. The modification is covered by complete unit tests. If not, please add more unit tests to ensure the correctness.
  3. If the modification has a dependency on downstream projects of a newer version, this PR should be tested with all supported versions of downstream projects.
  4. The documentation has been modified accordingly, like docstring or example tutorials.

The engines return the model top-k logprobs plus the sampled token when
it falls outside the top-k. The chat completions builder always dropped
the sampled token from `top_logprobs`, so a request for `top_logprobs=k`
got only k-1 candidates whenever the sampled token was in the top-k, and
an empty list for greedy decoding with `top_logprobs=1`. With
`top_logprobs=0` it could still return a candidate.

Keep the sampled token when it is part of the top-k, drop it only when
it is the extra appended row, and cap the list at `top_logprobs`,
matching the `/generate` endpoint and the OpenAI API.

This branch has not been deployed

No deployments
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.

1 participant