Skip to content

[None][test] Remove selected GPT-OSS V1 KV cache tests from CI - #17252

Open
yizhang-nv wants to merge 4 commits into
NVIDIA:mainfrom
yizhang-nv:codex/remove-gptoss-v1-gb200-tests
Open

[None][test] Remove selected GPT-OSS V1 KV cache tests from CI#17252
yizhang-nv wants to merge 4 commits into
NVIDIA:mainfrom
yizhang-nv:codex/remove-gptoss-v1-gb200-tests

Conversation

@yizhang-nv

@yizhang-nv yizhang-nv commented Aug 4, 2026

Copy link
Copy Markdown
Member

@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:

  • Five test_w4_4gpus configurations covering TP4, DP4, and EP4
  • Two test_eagle3_4gpus configurations covering CUTLASS and TRTLLM MoE backends

The 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 --check
  • Pre-commit hooks, including YAML validation and test-list source validation

No runtime tests were run because this is a test-list-only change.

PR Checklist

  • 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: Yi Zhang <187001205+yizhang-nv@users.noreply.github.com>
@yizhang-nv
yizhang-nv requested a review from a team as a code owner August 4, 2026 10:09
@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

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

Changes

GPTOSS KV-cache tests

Layer / File(s) Summary
Remove GPTOSS v1 KV-cache declarations
tests/integration/test_lists/test-db/l0_gb200_multi_gpus.yml, tests/integration/test_lists/test-db/l0_dgx_b200.yml, tests/integration/test_lists/waives.txt
Removes selected pre-merge, post-merge, and waived GPTOSS v1 configurations. GPTOSS v2 tests remain enabled.
Replace CUTLASS coverage
tests/integration/test_lists/qa/llm_function_core.txt, tests/integration/test_lists/qa/llm_function_rtx6k.txt, tests/integration/test_lists/test-db/l0_dgx_h100.yml
Removes selected CUTLASS cases and retains or adds DP4 Triton, DP4 TRTLLM, TRTLLM FP8, and EP4 Triton cases.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Possibly related PRs

Suggested reviewers: brnguyen2, niukuo, bowenfu

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and concisely describes the removal of selected GPT-OSS V1 KV cache tests from CI.
Description check ✅ Passed The description explains the scope, rationale, affected test lists, validation steps, and lack of runtime testing.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@BowenFu

BowenFu commented Aug 4, 2026

Copy link
Copy Markdown

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. test_w4_4gpus[v1_kv_cache-dp4-cutlass-auto] is already globally waived at tests/integration/test_lists/waives.txt:65 (SKIP https://nvbugs/6388142), so it does not run on GB200 today. The material change here is the other line.

The other one is the last running V1 -auto case on that stage. In the pre_merge block of l0_gb200_multi_gpus.yml the V1 entries are tp4-trtllm-auto, dp4-cutlass-auto, ep4-cutlass-auto and dp4-trtllm-fp8. Two of those are waived (waives.txt:65-66), so what actually executes is tp4-trtllm-auto and dp4-trtllm-fp8. Removing tp4-trtllm-auto leaves dp4-trtllm-fp8 as the only running V1 case — a different parallelism mode and a different quantization, so it isn't substitutable coverage.

That matters right now because V1 is still the default KV cache manager for GPT-OSS on main#16942, which opts GPT-OSS into V2 by default, is still open. So this drops GB200 pre-merge signal on the default path while retaining the not-yet-default one. The V1 entries in l0_dgx_b200.yml and l0_dgx_h100.yml still exist, but that is other hardware.

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.

Comment thread tests/integration/test_lists/test-db/l0_gb200_multi_gpus.yml Outdated
Comment thread tests/integration/test_lists/test-db/l0_gb200_multi_gpus.yml Outdated
Signed-off-by: Yi Zhang <187001205+yizhang-nv@users.noreply.github.com>
@yizhang-nv yizhang-nv changed the title [None][test] Remove two GPT-OSS tests from GB200 pre-merge [None][test] Remove GPT-OSS V1 KV cache tests from GB200 Aug 5, 2026
@BowenFu

BowenFu commented Aug 5, 2026

Copy link
Copy Markdown

The scope grew from 2 cases to 7 in 85e46fc7f, which turns my earlier point from "one case" into the whole axis: after this commit no v1_kv_cache GPT-OSS case runs on GB200 pre-merge at alltp4-trtllm-auto, dp4-trtllm-auto, dp4-trtllm-fp8, ep4-cutlass-auto, dp4-cutlass-auto, plus both test_eagle3_4gpus[v1_kv_cache-*] variants are gone.

The part that still gives me pause: V1 is still GPT-OSS's default KV cache manager on main#16942 ("Opt GPT-OSS in to KV cache manager V2 by default") is open and unmerged. So as it stands this removes the GB200 pre-merge signal on the default path, and keeps only the not-yet-default V2 path.

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 (dp4-cutlass-auto, ep4-cutlass-auto) are already globally waived in waives.txt, so removing those is a no-op either way — it's the three running ones plus the two eagle3 cases that carry the signal.

Comment thread tests/integration/test_lists/test-db/l0_gb200_multi_gpus.yml
Signed-off-by: Yi Zhang <187001205+yizhang-nv@users.noreply.github.com>
@BowenFu

BowenFu commented Aug 5, 2026

Copy link
Copy Markdown

Re-reviewed at 0057ae48d. The new commit ("Remove GPT-OSS V1 test waivers") introduces a problem the earlier heads didn't have.

The two waiver removals unwaive tests that still run on other boards, against bugs that are still open.

waives.txt is global, but this PR only removes the entries from l0_gb200_multi_gpus.yml. Both tests whose waivers you delete are still scheduled elsewhere on main:

test_w4_4gpus[v1_kv_cache-dp4-cutlass-auto] — waived under https://nvbugs/6388142, still listed in

  • l0_dgx_h100.yml:256
  • l0_dgx_b200.yml:328
  • qa/llm_function_core.txt:508, qa/llm_function_rtx6k.txt

test_w4_4gpus[v1_kv_cache-ep4-cutlass-auto] — waived under https://nvbugs/5596343, still listed in

  • l0_dgx_h100.yml:254
  • qa/llm_function_core.txt:512, qa/llm_function_rtx6k.txt

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 test_w4_4gpus[v1_kv_cache-tp4-cutlass-auto] SKIP (https://nvbugs/5596343) stays in the file, which is direct evidence that 5596343 is still live — so the ep4 variant is being unwaived while the tp4 variant under the identical bug is not.

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 waives.txt lines, or remove the tests from the H100/B200/QA lists too — but the second is a much bigger change than this PR's title suggests.

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 main. This PR removes 7 v1_kv_cache entries from GB200 pre-merge — including both test_eagle3_4gpus V1 variants — leaving pre-merge coverage only on the configuration that is not the default yet. Landing #16942 first makes this straightforwardly correct; landing this first deletes the default path's signal for however long the gap is.

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>
@yizhang-nv yizhang-nv changed the title [None][test] Remove GPT-OSS V1 KV cache tests from GB200 [None][test] Remove selected GPT-OSS V1 KV cache tests from CI Aug 5, 2026
@yizhang-nv

yizhang-nv commented Aug 5, 2026

Copy link
Copy Markdown
Member Author

@BowenFu Addressed the waiver consistency issue in 8fa9ac72ff.

  • Removed v1_kv_cache-dp4-cutlass-auto and v1_kv_cache-ep4-cutlass-auto from the remaining DGX H100, DGX B200, llm_function_core, and llm_function_rtx6k test lists. Removing their waivers therefore no longer enables scheduled cases elsewhere.
  • This cleanup is intentionally independent of [None][feat] Opt GPT-OSS in to KV cache manager V2 by default #16942. Other GPT-OSS V1 configurations remain scheduled on DGX H100, DGX B200, and QA lists.

Could you please take another look?

@QiJune QiJune left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@QiJune

QiJune commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

/bot skip --comment "trivial changes"

@QiJune
QiJune enabled auto-merge (squash) August 5, 2026 08:37
@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #64005 [ skip ] triggered by Bot. Commit: 8fa9ac7 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #64005 [ skip ] completed with state SUCCESS. Commit: 8fa9ac7
Skipping testing for commit 8fa9ac7

Link to invocation

@BowenFu BowenFu left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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 in modeling_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 skip on the last commit is fine here: Pre-commit still ran on the exact final SHA, and that's what enforces waives.txt sorting, duplicate-waiver detection and AST test-ID validation — the failure modes a list-only diff could actually have.

@brnguyen2 brnguyen2 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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.

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.

5 participants