[None][test] Remove selected GPT-OSS V1 KV cache tests from CI - #17252
[None][test] Remove selected GPT-OSS V1 KV cache tests from CI#17252yizhang-nv wants to merge 4 commits into
Conversation
Signed-off-by: Yi Zhang <187001205+yizhang-nv@users.noreply.github.com>
WalkthroughThe change updates GPTOSS v1 KV-cache coverage across integration test lists. It removes selected CUTLASS and other configurations, adds or retains Triton and TRTLLM variants, and leaves GPTOSS v2 tests enabled. ChangesGPTOSS KV-cache tests
Estimated code review effort: 1 (Trivial) | ~2 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Two things worth knowing before this lands, since the description doesn't name a requester or a rationale. One of the two removals is a no-op. The other one is the last running V1 That matters right now because V1 is still the default KV cache manager for GPT-OSS on If the intent is "V2 is where GB200 is going, and #16942 lands first", that's reasonable — but it'd be worth saying so in the description and sequencing it after #16942, or moving the case to post-merge rather than dropping it. If the driver is stage runtime, that's also fine to say; it just isn't in the PR today. |
Signed-off-by: Yi Zhang <187001205+yizhang-nv@users.noreply.github.com>
|
The scope grew from 2 cases to 7 in The part that still gives me pause: V1 is still GPT-OSS's default KV cache manager on Suggest either sequencing this behind #16942, or moving the V1 entries to post-merge rather than deleting them, so the default path keeps some GB200 coverage until the flip lands. Two of the entries ( |
Signed-off-by: Yi Zhang <187001205+yizhang-nv@users.noreply.github.com>
|
Re-reviewed at The two waiver removals unwaive tests that still run on other boards, against bugs that are still open.
So after this merges, both start running on DGX_H100 pre-merge (and dp4 on DGX_B200) with no waiver and no fix. The sibling If the intent was to drop waivers that no longer point at anything, that only holds for a test removed from every list. Either keep both The sequencing point from my earlier comment still stands and is now sharper. #16942 (opt GPT-OSS in to KV cache manager V2 by default) is still unmerged, so V1 remains GPT-OSS's default on Happy to re-look once the waiver lines are restored or the other lists are squared up. |
Signed-off-by: Yi Zhang <187001205+yizhang-nv@users.noreply.github.com>
|
@BowenFu Addressed the waiver consistency issue in
Could you please take another look? |
|
/bot skip --comment "trivial changes" |
|
PR_Github #64005 [ skip ] triggered by Bot. Commit: |
|
PR_Github #64005 [ skip ] completed with state |
BowenFu
left a comment
There was a problem hiding this comment.
Thanks for taking the waiver point seriously — that closes it.
I re-checked mechanically at 8fa9ac72f: I pulled all 69 files under tests/integration/test_lists/ at this head and grepped for both v1_kv_cache-dp4-cutlass-auto and v1_kv_cache-ep4-cutlass-auto. Zero hits. So removing their waives.txt entries can no longer schedule anything against nvbugs 6388142 / 5596343, which was my only blocker.
Two notes, neither blocking:
- The default path keeps pre-merge signal — GPT-OSS still resolves
"auto"to V1 (no V2 override inmodeling_gpt_oss.py, and #16942 is unmerged), and runnable V1 variants remain on DGX H100, B200, DGX B300 and GB300. What this actually does is remove V1 coverage on GB200 specifically, which matches the stated intent; it just isn't quite what "remove selected tests" implies. /bot skipon the last commit is fine here: Pre-commit still ran on the exact final SHA, and that's what enforceswaives.txtsorting, duplicate-waiver detection and AST test-ID validation — the failure modes a list-only diff could actually have.
brnguyen2
left a comment
There was a problem hiding this comment.
Approving — the comments below are optional touch-ups, not blockers.
Checked the removals against the rest of the tree: after this PR neither test_w4_4gpus[v1_kv_cache-dp4-cutlass-auto] nor [v1_kv_cache-ep4-cutlass-auto] is referenced by any file under tests/integration/test_lists/, so the two waive deletions are pure dead-line cleanup and don't silently re-enable anything. The GB200 list keeps a matching V2 entry for each removed V1 case.
One bookkeeping item, not a blocker: the bugs behind the deleted waives. 6388142 now has no reference anywhere in the repo — it should be closed or updated to note the test was removed, otherwise CI triage will regenerate the waive line. 5596343 still backs five other GPT-OSS waives, so it stays open regardless.
| accuracy/test_llm_api_pytorch.py::TestGPTOSS::test_w4_1gpu[v1_kv_cache-True-True-triton-auto] SKIP (https://nvbugs/6026676) | ||
| accuracy/test_llm_api_pytorch.py::TestGPTOSS::test_w4_4gpus[v1_kv_cache-dp4-cutlass-auto] SKIP (https://nvbugs/6388142) | ||
| accuracy/test_llm_api_pytorch.py::TestGPTOSS::test_w4_4gpus[v1_kv_cache-ep4-cutlass-auto] SKIP (https://nvbugs/5596343) | ||
| accuracy/test_llm_api_pytorch.py::TestGPTOSS::test_w4_4gpus[v1_kv_cache-tp4-cutlass-auto] SKIP (https://nvbugs/5596343) |
There was a problem hiding this comment.
After this PR https://nvbugs/6388142 has zero references left in the repo. Worth closing it (or commenting "test removed from all lists") so a future triage pass doesn't re-add the waive. 5596343 is fine — it still backs the tp4-cutlass-auto and V2 entries below.
@coderabbitai summary
Description
Remove all seven GPT-OSS V1 KV cache entries from the GB200 multi-GPU test list across its pre-merge and post-merge sections:
test_w4_4gpusconfigurations covering TP4, DP4, and EP4test_eagle3_4gpusconfigurations covering CUTLASS and TRTLLM MoE backendsThe corresponding V2 entries remain scheduled, and the parametrized test definitions remain unchanged.
This test-list cleanup is independent of #16942. Other GPT-OSS V1 configurations remain scheduled on DGX H100, DGX B200, and QA lists.
Also remove these two V1 cases from all remaining DGX H100, DGX B200, and QA test lists:
test_w4_4gpus[v1_kv_cache-dp4-cutlass-auto](NVBug 6388142)test_w4_4gpus[v1_kv_cache-ep4-cutlass-auto](NVBug 5596343)Their global waivers are removed after the test-list references are gone.
Test Coverage
git diff --checkNo runtime tests were run because this is a test-list-only change.
PR Checklist
GitHub Bot Help
To see a list of available CI bot commands, please comment
/bot help.