[https://nvbugs/6544407][fix] unwaive glm5 stress test - #17276
Conversation
Signed-off-by: Chuang Zhu <111838961+chuangz0@users.noreply.github.com>
WalkthroughThe integration waive list removes the skip entry for the B200 GLM5 NVFP4 TP4/EP4/DP stress test. Changes
Estimated code review effort: 1 (Trivial) | ~2 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
|
/bot run |
|
PR_Github #63916 [ run ] triggered by Bot. Commit: |
brnguyen2
left a comment
There was a problem hiding this comment.
The deletion is correct and complete — the removed entry matches the pytest node ID exactly, and no duplicate or leftover waiver for this test exists elsewhere in test_lists.
The gap is evidence. This test is listed only in tests/integration/test_lists/qa/llm_function_stress.txt:8, not in any test-db/l0_*.yml, so /bot run on this PR will not execute it — nothing in CI can confirm the unwaive. Combined with what the test does (tests/integration/defs/disaggregated/test_disaggregated.py:2807: 8×B200, 35k requests at concurrency 512, 0.90 accuracy gate, 10% request cancellation), "unable to reproduce the error on latest main" is thin: that profile fails intermittently, which is usually why it gets waived in the first place.
Before merging, please add to the description: how many runs of disaggregated/test_disaggregated.py::test_disaggregated_stress_test[input8k-output1k-conc512-glm5_nvfp4_tp4_ep4_dp_stress] you did on main, and a link to at least one passing run. Also state the disposition of https://nvbugs/6544407 — if the failure is genuinely gone the bug should be closed (or closed as not-repro) in the same pass, otherwise the waiver and the bug drift apart and the next person hitting it has no trail. If the root cause was actually fixed by a specific commit, naming it would make this a one-line approve.
| full:B200/accuracy/test_llm_api_pytorch.py::TestQwen3_30B_A3B::test_dummy_load_format SKIP (https://nvbugs/6525059) | ||
| full:B200/disaggregated/test_disaggregated.py::test_disaggregated_stress_test[input8k-output1k-conc512-deepseek_r1_v2_fp4_stress] SKIP (https://nvbugs/6472256) | ||
| full:B200/disaggregated/test_disaggregated.py::test_disaggregated_stress_test[input8k-output1k-conc512-glm5_nvfp4_tp4_ep4_dp_stress] SKIP (https://nvbugs/6544407) | ||
| full:B200/disaggregated/test_disaggregated.py::test_disaggregated_stress_test[input8k-output1k-conc512-qwen3_32b_fp8_stress] SKIP (https://nvbugs/6472256) |
There was a problem hiding this comment.
Worth noting for whoever reviews the QA side: the two sibling stress waivers here (deepseek_r1_v2_fp4_stress and qwen3_32b_fp8_stress, both https://nvbugs/6472256) stay in place, so this unwaive is genuinely scoped to the GLM5 case — good. But since none of these run in pre-merge CI, the first signal that the GLM5 unwaive was premature will be a red QA stress campaign. If you're not confident the underlying issue is fixed (vs. just not reproducing), consider holding this until a couple of clean QA stress runs land rather than using the campaign as the experiment.
|
Holding on this one — I don't think the evidence retires the risk yet. nvbugs/6544407 is still open P0 ( The triage on the bug also names a concrete, still-unapplied mitigation: the run starts Two ways I'd be happy to approve:
Nothing here is blocking on my side beyond that evidence — the diff itself is a one-line removal of a |
|
PR_Github #63916 [ run ] completed with state |
crazydemo
left a comment
There was a problem hiding this comment.
Review summary - CONCERNS
Verdict: The one-line deletion is correct and matches the pytest node ID exactly, but it cannot merge as-is (mergeable_state: dirty — rebase needed) and, more importantly, the unwaive is unverifiable by this PR's CI, so it should not be trusted without QA evidence.
Issues
- [MAJOR]
tests/integration/test_lists/waives.txt:165- unwaive not verifiable by CI; intermittent stress test may regress
QA view
- Test coverage: missing - the unwaived test lives only in
qa/llm_function_stress.txt, not in anytest-db/l0_*.yml, so/bot runon this PR does not execute it. Nothing here can confirm the test passes. - SM coverage: touches B200 (sm100/nvfp4); this PR runs nothing on that arch for this test. Verification depends entirely on out-of-band QA cluster runs.
- Test code: this is an unwaive, not a fix — the intermittent failure itself is not addressed in code.
- Test time: unknown - not added to any l0 list, so CI runtime is unchanged; QA-run duration cannot be inferred from the diff.
- Needs
/qa-verify: yes - a human QA must re-run the glm5 stress case on 8xB200 multiple times and confirm reliable passing, plus confirm the disposition of the bug.
Does this actually fix nvbugs/6544407?
Partial / unclear. The PR removes the SKIP waiver on the basis of "unable to reproduce on latest main" after 5 manual runs, but names no root-cause fix commit and does not state whether the bug is closed. For an 8xB200 / 35k-request / conc512 / 0.90-accuracy / 10%-cancellation stress test, intermittent failure is the norm and is why such tests get waived. The failing path is therefore not demonstrably closed — it may simply not have recurred in 5 runs.
Possible new issues
- Re-exposes the intermittent failure with no CI gate: the next scheduled QA stress run can flake exactly as before, now un-waived, with no trail back to this change.
- Waiver and bug drift apart: waiver removed while
nvbugs/6544407disposition is unrecorded, leaving the next person hitting it without context.
What I could not verify
- The actual pass/fail history on
main— only the description's claim and a Jenkins link are given; I cannot see run results. - Whether a specific commit fixed the root cause (none is named).
- The current state of
nvbugs/6544407.
Automated review by NVCortex Lite, run by @crazydemo.
| full:B200/accuracy/test_llm_api_pytorch.py::TestQwen3_30B_A3B::test_dummy_load_format SKIP (https://nvbugs/6525059) | ||
| full:B200/disaggregated/test_disaggregated.py::test_disaggregated_stress_test[input8k-output1k-conc512-deepseek_r1_v2_fp4_stress] SKIP (https://nvbugs/6472256) | ||
| full:B200/disaggregated/test_disaggregated.py::test_disaggregated_stress_test[input8k-output1k-conc512-glm5_nvfp4_tp4_ep4_dp_stress] SKIP (https://nvbugs/6544407) | ||
| full:B200/disaggregated/test_disaggregated.py::test_disaggregated_stress_test[input8k-output1k-conc512-qwen3_32b_fp8_stress] SKIP (https://nvbugs/6472256) |
There was a problem hiding this comment.
[MAJOR] Unwaive not verifiable by CI; intermittent stress test may regress
The deleted line unwaived test_disaggregated_stress_test[...glm5_nvfp4_tp4_ep4_dp_stress], but this test appears only in the QA stress list (tests/integration/test_lists/qa/llm_function_stress.txt), not in any test-db/l0_*.yml. Concretely: run /bot run on this PR and the test never executes, so CI cannot confirm the unwaive — the only evidence is 5 manual runs on a b200 node for a workload (8xB200, 35k requests, conc512, 0.90 accuracy gate, 10% cancellation) whose known failure mode is intermittent. Trigger for regression: the next scheduled QA stress run flakes exactly as before, now un-waived, and fails the suite with no CI trail pointing back here. Fix: before merging, (1) link at least one passing run and state the run count, and (2) resolve nvbugs/6544407 explicitly (close / close-not-repro) so the waiver and bug do not drift apart. If a specific commit fixed the root cause, name it in the description.
crazydemo
left a comment
There was a problem hiding this comment.
Review summary - Approve (non-blocking)
Approving so this is not blocked on me. The points raised in my review comment above are non-blocking — please read them and address what you agree with before merging.
Worth doing before this is relied on: Unwaive of an intermittent stress test with no CI coverage on this PR; a human QA must re-run the glm5 stress case on 8xB200 and confirm it passes reliably (multiple runs) before this is trusted, and confirm the disposition of nvbugs/6544407.
Automated review by NVCortex Lite, run by @crazydemo.
Dev Engineer Review
tests/integration/test_lists/waives.txt.main.The error was not reproduced after running the test five times on the b200 x8 node,
link https://prod.blsm.nvidia.com/swqa-tensorrt-qa-test/job/DEBUG_LLM_FUNCTION_CLUSTER_TEST/1845/
QA Engineer Review
tests/integration/test_lists/waives.txt.Description
unable to reproduce the error on latest main
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)
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.