Skip to content

Conversation

@kthui
Copy link
Contributor

@kthui kthui commented Dec 3, 2024

What does the PR do?

Add log probabilities and number of input tokens to additional outputs.

Checklist

  • PR title reflects the change and is of format <commit_type>: <Title>
  • Changes are described in the pull request.
  • Related issues are referenced.
  • Populated github labels field
  • Added test plan and verified test passes.
  • Verified that the PR passes existing CI.
  • Verified copyright is correct on all changed files.
  • Added succinct git squash message before merging ref.
  • All template sections are filled out.
  • Optional: Additional screenshots for behavior/output changes with before/after.

Commit Type:

Check the conventional commit type
box here and add the label to the github PR.

  • build
  • ci
  • docs
  • feat
  • fix
  • perf
  • refactor
  • revert
  • style
  • test

Related PRs:

N/A

Where should the reviewer start?

Start with the documentation, and then move to the implementation and finally the tests.

Test plan:

New tests for the new additional outputs will be added.

  • CI Pipeline ID: 21025199

Caveats:

N/A

Background

N/A

Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)

N/A

@kthui kthui force-pushed the jacky-vllm-logprobs branch from 2459e0a to 17ae25c Compare December 3, 2024 03:25
@kthui kthui added the PR: feat A new feature label Dec 3, 2024
@kthui kthui marked this pull request as ready for review December 3, 2024 21:25
@kthui
Copy link
Contributor Author

kthui commented Dec 3, 2024

For example:

input parameter logprobs = 2

text_output: the
logprobs:

[{
    '5': {'logprob': -1.0246853828430176, 'rank': 1, 'decoded_token': ' the'},
    '10': {'logprob': -2.2121853828430176, 'rank': 2, 'decoded_token': ' a'}
}]

text_output: term
logprobs:

[{
    '1385': {'logprob': -3.2646501064300537, 'rank': 1, 'decoded_token': ' term'},
    '78': {'logprob': -3.7529313564300537, 'rank': 2, 'decoded_token': ' first'}
}]

Copy link
Contributor

@rmccorm4 rmccorm4 left a comment

Choose a reason for hiding this comment

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

Minor doc comments - LGTM otherwise

Co-authored-by: Ryan McCormick <[email protected]>
@kthui kthui requested a review from rmccorm4 December 4, 2024 00:22
rmccorm4
rmccorm4 previously approved these changes Dec 4, 2024
Copy link
Contributor

@rmccorm4 rmccorm4 left a comment

Choose a reason for hiding this comment

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

@oandreeva-nv for second opinion

Copy link
Contributor

@oandreeva-nv oandreeva-nv left a comment

Choose a reason for hiding this comment

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

LGTM! :shipit:

@kthui kthui merged commit 0b9c8e2 into main Dec 5, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR: feat A new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants