[TRTLLM-14417][fix] Exclude ADP/cuda-graph dummy requests from speculative-decode acceptance stats - #16571
Conversation
…e stats The per-iteration speculative-decoding stats loop in _append_iter_stats counted attention-DP (and cuda-graph padding) dummy generation requests as real drafted requests. On a disagg-GEN worker under attention-DP, an idle rank is padded with a *generation* dummy that carries synthetic py_draft_tokens=[1]*k (resource_manager.add_dummy_requests is_gen branch) and is assigned a spec-sampler accepted count, so it entered the AR/AL aggregate as a mostly-rejected drafted request and depressed the reported acceptance rate/length. The parallel speculation-gate loop already filters is_dummy; mirror that here. Signed-off-by: Xiao Wang <24860335+xwang233@users.noreply.github.com>
📝 WalkthroughWalkthroughSpeculative decoding per-iteration statistics now exclude dummy generation requests from draft-token and accepted-token aggregation. ChangesSpeculative decoding metrics
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
/bot run |
|
PR_Github #60139 [ run ] triggered by Bot. Commit: |
|
PR_Github #60139 [ run ] completed with state
|
|
/bot run --reuse-test |
|
PR_Github #60181 [ run ] triggered by Bot. Commit: |
|
PR_Github #60181 [ run ] completed with state
|
chienchunhung
left a comment
There was a problem hiding this comment.
Thanks for the fix!
Added a comment: we should add an unit test to guard the code change.
…ltering Address review feedback on the specdec AR/AL dummy-request fix: - Add focused unit tests in test_iter_stats_populate.py covering the speculative-decoding aggregation branch of _update_iter_stats. The existing dummy-request tests leave stats.specdec_stats as None, so that branch was previously uncovered. New tests allocate specdec_stats and verify that attention-DP / CUDA-graph dummy generation requests are excluded from draft/accepted token counts and acceptance length. - Collapse the verbose filter comment to a concise one-liner per reviewer suggestion. Signed-off-by: Xiao Wang <24860335+xwang233@users.noreply.github.com>
|
/bot run |
|
PR_Github #60480 [ run ] triggered by Bot. Commit: |
|
PR_Github #60480 [ run ] completed with state
|
|
/bot run |
|
PR_Github #60496 [ run ] triggered by Bot. Commit: |
|
PR_Github #60496 [ run ] completed with state
|
|
/bot run |
|
/bot run |
|
PR_Github #61059 [ run ] triggered by Bot. Commit: |
|
PR_Github #61059 [ run ] completed with state
|
|
/bot run |
|
PR_Github #61182 [ run ] triggered by Bot. Commit: |
|
PR_Github #61182 [ run ] completed with state
|
|
/bot run |
|
PR_Github #61193 [ run ] triggered by Bot. Commit: |
|
PR_Github #61193 [ run ] completed with state
|
|
/bot run --reuse-test |
|
PR_Github #61324 [ run ] triggered by Bot. Commit: |
|
PR_Github #61324 [ run ] completed with state
|
|
/bot run --reuse-test |
|
PR_Github #61337 [ run ] triggered by Bot. Commit: |
|
PR_Github #61337 [ run ] completed with state
|
|
/bot run --reuse-test --disable-fail-fast |
|
PR_Github #61359 [ run ] triggered by Bot. Commit: |
|
PR_Github #61359 [ run ] completed with state
|
|
/bot run --reuse-test --disable-fail-fast |
|
PR_Github #61440 [ run ] triggered by Bot. Commit: |
|
PR_Github #61440 [ run ] completed with state
|
|
/bot run --reuse-test --disable-fail-fast |
|
PR_Github #61618 [ run ] triggered by Bot. Commit: |
|
PR_Github #61618 [ run ] completed with state |
…ative-decode acceptance stats (NVIDIA#16571) Signed-off-by: Xiao Wang <24860335+xwang233@users.noreply.github.com>
…nnahz/dep-1083-port-flashinfer-stable-va-lifecycle-for-native-all-reduce * 'main' of https://github.com/NVIDIA/TensorRT-LLM: (54 commits) [NVIDIA#15673][fix] Enable CUDA core fast path for SM89/SM120/SM121 (NVIDIA#12705) [None][test] Adjust timeout cases in QA perf test (NVIDIA#16894) [https://nvbugs/6157892][fix] Mistral format refactor (NVIDIA#15123) [None][feat] Add kimi_k2/glm_5 grouped routing and fused router to bench_moe (NVIDIA#16830) [https://nvbugs/6501376][fix] Test-only fix — drop the `if hidden_size % 2 != 0: with pytest.raises(...)`… (NVIDIA#16844) [TRTLLM-13642][feat] Add perf sanity tests for Llama-3.1-8B and Gemma-3-1B and verify cache transceiver V2 support (NVIDIA#16355) [https://nvbugs/6433376][fix] Update the Dense test to mirror the MoE sibling — assert `bfloat16` under… (NVIDIA#16203) [None][fix] Resolve NVFP4 mixed-precision base layers for the DSpark draft (NVIDIA#16831) [https://nvbugs/6479324][test] Remove waiver for fixed qwen3_5_4b_fp8_stress disaggregated stress test (NVIDIA#16878) [https://nvbugs/6507109][infra] Split slow DGX B300 attention unit tests (NVIDIA#16838) [None][infra] Waive 21 failed cases for main in post-merge 2862 (NVIDIA#16882) [None][perf] prepare_inputs: avoid O(seq_len) get_tokens(0) marshalling on the host (NVIDIA#16791) [None][perf] Optimize Blackwell fused MHC half-MMA kernel (NVIDIA#16799) [None][infra] Auto-update test durations from OpenSearch (last 7 days) [None][perf] Skip DeepGEMM clean_logits in DSA indexer prefill on custom top-k path (NVIDIA#16789) [None][feat] Support DeepSeek-V4 in layer_wise_benchmarks (NVIDIA#16774) [https://nvbugs/6465993][fix] use attention cache dtype for disaggregated transfer (NVIDIA#16505) [https://nvbugs/6463822][fix] Fix LTX2 CUDA graph test leak issue (NVIDIA#16775) [https://nvbugs/5948435][chore] Unwaive DeepSeekV3Lite test_nvfp4_4gpus CUTLASS ep4 fp8kv on RTXPro6000D (NVIDIA#16621) [TRTLLM-14417][fix] Exclude ADP/cuda-graph dummy requests from speculative-decode acceptance stats (NVIDIA#16571) ... Signed-off-by: Hannah Zhang <hannahz@nvidia.com>
…nnahz/dep-1082-shared-mnnvl-moe-lifecycle * 'main' of https://github.com/NVIDIA/TensorRT-LLM: (142 commits) [NVIDIA#15673][fix] Enable CUDA core fast path for SM89/SM120/SM121 (NVIDIA#12705) [None][test] Adjust timeout cases in QA perf test (NVIDIA#16894) [https://nvbugs/6157892][fix] Mistral format refactor (NVIDIA#15123) [None][feat] Add kimi_k2/glm_5 grouped routing and fused router to bench_moe (NVIDIA#16830) [https://nvbugs/6501376][fix] Test-only fix — drop the `if hidden_size % 2 != 0: with pytest.raises(...)`… (NVIDIA#16844) [TRTLLM-13642][feat] Add perf sanity tests for Llama-3.1-8B and Gemma-3-1B and verify cache transceiver V2 support (NVIDIA#16355) [https://nvbugs/6433376][fix] Update the Dense test to mirror the MoE sibling — assert `bfloat16` under… (NVIDIA#16203) [None][fix] Resolve NVFP4 mixed-precision base layers for the DSpark draft (NVIDIA#16831) [https://nvbugs/6479324][test] Remove waiver for fixed qwen3_5_4b_fp8_stress disaggregated stress test (NVIDIA#16878) [https://nvbugs/6507109][infra] Split slow DGX B300 attention unit tests (NVIDIA#16838) [None][infra] Waive 21 failed cases for main in post-merge 2862 (NVIDIA#16882) [None][perf] prepare_inputs: avoid O(seq_len) get_tokens(0) marshalling on the host (NVIDIA#16791) [None][perf] Optimize Blackwell fused MHC half-MMA kernel (NVIDIA#16799) [None][infra] Auto-update test durations from OpenSearch (last 7 days) [None][perf] Skip DeepGEMM clean_logits in DSA indexer prefill on custom top-k path (NVIDIA#16789) [None][feat] Support DeepSeek-V4 in layer_wise_benchmarks (NVIDIA#16774) [https://nvbugs/6465993][fix] use attention cache dtype for disaggregated transfer (NVIDIA#16505) [https://nvbugs/6463822][fix] Fix LTX2 CUDA graph test leak issue (NVIDIA#16775) [https://nvbugs/5948435][chore] Unwaive DeepSeekV3Lite test_nvfp4_4gpus CUTLASS ep4 fp8kv on RTXPro6000D (NVIDIA#16621) [TRTLLM-14417][fix] Exclude ADP/cuda-graph dummy requests from speculative-decode acceptance stats (NVIDIA#16571) ... Signed-off-by: Hannah Zhang <hannahz@nvidia.com>
Description
The per-iteration speculative-decoding stats loop in
PyExecutor._append_iter_stats(tensorrt_llm/_torch/pyexecutor/py_executor.py) aggregates acceptance rate (AR) and acceptance length (AL) overscheduled_batch.generation_requestswithout filtering out dummy requests.On a disaggregated-generation worker running under attention-DP (and likewise for cuda-graph padding), an idle rank is padded with a generation dummy request. That dummy carries synthetic
py_draft_tokens = [1] * k(theis_genbranch ofresource_manager.add_dummy_requests) and is assigned a spec-sampler accepted count, so it enters the AR/AL aggregate as a mostly-rejected drafted request. The result is a depressed, misleading reported acceptance rate/length — the more idle-rank dummies an iteration accumulates, the lower the reported AR/AL, even though real model behavior is unchanged.The fix mirrors the filter that already exists in the parallel speculation-gate loop in the same file (
if draft_len <= 0 or request.is_dummy: continue): skip any request whoseis_dummyis set before it contributes to the specdec metric.This is a metrics-only change. It touches only the stats-aggregation loop — not scheduling, forward, sampling, or resource management — so model outputs and throughput are unaffected; only the reported
specDecodingStatsare corrected.Test Coverage
request.is_dummy) already applied in the speculation-gate loop of the same method, so no new code path or public API is introduced.PR Checklist