[https://nvbugs/6640875][test] Stabilize LoRA KV scheduler tests and re-enable seven cases - #18949
Conversation
Signed-off-by: Yi Zhang <187001205+yizhang-nv@users.noreply.github.com>
|
/bot run --disable-fail-fast --gpu-type DGX_B200 --test-backend pytorch |
|
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 (2)
💤 Files with no reviewable changes (1)
Included review availability: Your plan provides up to 12 included reviews per hour; 9 remain after this review. WalkthroughLoRA KV-cache v2 scheduler tests now use an 8192-token cache limit in default and multi-adapter configurations. The integration waiver list removes the LoRA chunked-prefill skip. ChangesKV-cache v2 LoRA scheduler tests
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This change bounds LoRA KV-cache integration tests to 8192 tokens and re-enables the chunked-prefill test. It does not alter product runtime behavior, and no merge-blocking risk is currently identified. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
PR_Github #72372 [ run ] triggered by Bot. Commit: |
Signed-off-by: Yi Zhang <187001205+yizhang-nv@users.noreply.github.com>
yizhang-nv
changed the title
|
/bot run --disable-fail-fast --gpu-type DGX_B200 --test-backend pytorch |
|
PR_Github #72384 [ run ] triggered by Bot. Commit: |
|
PR_Github #72372 [ run ] completed with state |
… bound KV cache Signed-off-by: Yi Zhang <187001205+yizhang-nv@users.noreply.github.com>
yizhang-nv
changed the title
|
/bot run --disable-fail-fast --gpu-type DGX_B200 --test-backend pytorch |
|
PR_Github #72393 [ run ] triggered by Bot. Commit: |
|
PR_Github #72384 [ run ] completed with state |
|
/bot run --disable-fail-fast |
|
PR_Github #72402 [ run ] triggered by Bot. Commit: |
|
PR_Github #72393 [ run ] completed with state |
|
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. |
|
PR_Github #72402 [ run ] completed with state |
4 participants
Description
Stabilize the LoRA V1/V2 scheduler comparisons and restore execution of seven waived tests. The LoRA class now uses private MPI sessions so each LLM has a fresh worker lifecycle, and its default KV pool is capped at 8192 tokens (approximately 4 GiB for llama-7b-hf). The explicit eviction configuration and all prompts, scheduler options, and output assertions are preserved. Remove the LoRA chunked-prefill waiver for https://nvbugs/6640875.
Also restore execution of all six
TestKVCacheV2Llamacases waived under https://nvbugs/6600098, https://nvbugs/6428002, and https://nvbugs/6462303 by removing their entries fromtests/integration/test_lists/waives.txt.#17823 removed these waivers after adding private MPI sessions and bounding the default Llama comparison KV pool. Commit 7583916 in #17975 subsequently restored all six removed Llama waivers, although its message and PR discussion described waiving the separate LoRA test. This change restores coverage for all six cases so their current behavior can be validated.
Historical evidence does not establish that all failures are fixed: all six tests passed in post-merge #2921 with commit e189237, which contains #17823. After the fix merged, pre-merge #55310 recorded an unexpected termination for
test_eviction[cuda_graph], #55544 recorded a chunked-prefill timeout, and #55696 recorded a V1/V2 output mismatch for eviction with block reuse; all three tested commits include the fix. The last failure occurred on #17668, which changes KV cache V2 storage/lifecycle code, so it cannot be attributed to main solely from that PR result. Subsequent post-merge runs skipped these tests after the waivers were restored.The original LoRA chunked-prefill test passed in the B200 baseline run; its historical hang was not reproduced. The baseline logs showed V1/V2 worker reuse and a 65.52 GiB KV allocation. These changes reduce memory pressure and isolate worker lifetimes, but do not establish the cause of the historical timeout or prove that the bug is resolved.
Test Coverage
B200 runtime validation: all four
TestKVCacheV2LoRAcases passed in 283.06 seconds; chunked prefill took 73.06 seconds. Command:python3 -m pytest -s -vv tests/integration/defs/kv_cache/test_kv_cache_v2_scheduler.py::TestKVCacheV2LoRA --timeout=600 --timeout-method=thread.Validation used this branch's Python source with existing compiled artifacts from the
tekit-devel-yizhan1.3.0rc26 runtime; it was not a fresh native build of the PR. Full stdout/stderr logs were retained. Fresh PR CI remains necessary.Commit hooks passed, including waiver sorting/duplicate checks, AST test-list validation, and DCO validation.
Existing coverage re-enabled by this PR:
kv_cache/test_kv_cache_v2_scheduler.py::TestKVCacheV2LoRA::test_lora_chunked_prefillkv_cache/test_kv_cache_v2_scheduler.py::TestKVCacheV2Llama::test_eviction[cuda_graph]kv_cache/test_kv_cache_v2_scheduler.py::TestKVCacheV2Llama::test_overlap_scheduler[non_overlap]kv_cache/test_kv_cache_v2_scheduler.py::TestKVCacheV2Llama::test_overlap_scheduler[overlap]kv_cache/test_kv_cache_v2_scheduler.py::TestKVCacheV2Llama::test_token_budget_limitedkv_cache/test_kv_cache_v2_scheduler.py::TestKVCacheV2Llama::test_chunked_prefillkv_cache/test_kv_cache_v2_scheduler.py::TestKVCacheV2Llama::test_eviction_with_block_reuseFresh DGX B200 CI validation is pending. Historical passes are not a substitute for validating this PR.
PR Checklist
Dev Engineer Review
The change removes six obsolete
TestKVCacheV2Llamawaivers and adds an 8192-token KV-cache limit for LoRA scheduler tests across default and multi-adapter V1/V2 configurations. No public APIs change. Fresh DGX B200 validation remains pending because historical results include failures.QA Engineer Review
The change modifies the waiver list and KV-cache scheduler integration tests. It removes six
TestKVCacheV2Llamawaivers and one LoRA chunked-prefill waiver. The new cache limit covers LoRA scheduler behavior and reduces unbounded-cache risk. Commit hooks, waiver checks, AST validation, and DCO validation passed. Fresh DGX B200 PyTorch-backend CI is required. Coverage verdict:needs follow-up.Per-File QA Perspective
tests/integration/test_lists/waives.txt: Removes seven waiver entries. QA should verify the unwaived cases in fresh DGX B200 CI.tests/integration/defs/kv_cache/test_kv_cache_v2_scheduler.py: Adds an 8192-token KV-cache limit to default and multi-adapter LoRA V1/V2 configurations. QA should verify LoRA scheduler behavior and cache bounding across all affected configurations.