Skip to content

[feat]: support logit_bias #5354

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 16 commits into from
Jul 25, 2025
Merged

[feat]: support logit_bias #5354

merged 16 commits into from
Jul 25, 2025

Conversation

xq25478
Copy link
Contributor

@xq25478 xq25478 commented Jun 19, 2025 •
edited by coderabbitai bot
Loading

feat(openai protocol):support logitbias

Summary by CodeRabbit

  • New Features
    • Added support for the logit_bias parameter in both chat and completion APIs, allowing users to influence token generation by biasing specific tokens.
  • Bug Fixes
    • Removed previous restriction that disallowed the use of logit_bias in chat completions.
  • Tests
    • Introduced new asynchronous tests to verify correct handling and error reporting for valid and invalid logit_bias inputs in chat and completion APIs.
    • Added a timeout constraint to an end-to-end chat test.

@xq25478 xq25478 force-pushed the support_logitbias branch from 0268d72 to 147fb84 Compare June 19, 2025 06:42
@poweiw poweiw added the Community want to contribute PRs initiated from Community label Jun 24, 2025
@LinPoly LinPoly self-requested a review July 1, 2025 05:48
@LinPoly LinPoly changed the title feat(openai protocol):support logitbias [feat]: support logit_bias Jul 3, 2025
@LinPoly LinPoly requested a review from Copilot July 3, 2025 07:22
Copilot
Copilot AI reviewed Jul 3, 2025
View reviewed changes
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

A concise description of the purpose of the PR, followed by summarized bullets of changes

  • Add support for logit_bias by integrating a new logits processor into sampling parameters
  • Remove the old validator blocking logit_bias
  • Define LogitBiasLogitsProcessor to apply per-token biases at generation time

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
tensorrt_llm/serve/openai_protocol.py Integrated logit_bias into both completion and chat sampling
tensorrt_llm/sampling_params.py Added LogitBiasLogitsProcessor class and updated imports to include Dict

LinPoly
LinPoly reviewed Jul 3, 2025
View reviewed changes
Copy link
Collaborator

@LinPoly LinPoly left a comment

Choose a reason for hiding this comment

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

Thanks for contribution! I leave a few comment about LogitBiasLogitsProcessor's implementation, we also need to add tests, vLLM has tests for chat and for completion, these could be your reference. Ping me if you need any help.

@xq25478 xq25478 force-pushed the support_logitbias branch from 7196cd0 to 5506bea Compare July 3, 2025 09:15
@LinPoly
Copy link
Collaborator

LinPoly commented Jul 4, 2025 •
edited
Loading

@xq25478 Can you please sign off your commits following the steps here. And I think it is acceptable to put the logit processor implementation in sampling_params.py for now, I'll loop in other guys for more comments if necessary.

@LinPoly LinPoly requested a review from netanel-haber July 4, 2025 07:40
@LinPoly
Copy link
Collaborator

LinPoly commented Jul 4, 2025

Add @netanel-haber as Nave suggested.

@xq25478 xq25478 force-pushed the support_logitbias branch 3 times, most recently from 2fd2d0e to f3c7a0b Compare July 7, 2025 07:47
@xq25478 xq25478 closed this Jul 7, 2025
@xq25478 xq25478 reopened this Jul 7, 2025
@xq25478
Copy link
Contributor Author

xq25478 commented Jul 7, 2025

Thanks for contribution! I leave a few comment about LogitBiasLogitsProcessor's implementation, we also need to add tests, vLLM has tests for chat and for completion, these could be your reference. Ping me if you need any help.

test code has been added.

@xq25478 xq25478 force-pushed the support_logitbias branch from f3c7a0b to 9a4f0b9 Compare July 7, 2025 07:51
netanel-haber
netanel-haber reviewed Jul 9, 2025
View reviewed changes
netanel-haber
netanel-haber reviewed Jul 9, 2025
View reviewed changes
netanel-haber
netanel-haber reviewed Jul 9, 2025
View reviewed changes
@xq25478 xq25478 force-pushed the support_logitbias branch 2 times, most recently from 7e4e5dd to 533f5b6 Compare July 10, 2025 08:06
LinPoly
LinPoly reviewed Jul 10, 2025
View reviewed changes
@xq25478 xq25478 force-pushed the support_logitbias branch from 775735c to b5a07f4 Compare July 10, 2025 09:49
@LinPoly
Copy link
Collaborator

LinPoly commented Jul 10, 2025

/bot run

@tensorrt-cicd
Copy link
Collaborator

PR_Github #11549 [ run ] triggered by Bot

@tensorrt-cicd
Copy link
Collaborator

PR_Github #11549 [ run ] completed with state FAILURE
/LLM/main/L0_MergeRequest_PR pipeline #8552 completed with status: 'FAILURE'

@venkywonka
Copy link
Collaborator

@xq25478 would you mind rebasing this (looks it can't be auto-rebased with main). Thanks!

@venkywonka
Copy link
Collaborator

/bot run

@tensorrt-cicd
Copy link
Collaborator

PR_Github #12710 [ run ] completed with state SUCCESS
/LLM/main/L0_MergeRequest_PR pipeline #9457 completed with status: 'FAILURE'

@venkywonka
Copy link
Collaborator

/bot run

@tensorrt-cicd
Copy link
Collaborator

PR_Github #12761 [ run ] triggered by Bot

@tensorrt-cicd
Copy link
Collaborator

PR_Github #12761 [ run ] completed with state SUCCESS
/LLM/main/L0_MergeRequest_PR pipeline #9503 completed with status: 'FAILURE'

@venkywonka
Copy link
Collaborator

@xq25478 , there was a tot breakage that was fixed recently by #6309 - could rebase again and push? thanks!

@LinPoly
Copy link
Collaborator

LinPoly commented Jul 24, 2025

/bot run

@tensorrt-cicd
Copy link
Collaborator

PR_Github #12817 [ run ] triggered by Bot

@tensorrt-cicd
Copy link
Collaborator

PR_Github #12817 [ run ] completed with state FAILURE
/LLM/main/L0_MergeRequest_PR pipeline #9551 completed with status: 'FAILURE'

@LinPoly
Copy link
Collaborator

LinPoly commented Jul 24, 2025

/bot run

@tensorrt-cicd
Copy link
Collaborator

PR_Github #12834 [ run ] triggered by Bot

@tensorrt-cicd
Copy link
Collaborator

PR_Github #12834 [ run ] completed with state SUCCESS
/LLM/main/L0_MergeRequest_PR pipeline #9567 completed with status: 'FAILURE'

@venkywonka
Copy link
Collaborator

CleanShot 2025-07-24 at 14 22 42

@xq25478
seeing this again, apologies for this repetition, but could you rebase agian? thanks!

@venkywonka
Copy link
Collaborator

/bot run

@tensorrt-cicd
Copy link
Collaborator

PR_Github #12898 [ run ] triggered by Bot

@tensorrt-cicd
Copy link
Collaborator

PR_Github #12898 [ run ] completed with state SUCCESS
/LLM/main/L0_MergeRequest_PR pipeline #9616 completed with status: 'SUCCESS'

@xq25478
Copy link
Contributor Author

xq25478 commented Jul 25, 2025

PR_Github #12898 [ run ] completed with state SUCCESS /LLM/main/L0_MergeRequest_PR pipeline #9616 completed with status: 'SUCCESS'

done

@coderabbitai coderabbitai bot requested a review from venkywonka July 25, 2025 04:51
@LinPoly LinPoly enabled auto-merge (squash) July 25, 2025 06:03
@LinPoly
Copy link
Collaborator

LinPoly commented Jul 25, 2025

/bot run

@tensorrt-cicd
Copy link
Collaborator

PR_Github #12961 [ run ] triggered by Bot

@tensorrt-cicd
Copy link
Collaborator

PR_Github #12961 [ run ] completed with state FAILURE
/LLM/main/L0_MergeRequest_PR pipeline #9669 completed with status: 'FAILURE'

@LinPoly
Copy link
Collaborator

LinPoly commented Jul 25, 2025

/bot skip --comment "Previous CI passed"

@tensorrt-cicd
Copy link
Collaborator

PR_Github #12980 [ skip ] triggered by Bot

@tensorrt-cicd
Copy link
Collaborator

PR_Github #12980 [ skip ] completed with state SUCCESS
Skipping testing for commit 8a70c10

@LinPoly LinPoly merged commit a0aecf0 into NVIDIA:main Jul 25, 2025
2 checks passed
NVShreyas pushed a commit to NVShreyas/TensorRT-LLM that referenced this pull request Jul 28, 2025
@xq25478 @venkywonka
@LinPoly @NVShreyas
[feat]: support logit_bias (NVIDIA#5354)
4951867 
Signed-off-by: xq25478 <[email protected]>
Signed-off-by: Venky Ganesh <[email protected]>
Signed-off-by: hexiao.xq <[email protected]>
Co-authored-by: Venky Ganesh <[email protected]>
Co-authored-by: hexiao.xq <[email protected]>
Co-authored-by: Pengyun Lin <[email protected]>
Signed-off-by: Shreyas Misra <[email protected]>
Ransiki pushed a commit to Ransiki/TensorRT-LLM that referenced this pull request Jul 29, 2025
@xq25478 @venkywonka
@LinPoly @Ransiki
[feat]: support logit_bias (NVIDIA#5354)
d0a78d1 
Signed-off-by: xq25478 <[email protected]>
Signed-off-by: Venky Ganesh <[email protected]>
Signed-off-by: hexiao.xq <[email protected]>
Co-authored-by: Venky Ganesh <[email protected]>
Co-authored-by: hexiao.xq <[email protected]>
Co-authored-by: Pengyun Lin <[email protected]>
Signed-off-by: Ransiki Zhang <[email protected]>
lancelly pushed a commit to lancelly/TensorRT-LLM that referenced this pull request Aug 6, 2025
@xq25478 @venkywonka
@LinPoly @lancelly
[feat]: support logit_bias (NVIDIA#5354)
83b316a 
Signed-off-by: xq25478 <[email protected]>
Signed-off-by: Venky Ganesh <[email protected]>
Signed-off-by: hexiao.xq <[email protected]>
Co-authored-by: Venky Ganesh <[email protected]>
Co-authored-by: hexiao.xq <[email protected]>
Co-authored-by: Pengyun Lin <[email protected]>
Signed-off-by: Lanyu Liao <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hchings hchings hchings left review comments

@syuoni syuoni syuoni left review comments

Copilot code review Copilot Copilot left review comments

@netanel-haber netanel-haber netanel-haber approved these changes

@LinPoly LinPoly LinPoly approved these changes

@venkywonka venkywonka Awaiting requested review from venkywonka

Assignees
No one assigned
Labels
Community want to contribute PRs initiated from Community
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@xq25478 @LinPoly @tensorrt-cicd @venkywonka @WilliamTambellini @hchings @syuoni @netanel-haber @poweiw