Skip to content

[None][test] fix perf test cases issue of incorrect match#12096

Open
ruodil wants to merge 1 commit intoNVIDIA:mainfrom
ruodil:user/ruodil/new_feature
Open

[None][test] fix perf test cases issue of incorrect match#12096
ruodil wants to merge 1 commit intoNVIDIA:mainfrom
ruodil:user/ruodil/new_feature

Conversation

@ruodil
Copy link
Collaborator

@ruodil ruodil commented Mar 11, 2026

Summary by CodeRabbit

  • Tests
    • Expanded performance testing infrastructure with new multi-GPU test configurations for 8-GPU benchmark scenarios.
    • Reorganized and realigned test entries across different GPU types and hardware configurations for improved consistency.
    • Enhanced test coverage for comprehensive performance benchmarking validation across multiple system configurations.

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)

  • 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.

Signed-off-by: Ruodi Lu <ruodil@users.noreply.github.com>
@ruodil ruodil requested a review from yufeiwu-nv March 11, 2026 05:39
@ruodil ruodil requested a review from a team as a code owner March 11, 2026 05:39
@ruodil
Copy link
Collaborator Author

ruodil commented Mar 11, 2026

/bot skip --comment "skip test as just modifying cases"

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 11, 2026

📝 Walkthrough

Walkthrough

Modifies test configuration to add multi-GPU test blocks for 8-GPU systems (L40S, H100, H20, H200), adds a new test entry for qwen model under RTX-6000 Server section, and removes a duplicate test entry while reintroducing an aligned 8-GPU variant block with compute capability conditions.

Changes

Cohort / File(s) Summary
GPU Test Configuration
tests/integration/test_lists/qa/llm_perf_core.yml
Adds new multi-GPU test block for 8 GPUs with system_gpu_count >= 8 and compute_capability 8.0–9.0 conditions; adds separate qwen3_235b_a22b_fp8 test entry for RTX-6000 Server section; removes existing duplicate qwen3_235b_a22b_fp8 entry; reorganizes 8-GPU block alignment in L40S/H100/H20/H200 section.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is entirely empty—all template sections including Description, Test Coverage, and PR Checklist are left blank or unchanged from the template. Provide a detailed description explaining what issue is being fixed, the solution implemented, relevant test cases, and confirm the PR checklist items have been reviewed.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title '[None][test] fix perf test cases issue of incorrect match' clearly identifies a test fix related to perf test case matching issues, which aligns with the changeset that modifies test configuration.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Comment @coderabbitai help to get the list of available commands and usage tips.

@tensorrt-cicd
Copy link
Collaborator

PR_Github #38539 [ skip ] triggered by Bot. Commit: c4905ec Link to invocation

system_gpu_count:
gte: 8
compute_capability:
gt: 8.0
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
gt: 8.0
gte: 9.0

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe remove 8XL40S because it's low-priority and 8XGPU is SYS topo which lead to unstable

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if remove 8XL40S, maybe you can merge 3b into 4

# 2: L20, L40S, H100, H20, H200
# 3: L40S, H100, H20, H200
# 3: L40S, H100, H20, H200 (4 GPUs)
# 3b: L40S, H100, H20, H200 (8 GPUs)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# 3b: L40S, H100, H20, H200 (8 GPUs)
# 3b: H100, H20, H200 (8 GPUs)

- perf/test_perf.py::test_perf[llama_v3.3_70b_instruct_fp8-bench-pytorch-float8-input_output_len:512,32-gpus:4] #llama_v3.3_70b_instruct_fp8


# 3b: L40S, H100, H20, H200 (8 GPUs)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# 3b: L40S, H100, H20, H200 (8 GPUs)
# 3b: H100, H20, H200 (8 GPUs)

@tensorrt-cicd
Copy link
Collaborator

PR_Github #38539 [ skip ] completed with state SUCCESS. Commit: c4905ec
Skipping testing for commit c4905ec

Link to invocation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants