[None][perf] Move greedy stop checks to host - #15920
Conversation
|
/bot run --disable-fail-fast |
📝 WalkthroughWalkthroughThis PR adds an optional host-side stop-criteria evaluation path for single-token, single-beam fast-greedy sampling in TorchSampler. A new ChangesHost stop criteria for fast-greedy sampling
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant SampleAsync as sample_async
participant ProcessRequests as _process_requests
participant SampleState as SampleStateTorch
participant UpdateRequests as update_requests
participant StopCriteria as _handle_stop_criteria
SampleAsync->>ProcessRequests: invoke sampling
ProcessRequests->>ProcessRequests: compute use_host_stop_criteria
ProcessRequests-->>SampleAsync: return tokens, use_host_stop_criteria
SampleAsync->>SampleState: set use_host_stop_criteria flag
SampleAsync->>UpdateRequests: pass sample state
UpdateRequests->>UpdateRequests: check state.use_host_stop_criteria
alt host stop criteria enabled
UpdateRequests->>UpdateRequests: add sampled token to request
UpdateRequests->>StopCriteria: evaluate stop criteria on host
else disabled
UpdateRequests->>UpdateRequests: run device finish-reasons + draft-token processing
end
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
PR_Github #57453 [ run ] triggered by Bot. Commit: |
|
PR_Github #57453 [ run ] completed with state
|
Signed-off-by: Mingyang Hao <200044211+mingyangHao@users.noreply.github.com>
cdbd8a3 to
60e8d0e
Compare
|
/bot run --disable-fail-fast |
|
PR_Github #57647 [ run ] triggered by Bot. Commit: |
|
PR_Github #57647 [ run ] completed with state |
Signed-off-by: Mingyang Hao <200044211+mingyangHao@users.noreply.github.com> Co-authored-by: Mingyang Hao <200044211+mingyangHao@users.noreply.github.com>
Remove the PP sampler waivers introduced by NVIDIA#16103, NVIDIA#16105, NVIDIA#16127, and NVIDIA#16146. Also remove overlapping PP and gpu2 waivers from NVIDIA#16109 so the restored pre-NVIDIA#15920 sampler path is exercised in CI. Signed-off-by: Mingyang Hao <200044211+mingyangHao@users.noreply.github.com>
This reverts commit 4f1c3cd. Signed-off-by: Mingyang Hao <200044211+mingyangHao@users.noreply.github.com>
Remove the PP sampler waivers introduced by NVIDIA#16103, NVIDIA#16105, NVIDIA#16127, and NVIDIA#16146. Also remove overlapping PP and gpu2 waivers from NVIDIA#16109, plus the PP2 waiver added by NVIDIA#16169, so the restored pre-NVIDIA#15920 sampler path is exercised in CI. Signed-off-by: Mingyang Hao <200044211+mingyangHao@users.noreply.github.com>
Reland the host-side greedy stop-criteria fast path from NVIDIA#15920 after its temporary revert in NVIDIA#16163. Signed-off-by: Mingyang Hao <200044211+mingyangHao@users.noreply.github.com>
Remove the PP sampler waivers introduced by NVIDIA#16103, NVIDIA#16105, NVIDIA#16127, and NVIDIA#16146. Also remove overlapping PP and gpu2 waivers from NVIDIA#16109, plus the PP2 waiver added by NVIDIA#16169, so the restored pre-NVIDIA#15920 sampler path is exercised in CI. Signed-off-by: Mingyang Hao <200044211+mingyangHao@users.noreply.github.com>
PR NVIDIA#15920 introduced a fast path in TorchSampler that skips writing finish_reasons to host when all requests are greedy, single-beam, and have no stop words. In pipeline-parallel mode with disable_overlap_scheduler=False, only the last PP rank runs sample_async and knows the fast path was taken; non-last PP ranks construct SampleState in _forward_step_inter_pp with the default use_host_stop_criteria=False. The ring-broadcast payload sent only sample_state.host, so non-last ranks ended up with finish_reasons=None but use_host_stop_criteria=False, falling through to process_draft_tokens -> finish_if_reason which indexed an empty list and raised IndexError. Include use_host_stop_criteria in the PP ring-broadcast payload so all ranks agree on which update_requests branch to take. Guard with getattr/hasattr so SampleStateTRTLLM (which lacks this field) is unaffected. Remove the waiver for the affected test. Signed-off-by: trtllm-agent <296075020+trtllm-agent@users.noreply.github.com>
PR NVIDIA#15920 introduced a fast path in TorchSampler that skips writing finish_reasons to host when all requests are greedy, single-beam, and have no stop words. In pipeline-parallel mode with disable_overlap_scheduler=False, only the last PP rank runs sample_async and knows the fast path was taken; non-last PP ranks construct SampleState in _forward_step_inter_pp with the default use_host_stop_criteria=False. The ring-broadcast payload sent only sample_state.host, so non-last ranks ended up with finish_reasons=None but use_host_stop_criteria=False, falling through to process_draft_tokens -> finish_if_reason which indexed an empty list and raised IndexError. Include use_host_stop_criteria in the PP ring-broadcast payload so all ranks agree on which update_requests branch to take. Guard with getattr/hasattr so SampleStateTRTLLM (which lacks this field) is unaffected. Remove the waiver for the affected test. Signed-off-by: trtllm-agent <296075020+trtllm-agent@users.noreply.github.com>
PR NVIDIA#15920 introduced a fast path in TorchSampler that skips writing finish_reasons to host when all requests are greedy, single-beam, and have no stop words. In pipeline-parallel mode with disable_overlap_scheduler=False, only the last PP rank runs sample_async and knows the fast path was taken; non-last PP ranks construct SampleState in _forward_step_inter_pp with the default use_host_stop_criteria=False. The ring-broadcast payload sent only sample_state.host, so non-last ranks ended up with finish_reasons=None but use_host_stop_criteria=False, falling through to process_draft_tokens -> finish_if_reason which indexed an empty list and raised IndexError. Include use_host_stop_criteria in the PP ring-broadcast payload so all ranks agree on which update_requests branch to take. Guard with getattr/hasattr so SampleStateTRTLLM (which lacks this field) is unaffected. Remove the waiver for the affected test. Signed-off-by: trtllm-agent <296075020+trtllm-agent@users.noreply.github.com>
The TorchSampler IndexError described in nvbug 6435097 was introduced by PR NVIDIA#15920 ("Move greedy stop checks to host") and cleanly reverted by PR NVIDIA#16163 (commit 9a8ec05). PR NVIDIA#16163 is already present at HEAD, so the underlying code bug is fixed. The waiver at nvbugs/6427411 for test_ctx_pp_gen_tp_asymmetric[GSM8K-gen_tp=1-ctx_pp=2] was added by PRs NVIDIA#16103 / NVIDIA#16105 / NVIDIA#16127 before the revert landed and was not cleaned up afterward. Verified locally on this HEAD (e523b43): pytest tests/integration/defs/accuracy/test_disaggregated_serving.py::\ TestLlama3_1_8BInstruct::test_ctx_pp_gen_tp_asymmetric[GSM8K-gen_tp=1-ctx_pp=2] -> PASSED, GSM8K accuracy = 73.465 (threshold 70.997). Signed-off-by: trtllm-agent <296075020+trtllm-agent@users.noreply.github.com>
The TorchSampler IndexError described in nvbug 6435097 was introduced by PR NVIDIA#15920 ("Move greedy stop checks to host") and cleanly reverted by PR NVIDIA#16163 (commit 9a8ec05). PR NVIDIA#16163 is already present at HEAD, so the underlying code bug is fixed. The waiver at nvbugs/6427411 for test_ctx_pp_gen_tp_asymmetric[GSM8K-gen_tp=1-ctx_pp=2] was added by PRs NVIDIA#16103 / NVIDIA#16105 / NVIDIA#16127 before the revert landed and was not cleaned up afterward. Verified locally on this HEAD (e523b43): pytest tests/integration/defs/accuracy/test_disaggregated_serving.py::\ TestLlama3_1_8BInstruct::test_ctx_pp_gen_tp_asymmetric[GSM8K-gen_tp=1-ctx_pp=2] -> PASSED, GSM8K accuracy = 73.465 (threshold 70.997). Signed-off-by: trtllm-agent <296075020+trtllm-agent@users.noreply.github.com>
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Description
A focused benchmark on NVIDIA B300 with 64 requests, using the actual TorchSampler backend, showed:
Test Coverage
PR Checklist
Please review the following before submitting your PR:
PR description clearly explains what and why. If using CodeRabbit's summary, please make sure it makes sense.
PR Follows TRT-LLM CODING GUIDELINES to the best of your knowledge.
Test cases are provided for new code paths (see test instructions)
If PR introduces API changes, an appropriate PR label is added - either
api-compatibleorapi-breaking. Forapi-breaking, includeBREAKINGin the PR title.Any new dependencies have been scanned for license and vulnerabilities
CODEOWNERS updated if ownership changes
Documentation updated as needed
Update tava architecture diagram if there is a significant design change in PR.
The reviewers assigned automatically/manually are appropriate for the PR.
Please check this after reviewing the above items as appropriate for this PR.
GitHub Bot Help
To see a list of available CI bot commands, please comment
/bot help.