[TRTLLM-15078][test] Prune non-Llama-3.1-8B Llama tests - #17392
Conversation
Signed-off-by: Xin He (SW-GPU) <200704525+xinhe-nv@users.noreply.github.com>
|
@coderabbitai review the change, we need to retire llama tests, if the test is must have, please show me in a table. |
WalkthroughThe PR removes obsolete model accuracy, performance, disaggregated-serving, Ray, LoRA, and API test coverage. It updates integration manifests and retargets selected unit tests from TinyLlama to Llama 3.1 8B Instruct. ChangesModel references and accuracy coverage
Disaggregated and Ray test removal
Integration matrix updates
Unit-test model migration
LLM API and LoRA cleanup
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/unittest/_torch/sampler/test_logits_logprobs.py (1)
819-836: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winRelease
llmbefore loadinghf_model.
KvCacheConfig.free_gpu_memory_fractiondefaults to0.9. The test keeps the TP=2LLMalive while moving the full bf16 model tocuda:0, which can cause an OOM. Generate and save the TRT-LLM results first, callllm.shutdown(), then loadhf_model. A fixedfree_gpu_memory_fraction=0.4does not guarantee sufficient memory across GPU sizes.Test coverage:
test_logits_logprobs.pyis listed intests/integration/test_lists/test-db/l0_a30.yml.🤖 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/unittest/_torch/sampler/test_logits_logprobs.py` around lines 819 - 836, Update test_logprobs_match_hf_tp2 to generate and store the TRT-LLM results before loading the Hugging Face model, then call llm.shutdown() to release TP=2 GPU resources. Remove reliance on a fixed free_gpu_memory_fraction and load hf_model only after the LLM has been shut down.
🤖 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/defs/kv_cache/test_final_single_token_context_cuda_graph.py`:
- Line 39: Remove the stale entries for
test_zero_runtime_draft_speculation_after_final_token_reuse[v1] and [v2] from
the l0_h100 CI test list, while leaving all other test-list and QA entries
unchanged.
In `@tests/unittest/_torch/sampler/test_best_of_n.py`:
- Around line 37-38: Regenerate the expected_outputs fixture used by
test_n_outputs for the migrated Llama-3.1-8B-Instruct model path, updating its
snapshots to match the model’s current output while leaving the test logic
unchanged.
In `@tests/unittest/llmapi/test_llm_pytorch.py`:
- Around line 774-781: The difference-based assertion in
tests/unittest/llmapi/test_llm_pytorch.py:774-781 for test_gqa_nemo_lora should
remain and be confirmed stable for Llama-3.1-8B-Instruct. Update the NOTE inside
create_mock_nemo_lora_checkpoint at tests/unittest/llmapi/lora_test_utils.py:6
to remove the hardcoded-output contract and state that the test depends on the
adapter changing generated text; retain that seed and WEIGHTS_COEFFICIENT affect
this behavior, but no output string is hardcoded.
- Around line 774-781: Stabilize the LoRA verification around the generation
calls by re-validating that the current Llama-3.1-8B-Instruct setup reliably
produces different outputs across repeated runs; if greedy text comparison
remains flaky, replace it with a lower-confidence prompt or a logits-based
comparison. Update or remove the “tuned/large enough” comment so it reflects the
validated behavior, while preserving the existing LoRA and base generation
paths.
In `@tests/unittest/llmapi/test_llm.py`:
- Around line 123-126: Update every collected shared 8B model loader, including
llama_model_path in the identified test modules, to pass a bounded
kv_cache_config; explicitly exclude any non-collected tests only if they are
outside the audit scope. Extend TestLlama3_1_8BInstruct.test_chunked_prefill to
validate token increments during streaming as well as chunked prefill, and
register the resulting coverage in the appropriate integration test list while
preserving existing stream_interval coverage.
In `@tests/unittest/test_pip_install.py`:
- Line 217: Update the model-link mapping in create_link_for_models, where
src_dst_dict is initialized, to restore an entry for
TinyLlama/TinyLlama-1.1B-Chat-v1.0 pointing to the existing local model path.
Preserve the current run_sanity_check quickstart flow and use the repository’s
established source path rather than introducing a new download or test change.
---
Outside diff comments:
In `@tests/unittest/_torch/sampler/test_logits_logprobs.py`:
- Around line 819-836: Update test_logprobs_match_hf_tp2 to generate and store
the TRT-LLM results before loading the Hugging Face model, then call
llm.shutdown() to release TP=2 GPU resources. Remove reliance on a fixed
free_gpu_memory_fraction and load hf_model only after the LLM has been shut
down.
🪄 Autofix
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: d4a4f9e3-c5b8-446f-bb00-0090feb5e9e1
📒 Files selected for processing (93)
tests/integration/defs/accuracy/references/SlimPajama-6B.yamltests/integration/defs/accuracy/references/cnn_dailymail.yamltests/integration/defs/accuracy/references/gpqa_diamond.yamltests/integration/defs/accuracy/references/gsm8k.yamltests/integration/defs/accuracy/references/mmlu.yamltests/integration/defs/accuracy/references/passkey_retrieval_128k.yamltests/integration/defs/accuracy/test_llm_api_pytorch.pytests/integration/defs/accuracy/test_llm_api_pytorch_encode.pytests/integration/defs/conftest.pytests/integration/defs/disaggregated/test_ad_disagg_trtllm_serve.pytests/integration/defs/disaggregated/test_configs/disagg_config.yamltests/integration/defs/disaggregated/test_configs/disagg_config_cache_aware_balance.yamltests/integration/defs/disaggregated/test_configs/disagg_config_cache_reuse.yamltests/integration/defs/disaggregated/test_configs/disagg_config_conditional.yamltests/integration/defs/disaggregated/test_configs/disagg_config_conversation.yamltests/integration/defs/disaggregated/test_configs/disagg_config_conversation_workers.yamltests/integration/defs/disaggregated/test_configs/disagg_config_ctxpp2_genpp2.yamltests/integration/defs/disaggregated/test_configs/disagg_config_ctxpp2_gentp2.yamltests/integration/defs/disaggregated/test_configs/disagg_config_ctxpp4_genpp4.yamltests/integration/defs/disaggregated/test_configs/disagg_config_ctxpp4_gentp4.yamltests/integration/defs/disaggregated/test_configs/disagg_config_ctxtp2_genpp2.yamltests/integration/defs/disaggregated/test_configs/disagg_config_ctxtp2_gentp1.yamltests/integration/defs/disaggregated/test_configs/disagg_config_ctxtp2pp2_gentp2pp2.yamltests/integration/defs/disaggregated/test_configs/disagg_config_cuda_graph_padding.yamltests/integration/defs/disaggregated/test_configs/disagg_config_diff_max_tokens.yamltests/integration/defs/disaggregated/test_configs/disagg_config_gen_only.yamltests/integration/defs/disaggregated/test_configs/disagg_config_gen_only_bs1.yamltests/integration/defs/disaggregated/test_configs/disagg_config_gen_only_insufficient_kv.yamltests/integration/defs/disaggregated/test_configs/disagg_config_gen_only_kv_cache_aware.yamltests/integration/defs/disaggregated/test_configs/disagg_config_llama4_kv_cache_overflow.yamltests/integration/defs/disaggregated/test_configs/disagg_config_load_balance.yamltests/integration/defs/disaggregated/test_configs/disagg_config_load_balancing.yamltests/integration/defs/disaggregated/test_configs/disagg_config_metrics.yamltests/integration/defs/disaggregated/test_configs/disagg_config_mixed.yamltests/integration/defs/disaggregated/test_configs/disagg_config_multi_orchestrator.yamltests/integration/defs/disaggregated/test_configs/disagg_config_ngram.yamltests/integration/defs/disaggregated/test_configs/disagg_config_overlap.yamltests/integration/defs/disaggregated/test_configs/disagg_config_overlap_gen_first.yamltests/integration/defs/disaggregated/test_configs/disagg_config_overlap_gen_first_pp4.yamltests/integration/defs/disaggregated/test_configs/disagg_config_overlap_transceiver_runtime_python.yamltests/integration/defs/disaggregated/test_configs/disagg_config_overlap_transceiver_runtime_python_bounce.yamltests/integration/defs/disaggregated/test_configs/disagg_config_python_transceiver_host_offload.yamltests/integration/defs/disaggregated/test_disaggregated.pytests/integration/defs/disaggregated/test_disaggregated_single_gpu.pytests/integration/defs/examples/test_llm_api_with_mpi.pytests/integration/defs/examples/test_ray.pytests/integration/defs/kv_cache/test_final_single_token_context_cuda_graph.pytests/integration/defs/kv_cache/test_kv_cache_iteration_stats.pytests/integration/defs/kv_cache/test_kv_cache_v2_scheduler.pytests/integration/defs/perf/_model_paths.pytests/integration/defs/perf/pytorch_model_config.pytests/integration/defs/perf/sampler_options_config.pytests/integration/defs/test_e2e.pytests/integration/test_lists/qa/llm_function_core.txttests/integration/test_lists/qa/llm_spark_core.txttests/integration/test_lists/qa/llm_spark_func.ymltests/integration/test_lists/qa/llm_spark_perf.ymltests/integration/test_lists/test-db/l0_a10.ymltests/integration/test_lists/test-db/l0_a100.ymltests/integration/test_lists/test-db/l0_b200.ymltests/integration/test_lists/test-db/l0_dgx_b200.ymltests/integration/test_lists/test-db/l0_dgx_h100.ymltests/integration/test_lists/test-db/l0_dgx_h200.ymltests/integration/test_lists/test-db/l0_gb200_multi_gpus.ymltests/integration/test_lists/test-db/l0_h100.ymltests/integration/test_lists/test-db/l0_l40s.ymltests/integration/test_lists/test-db/l0_sanity_check.ymltests/integration/test_lists/waives.txttests/scripts/perf-sanity/aggregated/llama_v3_3_70b_instruct_fp4_blackwell.yamltests/test_common/llm_data.pytests/unittest/_torch/multi_gpu/test_mpi_sleep_wakeup.pytests/unittest/_torch/ray_orchestrator/multi_gpu/test_inflight_weight_update.pytests/unittest/_torch/ray_orchestrator/single_gpu/test_llm_sleep.pytests/unittest/_torch/sampler/test_beam_search.pytests/unittest/_torch/sampler/test_best_of_n.pytests/unittest/_torch/sampler/test_logits_logprobs.pytests/unittest/_torch/sampler/test_penalties_e2e.pytests/unittest/_torch/sampler/test_trtllm_sampler.pytests/unittest/auto_deploy/singlegpu/smoke/test_ad_build_small_single.pytests/unittest/llmapi/apps/_test_openai_lora.pytests/unittest/llmapi/apps/_test_openai_prometheus.pytests/unittest/llmapi/apps/_test_trtllm_serve_example.pytests/unittest/llmapi/apps/_test_trtllm_serve_top_logprobs.pytests/unittest/llmapi/lora_test_utils.pytests/unittest/llmapi/test_async_llm.pytests/unittest/llmapi/test_executor.pytests/unittest/llmapi/test_grpc.pytests/unittest/llmapi/test_llm.pytests/unittest/llmapi/test_llm_kv_cache_events.pytests/unittest/llmapi/test_llm_multi_gpu_pytorch.pytests/unittest/llmapi/test_llm_pytorch.pytests/unittest/llmapi/test_memory_profiling.pytests/unittest/test_pip_install.py
💤 Files with no reviewable changes (67)
- tests/integration/defs/disaggregated/test_configs/disagg_config.yaml
- tests/integration/defs/disaggregated/test_configs/disagg_config_gen_only.yaml
- tests/integration/defs/disaggregated/test_configs/disagg_config_diff_max_tokens.yaml
- tests/integration/defs/disaggregated/test_configs/disagg_config_ctxpp2_genpp2.yaml
- tests/integration/defs/disaggregated/test_configs/disagg_config_multi_orchestrator.yaml
- tests/integration/defs/disaggregated/test_configs/disagg_config_gen_only_bs1.yaml
- tests/integration/defs/disaggregated/test_configs/disagg_config_load_balance.yaml
- tests/integration/defs/disaggregated/test_configs/disagg_config_cache_aware_balance.yaml
- tests/integration/defs/disaggregated/test_configs/disagg_config_load_balancing.yaml
- tests/integration/defs/disaggregated/test_configs/disagg_config_ctxtp2pp2_gentp2pp2.yaml
- tests/integration/defs/accuracy/test_llm_api_pytorch_encode.py
- tests/integration/defs/examples/test_llm_api_with_mpi.py
- tests/integration/defs/disaggregated/test_configs/disagg_config_overlap_transceiver_runtime_python.yaml
- tests/integration/defs/disaggregated/test_configs/disagg_config_overlap_gen_first_pp4.yaml
- tests/integration/defs/disaggregated/test_configs/disagg_config_conditional.yaml
- tests/integration/defs/disaggregated/test_configs/disagg_config_python_transceiver_host_offload.yaml
- tests/integration/defs/disaggregated/test_configs/disagg_config_conversation.yaml
- tests/integration/test_lists/test-db/l0_sanity_check.yml
- tests/integration/defs/disaggregated/test_configs/disagg_config_llama4_kv_cache_overflow.yaml
- tests/integration/test_lists/test-db/l0_l40s.yml
- tests/integration/defs/disaggregated/test_configs/disagg_config_ctxpp4_gentp4.yaml
- tests/integration/defs/disaggregated/test_configs/disagg_config_overlap_transceiver_runtime_python_bounce.yaml
- tests/integration/defs/accuracy/references/gpqa_diamond.yaml
- tests/integration/defs/disaggregated/test_configs/disagg_config_overlap_gen_first.yaml
- tests/integration/defs/disaggregated/test_configs/disagg_config_gen_only_kv_cache_aware.yaml
- tests/integration/defs/perf/sampler_options_config.py
- tests/integration/defs/disaggregated/test_configs/disagg_config_cuda_graph_padding.yaml
- tests/integration/defs/accuracy/references/cnn_dailymail.yaml
- tests/integration/defs/disaggregated/test_configs/disagg_config_ctxtp2_gentp1.yaml
- tests/integration/defs/accuracy/references/gsm8k.yaml
- tests/integration/defs/disaggregated/test_configs/disagg_config_conversation_workers.yaml
- tests/integration/test_lists/qa/llm_spark_core.txt
- tests/integration/test_lists/test-db/l0_b200.yml
- tests/integration/defs/disaggregated/test_configs/disagg_config_overlap.yaml
- tests/unittest/auto_deploy/singlegpu/smoke/test_ad_build_small_single.py
- tests/integration/defs/disaggregated/test_configs/disagg_config_gen_only_insufficient_kv.yaml
- tests/integration/defs/disaggregated/test_configs/disagg_config_mixed.yaml
- tests/integration/defs/disaggregated/test_configs/disagg_config_ctxtp2_genpp2.yaml
- tests/integration/defs/disaggregated/test_configs/disagg_config_metrics.yaml
- tests/integration/defs/perf/pytorch_model_config.py
- tests/integration/test_lists/test-db/l0_gb200_multi_gpus.yml
- tests/integration/defs/disaggregated/test_configs/disagg_config_ngram.yaml
- tests/integration/defs/disaggregated/test_configs/disagg_config_ctxpp2_gentp2.yaml
- tests/unittest/llmapi/apps/_test_trtllm_serve_top_logprobs.py
- tests/integration/defs/accuracy/test_llm_api_pytorch.py
- tests/integration/defs/accuracy/references/mmlu.yaml
- tests/unittest/llmapi/apps/_test_openai_lora.py
- tests/unittest/llmapi/apps/_test_trtllm_serve_example.py
- tests/test_common/llm_data.py
- tests/integration/defs/disaggregated/test_disaggregated_single_gpu.py
- tests/integration/defs/kv_cache/test_kv_cache_v2_scheduler.py
- tests/integration/defs/perf/_model_paths.py
- tests/integration/test_lists/qa/llm_spark_perf.yml
- tests/integration/test_lists/test-db/l0_dgx_h200.yml
- tests/integration/test_lists/test-db/l0_a100.yml
- tests/integration/test_lists/test-db/l0_dgx_b200.yml
- tests/integration/defs/kv_cache/test_kv_cache_iteration_stats.py
- tests/integration/defs/disaggregated/test_configs/disagg_config_cache_reuse.yaml
- tests/integration/test_lists/test-db/l0_a10.yml
- tests/integration/test_lists/waives.txt
- tests/unittest/llmapi/apps/_test_openai_prometheus.py
- tests/integration/test_lists/qa/llm_spark_func.yml
- tests/scripts/perf-sanity/aggregated/llama_v3_3_70b_instruct_fp4_blackwell.yaml
- tests/integration/test_lists/test-db/l0_dgx_h100.yml
- tests/integration/defs/disaggregated/test_configs/disagg_config_ctxpp4_genpp4.yaml
- tests/integration/test_lists/test-db/l0_h100.yml
- tests/integration/test_lists/qa/llm_function_core.txt
| from tensorrt_llm.llmapi import CudaGraphConfig, Eagle3DecodingConfig, KvCacheConfig, RequestOutput | ||
| from tensorrt_llm.llmapi.mpi_session import MpiSession | ||
| from tensorrt_llm.sampling_params import GuidedDecodingParams, SamplingParams | ||
| from tensorrt_llm.sampling_params import SamplingParams |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
for directory in \
tests/integration/test_lists/test-db \
tests/integration/test_lists/qa; do
if [[ -d "$directory" ]]; then
rg -n -i \
'test_final_single_token_context_cuda_graph|final_token|guided_decoding' \
"$directory" || true
fi
doneRepository: NVIDIA/TensorRT-LLM
Length of output: 11840
Remove stale CI test-list entries.
tests/integration/test_lists/test-db/l0_h100.yml still references the two deleted tests:
test_zero_runtime_draft_speculation_after_final_token_reuse[v1] and [v2]. Remove both entries. No matching stale entries appear in the QA lists.
🤖 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/kv_cache/test_final_single_token_context_cuda_graph.py`
at line 39, Remove the stale entries for
test_zero_runtime_draft_speculation_after_final_token_reuse[v1] and [v2] from
the l0_h100 CI test list, while leaving all other test-list and QA entries
unchanged.
Source: Path instructions
| return LLM(model=os.path.join(llm_models_root(), "llama-3.1-model", | ||
| "Llama-3.1-8B-Instruct"), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 5 \
'expected_outputs|TinyLlama|Llama-3\.1-8B-Instruct' \
tests/unittest/_torch/sampler tests || trueRepository: NVIDIA/TensorRT-LLM
Length of output: 50376
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
file='tests/unittest/_torch/sampler/test_best_of_n.py'
printf '%s\n' '--- file outline ---'
ast-grep outline "$file" || true
printf '%s\n' '--- relevant definitions and references ---'
rg -n -C 8 'expected_outputs|def test_n_outputs|TinyLlama|Llama-3\.1-8B-Instruct|best_of|n_outputs' "$file"
printf '%s\n' '--- repository references limited to the target test and nearby fixtures ---'
rg -l 'expected_outputs' tests/unittest/_torch/sampler tests/unittest/_torch | sort | head -100Repository: NVIDIA/TensorRT-LLM
Length of output: 3657
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- target file status ---'
git diff -- tests/unittest/_torch/sampler/test_best_of_n.py
printf '%s\n' '--- test-list entries ---'
rg -n -C 3 'test_best_of_n|best_of_n' tests/integration/test_lists tests 2>/dev/null | head -200 || true
printf '%s\n' '--- copyright header and file metadata ---'
head -20 tests/unittest/_torch/sampler/test_best_of_n.pyRepository: NVIDIA/TensorRT-LLM
Length of output: 772
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
file='tests/unittest/_torch/sampler/test_best_of_n.py'
printf '%s\n' '--- focused file history ---'
git log --oneline --follow -- "$file" | head -20
printf '%s\n' '--- commits that changed the model path or expected outputs ---'
git log --all --oneline -S'Llama-3.1-8B-Instruct' -- "$file" | head -20
git log --all --oneline -S'Born in north-east France, Soyer trained as a' -- "$file" | head -20
printf '%s\n' '--- current revision and file history metadata ---'
git rev-parse HEAD
git status --short
git blame -L 21,42 -- "$file"Repository: NVIDIA/TensorRT-LLM
Length of output: 2660
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
file='tests/unittest/_torch/sampler/test_best_of_n.py'
commit="$(git log --all --format='%H' -S'Llama-3.1-8B-Instruct' -- "$file" | head -1)"
if [ -n "$commit" ]; then
printf '%s\n' "--- model-change commit: $commit ---"
git show --stat --oneline "$commit" -- "$file"
git show --format= --unified=20 "$commit" -- "$file"
else
printf '%s\n' 'No model-path change commit found in local history.'
fiRepository: NVIDIA/TensorRT-LLM
Length of output: 2118
Regenerate expected_outputs for Llama-3.1-8B-Instruct. The fixture was not changed with the model-path migration. test_n_outputs can compare Llama-3.1 output with stale snapshots. Test coverage: test_n_outputs is affected; no test functions were added, modified, or removed.
🤖 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/unittest/_torch/sampler/test_best_of_n.py` around lines 37 - 38,
Regenerate the expected_outputs fixture used by test_n_outputs for the migrated
Llama-3.1-8B-Instruct model path, updating its snapshots to match the model’s
current output while leaving the test logic unchanged.
| assert len(lora_outputs) == 1 | ||
|
|
||
| # Generate without LoRA. | ||
| # The LoRA weights are tuned/large enough that | ||
| # they differ from a no-LoRA run. | ||
| base_outputs = llm.generate(test_prompts, sampling_params) | ||
| assert base_outputs[0].outputs[0].text != expected_lora_text_output, \ | ||
| f"No-LoRA output should differ from expected output text: {expected_lora_text_output}, " \ | ||
| f"got: {base_outputs[0].outputs[0].text}" | ||
| assert lora_outputs[0].outputs[0].text != base_outputs[0].outputs[0].text, \ | ||
| "No-LoRA output should differ from LoRA output — adapter may not be applied" |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
The removed hardcoded assertion leaves a stale contract note in the helper. test_gqa_nemo_lora no longer asserts a fixed expected output string, but create_mock_nemo_lora_checkpoint still documents that contract and warns that changing the seed or WEIGHTS_COEFFICIENT breaks the test. The two files now disagree.
tests/unittest/llmapi/test_llm_pytorch.py#L774-L781: keep the difference-based assertion, and confirm it is stable for Llama-3.1-8B-Instruct as described in the per-site comment.tests/unittest/llmapi/lora_test_utils.py#L6-L6: update the NOTE insidecreate_mock_nemo_lora_checkpointthat references hardcoded expected outputs intest_llm_pytorch.py::test_gqa_nemo_lora. State the current dependency instead: the test relies on the adapter changing the generated text, soseedandWEIGHTS_COEFFICIENTstill affect it, but no output string is hardcoded.
📍 Affects 2 files
tests/unittest/llmapi/test_llm_pytorch.py#L774-L781(this comment)tests/unittest/llmapi/lora_test_utils.py#L6-L6
🤖 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/unittest/llmapi/test_llm_pytorch.py` around lines 774 - 781, The
difference-based assertion in tests/unittest/llmapi/test_llm_pytorch.py:774-781
for test_gqa_nemo_lora should remain and be confirmed stable for
Llama-3.1-8B-Instruct. Update the NOTE inside create_mock_nemo_lora_checkpoint
at tests/unittest/llmapi/lora_test_utils.py:6 to remove the hardcoded-output
contract and state that the test depends on the adapter changing generated text;
retain that seed and WEIGHTS_COEFFICIENT affect this behavior, but no output
string is hardcoded.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
The LoRA-versus-base difference assertion can be flaky on this model.
The test now asserts only that the LoRA output text differs from the base output text. Three factors weaken that assertion for Llama-3.1-8B-Instruct:
create_mock_nemo_lora_checkpointscales the random adapter weights byWEIGHTS_COEFFICIENT = 0.02. The relative perturbation is smaller againsthidden_size=4096than against the previoushidden_size=2048.sampling_paramsusestemperature=0.0andmax_tokens=10, so decoding is greedy.- The prompt is
"The capital of France is", a completion the base model predicts with very high confidence.
If the adapter does not flip any of the 10 greedy tokens, the test fails even though LoRA is applied correctly. Confirm the assertion is stable across repeated runs. If it is not, use a lower-confidence prompt, or compare logits instead of decoded text.
The comment on lines 776-778 also states that the weights are "tuned/large enough" to differ. That claim was validated for the previous model. Re-validate it or remove it.
🤖 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/unittest/llmapi/test_llm_pytorch.py` around lines 774 - 781, Stabilize
the LoRA verification around the generation calls by re-validating that the
current Llama-3.1-8B-Instruct setup reliably produces different outputs across
repeated runs; if greedy text comparison remains flaky, replace it with a
lower-confidence prompt or a logits-based comparison. Update or remove the
“tuned/large enough” comment so it reflects the validated behavior, while
preserving the existing LoRA and base generation paths.
| default_model_name = "llama-3.1-model/Llama-3.1-8B-Instruct" | ||
| mixtral_model_name = "Mixtral-8x7B-v0.1" | ||
|
|
||
| llama_model_path = get_model_path(default_model_name) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all consumers of the shared model path and harness, and check their KV cache bounds.
rg -n -C4 --type=py 'llama_model_path|default_model_name|llm_return_logprobs_test_harness' tests
# Check whether streaming-interval / chunked-prefill streaming coverage still exists.
rg -n --type=py 'stream_interval' testsRepository: NVIDIA/TensorRT-LLM
Length of output: 50376
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- changed sections in test_llm.py ---'
git diff --unified=8 -- tests/unittest/llmapi/test_llm.py
printf '%s\n' '--- shared definitions ---'
rg -n -C6 '^(default_model_name|llama_model_path|global_kvcache_config|global_kv_cache_config|def llm_return_logprobs_test_harness|def .*stream.*interval)' \
tests/unittest/llmapi/test_llm.py
printf '%s\n' '--- direct imports and harness call sites ---'
rg -n --glob '*.py' 'from \.test_llm import|llama_model_path|llm_return_logprobs_test_harness' \
tests/unittest/llmapi tests/integration
printf '%s\n' '--- stream/chunked-prefill candidates ---'
rg -n --glob '*.py' -i 'stream.?interval|chunked.?prefill|streaming.*token|token.*increment|async.*stream' \
tests/unittest/llmapi tests/integrationRepository: NVIDIA/TensorRT-LLM
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -e
git diff --unified=8 -- tests/unittest/llmapi/test_llm.py
rg -n -C6 '^(default_model_name|llama_model_path|global_kvcache_config|global_kv_cache_config|def llm_return_logprobs_test_harness|def .*stream.*interval)' tests/unittest/llmapi/test_llm.py
rg -n --glob '*.py' 'from \.test_llm import|llama_model_path|llm_return_logprobs_test_harness' tests/unittest/llmapi
rg -n --glob '*.py' -i 'stream.?interval|chunked.?prefill|streaming.*token|token.*increment|async.*stream' tests/unittest/llmapi tests/integrationRepository: NVIDIA/TensorRT-LLM
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- diff ---'
git diff --unified=5 -- tests/unittest/llmapi/test_llm.py | head -240
printf '%s\n' '--- definitions ---'
rg -n -C5 'default_model_name|llama_model_path|global_kvcache_config|global_kv_cache_config|llm_return_logprobs_test_harness' tests/unittest/llmapi/test_llm.py
printf '%s\n' '--- stream interval references ---'
rg -n --glob '*.py' -i 'stream.?interval|chunked.?prefill|streaming.*token|token.*increment' tests/unittest/llmapi tests/integration || trueRepository: NVIDIA/TensorRT-LLM
Length of output: 48107
🏁 Script executed:
#!/bin/bash
set -e
python3 - <<'PY'
import ast
from pathlib import Path
files = [
Path("tests/unittest/llmapi/test_llm.py"),
Path("tests/unittest/llmapi/test_llm_pytorch.py"),
Path("tests/unittest/llmapi/test_llm_multi_gpu_pytorch.py"),
Path("tests/unittest/llmapi/test_llm_download.py"),
Path("tests/unittest/llmapi/test_llm_kv_cache_events.py"),
Path("tests/unittest/llmapi/apps/_test_openai_metrics.py"),
Path("tests/unittest/llmapi/apps/_test_llm_server.py"),
]
for path in files:
if not path.exists():
continue
tree = ast.parse(path.read_text(), filename=str(path))
parents = {}
for parent in ast.walk(tree):
for child in ast.iter_child_nodes(parent):
parents[child] = parent
print(f"--- {path} ---")
for node in ast.walk(tree):
if not isinstance(node, ast.Call):
continue
func = node.func
name = func.id if isinstance(func, ast.Name) else (
func.attr if isinstance(func, ast.Attribute) else "")
if name not in {"LLM", "TorchLLM", "PyTorchLLM"}:
continue
source = ast.get_source_segment(path.read_text(), node) or ""
has_kv = any(
isinstance(keyword, ast.keyword)
and keyword.arg == "kv_cache_config"
for keyword in node.keywords
)
print(f"{path}:{node.lineno}: {name}: explicit_kv_cache_config={has_kv}")
if not has_kv:
print(" ", " ".join(source.split())[:220])
PY
printf '%s\n' '--- relevant stream test bodies ---'
sed -n '280,380p' tests/unittest/llmapi/test_llm.py
sed -n '150,215p' tests/integration/defs/accuracy/test_llm_api_pytorch.py
printf '%s\n' '--- test-list references ---'
rg -n --glob '*.yaml' --glob '*.yml' --glob '*.json' \
'test_llm(_pytorch|_multi_gpu_pytorch)?|test_nvfp4_streaming|stream_interval' \
tests/integration/test_lists tests/unittest 2>/dev/null || trueRepository: NVIDIA/TensorRT-LLM
Length of output: 50376
Bound every shared 8B model loader and retain chunked-stream coverage.
tests/unittest/llmapi/test_llm.py:1057,tests/unittest/llmapi/test_llm_download.py:21, andtests/unittest/llmapi/apps/_test_llm_server.py:17instantiatellama_model_pathwithoutkv_cache_config. Use a bounded configuration or exclude non-collected tests from this audit.- Test coverage:
test_nvfp4_streamingtestsstream_interval, whileTestLlama3_1_8BInstruct.test_chunked_prefilldoes not stream. Add coverage that asserts token increments with both streaming and chunked prefill, and list it in the appropriate integration test list.
🤖 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/unittest/llmapi/test_llm.py` around lines 123 - 126, Update every
collected shared 8B model loader, including llama_model_path in the identified
test modules, to pass a bounded kv_cache_config; explicitly exclude any
non-collected tests only if they are outside the audit scope. Extend
TestLlama3_1_8BInstruct.test_chunked_prefill to validate token increments during
streaming as well as chunked prefill, and register the resulting coverage in the
appropriate integration test list while preserving existing stream_interval
coverage.
| f"{models_root}/llama-models-v2/TinyLlama-1.1B-Chat-v1.0": | ||
| f"{os.getcwd()}/TinyLlama/TinyLlama-1.1B-Chat-v1.0", | ||
| } | ||
| src_dst_dict = {} |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -i 'TinyLlama|tinyllama|create_link_for_models' . || trueRepository: NVIDIA/TensorRT-LLM
Length of output: 33044
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- tests/unittest/test_pip_install.py ---'
cat -n tests/unittest/test_pip_install.py | sed -n '190,250p'
printf '%s\n' '--- exact link/path references near pip-install sanity checks ---'
rg -n -i 'create_link_for_models|src_dst_dict|TinyLlama/TinyLlama|TinyLlama-1\.1B-Chat-v1\.0|quickstart|run_sanity_check' \
tests/unittest/test_pip_install.py tests/unittest tests/integration/defs/examples examples/llm-api \
--glob '*.py' --glob '*.sh' || true
printf '%s\n' '--- changed files and relevant diff summary ---'
git diff --stat
git diff -- tests/unittest/test_pip_install.py | sed -n '1,220p'Repository: NVIDIA/TensorRT-LLM
Length of output: 12957
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- quickstart model input ---'
cat -n examples/llm-api/quickstart_example.py | sed -n '1,80p'
printf '%s\n' '--- model-root and link-path definitions ---'
rg -n -C 4 'def llm_models_root|llm_models_root\(\)|os\.symlink|islink|TinyLlama/TinyLlama|TinyLlama-1\.1B-Chat-v1\.0' \
tests/unittest/test_pip_install.py examples/llm-api/quickstart_example.py \
tests/unittest/llmapi tests/integration/defs/examples \
--glob '*.py' || true
printf '%s\n' '--- repository state ---'
git status --short
git diff --name-statusRepository: NVIDIA/TensorRT-LLM
Length of output: 50375
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- previous model-link implementation ---'
if git rev-parse --verify HEAD^ >/dev/null 2>&1; then
git show HEAD^:tests/unittest/test_pip_install.py 2>/dev/null |
nl -ba | sed -n '205,230p' || true
else
echo 'No parent commit is available.'
fi
printf '%s\n' '--- current and parent quickstart references ---'
git grep -n -i -E 'TinyLlama|create_link_for_models|src_dst_dict' HEAD -- \
tests/unittest/test_pip_install.py examples/llm-api/quickstart_example.py || true
if git rev-parse --verify HEAD^ >/dev/null 2>&1; then
git grep -n -i -E 'TinyLlama|create_link_for_models|src_dst_dict' HEAD^ -- \
tests/unittest/test_pip_install.py examples/llm-api/quickstart_example.py || true
fiRepository: NVIDIA/TensorRT-LLM
Length of output: 1393
Restore the TinyLlama model link for the installation sanity check.
The quickstart uses the same TinyLlama/TinyLlama-1.1B-Chat-v1.0 path as the removed symlink destination. Without the link, it may download from Hugging Face and fail offline. Restore the mapping or use the local model path.
Test coverage: create_link_for_models() changed; no test functions were added or removed. run_sanity_check() still executes the quickstart. No integration test-list entry applies to this unit-test script.
🤖 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/unittest/test_pip_install.py` at line 217, Update the model-link
mapping in create_link_for_models, where src_dst_dict is initialized, to restore
an entry for TinyLlama/TinyLlama-1.1B-Chat-v1.0 pointing to the existing local
model path. Preserve the current run_sanity_check quickstart flow and use the
repository’s established source path rather than introducing a new download or
test change.
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== PR metadata and changed files ==='
gh pr view 17392 --repo NVIDIA/TensorRT-LLM --json title,baseRefName,headRefName,changedFiles,additions,deletions,files
echo '=== Diff summary ==='
git diff --stat "origin/$(gh pr view 17392 --repo NVIDIA/TensorRT-LLM --json baseRefName --jq .baseRefName)...HEAD"
echo '=== Full PR diff ==='
git diff --find-renames --find-copies --unified=80 "origin/$(gh pr view 17392 --repo NVIDIA/TensorRT-LLM --json baseRefName --jq .baseRefName)...HEAD"Length of output: 99400 🏁 Script executed: #!/bin/bash
set -euo pipefail
base_ref="$(gh pr view 17392 --repo NVIDIA/TensorRT-LLM --json baseRefName --jq .baseRefName)"
range="origin/${base_ref}...HEAD"
echo '=== Changed paths ==='
git diff --name-status "$range"
echo '=== Removed pytest test definitions ==='
git diff --unified=0 "$range" -- 'tests/**/*.py' \
| grep -E '^-([[:space:]]*def test_|[[:space:]]*async def test_)' \
|| true
echo '=== Current coverage for removed behavior and configuration ==='
rg -n -C 4 --glob 'tests/**/*.py' \
'stream_interval|enable_chunked_prefill|multi_lora|PeftCacheConfig|lora_config.*overrides|evict.*lora|tokenizer_decode_incrementally' \
tests
echo '=== Retired-model references still on the PR branch ==='
rg -n -i -C 2 --glob 'tests/**' \
'TinyLlama|llama-v2-7b|llama-7b-hf|CodeLlama-7b|Meta-Llama-3-8B|Llama-3\.2-1B' \
tests || true
echo '=== Changed exact-output and model-dependent assertions ==='
git diff --unified=25 "$range" -- 'tests/unittest/llmapi/test_llm.py' 'tests/unittest/llmapi/test_llm_pytorch.py' \
| grep -n -E -C 12 'default_model_name|llama_model_path|check_output|assert .*text|references|expected_.*output|embedding_bias|stop_words|logits_processor' \
|| trueLength of output: 201568 🏁 Script executed: #!/bin/bash
set -euo pipefail
base_ref="$(gh pr view 17392 --repo NVIDIA/TensorRT-LLM --json baseRefName --jq .baseRefName)"
range="origin/${base_ref}...HEAD"
echo '=== Compact changed-file summary ==='
git diff --numstat "$range" | awk '{print $1 "\t" $2 "\t" $3}'
echo '=== Removed test symbols only ==='
git diff --unified=0 "$range" -- 'tests/**/*.py' |
awk '
/^diff --git / { file=$4; sub("^b/", "", file) }
/^-[[:space:]]*(async[[:space:]]+)?def test_/ {
line=$0; sub(/^-/, "", line); gsub(/^[[:space:]]+/, "", line);
print file ": " line
}'
echo '=== Current model-output assertions in tests/unittest/llmapi/test_llm.py ==='
rg -n -C 5 \
'default_model_name|def test_llm_loading_from_hf|def test_generate_with_stop_words|def tinyllama_logits_processor_test_harness|D E F G H|Z Z Z|assert .*outputs' \
tests/unittest/llmapi/test_llm.py
echo '=== Current streaming tests with non-default stream_interval ==='
rg -n -U -C 8 --glob 'tests/**/*.py' \
'stream_interval\s*=\s*(?!1)|stream_interval\s*:\s*(?!1)' \
tests | head -n 400
echo '=== Direct current coverage for async streaming output chunk size ==='
rg -n -U -C 8 --glob 'tests/**/*.py' \
'generate_async\([\s\S]{0,500}streaming\s*=\s*True[\s\S]{0,1200}(len\(|token_ids|stream_interval)' \
tests/unittest/llmapi tests/integration/defs 2>/dev/null | head -n 400 || trueLength of output: 32812
Please replace these text-golden checks with one of the following:
I did not find a reason to retain the retired Llama model tests themselves. The replacement tests must preserve the behaviors in the table. I did not run GPU tests in this review environment.
|
| @@ -1,49 +0,0 @@ | |||
| # Disaggregated config that exercises the KV-cache bounce buffer. | |||
There was a problem hiding this comment.
This file is deleted here, but #17334 (Kimi K3 disagg) modifies it in the same window — both are open against main. Whichever lands second either conflicts or silently drops the other's intent. Worth syncing with @brnguyen2 on whether the bounce config stays and moves to a surviving model, or #17334 should stop depending on it.
There was a problem hiding this comment.
Kimi K3 is p0 model, I can keep the pr until #17334 is merged.
|
|
||
| @pytest.mark.parametrize("llama_model_root", ['TinyLlama-1.1B-Chat-v1.0'], | ||
| indirect=True) | ||
| def test_disaggregated_ngram(disaggregated_test_root, llm_venv, |
There was a problem hiding this comment.
Most of what's removed here has a DeepSeek-V3-Lite analog that survives (ctxpp2_gentp2, ctxtp2ep2pp2_gentp4, transceiver_runtime_python, overlap*, cache_aware_balance, conditional), so the prune reads as deliberate. But these look like they leave no surviving coverage at all, on any model:
test_disaggregated_ngram— ngram speculative decoding under disaggtest_disaggregated_load_balance— plain round-robin balancing (onlycache_aware_balancesurvives)test_disaggregated_perf_metricsandtest_disaggregated_kv_cache_time_outputtest_disaggregated_chat_completion_tool_callstest_disaggregated_python_transceiver_host_offload,..._bounce,..._fabric_memorytest_disaggregated_overlap_gen_first- the ctx/gen PP x TP topologies
ctxpp2_genpp2,ctxtp2_genpp2,ctxtp2pp2_gentp2pp2,ctxpp4_genpp4,ctxpp4_gentp4
These are feature and topology coverage that happened to run on TinyLlama rather than Llama coverage. Could you confirm each of those is either covered elsewhere or intentionally dropped? Anything in the second bucket would be better as a port onto DSv3-Lite in this PR than as a gap nobody notices until it regresses.
There was a problem hiding this comment.
should we use other models to cover these scenarios? for example gpt-oss-20b or qwen3-0.6b, because DSv3-Lite is also a obsoleted model
There was a problem hiding this comment.
Yes — that's the right shape, and @chuangz0's point is the same one. I'd split it by what the test is actually for:
Topology/mechanism coverage (ctxpp2_genpp2, ctxtp2_genpp2, ctxtp2pp2_gentp2pp2, ctxpp4_genpp4, ctxpp4_gentp4, multi_orchestrator, router, cuda_graph, mixed, genbs1, diff_max_tokens) is model-agnostic — Qwen3-0.6B is the cheapest home for it and porting is mostly a config swap.
Feature coverage is the part I'd not lose: ngram, load_balance, perf_metrics, kv_cache_time_output, chat_completion_tool_calls, python_transceiver_host_offload, ..._bounce, ..._fabric_memory, overlap_gen_first(_pp4), benchmark_gen_only(_insufficient_kv). Each of those is the only test exercising its code path, so dropping them removes the signal entirely rather than moving it.
llama4_long_context_kv_cache_overflow is the one that genuinely needs a big model, so it can't just move to Qwen3-0.6B.
No objection to the Llama-accuracy pruning itself — it's this second list I'd want landed somewhere before this merges.
|
The changes to the dis-agg testing logic are quite extensive, and I'd like to spend some more time reviewing them thoroughly. |
|
The tests in test_ray.py and the disaggregated-serving test suite are not TinyLlama accuracy tests. Many of them provide functional coverage and should be preserved by migrating them to a maintained model, such as Qwen3-0.6B or Llama 3.1 8B. |
Dev Engineer Review
QA Engineer Review
Test functions and classes were removed or modified across accuracy, disaggregated serving, E2E, KV-cache, LLM API, Ray, MPI, LoRA, sampler, and AutoDeploy tests. Retained tests that previously used TinyLlama now use Llama-3.1-8B-Instruct.
The affected CI and QA manifests include:
tests/integration/test_lists/qa/llm_function_core.txttests/integration/test_lists/qa/llm_spark_core.txttests/integration/test_lists/qa/llm_spark_func.ymltests/integration/test_lists/qa/llm_spark_perf.ymltests/integration/test_lists/test-db/l0_a10.ymltests/integration/test_lists/test-db/l0_a100.ymltests/integration/test_lists/test-db/l0_b200.ymltests/integration/test_lists/test-db/l0_dgx_b200.ymltests/integration/test_lists/test-db/l0_dgx_h100.ymltests/integration/test_lists/test-db/l0_dgx_h200.ymltests/integration/test_lists/test-db/l0_gb200_multi_gpus.ymltests/integration/test_lists/test-db/l0_h100.ymltests/integration/test_lists/test-db/l0_l40s.ymltests/integration/test_lists/test-db/l0_sanity_check.ymltests/integration/test_lists/waives.txtThe manifests remove obsolete tests and retain or add selected DeepSeek, Qwen3, Mixtral, and decoder-encoding coverage. The changed test functions appear to have corresponding manifest updates where applicable.
CBTS coverage data is unavailable. Verdict: needs follow-up.
Description
The background is https://docs.google.com/spreadsheets/d/17qLRr5oQESqn1rT9LrIUnpjvhH8wcC59273YEPT9k44/edit?gid=177162358#gid=177162358
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.