[None][chore] Add failed cases into waives.txt #7251
Conversation
📝 WalkthroughWalkthroughAdds a large block of Changes
Sequence Diagram(s)(omitted — changes are file-level test skip additions with no control-flow or runtime behavior changes) Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
tests/integration/test_lists/waives.txt (1)
1-330: Context note: duplicating waivers across CLI flow and LLM API is legitimate.Referencing our prior learning for this repo: it’s expected to maintain both CLI flow tests and PyTorch LLM API tests for the same model; they serve different purposes. No action needed — mentioning to preempt questions.
🧹 Nitpick comments (2)
tests/integration/test_lists/waives.txt (2)
331-333: Multimodal fuyu/llava waivers look consistent; keep bug-link format uniform.Entries are well-scoped (bs:8, True/False cpp_e2e variants). Minor nit: bug links oscillate between nvbugs and nvbugspro across the file. If there’s a preferred canonical form, standardize when convenient.
331-355: Optional: add owner/date metadata to speed future un-waive sweeps.If the parser tolerates inline metadata, appending “; owner=xinhe-nv; added=2025-08-26” inside the reason parentheses makes expiry and ownership explicit. Example for a single line:
-examples/test_phi.py::test_llm_phi_quantization_1gpu[Phi-3-mini-128k-instruct-fp8-float16] SKIP (https://nvbugs/5465143) +examples/test_phi.py::test_llm_phi_quantization_1gpu[Phi-3-mini-128k-instruct-fp8-float16] SKIP (https://nvbugs/5465143; owner=xinhe-nv; added=2025-08-26)If you prefer, I can bulk-apply this style to the new block.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
tests/integration/test_lists/waives.txt(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-28T17:06:08.621Z
Learnt from: moraxu
PR: NVIDIA/TensorRT-LLM#6303
File: tests/integration/test_lists/qa/examples_test_list.txt:494-494
Timestamp: 2025-07-28T17:06:08.621Z
Learning: In TensorRT-LLM testing, it's common to have both CLI flow tests (test_cli_flow.py) and PyTorch API tests (test_llm_api_pytorch.py) for the same model. These serve different purposes: CLI flow tests validate the traditional command-line workflow, while PyTorch API tests validate the newer LLM API backend. Both are legitimate and should coexist.
Applied to files:
tests/integration/test_lists/waives.txt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Pre-commit Check
🔇 Additional comments (4)
tests/integration/test_lists/waives.txt (4)
352-354: Waivers validated: no overlapping quickstart entries
- Existing non-chunked 8-GPU waivers at lines 234–235
- New
chunked_prefill_sq_22kentries at lines 352–354 are distinct
334-341: Verify NVBUGS/5465143 scope across all Phi skipsIt looks like all Phi-related skips point to NVBUGS/5465143, except one non-Phi test:
• Lines 334–337, 338–339, 341:
– examples/test_phi.py::{lora, quantization variants}
– accuracy/test_cli_flow.py::TestPhi4MiniInstruct::{test_auto_dtype, test_tp2}
– accuracy/test_llm_api.py::TestPhi4MiniInstruct::test_fp8
→ SKIP (https://nvbugs/5465143)• Line 340: accuracy/test_cli_flow.py::TestLongAlpaca7B::test_auto_dtype
→ SKIP (https://nvbugs/5481075) — this one’s for a different model and bug.Please confirm that NVBUGS/5465143 truly covers all of the Phi modalities (LoRA, quantization, CLI flow, LLM API). If it does, no action is needed; if not, split the Phi-focused skips by their actual root causes to simplify future cleanup.
331-355: All new waiver entries are unique; ready to merge.Verified that no duplicate nodeids exist in tests/integration/test_lists/waives.txt and that every entry in lines 331–355 appears exactly once. Ship it.
342-351: All GPTOSS and EXAONE4 nodeids verified—no typos detectedThe
TestGPTOSSclass (line 2684) defines bothtest_w4_1gpuandtest_w4_4gpuswith the exact parameter IDs used in the waivers (True-True-cutlass,True-True-trtllm,tp4-CUTLASS,tp4-TRTLLM,ep4-CUTLASS,ep4-TRTLLM,dp4-CUTLASS,dp4-TRTLLM), and theTestEXAONE4::test_auto_dtypemethod (line 2795) exists as specified. No mismatches or stale entries were found—these skips correctly target existing tests.
Signed-off-by: xinhe-nv <[email protected]>
Signed-off-by: Xin He (SW-GPU) <[email protected]>
xinhe-nv
force-pushed
the
user/qa/post_update_waive_20250826_DEBUG_LLM_FUNCTION_TEST_1662
branch
from
10bced5 to
873dde0
Compare
|
/bot run |
…_FUNCTION_TEST_1662
|
PR_Github #16542 [ ] completed with state |
xinhe-nv
deleted the
user/qa/post_update_waive_20250826_DEBUG_LLM_FUNCTION_TEST_1662
branch
3 participants
waive failed cases.
Summary by CodeRabbit