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

feat: Extend response parameters support to BLS in python backend #395

Merged
merged 9 commits into from
Feb 6, 2025

Conversation

ziqif-nv
Copy link
Contributor

@ziqif-nv ziqif-nv commented Jan 30, 2025

What does the PR do?

Add support for setting response parameters in regular and decoupled Python backend model response(s), when using BLS.

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:

triton-inference-server/server#7987

Where should the reviewer start?

Start with the test cases on the related server PR linked above to learn the context and usage. Then the major changes in this PR are only inside request_executor.cc

Test plan:

New tests are added to the related server PR.

  • CI Pipeline ID: 23548252

Caveats:

N/A

Background

N/A

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

https://jirasw.nvidia.com/browse/DLIS-7520

@ziqif-nv ziqif-nv requested a review from kthui January 30, 2025 20:54
Copy link
Contributor

@kthui kthui left a comment

Choose a reason for hiding this comment

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

A minor readme comment otherwise LGTM. Nice work on your first feature PR!

README.md Outdated Show resolved Hide resolved
Co-authored-by: Jacky <[email protected]>
@ziqif-nv
Copy link
Contributor Author

ziqif-nv commented Feb 5, 2025

A minor readme comment otherwise LGTM. Nice work on your first feature PR!

@kthui Thank you so much for sharing context and knowledge on this PR and beyond! Cannot do it w/o them.

@ziqif-nv ziqif-nv closed this Feb 5, 2025
@ziqif-nv ziqif-nv reopened this Feb 5, 2025
@kthui kthui requested a review from krishung5 February 5, 2025 01:12
Copy link
Contributor

@krishung5 krishung5 left a comment

Choose a reason for hiding this comment

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

Lgtm as well, nice work 🚀

@ziqif-nv ziqif-nv merged commit f61d423 into main Feb 6, 2025
5 checks passed
@ziqif-nv ziqif-nv deleted the ziqif-bls-resp-params branch February 6, 2025 22:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: feat A new feature
Development

Successfully merging this pull request may close these issues.

3 participants