-
Notifications
You must be signed in to change notification settings - Fork 35
feat: Log probabilities support #221
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Rui Vieira <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ruivieira I added some in-place comments
In addition, please add logprobs support for requests with streaming=true (sendStreamingResponse), please take into consideration comments in PR #215 relevant to streaming (don't pass/use request in streaming.go).
Please add tests for the simulator with logprobs:
completions and chat completions requests with logprobs with and without streaming, and check the response
| @@ -0,0 +1,150 @@ | |||
| /* | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please move this file to common
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implemented in e98ded1
Since there are some cross-package imports, I've opted to call the types from common (common.ChatLogprobs) into openaiserverapi rather than the other way around. Please let me know if you prefer the other way around, or another refactor.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The dependency between openaiserverapi and common was in the wrong direction. Ira fixed this in PR #224
Now it will be possible to move the logprobs_utils to common.
feat: Add streaming support chore: Add probability checks refactor: Use GetLogprobs unified interface refactor: Move to common package. Add decreasing probability common method. refactor: Use logprobData struct in createCompletionResponse Signed-off-by: Rui Vieira <[email protected]>
Signed-off-by: Rui Vieira <[email protected]>
This PR implements log probabilities support in llm-d-inference-sim enabling compatibility with evaluation frameworks and other API clients that require token-level probability.
API Coverage
/v1/completions: logprobs parameter withTextLogprobsresponse structure/v1/chat/completions: logprobs boolean + top_logprobs integer withChatLogprobsresponse structureRefer to #213
Replaces #215