[None][fix] Reserve one-engine speculative tokens before scheduling - #15973
[None][fix] Reserve one-engine speculative tokens before scheduling#15973yizhang-nv wants to merge 4 commits into
Conversation
Signed-off-by: Yi Zhang <187001205+yizhang-nv@users.noreply.github.com>
Unwaive TestDeepSeekV32::test_nvfp4_multi_gpus_piecewise_cuda_graph[mtp3_fp8kv_chunked] (nvbugs/5989920) to re-enable it in CI. Signed-off-by: Yi Zhang <187001205+yizhang-nv@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughPyExecutor's ChangesScheduler Draft Token Placeholder Refactor
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
/bot run --disable-fail-fast |
|
PR_Github #57916 [ run ] triggered by Bot. Commit: |
| if (self.drafter is None | ||
| and getattr(self.model_engine, 'spec_config', None) is None): | ||
| return | ||
|
|
There was a problem hiding this comment.
Should also early return if use_spec_decode is False
|
PR_Github #57916 [ run ] completed with state
|
Signed-off-by: Yi Zhang <187001205+yizhang-nv@users.noreply.github.com>
|
/bot run --disable-fail-fast |
|
PR_Github #58156 [ run ] triggered by Bot. Commit: |
Signed-off-by: Yi Zhang <187001205+yizhang-nv@users.noreply.github.com>
|
/bot run --disable-fail-fast |
|
PR_Github #58158 [ run ] triggered by Bot. Commit: |
|
PR_Github #58156 [ run ] completed with state |
|
PR_Github #58158 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #58362 [ run ] triggered by Bot. Commit: |
|
PR_Github #58362 [ run ] completed with state |
…upstream PR NVIDIA#15973) Squash of upstream PR NVIDIA#15973 (4 commits). One-engine speculative decoders (MTP / DSpark) keep the draft model inside ModelEngine, so PyExecutor.drafter is None; under the overlap scheduler the next draft tokens are produced by the previous batch and their count may not be copied to the C++ request before the next scheduling pass, so the scheduler can budget a gen request as 1 token though the model consumes 1+K -> oversized mixed context/generation batch (total_num_tokens > max_num_tokens). Run scheduler-facing draft-token reservation for every speculative decoder immediately before scheduling (_prepare_scheduler_draft_tokens); placeholder token values are not consumed, their length reserves the correct micro-batch capacity. Includes: skip when spec decode is disabled, handle lightweight executor fixtures, and unwaive the DeepSeekV3 nvfp4 mtp3_fp8kv_chunked test that exposed the bug. Signed-off-by: chungen28 <chung-en@deepinfra.com>
Description
One-engine speculative decoders keep their draft model inside
ModelEngine, soPyExecutor.drafterisNone. With the overlap scheduler, the next draft tokens can be produced by the previous batch before their count is copied to the C++ request. The scheduler can therefore budget a generation request as one token even though the model consumes the full1 + Kspeculative step.This can admit an oversized mixed context/generation batch. The observed MTP3 failure was
total_num_tokens (8195) should be <= max_num_tokens (8192).Move scheduler-facing draft-token preparation out of the executor-level
Drafterbranch and run it immediately before scheduling for every speculative decoder. The placeholder token values are not consumed by the model; their length reserves the correct micro-batch capacity.Add a regression test that enters through
_prepare_and_schedule_batchand verifies that a one-engine speculative request exposes all three draft tokens at the_scheduleboundary.Remove the NVBug 5989920 waiver for
test_nvfp4_multi_gpus_piecewise_cuda_graph[mtp3_fp8kv_chunked]so CI runs the configuration that exposed this bug.Test Coverage
pytest -s -q tests/unittest/_torch/executor/test_py_executor.py— 69 passed.pytest -s -vv --tb=short tests/integration/defs/accuracy/test_llm_api_pytorch.py::TestDeepSeekV32::test_nvfp4_multi_gpus_piecewise_cuda_graph[mtp3_fp8kv_chunked]on 8x B200 — passed; MMLU 87.890, GSM8K 95.679.pre-commit run --files tensorrt_llm/_torch/pyexecutor/py_executor.py tests/unittest/_torch/executor/test_py_executor.py tests/integration/test_lists/waives.txt— passed.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.Summary by CodeRabbit
Bug Fixes
Tests