[TRTLLM-6624][feat] skip post blackwell #6357
Conversation
📝 WalkthroughWalkthroughThis update introduces conditional skipping of tests for post Blackwell-Ultra GPU architectures (SM version ≥ 103) across numerous integration test modules. It achieves this by adding module-level Changes
Sequence Diagram(s)sequenceDiagram
participant Pytest
participant TestModule
participant defs.conftest
Pytest->>TestModule: Import module
TestModule->>defs.conftest: get_sm_version()
defs.conftest-->>TestModule: Return SM version
alt SM version >= 103
TestModule->>Pytest: pytest.skip("TRT workflow tests are not supported on post Blackwell-Ultra architecture", allow_module_level=True)
Pytest-->>TestModule: Skip all tests in module
else SM version < 103
TestModule->>Pytest: Proceed with test collection and execution
Note over TestModule,Pytest: Individual tests/params may still be skipped via @skip_post_blackwell
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (30)
💤 Files with no reviewable changes (1)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (28)
⏰ 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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
xinhe-nv
force-pushed
the
user/xinhe/b300
branch
2 times, most recently
from
ee4307b to
b1261c2
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
tests/integration/defs/accuracy/test_cli_flow.py(4 hunks)tests/integration/defs/conftest.py(1 hunks)tests/integration/defs/examples/test_bert.py(1 hunks)tests/integration/defs/examples/test_bindings.py(1 hunks)tests/integration/defs/examples/test_chatglm.py(1 hunks)tests/integration/defs/examples/test_commandr.py(2 hunks)tests/integration/defs/examples/test_draft_target_model.py(1 hunks)tests/integration/defs/examples/test_eagle.py(1 hunks)tests/integration/defs/examples/test_enc_dec.py(2 hunks)tests/integration/defs/examples/test_exaone.py(1 hunks)tests/integration/defs/examples/test_gpt.py(6 hunks)tests/integration/defs/examples/test_gptj.py(1 hunks)tests/integration/defs/examples/test_granite.py(1 hunks)tests/integration/defs/examples/test_internlm.py(1 hunks)tests/integration/defs/examples/test_llama.py(17 hunks)tests/integration/defs/examples/test_llm_api_with_mpi.py(1 hunks)tests/integration/defs/examples/test_mamba.py(1 hunks)tests/integration/defs/examples/test_medusa.py(2 hunks)tests/integration/defs/examples/test_mistral.py(1 hunks)tests/integration/defs/examples/test_mixtral.py(1 hunks)tests/integration/defs/examples/test_multimodal.py(3 hunks)tests/integration/defs/examples/test_nemotron.py(1 hunks)tests/integration/defs/examples/test_nemotron_nas.py(1 hunks)tests/integration/defs/examples/test_ngram.py(1 hunks)tests/integration/defs/examples/test_phi.py(2 hunks)tests/integration/defs/examples/test_qwen.py(1 hunks)tests/integration/defs/examples/test_qwenvl.py(1 hunks)tests/integration/defs/examples/test_recurrentgemma.py(1 hunks)tests/integration/defs/examples/test_redrafter.py(1 hunks)tests/integration/defs/examples/test_whisper.py(1 hunks)tests/integration/test_lists/waives.txt(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- tests/integration/defs/examples/test_bindings.py
- tests/integration/defs/examples/test_qwen.py
- tests/integration/defs/examples/test_medusa.py
🚧 Files skipped from review as they are similar to previous changes (12)
- tests/integration/defs/examples/test_phi.py
- tests/integration/defs/examples/test_recurrentgemma.py
- tests/integration/defs/examples/test_mamba.py
- tests/integration/defs/examples/test_redrafter.py
- tests/integration/defs/conftest.py
- tests/integration/defs/examples/test_enc_dec.py
- tests/integration/defs/examples/test_commandr.py
- tests/integration/defs/examples/test_multimodal.py
- tests/integration/defs/examples/test_gpt.py
- tests/integration/defs/examples/test_exaone.py
- tests/integration/defs/examples/test_llama.py
- tests/integration/defs/accuracy/test_cli_flow.py
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit Inference Engine (CODING_GUIDELINES.md)
**/*.py: Python code should conform to Python 3.8+.
Indent Python code with 4 spaces. Do not use tabs.
Always maintain the namespace when importing in Python, even if only one class or function from a module is used.
Python filenames should use snake_case (e.g., some_file.py).
Python classes should use PascalCase (e.g., class SomeClass).
Python functions and methods should use snake_case (e.g., def my_awesome_function():).
Python local variables should use snake_case. Prefix k for variable names that start with a number (e.g., k_99th_percentile).
Python global variables should use upper snake_case and prefix G (e.g., G_MY_GLOBAL).
Python constants should use upper snake_case (e.g., MY_CONSTANT).
Avoid shadowing variables declared in an outer scope in Python.
Initialize all externally visible members of a Python class in the constructor.
For interfaces that may be used outside a Python file, prefer docstrings over comments.
Comments in Python should be reserved for code within a function, or interfaces that are local to a file.
Use Google style docstrings for Python classes and functions, which can be parsed by Sphinx.
Attributes and variables in Python can be documented inline; attribute docstrings will be rendered under the docstring for the class.
Avoid using reflection in Python when functionality can be easily achieved without it.
When using try-except blocks in Python, limit the except to the smallest set of errors possible.
When using try-except blocks to handle multiple possible variable types in Python, keep the body of the try as small as possible, using the else block to implement the logic.
Files:
tests/integration/defs/examples/test_draft_target_model.pytests/integration/defs/examples/test_nemotron_nas.pytests/integration/defs/examples/test_eagle.pytests/integration/defs/examples/test_gptj.pytests/integration/defs/examples/test_bert.pytests/integration/defs/examples/test_mistral.pytests/integration/defs/examples/test_ngram.pytests/integration/defs/examples/test_llm_api_with_mpi.pytests/integration/defs/examples/test_mixtral.pytests/integration/defs/examples/test_qwenvl.pytests/integration/defs/examples/test_nemotron.pytests/integration/defs/examples/test_whisper.pytests/integration/defs/examples/test_granite.pytests/integration/defs/examples/test_internlm.pytests/integration/defs/examples/test_chatglm.py
**/*.{cpp,h,hpp,cc,cxx,cu,py}
📄 CodeRabbit Inference Engine (CODING_GUIDELINES.md)
All TensorRT-LLM Open Source Software code should contain an NVIDIA copyright header that includes the current year. This includes .cpp, .h, .cu, .py, and any other source files which are compiled or interpreted.
Files:
tests/integration/defs/examples/test_draft_target_model.pytests/integration/defs/examples/test_nemotron_nas.pytests/integration/defs/examples/test_eagle.pytests/integration/defs/examples/test_gptj.pytests/integration/defs/examples/test_bert.pytests/integration/defs/examples/test_mistral.pytests/integration/defs/examples/test_ngram.pytests/integration/defs/examples/test_llm_api_with_mpi.pytests/integration/defs/examples/test_mixtral.pytests/integration/defs/examples/test_qwenvl.pytests/integration/defs/examples/test_nemotron.pytests/integration/defs/examples/test_whisper.pytests/integration/defs/examples/test_granite.pytests/integration/defs/examples/test_internlm.pytests/integration/defs/examples/test_chatglm.py
🪛 Ruff (0.12.2)
tests/integration/defs/examples/test_draft_target_model.py
27-27: Undefined name get_sm_version
(F821)
tests/integration/defs/examples/test_chatglm.py
34-34: Undefined name skip_post_blackwell
(F821)
⏰ 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 (21)
tests/integration/defs/examples/test_llm_api_with_mpi.py (1)
20-26: LGTM: Proper module-level skip implementation for post Blackwell-Ultra architectures.The import and skip logic are correctly implemented:
- Clean addition to existing imports from
defs.conftest- Appropriate use of
allow_module_level=True- Clear, descriptive skip message
- Consistent with the PR's objective to exclude post Blackwell-Ultra platforms
tests/integration/defs/examples/test_nemotron_nas.py (1)
5-12: LGTM: Consistent implementation of module-level architecture skip.The changes correctly extend the existing import and implement the same skip pattern:
- Properly adds
get_sm_versionto existing import fromdefs.conftest- Maintains consistency with the skip condition and message format
- Follows the established pattern for excluding post Blackwell-Ultra architectures
tests/integration/defs/examples/test_nemotron.py (1)
18-25: LGTM: Consistent module-level skip implementation.The changes follow the established pattern correctly:
- Appropriately extends existing imports from
defs.conftest- Implements the same skip logic and message as other test modules
- Proper use of module-level skip functionality
tests/integration/defs/examples/test_eagle.py (1)
21-28: LGTM: Comprehensive skip implementation with both module and function-level markers.The changes are well-implemented:
- Correctly imports both
get_sm_versionandskip_post_blackwell- Consistent module-level skip for post Blackwell-Ultra architectures
- Individual test functions also use
@skip_post_blackwelldecorator for additional granularityThe dual approach provides flexibility for different skip conditions (post-Blackwell vs post-Blackwell-Ultra).
tests/integration/defs/examples/test_ngram.py (1)
21-28: LGTM: Consistent implementation following established pattern.The changes properly implement the skip functionality:
- Correctly adds required imports from
defs.conftest- Implements consistent module-level skip for post Blackwell-Ultra architectures
- Maintains the established pattern across test modules
- Combines with function-level
@skip_post_blackwelldecorator for comprehensive coveragetests/integration/defs/examples/test_draft_target_model.py (1)
26-30: LGTM! Module-level skip implementation is correct.The module-level skip logic is properly implemented using
pytest.skip()withallow_module_level=Trueto disable all TRT workflow tests on post-Blackwell-Ultra architectures (SM >= 103).tests/integration/defs/examples/test_internlm.py (2)
17-17: LGTM! Import addition is properly implemented.The
get_sm_versionfunction is correctly added to the existing import fromdefs.conftest.
20-24: LGTM! Module-level skip logic is correctly implemented.The skip logic properly disables all TRT workflow tests on post-Blackwell-Ultra architectures using the appropriate SM version threshold.
tests/integration/defs/examples/test_granite.py (2)
21-21: LGTM! Import statement is correctly added.The
get_sm_versionfunction is properly imported fromdefs.conftest.
24-28: LGTM! Module-level skip implementation is correct.The skip logic properly disables TRT workflow tests on post-Blackwell-Ultra architectures with the correct SM version threshold and appropriate skip message.
tests/integration/defs/examples/test_bert.py (1)
21-25: LGTM! Module-level skip logic is correctly implemented.The skip implementation properly disables all TRT workflow tests on post-Blackwell-Ultra architectures using the appropriate SM version threshold and skip message.
tests/integration/defs/examples/test_qwenvl.py (2)
23-23: LGTM! Import statement is correctly added.The
get_sm_versionfunction is properly imported fromdefs.conftest.
26-30: LGTM! Module-level skip implementation is correct.The skip logic properly disables all TRT workflow tests on post-Blackwell-Ultra architectures with the correct SM version threshold and consistent skip message.
tests/integration/defs/examples/test_mistral.py (2)
23-23: LGTM: Clean import additionThe import of
get_sm_versionfollows the existing import pattern and maintains namespace consistency.
26-30: LGTM: Proper module-level skip implementationThe conditional skip for post-Blackwell-Ultra architectures (SM >= 103) is correctly implemented with a clear, informative message. This aligns with the PR objective to exclude these platforms from TRT workflow tests.
tests/integration/defs/examples/test_whisper.py (1)
18-18: LGTM: Consistent implementation across test modulesThe import and module-level skip implementation matches the pattern established in other test files, ensuring consistent behavior for post-Blackwell-Ultra architecture handling.
Also applies to: 21-25
tests/integration/defs/examples/test_gptj.py (1)
18-18: LGTM: Consistent pattern implementationThe changes follow the established pattern for module-level skipping on post-Blackwell-Ultra architectures, maintaining consistency across the test suite.
Also applies to: 21-25
tests/integration/defs/examples/test_mixtral.py (2)
22-23: LGTM: Clean import modificationThe addition of
get_sm_versionto the existing import maintains proper formatting and alphabetical ordering.
26-30: LGTM: Standard module-level skip implementationConsistent with other test modules in this PR, the skip logic properly excludes post-Blackwell-Ultra architectures from TRT workflow tests.
tests/integration/defs/examples/test_chatglm.py (1)
21-21: LGTM: Consistent module-level skip implementationThe import of
get_sm_versionand the module-level skip logic follow the established pattern across other test files in this PR.Also applies to: 24-28
tests/integration/test_lists/waives.txt (1)
101-101: Waives-file parser correctly ignores commented linesConfirmed that in
parse_test_list_lineseach line is split at the first#(vialine.partition("#")[0].strip()) and empty results are dropped, so any line beginning with#—including#full:…—is treated as a comment and skipped. No changes are needed.
xinhe-nv
force-pushed
the
user/xinhe/b300
branch
from
b1261c2 to
0bf0938
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/integration/test_lists/waives.txt (1)
101-101: Prefer deleting waived-entry instead of prefixing with#full:Commenting the entry keeps stale platform-specific noise in the list and breaks the convention that removals disappear (and—per
mergeWaiveList.pylearning—collect at the tail).
A clean delete is clearer and avoids accidental future re-enabling through search-replace scripts.-#full:B200/examples/test_chatglm.py::test_llm_glm_4_9b_single_gpu_summary[glm-4-9b-chat-disable_weight_only] SKIP (https://nvbugs/5114743)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
tests/integration/defs/accuracy/test_cli_flow.py(4 hunks)tests/integration/defs/conftest.py(1 hunks)tests/integration/defs/examples/test_bert.py(1 hunks)tests/integration/defs/examples/test_bindings.py(1 hunks)tests/integration/defs/examples/test_chatglm.py(1 hunks)tests/integration/defs/examples/test_commandr.py(2 hunks)tests/integration/defs/examples/test_draft_target_model.py(1 hunks)tests/integration/defs/examples/test_eagle.py(1 hunks)tests/integration/defs/examples/test_enc_dec.py(2 hunks)tests/integration/defs/examples/test_exaone.py(1 hunks)tests/integration/defs/examples/test_gpt.py(6 hunks)tests/integration/defs/examples/test_gptj.py(1 hunks)tests/integration/defs/examples/test_granite.py(1 hunks)tests/integration/defs/examples/test_internlm.py(1 hunks)tests/integration/defs/examples/test_llama.py(17 hunks)tests/integration/defs/examples/test_llm_api_with_mpi.py(1 hunks)tests/integration/defs/examples/test_mamba.py(1 hunks)tests/integration/defs/examples/test_medusa.py(2 hunks)tests/integration/defs/examples/test_mistral.py(1 hunks)tests/integration/defs/examples/test_mixtral.py(1 hunks)tests/integration/defs/examples/test_multimodal.py(3 hunks)tests/integration/defs/examples/test_nemotron.py(1 hunks)tests/integration/defs/examples/test_nemotron_nas.py(1 hunks)tests/integration/defs/examples/test_ngram.py(1 hunks)tests/integration/defs/examples/test_phi.py(2 hunks)tests/integration/defs/examples/test_qwen.py(1 hunks)tests/integration/defs/examples/test_qwenvl.py(1 hunks)tests/integration/defs/examples/test_recurrentgemma.py(1 hunks)tests/integration/defs/examples/test_redrafter.py(1 hunks)tests/integration/defs/examples/test_whisper.py(1 hunks)tests/integration/test_lists/waives.txt(1 hunks)
✅ Files skipped from review due to trivial changes (5)
- tests/integration/defs/examples/test_llm_api_with_mpi.py
- tests/integration/defs/examples/test_bindings.py
- tests/integration/defs/examples/test_whisper.py
- tests/integration/defs/examples/test_draft_target_model.py
- tests/integration/defs/examples/test_qwen.py
🚧 Files skipped from review as they are similar to previous changes (25)
- tests/integration/defs/examples/test_ngram.py
- tests/integration/defs/examples/test_granite.py
- tests/integration/defs/examples/test_internlm.py
- tests/integration/defs/examples/test_bert.py
- tests/integration/defs/examples/test_mixtral.py
- tests/integration/defs/examples/test_nemotron_nas.py
- tests/integration/defs/examples/test_mistral.py
- tests/integration/defs/conftest.py
- tests/integration/defs/examples/test_chatglm.py
- tests/integration/defs/examples/test_phi.py
- tests/integration/defs/examples/test_exaone.py
- tests/integration/defs/examples/test_recurrentgemma.py
- tests/integration/defs/examples/test_mamba.py
- tests/integration/defs/examples/test_eagle.py
- tests/integration/defs/examples/test_nemotron.py
- tests/integration/defs/examples/test_gptj.py
- tests/integration/defs/examples/test_enc_dec.py
- tests/integration/defs/examples/test_qwenvl.py
- tests/integration/defs/examples/test_medusa.py
- tests/integration/defs/examples/test_multimodal.py
- tests/integration/defs/examples/test_redrafter.py
- tests/integration/defs/accuracy/test_cli_flow.py
- tests/integration/defs/examples/test_commandr.py
- tests/integration/defs/examples/test_gpt.py
- tests/integration/defs/examples/test_llama.py
🧰 Additional context used
🧠 Learnings (1)
tests/integration/test_lists/waives.txt (2)
Learnt from: yiqingy0
PR: #5198
File: jenkins/mergeWaiveList.py:0-0
Timestamp: 2025-07-22T08:33:49.109Z
Learning: In the TensorRT-LLM waive list merging system, removed lines are always located at the end of the merge waive lists, which is why the mergeWaiveList.py script uses reverse traversal - it's an optimization for this specific domain constraint.
Learnt from: moraxu
PR: #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.
xinhe-nv
force-pushed
the
user/xinhe/b300
branch
from
0bf0938 to
be9017e
Compare
|
/bot run |
|
PR_Github #13530 [ run ] triggered by Bot |
|
PR_Github #13530 [ run ] completed with state |
xinhe-nv
force-pushed
the
user/xinhe/b300
branch
from
be9017e to
afd09ac
Compare
|
/bot run |
|
PR_Github #13593 [ run ] triggered by Bot |
|
PR_Github #13593 [ run ] completed with state |
xinhe-nv
force-pushed
the
user/xinhe/b300
branch
from
afd09ac to
e27d15f
Compare
|
/bot run |
|
PR_Github #13599 [ run ] triggered by Bot |
|
PR_Github #13599 [ run ] completed with state |
xinhe-nv
force-pushed
the
user/xinhe/b300
branch
2 times, most recently
from
7f8b0b0 to
1780003
Compare
|
/bot run |
xinhe-nv
force-pushed
the
user/xinhe/b300
branch
from
1780003 to
9fcf5b3
Compare
|
/bot run |
|
PR_Github #13627 [ run ] triggered by Bot |
|
PR_Github #13627 [ run ] completed with state |
xinhe-nv
force-pushed
the
user/xinhe/b300
branch
from
4ca032d to
a7d652d
Compare
|
/bot run |
|
PR_Github #13745 [ run ] triggered by Bot |
xinhe-nv
force-pushed
the
user/xinhe/b300
branch
from
a7d652d to
952010d
Compare
|
PR_Github #13745 [ run ] completed with state |
xinhe-nv
force-pushed
the
user/xinhe/b300
branch
from
952010d to
2b8fffd
Compare
|
/bot run |
xinhe-nv
changed the title
|
PR_Github #13766 [ run ] triggered by Bot |
xinhe-nv
force-pushed
the
user/xinhe/b300
branch
from
2b8fffd to
0dd37a2
Compare
|
/bot run |
|
PR_Github #13785 [ run ] triggered by Bot |
|
PR_Github #13766 [ run ] completed with state |
xinhe-nv
force-pushed
the
user/xinhe/b300
branch
from
0dd37a2 to
441b126
Compare
|
PR_Github #13785 [ run ] completed with state |
Signed-off-by: Xin He (SW-GPU) <[email protected]>
xinhe-nv
force-pushed
the
user/xinhe/b300
branch
from
441b126 to
cd904c2
Compare
|
/bot run |
|
PR_Github #13810 [ run ] triggered by Bot |
|
PR_Github #13810 [ run ] completed with state |
Signed-off-by: Xin He (SW-GPU) <[email protected]> Signed-off-by: Lanyu Liao <[email protected]>
Signed-off-by: Xin He (SW-GPU) <[email protected]>
update test to skip test on blackwell and backwell ultra
Summary by CodeRabbit
Chores
Tests