Skip to content

[https://nvbugs/6428087][fix] Extend the PP send payload with the flag (backward-compatible 2/3-tuple… - #16145

Open
trtllm-agent wants to merge 1 commit into
NVIDIA:mainfrom
tensorrt-cicd:repair-bot-bug6428087
Open

[https://nvbugs/6428087][fix] Extend the PP send payload with the flag (backward-compatible 2/3-tuple…#16145
trtllm-agent wants to merge 1 commit into
NVIDIA:mainfrom
tensorrt-cicd:repair-bot-bug6428087

Conversation

@trtllm-agent

@trtllm-agent trtllm-agent commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Root cause: PP send/recv drops SampleStateTorch.use_host_stop_criteria, so non-last PP ranks default to False and call process_draft_tokens with an empty finish_reasons list from the last rank's host fast path.
  • Fix: Extend the PP send payload with the flag (backward-compatible 2/3-tuple handling) so all PP ranks pick the same update branch; remove the waiver.
  • Automated fix generated by repair-bot

Test plan

  • Verify fix on the same GPU type as the original failure
  • Check for regressions in related tests

Links

Summary by CodeRabbit

  • Bug Fixes

    • Improved consistency of generation stopping behavior across pipeline stages, reducing cases where host-based stop criteria could be lost during distributed execution.
  • Tests

    • Updated integration test waivers to reflect a revised supported test configuration.

The greedy host stop-criteria optimization (PR NVIDIA#15920) added a per-batch
flag `use_host_stop_criteria` on `SampleStateTorch` that gates whether
`update_requests` uses the host fast path or calls
`process_draft_tokens`. In the PP execution loop, `SampleStateTorch` is
constructed on non-last PP ranks via `_forward_step_inter_pp` without
setting this flag, and only `sample_state.host` and per-request result
diffs are shipped via ring send/recv from the last PP rank. Consequently,
when the last PP rank determined `use_host_stop_criteria=True` and
therefore skipped writing finish_reasons, earlier PP ranks still saw the
default False and entered `process_draft_tokens`, which then indexed an
empty finish_reasons list and raised
`IndexError: list index out of range` from `finish_if_reason`.

Extend the PP send payload with the flag (backward-compatible: recv
accepts both 2- and 3-tuple payloads, and send only appends the flag when
the sampler exposes the attribute) so that all ranks pick the same
branch. Also remove the associated waiver.

Signed-off-by: trtllm-agent <296075020+trtllm-agent@users.noreply.github.com>
@coderabbitai

coderabbitai Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: f8c8b5cb-ffe8-48d2-b4b2-2f97b8b26e5e

📥 Commits

Reviewing files that changed from the base of the PR and between 5ce293f and c9b9bea.

📒 Files selected for processing (2)
  • tensorrt_llm/_torch/pyexecutor/py_executor.py
  • tests/integration/test_lists/waives.txt
💤 Files with no reviewable changes (1)
  • tests/integration/test_lists/waives.txt

📝 Walkthrough

Walkthrough

The PR extends the PP (pipeline parallelism) ring broadcast for SAMPLE_STATE to propagate a use_host_stop_criteria flag alongside existing host and result-diff data across non-last PP ranks, and updates a test waiver entry for a DeepSeekV3Lite accuracy test with different parameter settings.

Changes

Sample State Broadcast Fix

Layer / File(s) Summary
Propagate use_host_stop_criteria in PP ring broadcast
tensorrt_llm/_torch/pyexecutor/py_executor.py
Send path now includes use_host_stop_criteria (via getattr) in the SAMPLE_STATE payload; receive path unpacks it and assigns it to sample_state.use_host_stop_criteria when present.
Test waiver update
tests/integration/test_lists/waives.txt
Replaced the waived parameter combination for TestDeepSeekV3Lite::test_nvfp4_4gpus with moe_backend=CUTEDSL (tp2pp2), changing fp8kv, attention_dp, cuda_graph, overlap_scheduler, and torch_compile settings.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • NVIDIA/TensorRT-LLM#16103: Both PRs modify tests/integration/test_lists/waives.txt to adjust DeepSeekV3Lite test waiver configurations.
  • NVIDIA/TensorRT-LLM#16127: Directly related to the SAMPLE_STATE use_host_stop_criteria propagation fix, addressing the same regression this PR resolves.

Suggested reviewers: shuyixiong, mingyangHao, brb-nv

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title is specific and matches the main bug fix, including the NVBugs reference and fix type.
Description check ✅ Passed It covers the summary, test plan, and bug link, but it omits the PR checklist section from the template.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants