[None][fix] Clean up Triton related tests - #13163
Conversation
|
/bot run --disable-fail-fast |
📝 WalkthroughWalkthroughThis pull request removes a significant number of Triton integration tests from both test definitions and test manifests, while also replacing a few tests with new variants. Changes include deletion of 23 test functions and restructuring of test configurations across multiple test files and test-list registries. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 the current code and only fix it if needed.
Inline comments:
In `@tests/integration/defs/triton_server/test_triton_llm.py`:
- Around line 2034-2037: The test contains an in-test workaround that invokes
pip ("pip3 install -U nvidia-cublas-cu12") based on query_gpu_name(), which
mutates the shared Python environment and requires network access; remove that
conditional pip install from the test body (the code block referencing
query_gpu_name() and the pip call) and instead apply the workaround outside the
test runtime — either bake nvidia-cublas-cu12 into the runner/CI image or move
installation into an isolated setup step that launches Triton (e.g., a dedicated
session/module pytest fixture or the Triton start script that runs once per
job), ensuring no runtime pip installs occur inside tests like
test_triton_llm.py and that query_gpu_name() is only used for skips/assumptions,
not environment mutation.
🪄 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: Pro Plus
Run ID: d4825b79-3f87-40c5-96b1-7469ff56beff
📒 Files selected for processing (7)
tests/integration/defs/triton_server/test_triton.pytests/integration/defs/triton_server/test_triton_llm.pytests/integration/test_lists/qa/llm_triton_integration.txttests/integration/test_lists/test-db/l0_a100.ymltests/integration/test_lists/test-db/l0_a30.ymltests/integration/test_lists/test-db/l0_b200.ymltests/integration/test_lists/waives.txt
💤 Files with no reviewable changes (5)
- tests/integration/test_lists/test-db/l0_b200.yml
- tests/integration/test_lists/test-db/l0_a100.yml
- tests/integration/test_lists/waives.txt
- tests/integration/test_lists/qa/llm_triton_integration.txt
- tests/integration/defs/triton_server/test_triton.py
|
PR_Github #44072 [ run ] triggered by Bot. Commit: |
|
PR_Github #44072 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #44409 [ run ] triggered by Bot. Commit: |
|
PR_Github #44409 [ run ] completed with state
|
937eca9 to
26f8481
Compare
|
/bot run --disable-fail-fast |
|
PR_Github #44825 [ run ] triggered by Bot. Commit: |
|
PR_Github #44825 [ run ] completed with state
|
…usa coverage Drops waived triton_server test entries from QA and l0 lists, then removes the corresponding multimodal, speculative-decoding-BLS, disaggregated-BLS, and medusa test functions that are no longer referenced by any active list. Keeps t5 and llmapi coverage. Signed-off-by: Iman Tabrizian <10105175+tabrizian@users.noreply.github.com>
Remove triton_server waives whose test functions were deleted in the prior commit, plus specific parametrizations that no longer appear in any active test list. Signed-off-by: Iman Tabrizian <10105175+tabrizian@users.noreply.github.com>
After removing the remaining triton_server entries, the post_merge triton-backend blocks in l0_a100.yml and l0_b200.yml were empty. Remove them entirely. Signed-off-by: Iman Tabrizian <10105175+tabrizian@users.noreply.github.com>
26f8481 to
ec7da0b
Compare
|
/bot run --disable-fail-fast |
|
PR_Github #44900 [ run ] triggered by Bot. Commit: |
|
PR_Github #44900 [ run ] completed with state
|
| @pytest.mark.parametrize("MAX_BEAM_WIDTH", ["1"]) | ||
| @pytest.mark.parametrize("EXCLUDE_INPUT_IN_OUTPUT", ["False"]) | ||
| @pytest.mark.parametrize("model_setup", | ||
| list(MODEL_CONFIGS.keys()), |
There was a problem hiding this comment.
Looks like tests are failing because MODEL_CONFIGS no longer exist.
|
@Tabrizian TRTLLM is migrating QA role to L0 post-merge, could you please pick necessary test cases from QA test list into L0 post-merge coverage? Eventually QA test list will be depricated. cc @schetlur-nv for awareness. |
The triton test cleanup accidentally removed the MODEL_CONFIGS dict, the model_setup fixture, and the LATENCY_THRESHOLDS dict while test_benchmark_core_model (still present and still listed in l0_a30.yml) references all three. This caused a NameError at collection time, failing the whole module. Restore the three definitions. Signed-off-by: Iman Tabrizian <10105175+tabrizian@users.noreply.github.com>
Following the removal of the multimodal, speculative-decoding-BLS, and disaggregated-serving-BLS triton tests, delete the now-unused helper code: - build_engines.py: prepare_blip2_opt_engine, prepare_llava_engine, prepare_llava_onevision_engine, prepare_mllama_engine, prepare_mistral3_pixtral_engine - common.py: prepare_multimodal_model_repo, prepare_disaggregated_serving_model_repo, modify_disaggregated_serving_config_pbtxt, and the multimodal / disaggregated_serving_bls branches inside modify_ib_config_pbtxt - conftest.py: multimodal/mllama example-root and llava/llava_onevision/ blip2/mllama/test_video model-root fixtures All removed symbols were verified to have no remaining references. Signed-off-by: Iman Tabrizian <10105175+tabrizian@users.noreply.github.com>
QA coverage is being migrated to L0 post-merge and the QA test list (tests/integration/test_lists/qa/llm_triton_integration.txt) will be deprecated. Promote all still-valid QA triton cases into the a30 post_merge triton section (505 cases not already present). Hardware-gated cases (8-GPU 175b/70b, the 80GB return-logits case) are gated at runtime by their own skip_less_device/skip_less_device_memory markers, consistent with the cases already in this section. The multi-node case (test_triton_multi_node.py) is excluded as it needs dedicated multi-node orchestration that a single-node condition cannot express. Signed-off-by: Iman Tabrizian <10105175+tabrizian@users.noreply.github.com>
|
/bot run --disable-fail-fast |
…iton-server-tests Signed-off-by: Iman Tabrizian <10105175+tabrizian@users.noreply.github.com> # Conflicts: # tests/integration/defs/triton_server/conftest.py # tests/integration/test_lists/qa/llm_triton_integration.txt # tests/integration/test_lists/test-db/l0_a30.yml # tests/integration/test_lists/waives.txt
|
/bot run --disable-fail-fast |
|
PR_Github #56751 [ run ] triggered by Bot. Commit: |
|
PR_Github #56751 [ run ] completed with state
|
main removed prepare_llama_v2_7b_engine and trimmed MODEL_CONFIGS to gptj_6b only, so test_benchmark_core_model no longer has a llama_v2_7b parametrization. Remove the now-invalid l0_a30 entry that the CI "Check Test List" stage flagged as an invalid test name filter. Signed-off-by: Iman Tabrizian <10105175+tabrizian@users.noreply.github.com>
|
/bot run --disable-fail-fast |
|
PR_Github #56780 [ run ] triggered by Bot. Commit: |
This PR removes all triton post-merge tests from l0_a100.yml and l0_b200.yml (they only covered multimodal / disaggregated-serving-BLS tests, which are deleted). The DGX_A100-Triton-Post-Merge-1/2 and DGX_B200-Triton-Post-Merge-1 stages in L0_Test.groovy still point at those lists, so they collect zero tests and fail with pytest exit code 5. Remove the now-empty stage definitions. Single-GPU triton coverage remains on the A30-Triton-Post-Merge stages (l0_a30) and the DGX-H200 stage keeps its llmapi_backend test. Signed-off-by: Iman Tabrizian <10105175+tabrizian@users.noreply.github.com>
The A30-Triton-Post-Merge stages surfaced runtime failures in four functions migrated from the QA list, deterministic across both shards: test_gpt_350m_speculative_decoding, test_gpt_next_ptuning_ifb, test_mistral_v1_multi_models, and test_rcca_bug_4934893 (the latter is tracked by nvbugs/5619369). These legacy tests do not pass in the L0 a30 environment, so exclude them from the migration; the remaining migrated triton tests pass. They can be re-added once fixed/triaged separately. Signed-off-by: Iman Tabrizian <10105175+tabrizian@users.noreply.github.com>
|
/bot run --disable-fail-fast |
|
PR_Github #56849 [ run ] triggered by Bot. Commit: |
|
PR_Github #56780 [ run ] completed with state |
|
PR_Github #56849 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #56963 [ run ] triggered by Bot. Commit: |
|
PR_Github #56963 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
Per the QA->L0 migration rule, move all QA triton tests to l0_a30 post-merge except multi-model (test_mistral_v1_multi_models) and speculative-decoding (test_gpt_350m_speculative_decoding[_return_logits]) tests. test_gpt_next_ptuning_ifb is dropped (fails on a30). Re-add the test_rcca_bug_4934893 cases and waive all 8 param combos under nvbugs/5619369 (known-failing RCCA), and drop the spec-dec return_logits entries that remained. Signed-off-by: Iman Tabrizian <10105175+tabrizian@users.noreply.github.com>
|
/bot run --disable-fail-fast |
…iton-server-tests Signed-off-by: Iman Tabrizian <10105175+tabrizian@users.noreply.github.com> # Conflicts: # jenkins/L0_Test.groovy
|
/bot run --disable-fail-fast |
2 similar comments
|
/bot run --disable-fail-fast |
|
/bot run --disable-fail-fast |
|
Closing, since this was already completed here: #15814 (comment) |
Description
Cleans up the Triton-server integration tests and migrates QA coverage to L0 post-merge:
test_triton.py/test_triton_llm.py, along with their entries in the test manifests and waives.prepare_blip2_opt_engine,prepare_llava_engine,prepare_llava_onevision_engine,prepare_mllama_engine,prepare_mistral3_pixtral_engine), theprepare_multimodal_model_repo/prepare_disaggregated_serving_model_repo/modify_disaggregated_serving_config_pbtxthelpers, the multimodal &disaggregated_serving_blsbranches insidemodify_ib_config_pbtxt, and the multimodal example-root / model-root fixtures. All removed symbols were verified to have no remaining references.test_benchmark_core_modeldependencies: the cleanup had accidentally removedMODEL_CONFIGS, themodel_setupfixture, andLATENCY_THRESHOLDS, whichtest_benchmark_core_modelstill references — this caused aNameErrorat collection time. These are restored.tests/integration/test_lists/qa/llm_triton_integration.txtinto thel0_a30post_merge/tritonsection (505 cases not already present), since the QA list is being deprecated. Hardware-gated cases (8-GPU 175b/70b, 80 GB return-logits) are gated by their ownskip_less_device/skip_less_device_memorymarkers, consistent with existing cases in that section. The multi-node case is excluded (needs dedicated multi-node orchestration).Test Coverage
This PR only touches the Triton integration tests and their manifests; it adds no product code. Coverage is validated by the standard CI test-list validation (collection + manifest consistency) and by the migrated
l0_a30post_merge/tritoncases.PR Checklist
GitHub Bot Help
To see a list of available CI bot commands, please comment
/bot help.