[TRTLLM-14198][refactor] Make FlashInfer a hard dependency for the Torch sampler#16160
[TRTLLM-14198][refactor] Make FlashInfer a hard dependency for the Torch sampler#16160zhaoyangwang-nvidia wants to merge 3 commits into
Conversation
78f0aed to
0fe449d
Compare
4093d14 to
ad0e8b5
Compare
📝 WalkthroughWalkthroughThis PR removes the ChangesFlashInfer-only sampling refactor
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/unittest/_torch/sampler/test_torch_sampler.py`:
- Around line 1888-1917: The `_sample_grouped_strategies` wrapper is accepting
`group_metadata` but not passing it through to `sample_grouped_strategies_orig`,
so the helper silently drops metadata. Update the delegation in
`_sample_grouped_strategies` to forward the `group_metadata` argument alongside
the existing `group_logit_indices`, `generator`, and `return_probs` parameters
so the instrumentation helper preserves real metadata.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: ff798a0a-64d7-499d-9610-83539c3ce283
📒 Files selected for processing (16)
docs/source/developer-guide/telemetry.mddocs/source/features/sampling.mdtensorrt_llm/_torch/pyexecutor/_util.pytensorrt_llm/_torch/pyexecutor/py_executor_creator.pytensorrt_llm/_torch/pyexecutor/sampler/ops/flashinfer.pytensorrt_llm/_torch/pyexecutor/sampler/ops/interface.pytensorrt_llm/_torch/pyexecutor/sampler/ops/vanilla.pytensorrt_llm/_torch/pyexecutor/sampler/sampler.pytensorrt_llm/_torch/pyexecutor/sampler/sampling_utils.pytensorrt_llm/llmapi/llm_args.pytensorrt_llm/usage/llm_args_golden_manifest.jsontests/unittest/_torch/executor/test_py_executor_creator_mla_cache_reuse_sync.pytests/unittest/_torch/sampler/test_beam_search.pytests/unittest/_torch/sampler/test_logits_logprobs.pytests/unittest/_torch/sampler/test_torch_sampler.pytests/unittest/api_stability/references/llm.yaml
💤 Files with no reviewable changes (9)
- tests/unittest/api_stability/references/llm.yaml
- tests/unittest/_torch/executor/test_py_executor_creator_mla_cache_reuse_sync.py
- tensorrt_llm/_torch/pyexecutor/sampler/ops/interface.py
- tensorrt_llm/llmapi/llm_args.py
- tensorrt_llm/usage/llm_args_golden_manifest.json
- docs/source/developer-guide/telemetry.md
- tensorrt_llm/_torch/pyexecutor/py_executor_creator.py
- tensorrt_llm/_torch/pyexecutor/_util.py
- tensorrt_llm/_torch/pyexecutor/sampler/ops/vanilla.py
ad0e8b5 to
f979046
Compare
|
/bot run --disable-fail-fast |
…sampler and spec Convert the Torch sampler and one-model speculative decoding from "FlashInfer-optional with torch fallback" to a hard FlashInfer dependency. The failure is raised at sampler / speculative-worker construction time (TorchSampler.__init__, SpecWorkerBase) with a clear ImportError, so sampling_utils stays importable for torch-only consumers (AutoDeploy demollm, drafting greedy helpers). - IS_FLASHINFER_AVAILABLE semantics are unchanged (its ~46 legitimate fallback consumers are not touched). - ops/flashinfer.py: keep flashinfer.sampling behind an import guard so the module stays importable without flashinfer; ops are only reached after the hard-dependency check at construction time. - TorchSampler.__init__ enforces the hard dependency once (out of the CUDA-graph-captured loop) before selecting the grouped sampler class. - Flatten the single-implementation grouped sampler: drop the GroupedStrategySampler ABC and keep only FlashInferGroupedStrategySampler. - Remove the SimpleGroupedStrategySampler torch backend and the disable_flashinfer_sampling LlmArg + its plumbing; update the api_stability and usage golden files and the sampling/telemetry docs. - compute_probs_from_logits becomes FlashInfer-only (drop the CUDA C++ op and CPU vanilla branches). - speculative/interface.py: drop the availability guards, keep the flashinfer>=0.6.4 version gate and its version-gated branches. - Delete now-dead fallback kernels (forward_native_sampling, _apply_top_k_top_p, _random_sample, vanilla.compute_probs_from_logits_op); keep beam-search/greedy/rejection/_Fusions and the torch sample() path used by rejection-sampling draft probs and demollm. - Prune torch-vs-flashinfer parametrized sampler tests. Signed-off-by: ZhaoyangWang <zhaoyangw@nvidia.com>
f979046 to
5d97c03
Compare
|
PR_Github #58426 [ run ] triggered by Bot. Commit: |
|
/bot kill |
|
/bot run --disable-fail-fast |
|
PR_Github #58427 [ kill ] triggered by Bot. Commit: |
|
PR_Github #58429 [ run ] triggered by Bot. Commit: |
|
PR_Github #58427 [ kill ] completed with state |
|
PR_Github #58426 [ run ] completed with state |
…vailable One-model speculative sampling (sampling_batch_spec_dec_one_model) now calls flashinfer unconditionally after the torch fallback was removed. SpecWorkerBase previously only set use_flashinfer=False when flashinfer was missing or <0.6.4, so the worker could initialize successfully and then crash mid-inference on the first non-greedy sampling step. Raise ImportError at SpecWorkerBase.__init__ when flashinfer is unavailable or below 0.6.4, and drop the now-redundant use_flashinfer flag (always True past the check). Signed-off-by: ZhaoyangWang <zhaoyangw@nvidia.com>
|
Hi @sunnyqgg Added some spec-side changes here touching the spec ↔ sampler integration — could you help review? Thanks! |
|
PR_Github #58429 [ run ] completed with state
|
sampler.py lives in tensorrt_llm/_torch/pyexecutor/sampler/, so the relative import 'from ..flashinfer_utils' resolved to the wrong package and broke 'import tensorrt_llm' during pytest collection. Use the absolute import 'from tensorrt_llm._torch.flashinfer_utils import IS_FLASHINFER_AVAILABLE', matching the sibling ops/flashinfer.py. Signed-off-by: ZhaoyangWang <zhaoyangw@nvidia.com>
|
/bot run --disable-fail-fast |
|
PR_Github #58454 [ run ] triggered by Bot. Commit: |
…sampler and spec
Convert the Torch sampler and one-model speculative decoding from "FlashInfer-optional with torch fallback" to a hard FlashInfer dependency. The failure is raised at sampler / speculative-worker construction time (resolve_sampling_backend, SpecWorkerBase) via a new require_flashinfer helper and a sentinel-based bind_flashinfer_sampling, so sampling_utils stays importable for torch-only consumers (AutoDeploy demollm, drafting greedy helpers).
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests
Description
Make FlashInfer required for the PyTorch-backend Torch sampler, and delete
the now-dead torch fallback paths and their tests. When flashinfer is missing,
TorchSampler.__init__now raises a clearImportErrorinstead of silentlyrunning a slower/divergent torch path.
IS_FLASHINFER_AVAILABLEsemantics are unchanged — its ~46 non-samplerfallback consumers are not touched.
What changed
TorchSampler.__init__hard-fails (ImportError) when flashinfer is missing,before entering the CUDA-graph-captured sampling loop.
SimpleGroupedStrategySamplerand theGroupedStrategySamplerABC;FlashInferGroupedStrategySampleris the onlyimplementation. The one-line factory is inlined and
ops/interface.pydeleted.disable_flashinfer_samplingLlmArg and its plumbing (updatesapi_stability / usage golden files and sampling & telemetry docs).
compute_probs_from_logitsbecomes FlashInfer-only; delete dead fallbackkernels (
forward_native_sampling,_apply_top_k_top_p,_random_sample,vanilla.compute_probs_from_logits_op).Intentionally kept (not fallbacks)
Torch
sample()for two-model speculative rejection sampling and AutoDeploydemollm; beam search (torch-only by design);
_Fusionslog-prob machinery;and the
flashinfer>=0.6.4version gate in one-model spec decoding.Test Coverage
Updated sampler unit tests (removed
use_flashinfer/disable_flashinfer_samplingparametrizations); updated api_stability references and usage golden manifest.
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.