Skip to content

[None][chore] Remove two-model speculative decoding leftovers - #20464

Merged
lori-ren merged 19 commits into
NVIDIA:mainfrom
lori-ren:chore/remove-two-model-sampler-leftovers
Oct 22, 2026
Merged

lori-ren merged 19 commits into
NVIDIA:mainfrom
lori-ren:chore/remove-two-model-sampler-leftovers

Conversation

@lori-ren

@lori-ren lori-ren commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Dev Engineer Review

The inspected working-tree diff contains formatting-only changes in three files. It does not show the speculative-decoding cleanup described in the PR objectives. No behavior or API changes are evident in this diff.

QA Engineer Review

Two test files changed. The edits reformat imports and one skip message; they do not change test logic or add/remove test cases. No test-list files changed. The Eagle3 test module appears in the l0_h100.yml and l0_b200.yml CI lists; no matching CI or manual-QA list entry was found for test_pytorch_model_engine.py. Coverage is sufficient for the formatting-only changes. Test execution results were not provided.

Per-File QA Perspective

  • tensorrt_llm/llmapi/llm_args.py — Import and line-wrapping formatting only. No behavior change is evident.
  • tests/unittest/_torch/executor/test_pytorch_model_engine.py — Import and line-wrapping formatting only; test coverage is unchanged. No matching CI or manual-QA list entry was found.
  • tests/unittest/_torch/speculative/test_eagle3.py — Import formatting and a test skip-message formatting change; test behavior is unchanged. The module is listed in l0_h100.yml and l0_b200.yml.

Description

#18721 removed the two-model speculative decoding path but left behind the code that only served it. This is the follow-up promised in the review of #20188. No LlmRequest is created with is_draft=True any more, so everything removed here was unreachable:

  • the draft-batch branches in TorchSampler, finish reasons, penalties and the seed manager;
  • py_is_first_draft and the CUDA graph key field it fed;
  • d2t handling in TorchSampler and LlmRequest: only the removed drafter put d2t into the model outputs;
  • the executor creator's draft_model_engine, which was always None;
  • the drafter branches of the overlap executor loop. Drafter modes force the overlap scheduler off, and the hooks those branches called (should_forward_draft_model, generate_draft_tokens_with_overlap, cleanup_previous_draft_resources) were defined only by the removed ModelDrafter.

Unchanged on purpose:

  • Drafter-based modes. NGram and UserProvidedDecodingConfig still verify draft tokens by greedy match in TorchSampler. The old two_model_spec_dec module had one stateless method that only used sampler state, so it is now a TorchSampler method.
  • User drafter hooks. PyExecutor still reads drafter.draft_model_engine and drafter.guided_decoder, so a user-provided drafter that runs its own engine keeps its warmup, CUDA graph release, sleep/wakeup, disagg request-state and startup-metric hooks.
  • One-model draft KV cache. The is_draft handling on the KV cache managers is untouched.

Comments that still described two-model modes are updated.

API compatibility. No public API changes: the removed symbols are _torch internals, and the LLM API arguments and the Drafter interface are unchanged. The startup-metric keys dropped from docs/source/llm-api/index.md (draft_model_engine_creation_seconds, initial_draft_model_engine, final_draft_model_engine) were only emitted by the two-model path that #18721 removed, so no current configuration produces them.

Test Coverage

  • tests/unittest/_torch/sampler/
  • tests/unittest/_torch/speculative/hw_agnostic/test_ngram.py, test_user_provided.py: drafter-based verification end to end.
  • tests/unittest/_torch/executor/test_py_executor.py, tests/unittest/executor/test_base_worker.py: the kept drafter-engine hooks.

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-compatible or api-breaking. For api-breaking, include BREAKING in 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.

No LlmRequest is created with is_draft=True since the two-model path was removed, so py_is_draft and every branch on it are dead.

Signed-off-by: Lori Ren <lorir@nvidia.com>
The executor's draft_model_engine is always None since the two-model path was removed; drop it from PyExecutor, its creator, KvCacheCreator and the startup metrics.

Signed-off-by: Lori Ren <lorir@nvidia.com>
The flag was only set by the removed two-model drafter, so it is always False and its graph-key field is constant.

Signed-off-by: Lori Ren <lorir@nvidia.com>
Nothing reads the per-request d2t since the two-model rejection path was removed, and the Torch Sampler no longer computes probs for draft verification.

Signed-off-by: Lori Ren <lorir@nvidia.com>
The module now only verifies drafter-produced tokens by greedy match; the two-model name is stale.

Signed-off-by: Lori Ren <lorir@nvidia.com>
A user-provided Drafter can still expose draft_model_engine and guided_decoder, so restore the executor hooks that warm up, checkpoint, release and clean up that engine.

Signed-off-by: Lori Ren <lorir@nvidia.com>
The two-model path is gone; describe the drafter-based (NGram, user-provided) and one-model modes that remain.

Signed-off-by: Lori Ren <lorir@nvidia.com>
It was one stateless method that reached back into TorchSampler for everything it used.

Signed-off-by: Lori Ren <lorir@nvidia.com>
Drafter modes force the overlap scheduler off, so its drafter is always None and the hooks it called were defined only by the removed ModelDrafter.

Signed-off-by: Lori Ren <lorir@nvidia.com>
Only the removed two-model drafter put d2t into the model outputs; one-model Eagle3 maps draft tokens in its own worker.

Signed-off-by: Lori Ren <lorir@nvidia.com>
@lori-ren lori-ren added the api-compatible Accepted LLM API contract change that is backwards-compatible label Oct 16, 2026
@lori-ren

Copy link
Copy Markdown
Contributor Author

/bot run

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #78391 [ run ] triggered by Bot. Commit: e360a27 Link to invocation

… wording

test_llama_eagle3 skipped every use_one_model=False case; the mocked HF download case now runs one-model.

Signed-off-by: Lori Ren <lorir@nvidia.com>
Their remaining cases pin these params to one value, so the branches they selected never run.

Signed-off-by: Lori Ren <lorir@nvidia.com>
@lori-ren

Copy link
Copy Markdown
Contributor Author

/bot run

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #78420 [ run ] triggered by Bot. Commit: bc63b35 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #78391 [ run ] completed with state ABORTED. Commit: e360a27

Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #78420 [ run ] completed with state FAILURE. Commit: bc63b35
/LLM/main/L0_MergeRequest_PR pipeline #64639 completed with status: 'FAILURE'

CI Report

⚠️ Action Required:

  • Please check the failed tests and fix your PR
  • If you cannot view the failures, ask the CI triggerer to share details
  • Once fixed, request an NVIDIA team member to trigger CI again

CI Agent Failure Analysis

Link to invocation

@lori-ren

Copy link
Copy Markdown
Contributor Author

/bot run

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #78433 [ run ] triggered by Bot. Commit: bc63b35 Link to invocation

@lori-ren
lori-ren marked this pull request as ready for review October 16, 2026 09:32
@lori-ren
lori-ren requested review from a team as code owners October 16, 2026 09:32
@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #78925 [ run ] triggered by Bot. Commit: c8f328e Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #78925 [ run ] completed with state SUCCESS. Commit: c8f328e
/LLM/main/L0_MergeRequest_PR pipeline #65024 completed with status: 'FAILURE'

CI Report

⚠️ Action Required:

  • Please check the failed tests and fix your PR
  • If you cannot view the failures, ask the CI triggerer to share details
  • Once fixed, request an NVIDIA team member to trigger CI again

CI Agent Failure Analysis

Link to invocation

@lori-ren

Copy link
Copy Markdown
Contributor Author

/bot run --disable-fail-fast

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #79058 [ run ] triggered by Bot. Commit: c8f328e Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #79058 [ run ] completed with state SUCCESS. Commit: c8f328e
/LLM/main/L0_MergeRequest_PR pipeline #65129 completed with status: 'SUCCESS'
Pipeline passed with automatic retried tests. Check the rerun report for details.

CI Report

Link to invocation

@lori-ren

Copy link
Copy Markdown
Contributor Author

/bot run --disable-fail-fast

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #79141 [ run ] triggered by Bot. Commit: 125a2fe Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #79141 [ run ] completed with state SUCCESS. Commit: 125a2fe
/LLM/main/L0_MergeRequest_PR pipeline #65192 completed with status: 'FAILURE'

CI Report

⚠️ Action Required:

  • Please check the failed tests and fix your PR
  • If you cannot view the failures, ask the CI triggerer to share details
  • Once fixed, request an NVIDIA team member to trigger CI again

CI Agent Failure Analysis

Link to invocation

@lori-ren

Copy link
Copy Markdown
Contributor Author

/bot run --disable-fail-fast

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #79296 [ run ] triggered by Bot. Commit: 125a2fe Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #79296 [ run ] completed with state SUCCESS. Commit: 125a2fe
/LLM/main/L0_MergeRequest_PR pipeline #65317 completed with status: 'FAILURE'

CI Report

⚠️ Action Required:

  • Please check the failed tests and fix your PR
  • If you cannot view the failures, ask the CI triggerer to share details
  • Once fixed, request an NVIDIA team member to trigger CI again

CI Agent Failure Analysis

Link to invocation

@lori-ren

Copy link
Copy Markdown
Contributor Author

/bot run --disable-fail-fast

1 similar comment
@lori-ren

Copy link
Copy Markdown
Contributor Author

/bot run --disable-fail-fast

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #79434 [ run ] triggered by Bot. Commit: 125a2fe Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #79434 [ run ] completed with state SUCCESS. Commit: 125a2fe
/LLM/main/L0_MergeRequest_PR pipeline #65424 completed with status: 'FAILURE'

CI Report

⚠️ Action Required:

  • Please check the failed tests and fix your PR
  • If you cannot view the failures, ask the CI triggerer to share details
  • Once fixed, request an NVIDIA team member to trigger CI again

CI Agent Failure Analysis

Link to invocation

@lori-ren

Copy link
Copy Markdown
Contributor Author

/bot run --disable-fail-fast

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #79491 [ run ] triggered by Bot. Commit: 125a2fe Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #79491 [ run ] completed with state SUCCESS. Commit: 125a2fe
/LLM/main/L0_MergeRequest_PR pipeline #65467 completed with status: 'FAILURE'

CI Report

⚠️ Action Required:

  • Please check the failed tests and fix your PR
  • If you cannot view the failures, ask the CI triggerer to share details
  • Once fixed, request an NVIDIA team member to trigger CI again

CI Agent Failure Analysis

Link to invocation

@lori-ren

Copy link
Copy Markdown
Contributor Author

/bot run --disable-fail-fast

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #79585 [ run ] triggered by Bot. Commit: 125a2fe Link to invocation

@lori-ren

Copy link
Copy Markdown
Contributor Author

/bot run --disable-fail-fast

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #79649 [ run ] triggered by Bot. Commit: 125a2fe Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #79585 [ run ] completed with state ABORTED. Commit: 125a2fe

Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #79649 [ run ] completed with state SUCCESS. Commit: 125a2fe
/LLM/main/L0_MergeRequest_PR pipeline #65579 completed with status: 'SUCCESS'

CI Report

Link to invocation

@lori-ren
lori-ren merged commit 53edc7d into NVIDIA:main Oct 22, 2026
7 checks passed
vsabavat added a commit to vsabavat/TensorRT-LLM that referenced this pull request Oct 22, 2026
…K3 kernels branch

Propagate k3-cons/b-runtime 8c2cefa (main 53edc7d, NVIDIA#20464) into the K3 kernels branch. No conflicts.

Signed-off-by: Vasanth Sabavat <vsabavat@nvidia.com>
vsabavat added a commit to vsabavat/TensorRT-LLM that referenced this pull request Oct 22, 2026
Merge the K3 kernels branch after it absorbed k3-cons/b-runtime 8c2cefa (main 53edc7d, NVIDIA#20464). No conflicts.

Signed-off-by: Vasanth Sabavat <vsabavat@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api-compatible Accepted LLM API contract change that is backwards-compatible ci: full pre-merge approved

Projects

None yet

Development

Successfully merging this pull request may close these issues.