Skip to content

Conversation

xinhe-nv
Copy link
Collaborator

@xinhe-nv xinhe-nv commented Jul 28, 2025

addd nemotronH-8b, nemotronH-47b and nemotronH-56b cuda graph tests
test result with H100 https://prod.blsm.nvidia.com/swqa-tensorrt-qa-test/job/LLM_FUNCTION_TEST_DEBUG/1512/testReport/

Summary by CodeRabbit

Summary by CodeRabbit

  • Tests
    • Enhanced test coverage by parameterizing tests to run with and without CUDA graph enabled.
    • Updated test decorators for improved environment compatibility.
    • Added new test classes for larger model configurations with multiple parallelism settings.
    • Expanded the test list to explicitly include parameterized test variants for greater granularity.
  • New Features
    • Added accuracy benchmark entries for new 47B and 56B Nemotron-H model variants on GSM8K and MMLU datasets.

Copy link
Contributor

coderabbitai bot commented Jul 28, 2025

📝 Walkthrough

"""

Walkthrough

The changes introduce parameterization over the cuda_graph flag for two test methods in the TestNemotronH class within the PyTorch LLM API integration tests. Two new test classes for 47B and 56B NemotronH models are added with parameterized tests over cuda_graph and parallelism configurations. The test list file is updated to explicitly enumerate these parameterized test cases, ensuring coverage for both cuda_graph=True and cuda_graph=False scenarios. Accuracy reference YAML files for GSM8K and MMLU benchmarks are updated with entries for the new 47B and 56B models, including baseline and FP8 quantized results.

Changes

Cohort / File(s) Change Summary
Test Parameterization and New Test Classes
tests/integration/defs/accuracy/test_llm_api_pytorch.py
Parameterized test_auto_dtype and test_reasoning_fp8_prequantized over cuda_graph (True/False) in TestNemotronH. Added TestNemotronH_47B_Base and TestNemotronH_56B_Base classes with parameterized tests over cuda_graph and parallelism sizes, including model-specific configurations and assertions for FP8 quantization. Removed max_batch_size argument. Conditional passing of cuda_graph_config to LLM constructor.
Test List Parameterization
tests/integration/test_lists/qa/benchmark_test_list.txt
Expanded test entries to explicitly list parameterized variants for cuda_graph=True and cuda_graph=False for TestNemotronH and new 47B/56B test classes. No changes to exported entities.
Accuracy Reference Updates
tests/integration/defs/accuracy/references/gsm8k.yaml, tests/integration/defs/accuracy/references/mmlu.yaml
Added accuracy entries for new models nvidia/Nemotron-H-47B-Base-8K and nvidia/Nemotron-H-56B-Base-8K with baseline and FP8 quantized results for GSM8K and MMLU benchmarks. No modifications to existing entries.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~15 minutes

Suggested reviewers

  • syuoni
  • crazydemo
  • LarryXFly
    """

Note

⚡️ Unit Test Generation is now available in beta!

Learn more here, or try it out under "Finishing Touches" below.


📜 Recent review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ddca496 and 885a7ad.

📒 Files selected for processing (4)
  • tests/integration/defs/accuracy/references/gsm8k.yaml (1 hunks)
  • tests/integration/defs/accuracy/references/mmlu.yaml (1 hunks)
  • tests/integration/defs/accuracy/test_llm_api_pytorch.py (1 hunks)
  • tests/integration/test_lists/qa/benchmark_test_list.txt (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
  • tests/integration/defs/accuracy/references/mmlu.yaml
  • tests/integration/defs/accuracy/references/gsm8k.yaml
  • tests/integration/test_lists/qa/benchmark_test_list.txt
  • tests/integration/defs/accuracy/test_llm_api_pytorch.py
⏰ 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
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need 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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai or @coderabbitai title anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@xinhe-nv xinhe-nv requested review from crazydemo and LarryXFly July 28, 2025 04:53
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between dc75779 and 394dcaf.

📒 Files selected for processing (2)
  • tests/integration/defs/accuracy/test_llm_api_pytorch.py (1 hunks)
  • tests/integration/test_lists/qa/benchmark_test_list.txt (1 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
🔇 Additional comments (3)
tests/integration/defs/accuracy/test_llm_api_pytorch.py (3)

1583-1584: LGTM! Parameterization implemented correctly.

The test_auto_dtype method has been properly parameterized with the cuda_graph flag, allowing the test to run with both CUDA graph enabled and disabled configurations.


1590-1590: LGTM! Conditional CUDA graph configuration implemented correctly.

The conditional logic properly enables CUDA graph configuration when cuda_graph=True and disables it when cuda_graph=False, following the same pattern used elsewhere in the codebase.


1597-1603: LGTM! Method parameterized correctly with appropriate hardware requirements.

The test_reasoning_fp8_prequantized method has been properly parameterized with the cuda_graph flag using the same pattern as test_auto_dtype. The decorator change from @skip_pre_ada to @skip_pre_hopper is appropriate for FP8 prequantized tests, which typically require Hopper architecture or newer for optimal performance.

@xinhe-nv xinhe-nv force-pushed the user/xinhe/NemotronH branch from 394dcaf to 9e50514 Compare July 28, 2025 09:05
@xinhe-nv xinhe-nv force-pushed the user/xinhe/NemotronH branch 2 times, most recently from b08eda7 to 162176b Compare July 28, 2025 09:59
@xinhe-nv xinhe-nv force-pushed the user/xinhe/NemotronH branch from 0a408ad to 98fcc4f Compare July 28, 2025 23:52
@coderabbitai coderabbitai bot requested a review from LarryXFly July 28, 2025 23:53
@xinhe-nv xinhe-nv force-pushed the user/xinhe/NemotronH branch 4 times, most recently from 0b96cd4 to 596c055 Compare July 29, 2025 06:51
@xinhe-nv xinhe-nv requested a review from tomeras91 July 29, 2025 06:56
@xinhe-nv xinhe-nv enabled auto-merge (squash) July 29, 2025 06:58
@xinhe-nv
Copy link
Collaborator Author

/bot run

@tensorrt-cicd
Copy link
Collaborator

PR_Github #13319 [ run ] triggered by Bot

@tensorrt-cicd
Copy link
Collaborator

PR_Github #13319 [ run ] completed with state SUCCESS
/LLM/main/L0_MergeRequest_PR pipeline #9953 completed with status: 'FAILURE'

@xinhe-nv xinhe-nv force-pushed the user/xinhe/NemotronH branch from 596c055 to e78535c Compare July 29, 2025 09:11
@coderabbitai coderabbitai bot requested a review from litaotju July 29, 2025 09:12
@xinhe-nv xinhe-nv force-pushed the user/xinhe/NemotronH branch from e78535c to ddca496 Compare July 30, 2025 03:49
@xinhe-nv
Copy link
Collaborator Author

/bot run

Signed-off-by: Xin He (SW-GPU) <[email protected]>
@xinhe-nv xinhe-nv force-pushed the user/xinhe/NemotronH branch from ddca496 to 885a7ad Compare July 30, 2025 05:42
@tensorrt-cicd
Copy link
Collaborator

PR_Github #13485 [ run ] triggered by Bot

Copy link
Collaborator

@crazydemo crazydemo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tensorrt-cicd
Copy link
Collaborator

PR_Github #13485 [ run ] completed with state SUCCESS
/LLM/main/L0_MergeRequest_PR pipeline #10099 completed with status: 'FAILURE'

@LarryXFly LarryXFly disabled auto-merge July 30, 2025 08:45
@LarryXFly LarryXFly merged commit d9ab3fd into NVIDIA:main Jul 30, 2025
2 checks passed
@xinhe-nv xinhe-nv deleted the user/xinhe/NemotronH branch July 30, 2025 08:53
lancelly pushed a commit to lancelly/TensorRT-LLM that referenced this pull request Aug 6, 2025
Signed-off-by: Xin He (SW-GPU) <[email protected]>
Co-authored-by: Larry <[email protected]>
Signed-off-by: Lanyu Liao <[email protected]>
jain-ria pushed a commit to jain-ria/TensorRT-LLM that referenced this pull request Aug 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants