[TRTLLM-6473][test] add speculative decoding and ep load balance cases into QA test list #6436
Conversation
📝 WalkthroughWalkthroughNew accuracy reference entries for Eagle3 speculative decoding were added for large LLaMA models in CNN/DailyMail and MMLU YAMLs. Two new parameterized Eagle3 test methods were introduced for Llama 3.3 70B and Llama 4 Maverick 17B models, with expanded test list entries and a new waiver for a known FP8 Eagle3 test bug. No existing tests or accuracy data were modified or removed. Changes
Sequence Diagram(s)sequenceDiagram
participant Tester
participant TestRunner
participant LlamaModel
participant Eagle3Decoder
Tester->>TestRunner: Run Eagle3 test (parameterized)
TestRunner->>LlamaModel: Initialize model (Llama3.3-70B or Llama4 Maverick 17B)
TestRunner->>Eagle3Decoder: Configure Eagle3 decoding (single or two-model, FP8, etc.)
Eagle3Decoder->>LlamaModel: Perform speculative decoding
LlamaModel-->>Eagle3Decoder: Return decoded outputs
Eagle3Decoder-->>TestRunner: Return results
TestRunner-->>Tester: Report test outcome
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (7)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (5)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
coderabbitai
bot
requested review from
brb-nv,
LarryXFly,
syuoni,
yilin-void and
yuxianq
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
tests/integration/defs/accuracy/references/gpqa_diamond.yaml (1)
3-4: Check uniqueness guarantees in GPQA reference list.The new Eagle variant again mirrors the baseline score (45.96). Ensure the reference-reader differentiates these two list items and that downstream reports won’t double-count or randomly pick one.
tests/integration/defs/accuracy/references/mmlu.yaml (1)
70-74: Key ordering doesn’t affect YAML, but keep it consistent.Most other entries list
spec_dec_algobeforekv_cache_quant_algo. Maintaining the same key order aids quick diff-scans and reduces churn in future patches.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
tests/integration/defs/accuracy/references/cnn_dailymail.yaml(1 hunks)tests/integration/defs/accuracy/references/gpqa_diamond.yaml(1 hunks)tests/integration/defs/accuracy/references/gsm8k.yaml(1 hunks)tests/integration/defs/accuracy/references/mmlu.yaml(1 hunks)tests/integration/defs/accuracy/test_llm_api_pytorch.py(4 hunks)tests/integration/defs/examples/test_llama.py(1 hunks)tests/integration/test_lists/qa/examples_test_list.txt(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit Inference Engine (CODING_GUIDELINES.md)
**/*.py: Python code should conform to Python 3.8+.
Indent Python code with 4 spaces. Do not use tabs.
Always maintain the namespace when importing in Python, even if only one class or function from a module is used.
Python filenames should use snake_case (e.g., some_file.py).
Python classes should use PascalCase (e.g., class SomeClass).
Python functions and methods should use snake_case (e.g., def my_awesome_function():).
Python local variables should use snake_case, and prefix k for variable names that start with a number (e.g., k_99th_percentile).
Python global variables should use upper snake_case and prefix G (e.g., G_MY_GLOBAL).
Python constants should use upper snake_case (e.g., MY_CONSTANT).
Avoid shadowing variables declared in an outer scope in Python.
Initialize all externally visible members of a Python class in the constructor.
For interfaces that may be used outside a Python file, prefer docstrings over comments.
Comments in Python should be reserved for code within a function, or interfaces that are local to a file.
Use Google style docstrings for Python classes and functions, which can be parsed by Sphinx.
Attributes and variables in Python can be documented inline; attribute docstrings will be rendered under the docstring for the class.
Avoid using reflection in Python when functionality can be easily achieved without it.
When using try-except blocks in Python, limit the except to the smallest set of errors possible.
When using try-except blocks to handle multiple possible variable types in Python, keep the body of the try as small as possible, using the else block to implement the logic.
Files:
tests/integration/defs/examples/test_llama.pytests/integration/defs/accuracy/test_llm_api_pytorch.py
**/*.{cpp,h,hpp,cc,cxx,cu,py}
📄 CodeRabbit Inference Engine (CODING_GUIDELINES.md)
All TensorRT-LLM Open Source Software code should contain an NVIDIA copyright header that includes the current year. This includes .cpp, .h, .cu, .py, and any other source files which are compiled or interpreted.
Files:
tests/integration/defs/examples/test_llama.pytests/integration/defs/accuracy/test_llm_api_pytorch.py
🧠 Learnings (4)
📓 Common learnings
Learnt from: moraxu
PR: NVIDIA/TensorRT-LLM#6303
File: tests/integration/test_lists/qa/examples_test_list.txt:494-494
Timestamp: 2025-07-28T17:06:08.621Z
Learning: In TensorRT-LLM testing, it's common to have both CLI flow tests (test_cli_flow.py) and PyTorch API tests (test_llm_api_pytorch.py) for the same model. These serve different purposes: CLI flow tests validate the traditional command-line workflow, while PyTorch API tests validate the newer LLM API backend. Both are legitimate and should coexist.
tests/integration/defs/examples/test_llama.py (1)
Learnt from: moraxu
PR: #6303
File: tests/integration/test_lists/qa/examples_test_list.txt:494-494
Timestamp: 2025-07-28T17:06:08.621Z
Learning: In TensorRT-LLM testing, it's common to have both CLI flow tests (test_cli_flow.py) and PyTorch API tests (test_llm_api_pytorch.py) for the same model. These serve different purposes: CLI flow tests validate the traditional command-line workflow, while PyTorch API tests validate the newer LLM API backend. Both are legitimate and should coexist.
tests/integration/defs/accuracy/test_llm_api_pytorch.py (1)
Learnt from: moraxu
PR: #6303
File: tests/integration/test_lists/qa/examples_test_list.txt:494-494
Timestamp: 2025-07-28T17:06:08.621Z
Learning: In TensorRT-LLM testing, it's common to have both CLI flow tests (test_cli_flow.py) and PyTorch API tests (test_llm_api_pytorch.py) for the same model. These serve different purposes: CLI flow tests validate the traditional command-line workflow, while PyTorch API tests validate the newer LLM API backend. Both are legitimate and should coexist.
tests/integration/test_lists/qa/examples_test_list.txt (1)
Learnt from: moraxu
PR: #6303
File: tests/integration/test_lists/qa/examples_test_list.txt:494-494
Timestamp: 2025-07-28T17:06:08.621Z
Learning: In TensorRT-LLM testing, it's common to have both CLI flow tests (test_cli_flow.py) and PyTorch API tests (test_llm_api_pytorch.py) for the same model. These serve different purposes: CLI flow tests validate the traditional command-line workflow, while PyTorch API tests validate the newer LLM API backend. Both are legitimate and should coexist.
🔇 Additional comments (11)
tests/integration/defs/accuracy/references/gsm8k.yaml (1)
12-13: Duplicate accuracy entry – confirm necessity.You add a variant with
spec_dec_algo: Eaglethat carries the exact same accuracy (83.78) as the baseline just above.
If the QA harness keys on(model-id, accuracy)only, the two records may be indistinguishable and the lookup could return either mapping nondeterministically.Please confirm:
- The harness disambiguates entries with identical accuracy by checking all keys (including
spec_dec_algo).- Having two identical-score rows is intentional and won’t break downstream aggregation.
If either point is uncertain, deduplicate or adjust the accuracy to the authoritative value measured for the Eagle run.
tests/integration/defs/accuracy/references/cnn_dailymail.yaml (1)
121-123: Potential ambiguity from identical baseline & Eagle row.As with gsm8k, this new Eagle row repeats the baseline accuracy (33.640).
Verify that the accuracy parser treats(spec_dec_algo, …)as part of the uniqueness key, otherwise one row may mask the other.tests/integration/defs/accuracy/references/mmlu.yaml (1)
62-63: Duplicate accuracy value for Eagle variant.Same concern as previous files – identical score (81.31) may collide with the baseline entry. Confirm harness behaviour.
tests/integration/defs/examples/test_llama.py (1)
4071-4072: Verify public API re-export for LLM importI wasn’t able to confirm that
LLMis re-exported fromtensorrt_llm.llmapi. Please check and then:
If
LLMis already exposed intensorrt_llm.llmapi, update the import in
tests/integration/defs/examples/test_llama.py(≈ lines 4071–4072):-from tensorrt_llm._tensorrt_engine import LLM -from tensorrt_llm.llmapi import (BuildConfig, KvCacheConfig, - LookaheadDecodingConfig, SamplingParams) +from tensorrt_llm.llmapi import (LLM, BuildConfig, KvCacheConfig, + LookaheadDecodingConfig, SamplingParams)Otherwise, preserve compatibility with a fallback in case the re-export isn’t available yet:
try: from tensorrt_llm.llmapi import LLM except ImportError: # Fallback for transitional builds; remove once `llmapi` re-exports LLM from tensorrt_llm._tensorrt_engine import LLMtests/integration/test_lists/qa/examples_test_list.txt (3)
476-480: Ensure explicit 4-GPU marks on new EPLB testsThe new 4-GPU EPLB tests in accuracy/test_llm_api_pytorch.py will hang or fail on machines with fewer than 4 GPUs. Please manually verify that each of the following methods is decorated with either
@pytest.mark.world_size(4) # or @pytest.mark.skip_if_cuda_less_than(4)– otherwise they may block CI on smaller setups.
• TestDeepSeekV3Lite.test_fp8_block_scales_4gpus_static_eplb
• TestDeepSeekV3Lite.test_bfloat16_4gpus_online_eplb[mtp_nextn=0]
• TestDeepSeekV3Lite.test_bfloat16_4gpus_online_eplb[mtp_nextn=2]
• TestDeepSeekV3Lite.test_nvfp4_4gpus_online_eplb[fp8kv=False]
• TestDeepSeekV3Lite.test_nvfp4_4gpus_online_eplb[fp8kv=True]
462-462: Naming for test_fp8_eagle3 is consistentAll FP8 tests in TestLlama4MaverickInstruct follow the
test_fp8_*pattern, and there is notest_eagle3_fp8or similar variant. No changes required.
444-445: Confirmedtest_eagle3Exists in TestLlama3_1_8BInstructThe
test_eagle3method is implemented in tests/integration/defs/accuracy/test_llm_api_pytorch.py within the TestLlama3_1_8BInstruct class (around lines 232–235), parameterized byeagle3_one_modelwith IDsone_model/two_model. The QA examples list is accurate and will not fail at collection time.tests/integration/defs/accuracy/test_llm_api_pytorch.py (4)
233-248: LGTM: Proper parametrization for Eagle3 configuration testing.The parametrization correctly tests both single-model and two-model Eagle3 configurations. The parameter is properly passed to
EagleDecodingConfigand follows consistent naming conventions used throughout the file.
255-256: LGTM: Enhanced test coverage with CnnDailymail task.The addition of CnnDailymail task evaluation before MMLU expands the test coverage for Eagle3 configurations. The implementation follows the established pattern used throughout the test file.
371-389: LGTM: Well-structured Eagle3 test for large model with tensor parallelism.The new test method properly:
- Uses appropriate MPI world size requirement for 8-way tensor parallelism
- Follows established parametrization patterns with
eagle3_one_model- Tests comprehensive task coverage (MMLU, GSM8K, GPQADiamond)
- Correctly applies chat template for GPQADiamond evaluation
- Uses proper model paths and configuration
1-2029: LGTM: Code adheres to established patterns and coding guidelines.The changes follow the established test patterns in the file and properly adhere to the coding guidelines:
- Proper Python 3.8+ conventions with 4-space indentation
- Correct naming conventions (PascalCase for classes, snake_case for methods)
- Appropriate pytest decorators and parametrization usage
- Consistent LLM configuration and task evaluation patterns
- Proper NVIDIA copyright header with current year
crazydemo
force-pushed
the
add_decoding_case
branch
from
eaaf9b1 to
e526c60
Compare
coderabbitai
bot
requested review from
litaotju,
StanleySun639 and
pamelap-nvidia
crazydemo
force-pushed
the
add_decoding_case
branch
from
8917620 to
3c8b77d
Compare
crazydemo
force-pushed
the
add_decoding_case
branch
2 times, most recently
from
4ab061f to
9cbab54
Compare
crazydemo
changed the title
crazydemo
force-pushed
the
add_decoding_case
branch
from
2f7e23e to
219ddd2
Compare
|
/bot run |
|
PR_Github #13631 [ run ] triggered by Bot |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
tests/integration/test_lists/qa/llm_sanity_test.txt (1)
20-24: Re-evaluate adding 4-GPU EPLB variants to the sanity suiteThese five new DeepSeek-V3-Lite entries all require at least four GPUs and exercise endpoint load-balancing code paths that are generally much slower than the existing single-node sanity cases. They will noticeably extend the qa/llm_sanity stage wall-time and GPU occupancy.
If the objective is merely to make sure the EPLB paths do not regress, consider:
• Moving them to the heavier qa/llm_extended (or similar) list,
• Or gating them behind an env-flag so they are skipped on the quick sanity workers.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
tests/integration/defs/accuracy/references/cnn_dailymail.yaml(2 hunks)tests/integration/defs/accuracy/references/mmlu.yaml(1 hunks)tests/integration/defs/accuracy/test_llm_api_pytorch.py(2 hunks)tests/integration/test_lists/qa/examples_test_list.txt(3 hunks)tests/integration/test_lists/qa/llm_sanity_test.txt(3 hunks)tests/integration/test_lists/waives.txt(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- tests/integration/test_lists/waives.txt
🚧 Files skipped from review as they are similar to previous changes (4)
- tests/integration/defs/accuracy/references/cnn_dailymail.yaml
- tests/integration/defs/accuracy/references/mmlu.yaml
- tests/integration/defs/accuracy/test_llm_api_pytorch.py
- tests/integration/test_lists/qa/examples_test_list.txt
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: moraxu
PR: NVIDIA/TensorRT-LLM#6303
File: tests/integration/test_lists/qa/examples_test_list.txt:494-494
Timestamp: 2025-07-28T17:06:08.621Z
Learning: In TensorRT-LLM testing, it's common to have both CLI flow tests (test_cli_flow.py) and PyTorch API tests (test_llm_api_pytorch.py) for the same model. These serve different purposes: CLI flow tests validate the traditional command-line workflow, while PyTorch API tests validate the newer LLM API backend. Both are legitimate and should coexist.
tests/integration/test_lists/qa/llm_sanity_test.txt (3)
Learnt from: moraxu
PR: #6303
File: tests/integration/test_lists/qa/examples_test_list.txt:494-494
Timestamp: 2025-07-28T17:06:08.621Z
Learning: In TensorRT-LLM testing, it's common to have both CLI flow tests (test_cli_flow.py) and PyTorch API tests (test_llm_api_pytorch.py) for the same model. These serve different purposes: CLI flow tests validate the traditional command-line workflow, while PyTorch API tests validate the newer LLM API backend. Both are legitimate and should coexist.
Learnt from: yiqingy0
PR: #5198
File: jenkins/mergeWaiveList.py:0-0
Timestamp: 2025-07-22T08:33:49.109Z
Learning: In the TensorRT-LLM waive list merging system, removed lines are always located at the end of the merge waive lists, which is why the mergeWaiveList.py script uses reverse traversal - it's an optimization for this specific domain constraint.
Learnt from: CR
PR: NVIDIA/TensorRT-LLM#0
File: CODING_GUIDELINES.md:0-0
Timestamp: 2025-07-31T04:50:23.290Z
Learning: Applies to **/*.py : The code developed for TensorRT-LLM should conform to Python 3.8+.
🔇 Additional comments (1)
tests/integration/test_lists/qa/llm_sanity_test.txt (1)
50-51: Consider CI resource impact of the new FP8 Eagle3 testsThe two added
test_fp8_eagle3cases each consume 8 GPUs and roughly double the runtime of existing auto-dtype checks. Please verify that:
- Your CI infrastructure can handle two concurrent 8-GPU jobs
- Or merge them into a single parametrized test (tp8-torch_compile = True/False) that reuses the same accuracy target to reduce CI load
Affected file/location:
• tests/integration/test_lists/qa/llm_sanity_test.txt (lines 50–51)
|
PR_Github #13631 [ run ] completed with state |
|
/bot run --disable-fail-fast |
|
PR_Github #13647 [ run ] triggered by Bot |
|
PR_Github #13647 [ run ] completed with state |
crazydemo
force-pushed
the
add_decoding_case
branch
from
75bdb6c to
65cc9d5
Compare
|
/bot run --skip-test |
|
PR_Github #13749 [ run ] triggered by Bot |
crazydemo
force-pushed
the
add_decoding_case
branch
from
65cc9d5 to
6efa7b0
Compare
|
/bot run --skip-test |
Signed-off-by: Ivy Zhang <[email protected]>
Signed-off-by: Ivy Zhang <[email protected]>
Signed-off-by: Ivy Zhang <[email protected]>
Signed-off-by: Ivy Zhang <[email protected]>
Signed-off-by: Ivy Zhang <[email protected]>
Signed-off-by: Ivy Zhang <[email protected]>
crazydemo
force-pushed
the
add_decoding_case
branch
from
a11c3be to
c3e7bb8
Compare
|
/bot reuse-pipeline |
|
PR_Github #13896 [ reuse-pipeline ] triggered by Bot |
|
PR_Github #13896 [ reuse-pipeline ] completed with state |
…s into QA test list (NVIDIA#6436) Signed-off-by: Ivy Zhang <[email protected]> Signed-off-by: Lanyu Liao <[email protected]>
…s into QA test list (NVIDIA#6436) Signed-off-by: Ivy Zhang <[email protected]>
Summary by CodeRabbit
Tests
Chores
Description
Test Coverage
GitHub Bot Help
/bot [-h] ['run', 'kill', 'skip', 'reuse-pipeline'] ...Provide a user friendly way for developers to interact with a Jenkins server.
Run
/bot [-h|--help]to print this help message.See details below for each supported subcommand.
run [--reuse-test (optional)pipeline-id --disable-fail-fast --skip-test --stage-list "A10-PyTorch-1, xxx" --gpu-type "A30, H100_PCIe" --test-backend "pytorch, cpp" --add-multi-gpu-test --only-multi-gpu-test --disable-multi-gpu-test --post-merge --extra-stage "H100_PCIe-TensorRT-Post-Merge-1, xxx" --detailed-log --debug(experimental)]Launch build/test pipelines. All previously running jobs will be killed.
--reuse-test (optional)pipeline-id(OPTIONAL) : Allow the new pipeline to reuse build artifacts and skip successful test stages from a specified pipeline or the last pipeline if no pipeline-id is indicated. If the Git commit ID has changed, this option will be always ignored. The DEFAULT behavior of the bot is to reuse build artifacts and successful test results from the last pipeline.--disable-reuse-test(OPTIONAL) : Explicitly prevent the pipeline from reusing build artifacts and skipping successful test stages from a previous pipeline. Ensure that all builds and tests are run regardless of previous successes.--disable-fail-fast(OPTIONAL) : Disable fail fast on build/tests/infra failures.--skip-test(OPTIONAL) : Skip all test stages, but still run build stages, package stages and sanity check stages. Note: Does NOT update GitHub check status.--stage-list "A10-PyTorch-1, xxx"(OPTIONAL) : Only run the specified test stages. Examples: "A10-PyTorch-1, xxx". Note: Does NOT update GitHub check status.--gpu-type "A30, H100_PCIe"(OPTIONAL) : Only run the test stages on the specified GPU types. Examples: "A30, H100_PCIe". Note: Does NOT update GitHub check status.--test-backend "pytorch, cpp"(OPTIONAL) : Skip test stages which don't match the specified backends. Only support [pytorch, cpp, tensorrt, triton]. Examples: "pytorch, cpp" (does not run test stages with tensorrt or triton backend). Note: Does NOT update GitHub pipeline status.--only-multi-gpu-test(OPTIONAL) : Only run the multi-GPU tests. Note: Does NOT update GitHub check status.--disable-multi-gpu-test(OPTIONAL) : Disable the multi-GPU tests. Note: Does NOT update GitHub check status.--add-multi-gpu-test(OPTIONAL) : Force run the multi-GPU tests in addition to running L0 pre-merge pipeline.--post-merge(OPTIONAL) : Run the L0 post-merge pipeline instead of the ordinary L0 pre-merge pipeline.--extra-stage "H100_PCIe-TensorRT-Post-Merge-1, xxx"(OPTIONAL) : Run the ordinary L0 pre-merge pipeline and specified test stages. Examples: --extra-stage "H100_PCIe-TensorRT-Post-Merge-1, xxx".--detailed-log(OPTIONAL) : Enable flushing out all logs to the Jenkins console. This will significantly increase the log volume and may slow down the job.--debug(OPTIONAL) : Experimental feature. Enable access to the CI container for debugging purpose. Note: Specify exactly one stage in thestage-listparameter to access the appropriate container environment. Note: Does NOT update GitHub check status.For guidance on mapping tests to stage names, see
docs/source/reference/ci-overview.mdand the
scripts/test_to_stage_mapping.pyhelper.kill
killKill all running builds associated with pull request.
skip
skip --comment COMMENTSkip testing for latest commit on pull request.
--comment "Reason for skipping build/test"is required. IMPORTANT NOTE: This is dangerous since lack of user care and validation can cause top of tree to break.reuse-pipeline
reuse-pipelineReuse a previous pipeline to validate current commit. This action will also kill all currently running builds associated with the pull request. IMPORTANT NOTE: This is dangerous since lack of user care and validation can cause top of tree to break.