Repository navigation
[https://nvbugs/7044831][test] Unwaive the mixed-precision disaggregation tests fixed by #21597 - #21774
Conversation
|
No actionable comments were generated in the recent review. WalkthroughThe integration test waiver list adds skips for a CPU subprocess-timeout test and a DGX_B200 DeepSeek V3 Lite test. It removes the skip for a disaggregation test. ChangesIntegration test waivers
Estimated code review effort: 1 (Trivial) | ~4 minutes Change: Other Merge Risk:
|
|
/bot run --disable-fail-fast |
|
PR_Github #82986 [ run ] triggered by Bot. Commit: |
be60a67 to
9ad8a9b
Compare
|
Automatically added "ci: full pre-merge approved" because this PR has satisfied the required GitHub review approvals. Unresolved review conversations and other required checks remain independent merge requirements. |
|
/bot run --disable-fail-fast |
|
PR_Github #83000 [ run ] triggered by Bot. Commit: |
|
PR_Github #82986 [ run ] completed with state |
|
PR_Github #83000 [ run ] completed with state |
sunnyqgg
left a comment
There was a problem hiding this comment.
Checked this unwaive against #21597 (7d7b098, already on this branch's base), the four NVBugs involved and this PR's CI: 0 blockers, 0 possibly blocking, 3 required, 0 nice-to-have. The fix covers both error paths the three tests hit (_kv_heads_for_layer now maps each mixed-precision high layer to its local layer first, kv_extractor.py:425-449), and in L0_MergeRequest_PR #68556 (head 9ad8a9b) B300-PyTorch-1 passed all three unwaived tests and DGX_B200-PyTorch-1 passed both test_mixed_precision_kv_cache.py ones (H100 skips them).
- [Required] Please drop
unittest/disaggregated SKIP (https://nvbugs/7042835)(waives.txt:263) as well, or narrow it to the node ids that still fail. #21597 also fixed 7042835's root cause (context_first_profile.py:116now readsmixed_precision_low_dtype), and this PR's own L0_MergeRequest_PR #68540 (head be60a67, which removed this waive) ranunittest/disaggregatedon CPU-Generic-x86-1 and CPU-Generic-arm-1 with 1793 passed / 0 failed, so CI does not bear out the description's "the directory now has failures unrelated to mixed precision". The waive matches only theunittest/disaggregatedentry inl0_cpu.yml:226: outer waive matching is exact (tests/integration/defs/test_list_parser.py:698), the unittest-level matcher has no directory match (tests/unittest/conftest.py:336), and GPU stages runnot cpu_only(jenkins/L0_Test.groovy:1962). So while it stays, #21597's own regression testtest_extractor.py::test_v2_builder_resolves_mixed_precision_high_layer_heads[False/True](andtest_context_first_profile.py) runs in no CI stage, and a KV-head regression in a layout the three Blackwell-only tests do not build would merge unnoticed. The MPI timeout recorded on 7042835 (test_transfer_lifecycle_gpu[baseline]) is a GPU test listed by file (l0_dgx_b200.yml:71) that this waive does not cover anyway. If something in the directory still fails, waive those node ids under their own NVBug and name them in the description. - [Required] This PR removes the last waive of https://nvbugs/7044232, https://nvbugs/7043616 and https://nvbugs/7044831, but all three are still open, none mentions #21597, and only 7044831 is linked from the title (7044232 and 7043616 were auto-filed and never triaged). Please annotate each with the fix (#21597) and the passing runs in L0_MergeRequest_PR #68556 (B300-PyTorch-1: all three tests; DGX_B200-PyTorch-1: both
test_mixed_precision_kv_cache.pytests) so they are closed when this merges.
9ad8a9b to
b81f4ec
Compare
|
/bot run --disable-fail-fast |
|
PR_Github #83031 [ run ] triggered by Bot. Commit: |
|
PR_Github #83031 [ run ] completed with state |
… fixed by NVIDIA#21597 NVIDIA#21597 fixed the KV-head count of mixed-precision high layers in disaggregated transfer. Remove the waives of the tests that failed on it: - B300: test_mamba_cache_manager.py:: test_v2_hybrid_mixed_precision_appends_high_group (https://nvbugs/7044232) - B300 and DGX_B200: test_mixed_precision_kv_cache.py:: test_deepseek_v4_mixed_disagg_names_every_layer (https://nvbugs/7043616) - All platforms: test_mixed_precision_kv_cache.py:: test_disagg_exports_both_pools_and_pairs_sink_tail (https://nvbugs/7044831) Signed-off-by: tianruih <tianruih@nvidia.com>
b81f4ec to
9e4c570
Compare
|
/bot run --disable-fail-fast |
|
PR_Github #83076 [ run ] triggered by Bot. Commit: |
|
PR_Github #83076 [ run ] completed with state |
Dev Engineer Review
The change removes three waivers from
tests/integration/test_lists/waives.txt. It enables the B300 hybrid mixed-precision append test, the B300 and DGX_B200 DeepSeek V4 mixed-disaggregation test, and the mixed-precision disaggregation pool-pairing test to run. The broaderunittest/disaggregatedwaiver remains. No source-code or test-code files changed.QA Engineer Review
The change removes three test waivers and adds no test functions or test IDs. The affected tests cover mixed-precision KV-cache handling and disaggregation. The supplied evidence does not establish results from completed checks on the current head. Coverage verdict: needs follow-up.
Per-File QA Perspective
tests/integration/test_lists/waives.txt: Removes the three test waivers described above. This test-list-only change does not require test-db or manual-QA list membership evidence.Description
#21597 fixed the KV-head count of mixed-precision high layers in disaggregated transfer. This PR removes the waives of the tests that failed on it.
unittest/_torch/pyexecutor/kv_cache/test_mamba_cache_manager.py::test_v2_hybrid_mixed_precision_appends_high_groupunittest/_torch/pyexecutor/kv_cache/test_mixed_precision_kv_cache.py::test_deepseek_v4_mixed_disagg_names_every_layerunittest/_torch/pyexecutor/kv_cache/test_mixed_precision_kv_cache.py::test_disagg_exports_both_pools_and_pairs_sink_tailThe
unittest/disaggregatedwaive (https://nvbugs/7042835) stays: #21597 fixed its original break, but the directory now has failures unrelated to mixed precision.Test Coverage
The unwaived tests run in this PR's CI. On B200, with this branch built from source, the three tests pass (
test_mixed_precision_kv_cache.pyplus the hybrid test: 21 passed).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.