test: skip llama3.3 70b test on cg4 #6293
Conversation
📝 Walkthrough""" WalkthroughA test class in the integration suite was updated to add a skip marker based on host memory, replace device-specific skip decorators with custom ones, and modify a test method to instantiate an object with an explicit configuration for GPU memory usage. The rest of the test logic remains unchanged. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Suggested reviewers
✨ Finishing Touches
🧪 Generate unit tests
🪧 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
changed the title
|
/bot run |
|
PR_Github #12695 [ run ] triggered by Bot |
|
PR_Github #12695 [ run ] completed with state |
![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
🧹 Nitpick comments (1)
tests/integration/defs/accuracy/test_llm_api_pytorch.py (1)
342-343: LGTM! Memory-based skip marker correctly addresses CG4 limitations.The skip marker and explanatory comment effectively implement the PR objective of preventing system crashes on CG4 hardware with insufficient host memory. The 1TB requirement is well-justified given CG4's 800G total with 480G shared with GPUs.
Fix the line length violation:
-# 1TB is basic requirement for large model tests. CG4 120G only has 800G host memory, and 480G is shared with GPUs. the test will cause the system crash. +# 1TB is basic requirement for large model tests. CG4 120G only has 800G host memory, +# and 480G is shared with GPUs. The test will cause the system crash.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
tests/integration/defs/accuracy/test_llm_api_pytorch.py(3 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)
🚧 Files skipped from review as they are similar to previous changes (3)
- tests/integration/defs/examples/test_bert.py
- tests/integration/defs/conftest.py
- tests/integration/defs/examples/test_bindings.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/accuracy/test_llm_api_pytorch.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/accuracy/test_llm_api_pytorch.py
🪛 Ruff (0.12.2)
tests/integration/defs/accuracy/test_llm_api_pytorch.py
343-343: Line too long (153 > 120)
(E501)
🔇 Additional comments (3)
tests/integration/defs/accuracy/test_llm_api_pytorch.py (3)
360-360: Good refactoring! Semantic skip decorator improves maintainability.Replacing the explicit device list with
@skip_pre_hoppermakes the code more readable and follows the DRY principle. This semantic approach is easier to maintain than hardcoded device lists.
363-366: Practical memory configuration improves test stability.Adding explicit
KvCacheConfigwithfree_gpu_memory_fraction=0.6is a sensible adjustment for the 70B model on 4 GPUs. This reserves sufficient memory headroom to prevent OOM issues while maintaining test functionality.
377-377: Consistent refactoring with semantic skip decorators.The replacement of explicit device list with
@skip_pre_blackwellmaintains consistency with the semantic decorator approach used elsewhere in the changes. This improves code maintainability and readability.
|
/bot run |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/integration/defs/accuracy/test_llm_api_pytorch.py (1)
342-343: Fix line length violation in comment.The comment exceeds the 120-character limit. Consider breaking it into multiple lines for better readability and compliance with coding standards.
-# 1TB is basic requirement for large model tests. CG4 120G only has 800G host memory, and 480G is shared with GPUs. the test will cause the system crash. +# 1TB is basic requirement for large model tests. CG4 120G only has 800G host memory, +# and 480G is shared with GPUs. The test will cause the system crash.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/integration/defs/accuracy/test_llm_api_pytorch.py(3 hunks)
🧰 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/accuracy/test_llm_api_pytorch.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/accuracy/test_llm_api_pytorch.py
🪛 Ruff (0.12.2)
tests/integration/defs/accuracy/test_llm_api_pytorch.py
343-343: Line too long (153 > 120)
(E501)
🔇 Additional comments (3)
tests/integration/defs/accuracy/test_llm_api_pytorch.py (3)
360-360: Good refactoring to use semantic skip decorators.Replacing device-specific lists with
@skip_pre_hopperimproves code maintainability and readability. This aligns with the existing pattern used throughout the codebase.
363-366: Appropriate memory configuration for large model testing.Adding explicit
KvCacheConfigwithfree_gpu_memory_fraction=0.6is a sensible optimization for the 70B model test, ensuring adequate memory headroom for successful execution.
377-377: Consistent refactoring to semantic skip decorators.The change from device-specific
@pytest.mark.skip_device_not_contain(["B200"])to@skip_pre_blackwellmaintains consistency with the previous refactoring and improves code maintainability.
|
PR_Github #12838 [ run ] triggered by Bot |
|
PR_Github #12838 [ run ] completed with state |
|
/bot run |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/integration/defs/accuracy/test_llm_api_pytorch.py (1)
342-343: Fix line length violation and improve comment formatting.The host memory skip marker and explanatory comment are valuable additions that address the previous review feedback. However, the comment exceeds the 120-character line limit.
@pytest.mark.timeout(7200) @pytest.mark.skip_less_host_memory(1000000) -# 1TB is basic requirement for large model tests. CG4 120G only has 800G host memory, and 480G is shared with GPUs. the test will cause the system crash. +# 1TB is basic requirement for large model tests. CG4 120G only has 800G host memory, +# and 480G is shared with GPUs. The test will cause the system crash. class TestLlama3_3_70BInstruct(LlmapiAccuracyTestHarness):
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/integration/defs/accuracy/test_llm_api_pytorch.py(3 hunks)
🧰 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/accuracy/test_llm_api_pytorch.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/accuracy/test_llm_api_pytorch.py
🪛 Ruff (0.12.2)
tests/integration/defs/accuracy/test_llm_api_pytorch.py
343-343: Line too long (153 > 120)
(E501)
🔇 Additional comments (3)
tests/integration/defs/accuracy/test_llm_api_pytorch.py (3)
360-360: LGTM: Semantic decorator improves maintainability.Replacing the device-specific skip decorator with
@skip_pre_hoppermakes the code more maintainable and semantic. This abstraction allows for centralized device compatibility logic.
363-366: LGTM: Explicit memory configuration improves test reliability.Adding
KvCacheConfigwithfree_gpu_memory_fraction=0.6provides explicit GPU memory management for the large 70B model test. This configuration should help prevent out-of-memory issues during test execution.
377-377: LGTM: Consistent semantic decorator usage.Replacing the B200-specific skip decorator with
@skip_pre_blackwellmaintains consistency with the previous decorator refactoring and improves code maintainability.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/integration/defs/accuracy/test_llm_api_pytorch.py (1)
342-343: LGTM! Host memory skip marker correctly addresses CG4 limitations.The skip marker and explanatory comment effectively address the PR objective to skip the llama3.3 70B test on CG4 systems with insufficient host memory. The explanation aligns with the past review discussion about the 1TB requirement.
Consider breaking the long comment to comply with the 120-character line limit:
-# 1TB is basic requirement for large model tests. CG4 120G only has 800G host memory, and 480G is shared with GPUs. the test will cause the system crash. +# 1TB is basic requirement for large model tests. CG4 120G only has 800G host memory, +# and 480G is shared with GPUs. The test will cause the system crash.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/integration/defs/accuracy/test_llm_api_pytorch.py(3 hunks)
🧰 Additional context used
🪛 Ruff (0.12.2)
tests/integration/defs/accuracy/test_llm_api_pytorch.py
343-343: Line too long (153 > 120)
(E501)
🔇 Additional comments (3)
tests/integration/defs/accuracy/test_llm_api_pytorch.py (3)
360-360: Good refactoring to use semantic skip decorator.Replacing the device-specific skip decorator with
@skip_pre_hopperimproves code maintainability by using a more semantic decorator that captures the intent (requiring Hopper or newer architecture) rather than listing specific device models.
363-366: Appropriate memory optimization for large model testing.Adding the
KvCacheConfigwithfree_gpu_memory_fraction=0.6is a sensible memory management optimization for the 70B model. This configuration helps ensure the test can run successfully on 4 GPUs without encountering out-of-memory issues.
377-377: Consistent refactoring to semantic skip decorator.Replacing the B200-specific skip decorator with
@skip_pre_blackwellmaintains consistency with the previous refactoring and improves code maintainability by using semantic architecture-based skip conditions.
|
PR_Github #12862 [ run ] triggered by Bot |
|
PR_Github #12862 [ run ] completed with state |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/integration/defs/accuracy/test_llm_api_pytorch.py (1)
342-343: LGTM! Host memory requirement properly documented.The skip decorator and explanatory comment address the CG4 memory constraints effectively.
Consider breaking the long comment to adhere to line length limits:
-# 1TB is basic requirement for large model tests. CG4 120G only has 800G host memory, and 480G is shared with GPUs. the test will cause the system crash. +# 1TB is basic requirement for large model tests. CG4 120G only has 800G host memory, +# and 480G is shared with GPUs. The test will cause system crash on insufficient memory.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/integration/defs/accuracy/test_llm_api_pytorch.py(3 hunks)
🧰 Additional context used
🪛 Ruff (0.12.2)
tests/integration/defs/accuracy/test_llm_api_pytorch.py
343-343: Line too long (153 > 120)
(E501)
⏰ 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 (3)
tests/integration/defs/accuracy/test_llm_api_pytorch.py (3)
360-360: Good improvement: Architecture-based skip is more maintainable.Replacing device-specific skip decorators with architecture-based ones (
@skip_pre_hopper) improves code maintainability and semantic clarity.
363-366: Appropriate memory optimization for large model testing.The explicit KV cache configuration with 60% GPU memory fraction is a sensible addition for the 70B model to prevent OOM issues during testing.
377-377: Consistent improvement: Architecture-based skip enhances maintainability.The replacement of device-specific skip with
@skip_pre_blackwellfollows the same maintainability improvement pattern as the previous decorator change.
|
PR_Github #12916 [ reuse-pipeline ] triggered by Bot |
|
PR_Github #12916 [ reuse-pipeline ] completed with state |
|
/bot reuse-pipeline |
|
PR_Github #12920 [ reuse-pipeline ] triggered by Bot |
Signed-off-by: Xin He (SW-GPU) <[email protected]>
Signed-off-by: Xin He (SW-GPU) <[email protected]>
|
/bot reuse-pipeline |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/integration/defs/accuracy/test_llm_api_pytorch.py (1)
342-343: Good addition of explanatory comment, but fix line length.The added host memory skip marker and explanatory comment effectively address the previous review feedback. However, the comment exceeds the 120-character line limit.
Apply this diff to fix the line length:
-# 1TB is basic requirement for large model tests. CG4 120G only has 800G host memory, and 480G is shared with GPUs. the test will cause the system crash. +# 1TB is basic requirement for large model tests. CG4 120G only has 800G host memory, +# and 480G is shared with GPUs. The test will cause the system crash.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/integration/defs/accuracy/test_llm_api_pytorch.py(3 hunks)
🧰 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/accuracy/test_llm_api_pytorch.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/accuracy/test_llm_api_pytorch.py
🪛 Ruff (0.12.2)
tests/integration/defs/accuracy/test_llm_api_pytorch.py
343-343: Line too long (153 > 120)
(E501)
🔇 Additional comments (3)
tests/integration/defs/accuracy/test_llm_api_pytorch.py (3)
360-360: Good abstraction with custom skip decorator.Replacing the device-specific skip decorator with
@skip_pre_hopperimproves code maintainability by using a semantic abstraction rather than hardcoded device lists.
363-366: Appropriate memory configuration for large model testing.The explicit KV cache configuration with
free_gpu_memory_fraction=0.6is a good practice for large model tests, helping prevent OOM issues by reserving adequate memory space for the 70B model.
377-377: Good abstraction with custom skip decorator.Replacing the device-specific skip decorator with
@skip_pre_blackwellimproves code maintainability by using a semantic abstraction rather than hardcoded device lists.
|
PR_Github #12920 [ reuse-pipeline ] completed with state |
|
PR_Github #12922 [ reuse-pipeline ] triggered by Bot |
|
PR_Github #12922 [ reuse-pipeline ] completed with state |
|
/bot reuse-pipeline |
|
PR_Github #12925 [ reuse-pipeline ] triggered by Bot |
|
PR_Github #12925 [ reuse-pipeline ] completed with state |
Signed-off-by: Xin He (SW-GPU) <[email protected]> Signed-off-by: Shreyas Misra <[email protected]>
Signed-off-by: Xin He (SW-GPU) <[email protected]> Signed-off-by: Ransiki Zhang <[email protected]>
Signed-off-by: Xin He (SW-GPU) <[email protected]> Signed-off-by: Lanyu Liao <[email protected]>
skil llama3.3 70B test on CG4
Summary by CodeRabbit