[TRTLLM-13696][test] Part2.4: Migrate CPU only tests - others - #17339
[TRTLLM-13696][test] Part2.4: Migrate CPU only tests - others#17339tongyuantongyu wants to merge 3 commits into
Conversation
|
/bot run --stage-list "CPU-Generic-x86-1, CPU-Generic-arm-1" --disable-fail-fast |
|
PR_Github #64218 [ run ] triggered by Bot. Commit: |
4cd04e1 to
c1f6428
Compare
|
PR_Github #64218 [ run ] completed with state |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (59)
💤 Files with no reviewable changes (7)
🚧 Files skipped from review as they are similar to previous changes (52)
WalkthroughThe pull request classifies compatible tests as CPU-only, makes MLA capability checks CPU-safe, and updates hardware-specific integration test lists. CPU coverage expands, while selected GPU-specific test entries are removed or replaced. ChangesCPU test execution
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
/bot run --disable-fail-fast |
|
PR_Github #64293 [ run ] triggered by Bot. Commit: |
BowenFu
left a comment
There was a problem hiding this comment.
Checked the one thing that can actually go wrong here: all 31 entries removed from the GPU stage lists are covered by l0_cpu.yml at this head — unittest/_torch/executor and unittest/_torch/modules bridge the test_kv_cache_*/test_moe_host_sharer/test_cute_dsl_b12x_moe_backend removals, unittest/usage and unittest/dynamo the rest — so this is a move, not a drop, and nothing falls out of every stage.
Also confirmed the newly-marked scopes are genuinely GPU-free rather than GPU tests hidden behind a marker: the mixed files use narrow class/method markers and leave the real CUDA cases unmarked. The cuda_capability hoist in test_attention_mla.py reads the same get_device_capability() on a GPU box, so tokens_per_block and the FP8 parameterization are unchanged there.
Not blocking, just noting: that hoist caches capability at import time. Fine today since nothing in the file switches device, but it is now an assumption the file depends on.
DomBrown
left a comment
There was a problem hiding this comment.
Approving from API committee perspective
WeiHaocheng
left a comment
There was a problem hiding this comment.
LGTM for scaffolding part.
|
PR_Github #64293 [ run ] completed with state
|
brnguyen2
left a comment
There was a problem hiding this comment.
Approving — the comments below are optional touch-ups, not blockers.
Mechanically clean, and the marker/test-list bookkeeping lines up on the files I spot-checked. I verified that the fully-marked files really are GPU-free (kv-cache estimation/rebalance/dual-pool mock torch.cuda.*, test_visual_gen_multinode.py patches set_device, TestSequenceSharderDistributed uses gloo), and that no GPU list is left with a directory entry that would now collect zero tests — weight_sharing and api_stability both keep unmarked files.
One gap worth closing before merge: three files get pytestmark = pytest.mark.cpu_only but appear in no test list at all, so they still never run — and the marker now also locks them out of any future GPU-stage directory entry. Details inline.
Since GPU stages run with -m "not cpu_only", the marker alone is enough to drop a test from GPU; the list deletions are cosmetic, so the two halves of this change can't drift out of sync in a dangerous direction. Good.
Test-only, no docs or release notes owed.
Signed-off-by: Yuan Tong <13075180+tongyuantongyu@users.noreply.github.com>
Signed-off-by: Yuan Tong <13075180+tongyuantongyu@users.noreply.github.com>
Signed-off-by: Yuan Tong <13075180+tongyuantongyu@users.noreply.github.com>
c1f6428 to
19e3f6b
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
/bot run --disable-fail-fast |
|
PR_Github #64937 [ run ] triggered by Bot. Commit: |
|
PR_Github #64937 [ run ] completed with state |
Dev Engineer Review
pytest.mark.cpu_onlyat module or class scope.test_attention_mla.py.l0_cpu.yml.QA Engineer Review
tests/integration/test_lists/test-db/l0_cpu.yml.Description
Move the rest of test cases don't need GPU to CPU stage.
Test Coverage
The CPU stage.
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.