[None][test] add e2e key model perf test - #17047
Conversation
WalkthroughAdds NVTX markers for DeepSeek V4 and mHC, and expands performance benchmark configuration and test coverage for new GLM, MiniMax, Gemma, and DeepSeek model variants. ChangesNVTX instrumentation
Performance model coverage
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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/test_lists/qa/llm_perf_core.yml (1)
203-319: 📐 Maintainability & Code Quality | 🔵 TrivialTest coverage summary: needs follow-up.
No test functions were added, modified, or removed. The new DeepSeek V4, GLM-5.2, MiniMax-M3, and Gemma 4 parameterized cases are listed in this QA file and exercise the corresponding model paths and configuration patterns. No
test-db/mirror is required; QA lists are maintained independently. Hardware gating and the M3 token-cap discrepancy still require confirmation.As per path instructions, QA-list coverage is independent of CI
test-dbcoverage.🤖 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/test_lists/qa/llm_perf_core.yml` around lines 203 - 319, Follow up on the QA-list additions for DeepSeek V4, GLM-5.2, MiniMax-M3, and Gemma 4 by confirming their hardware gating and configuration validity, especially the MiniMax-M3 ISL+OSL token cap versus max_seq_len requirement. Keep coverage represented through the existing perf/test_perf.py parameterized entries; do not add test functions or a test-db mirror.Source: Path instructions
🤖 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/integration/test_lists/qa/llm_perf_core.yml`:
- Around line 314-315: Align the MiniMax-M3 test cases in the perf workload
definitions with the documented 2048-token cap: update the input/output lengths
for both `2000,500` and `500,2000` cases so their totals do not exceed 2048, or
update and validate the corresponding safety-envelope configuration before
retaining them.
---
Nitpick comments:
In `@tests/integration/test_lists/qa/llm_perf_core.yml`:
- Around line 203-319: Follow up on the QA-list additions for DeepSeek V4,
GLM-5.2, MiniMax-M3, and Gemma 4 by confirming their hardware gating and
configuration validity, especially the MiniMax-M3 ISL+OSL token cap versus
max_seq_len requirement. Keep coverage represented through the existing
perf/test_perf.py parameterized entries; do not add test functions or a test-db
mirror.
🪄 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: 4e3762f4-2f75-4ebe-9b3f-b8de70fb9d1b
📒 Files selected for processing (5)
tensorrt_llm/tools/layer_wise_benchmarks/mark_utils.pytests/integration/defs/perf/_model_paths.pytests/integration/defs/perf/pytorch_model_config.pytests/integration/defs/perf/test_perf.pytests/integration/test_lists/qa/llm_perf_core.yml
… MiniMax-M3 and Gemma-4
Four models that TensorRT LLM already supports had no coverage, or only
partial coverage, in the QA perf core list:
* DeepSeek-V4-Pro: only the DSpark speculative-decoding variant was listed,
so the plain Pro checkpoint was untracked. Adds throughput and latency
cases with the knobs from
examples/configs/curated/deepseek-v4-pro-{throughput,latency}.yaml, at the
8K/1K fixed shape used for DeepSeek-V4 Pro in tech blog 26.
* GLM-5.2: only glm_5_fp8 was listed. Adds the NVFP4 checkpoint with the
CuteDSL MoE backend used by TestGLM52::test_nvfp4, mirroring the glm_5_fp8
ISL/OSL sweep so the two are directly comparable. Speculative decoding is
intentionally left off so the sweep measures kernel time rather than MTP
acceptance rate.
* MiniMax-M3: the MXFP8 checkpoint is already covered at 4 GPUs. Adds the
NVFP4 checkpoint (MXFP8 base layers with NVFP4 routed experts on CUTLASS)
on the MSA sparse-attention path, at 4 GPUs and at the 8-GPU TP8/EP8 scale
from
docs/source/deployment-guide/deployment-guide-for-minimax-m3-on-trtllm.md.
The MSA kernels are SM100/SM103 only, so the cases are restricted to
B200/GB200/B300/GB300. The pattern is scoped to the NVFP4 label so the
existing MXFP8 configuration is unchanged. ISL+OSL is kept at or below
2048 because the sparse path requires max_seq_len capped just above it,
otherwise CUDA-graph capture fails on gigabyte-scale warmup temporaries.
* Gemma-4: no perf coverage at all. Adds the NVFP4 MoE (26B-A4B) and dense
(31B) checkpoints on a single GPU, reusing the existing
gemma_4_26b_a4b_nvfp4 path entry and registering the 31B one alongside it.
The FLASHINFER backend is selected by the model's own get_model_defaults(),
so it is not repeated in the perf config. An 8000-token ISL is included to
exercise the 1024-token sliding window. Coverage is limited to Blackwell
because the model dispatches trtllm-gen cubins for all layers.
Newer versions of these model families are not covered because the
repository currently supports Kimi K2.5, Qwen3.5 and Nemotron 3.
Signed-off-by: Ruodi Lu <ruodil@users.noreply.github.com>
abefef3 to
5a2dc86
Compare
fredricz-20070104
left a comment
fredricz-20070104
left a comment
There was a problem hiding this comment.
Review summary - Approve
Reviewed the full diff; no blocking or major issues found.
Minor, non-blocking notes:
tests/integration/defs/perf/pytorch_model_config.py: MiniMax-M3 comment claims ISL+OSL <= 2048 but real cases reach 2500tests/integration/defs/perf/pytorch_model_config.py: Very broad 'gemma_4_' pattern
Automated review by NVCortex Lite, run by @fredricz-20070104.
|
Reviewed. The thing I check hardest on a This one is clean. I resolved both versions of On the open CodeRabbit thread about the M3 token cap — it's a comment bug, not a functional one, so please just fix the comments rather than the cases. Holding my approval only on that one open thread; the substance looks right to me. |
BowenFu
left a comment
BowenFu
left a comment
There was a problem hiding this comment.
Approving over the one open CodeRabbit thread: the 2048 issue is a stale comment, not a functional one (M3 cases are 2500 tokens and the block pins max_seq_len: 2560, and sizing keys off max_seq_len) — see issuecomment-5133921492. Still worth fixing the <= 2048 comments in the config block and llm_perf_core.yml:255, non-blocking. Verified no pre-existing scheduled test changes config: resolve-and-diff over every scheduled label returned zero deltas, and the new cases are QA-list only (nothing under test-db/**).
Signed-off-by: ruodil <200874449+ruodil@users.noreply.github.com>
|
/bot skip --comment "skip CI as just adding test cases" |
|
PR_Github #63340 [ skip ] triggered by Bot. Commit: |
|
PR_Github #63340 [ skip ] completed with state |
|
/bot skip --comment "skip CI as just adding test cases" |
|
PR_Github #63407 [ skip ] triggered by Bot. Commit: |
|
PR_Github #63407 [ skip ] completed with state |
5 participants
Dev Engineer Review
llm_perf_corecoverage for DeepSeek-V4-Pro, GLM-5.2, MiniMax-M3, and Gemma-4.git diff --checkpasses.QA Engineer Review
tests/integration/test_lists/qa/llm_perf_core.yml.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.