[#14649][feat] AutoDeploy: UltraV3 nvfp4 MTP - #14473
Conversation
govind-ramnarayan
changed the title
govind-ramnarayan
changed the title
7cad656 to
85a6f5d
Compare
govind-ramnarayan
changed the title
📝 WalkthroughWalkthroughThis PR adds end-to-end MTP speculative decoding support for the Ultra V3 model in TensorRT-LLM. The changes introduce quantization exclusion remapping for draft models, provide a complete runtime configuration, and validate the implementation through unit, smoke, and integration tests. ChangesUltra V3 MTP Implementation
🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/integration/defs/accuracy/test_llm_api_autodeploy.py (1)
878-914: Confirm QA scheduled-list intent for the new Ultra MTP integration coverage.Since this adds a new integration accuracy path, please explicitly confirm whether a corresponding entry is needed under
tests/integration/test_lists/qa/(or document that QA-list updates are intentionally unnecessary for this PR).As per coding guidelines: "If the change adds or materially alters an integration test under tests/integration/defs/, call out whether an entry is needed under tests/integration/test_lists/qa/."
🤖 Prompt for 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. In `@tests/integration/defs/accuracy/test_llm_api_autodeploy.py` around lines 878 - 914, Confirm whether this new integration test (test_mtp using AutoDeployLLM with MTP_CONFIG_YAML and GSM8K) should be scheduled in the QA scheduled-list; if yes, add a corresponding entry to the QA scheduled-list naming the test (e.g., test_mtp / "Ultra MTP" coverage) and include required runtime tags (world_size/device requirements) and skip conditions, otherwise state in the PR description why no QA-list entry is needed (for example: transient/experimental integration, gated by skip_pre_blackwell, or not a maintained nightly test) so reviewers know the omission is intentional.
🤖 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/auto_deploy/singlegpu/smoke/test_ad_speculative_decoding.py`:
- Around line 205-215: The DraftFactory stub uses a mutable class attribute
_quant_exclude_conversion_mapping which triggers RUF012 and may share state
across tests; convert that mapping into an instance attribute by moving
_quant_exclude_conversion_mapping from the class body into DraftFactory.__init__
(i.e., set self._quant_exclude_conversion_mapping = {r"^mtp(?=\.|\*)":
"model"}), update any references to use the instance attribute, and ensure other
tests/uses instantiate DraftFactory so no class-level mutable state remains.
---
Nitpick comments:
In `@tests/integration/defs/accuracy/test_llm_api_autodeploy.py`:
- Around line 878-914: Confirm whether this new integration test (test_mtp using
AutoDeployLLM with MTP_CONFIG_YAML and GSM8K) should be scheduled in the QA
scheduled-list; if yes, add a corresponding entry to the QA scheduled-list
naming the test (e.g., test_mtp / "Ultra MTP" coverage) and include required
runtime tags (world_size/device requirements) and skip conditions, otherwise
state in the PR description why no QA-list entry is needed (for example:
transient/experimental integration, gated by skip_pre_blackwell, or not a
maintained nightly test) so reviewers know the omission is intentional.
🪄 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: 7443049e-4fed-48d2-bddb-1ccc2b45548c
📒 Files selected for processing (8)
examples/auto_deploy/model_registry/configs/ultra_v3_mtp.yamltensorrt_llm/_torch/auto_deploy/models/custom/modeling_eagle.pytensorrt_llm/_torch/auto_deploy/models/eagle.pytests/integration/defs/accuracy/references/gsm8k.yamltests/integration/defs/accuracy/test_llm_api_autodeploy.pytests/integration/test_lists/test-db/l0_dgx_b200.ymltests/unittest/auto_deploy/_utils_test/_model_test_utils.pytests/unittest/auto_deploy/singlegpu/smoke/test_ad_speculative_decoding.py
|
/bot run --stage-list "A10-Build_Docs, A10-PackageSanityCheck-PY310-UB2204, A100X-PackageSanityCheck-PY312-UB2404, A30-AutoDeploy-1, H100_PCIe-AutoDeploy-1, DGX_B200-AutoDeploy-1, A100X-PyTorch-1, DGX_H100-4_GPUs-AutoDeploy-1, DGX_B200-4_GPUs-AutoDeploy-1" |
|
PR_Github #50639 [ run ] triggered by Bot. Commit: |
|
PR_Github #50639 [ run ] completed with state
|
85a6f5d to
d876bef
Compare
|
@coderabbitai Regarding the QA scheduled-list nitpick on |
|
Tip For best results, initiate chat on the files or code changes.
✏️ Learnings added
|
@coderabbitai Rather, the learning should be that when we add MTP tests in AutoDeploy for a model that is already tested with the base model only, we will just add the tests to the same lists that the base model tests are in, unless explicitly told otherwise. |
|
Tip For best results, initiate chat on the files or code changes.
(ᵔ·ᴥ·ᵔ) ✏️ Learnings added
|
d876bef to
151e453
Compare
|
/bot run --stage-list "A10-Build_Docs, A10-PackageSanityCheck-PY310-UB2204, A100X-PackageSanityCheck-PY312-UB2404, A30-AutoDeploy-1, H100_PCIe-AutoDeploy-1, DGX_B200-AutoDeploy-1, A100X-PyTorch-1, DGX_H100-4_GPUs-AutoDeploy-1, DGX_B200-4_GPUs-AutoDeploy-1, DGX_H100-4_GPUs-AutoDeploy-Post-Merge-1, DGX_B200-8_GPUs-AutoDeploy-Post-Merge-1" |
|
PR_Github #51095 [ run ] triggered by Bot. Commit: |
|
PR_Github #51095 [ run ] completed with state
|
|
/bot run --stage-list "A10-Build_Docs, A10-PackageSanityCheck-PY310-UB2204, A100X-PackageSanityCheck-PY312-UB2404, A30-AutoDeploy-1, H100_PCIe-AutoDeploy-1, DGX_B200-AutoDeploy-1, A100X-PyTorch-1, DGX_H100-4_GPUs-AutoDeploy-1, DGX_B200-4_GPUs-AutoDeploy-1, DGX_H100-4_GPUs-AutoDeploy-Post-Merge-1, DGX_B200-8_GPUs-AutoDeploy-Post-Merge-1" |
|
PR_Github #51128 [ run ] triggered by Bot. Commit: |
|
PR_Github #51128 [ run ] completed with state
|
|
/bot run --stage-list "A10-Build_Docs, A10-PackageSanityCheck-PY310-UB2204, A100X-PackageSanityCheck-PY312-UB2404, A30-AutoDeploy-1, H100_PCIe-AutoDeploy-1, DGX_B200-AutoDeploy-1, A100X-PyTorch-1, DGX_H100-4_GPUs-AutoDeploy-1, DGX_B200-4_GPUs-AutoDeploy-1, DGX_H100-4_GPUs-AutoDeploy-Post-Merge-1, DGX_B200-8_GPUs-AutoDeploy-Post-Merge-1" |
|
PR_Github #51395 [ run ] triggered by Bot. Commit: |
|
PR_Github #51395 [ run ] completed with state
|
|
/bot run --stage-list "A10-Build_Docs, A10-PackageSanityCheck-PY310-UB2204, A100X-PackageSanityCheck-PY312-UB2404, A30-AutoDeploy-1, H100_PCIe-AutoDeploy-1, DGX_B200-AutoDeploy-1, A100X-PyTorch-1, DGX_H100-4_GPUs-AutoDeploy-1, DGX_B200-4_GPUs-AutoDeploy-1, DGX_H100-4_GPUs-AutoDeploy-Post-Merge-1, DGX_B200-8_GPUs-AutoDeploy-Post-Merge-1" |
|
PR_Github #51435 [ run ] triggered by Bot. Commit: |
|
PR_Github #51435 [ run ] completed with state
|
151e453 to
114081c
Compare
|
/bot run --disable-fail-fast |
|
PR_Github #52440 [ run ] triggered by Bot. Commit: |
|
/bot run |
|
PR_Github #52440 [ run ] completed with state
|
|
PR_Github #52463 [ run ] triggered by Bot. Commit: |
|
PR_Github #52463 [ run ] completed with state
|
19aaa51 to
fbec431
Compare
Signed-off-by: Govind Ramnarayan <105831528+govind-ramnarayan@users.noreply.github.com>
Signed-off-by: Govind Ramnarayan <105831528+govind-ramnarayan@users.noreply.github.com>
Signed-off-by: Govind Ramnarayan <105831528+govind-ramnarayan@users.noreply.github.com>
Signed-off-by: Govind Ramnarayan <105831528+govind-ramnarayan@users.noreply.github.com>
Signed-off-by: Govind Ramnarayan <105831528+govind-ramnarayan@users.noreply.github.com>
Signed-off-by: Govind Ramnarayan <105831528+govind-ramnarayan@users.noreply.github.com>
- Move DraftFactory test stub quant-exclude mapping to an instance attribute to satisfy Ruff RUF012. - Drop stale TODO comment above gather_logits_before_lm_head in the Ultra V3 MTP config. Signed-off-by: Govind Ramnarayan <105831528+govind-ramnarayan@users.noreply.github.com>
…orm group_size The NVFP4 fused_gated_rmsnorm_quant op requires group_size in [256, 8192] and a multiple of 256. The shrunken Ultra-V3-NVFP4 smoke config had n_groups=2, giving gated-RMSNorm group_size = mamba d_inner / n_groups = 256 / 2 = 128, which trips the constraint once the default-on NVFP4 RMSNorm fusion routes the gated norm into the C++ op. Set n_groups=1 so group_size = 256 / 1 = 256. Signed-off-by: Govind Ramnarayan <105831528+govind-ramnarayan@users.noreply.github.com>
…g GSM8K Point the Nemotron-Ultra-V3-NVFP4 model mapping at the official NVIDIA-Nemotron-3-Ultra-550B-A55B-NVFP4 checkpoint location, and adapt TestNemotronUltraV3.test_mtp's GSM8K evaluation to the no-thinking prompt format (enable_thinking=False, fewshot_as_multiturn=True, max_tokens=GSM8K.MAX_OUTPUT_LEN) used for that checkpoint. test_accuracy is intentionally left unchanged. The MTP accuracy reference and acceptance-rate threshold are inherited and first validated in CI against the official checkpoint. Signed-off-by: Govind Ramnarayan <105831528+govind-ramnarayan@users.noreply.github.com>
…per) Address review comment: enable the FlashInfer SSM backend with the replay kernel for Ultra V3 MTP, matching the super configuration. - ultra_v3_mtp.yaml: insert_cached_ssm_attention backend=flashinfer_ssm with ssm_replay=true for the MTP extend path - ultra smoke test: switch insert_cached_ssm_attention to flashinfer_ssm + ssm_replay (was triton_ssm) - small-model registry: ssm_state_size 8 -> 64 to satisfy the FlashInfer SSM decode-path constraint Signed-off-by: Govind Ramnarayan <105831528+govind-ramnarayan@users.noreply.github.com>
d279cf2 to
8669bac
Compare
4 participants
Fixes: #14649
Summary by CodeRabbit
Description
Test Coverage
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.