Skip to content

[https://nvbugs/6055847][fix] Preserve Nemotron HF mamba cache dtype in bench tuning#12826

Open
hyukn wants to merge 1 commit intoNVIDIA:mainfrom
hyukn:fix/6055847
Open

[https://nvbugs/6055847][fix] Preserve Nemotron HF mamba cache dtype in bench tuning#12826
hyukn wants to merge 1 commit intoNVIDIA:mainfrom
hyukn:fix/6055847

Conversation

@hyukn
Copy link
Copy Markdown
Collaborator

@hyukn hyukn commented Apr 8, 2026

Summary by CodeRabbit

Bug Fixes

  • Fixed Mamba cache dtype configuration logic to properly respect default and automatic settings during benchmark operations, preventing unnecessary configuration application.

Description

Avoid overriding Nemotron hybrid mamba_ssm_cache_dtype with auto during benchmark settings generation, so heuristics keep the HF-configured float32 cache dtype unless the user explicitly overrides it.

Test Coverage

PR Checklist

Please review the following before submitting your PR:

  • PR description clearly explains what and why. If using CodeRabbit's summary, please make sure it makes sense.

  • PR Follows TRT-LLM CODING GUIDELINES to the best of your knowledge.

  • Test cases are provided for new code paths (see test instructions)

  • Any new dependencies have been scanned for license and vulnerabilities

  • CODEOWNERS updated if ownership changes

  • Documentation updated as needed

  • Update tava architecture diagram if there is a significant design change in PR.

  • The reviewers assigned automatically/manually are appropriate for the PR.

  • Please check this after reviewing the above items as appropriate for this PR.

GitHub Bot Help

To see a list of available CI bot commands, please comment /bot help.

@hyukn hyukn requested a review from a team as a code owner April 8, 2026 06:42
@hyukn hyukn requested a review from FrankD412 April 8, 2026 06:42
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 8, 2026

📝 Walkthrough

Walkthrough

Updated conditional logic in benchmark utility for configuring NemotronHybridConfig mamba cache dtype. The setter is now invoked only when both conditions are met: model_config is a NemotronHybridConfig AND mamba_ssm_cache_dtype is neither None nor "auto".

Changes

Cohort / File(s) Summary
NemotronHybridConfig dtype validation
tensorrt_llm/bench/benchmark/utils/general.py
Added conditional gating to prevent setting mamba cache dtype when value is None or "auto". Reformatted isinstance check across multiple lines for improved readability.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~7 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive The description explains the issue and solution concisely, but the Test Coverage section is empty and PR Checklist items are not marked as completed. Complete the Test Coverage section to document relevant test cases that safeguard these changes and address the unchecked PR Checklist items.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the issue (NVBugs ticket), type (fix), and main change (preserving Nemotron HF mamba cache dtype in bench tuning).
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
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

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tensorrt_llm/bench/benchmark/utils/general.py`:
- Around line 114-116: Formatting-only change required: run the project
formatter (yapf/pre-commit) on the file containing the conditional that checks
isinstance(model_config, NemotronHybridConfig) and mamba_ssm_cache_dtype, commit
the resulting formatting-only modifications, and push them to unblock CI; do not
alter logic in the if statement or surrounding functions—only apply the
automated formatting and include the formatted file in this PR.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: cffb9f9b-66ef-40d6-8ad5-3a8dfbf55cfe

📥 Commits

Reviewing files that changed from the base of the PR and between 0b5289f and a0a003a.

📒 Files selected for processing (1)
  • tensorrt_llm/bench/benchmark/utils/general.py

Comment on lines +114 to +116
if isinstance(
model_config, NemotronHybridConfig
) and mamba_ssm_cache_dtype not in (None, "auto"):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Run formatter and commit the result to unblock CI

This change appears functionally correct, but CI is currently blocked by formatting (pre-commit reported yapf modified this file). Please run the formatter and include the formatting-only update in this PR.
As per coding guidelines, "Code formatting is handled by automatic tooling; do not override it unless it substantially improves readability."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tensorrt_llm/bench/benchmark/utils/general.py` around lines 114 - 116,
Formatting-only change required: run the project formatter (yapf/pre-commit) on
the file containing the conditional that checks isinstance(model_config,
NemotronHybridConfig) and mamba_ssm_cache_dtype, commit the resulting
formatting-only modifications, and push them to unblock CI; do not alter logic
in the if statement or surrounding functions—only apply the automated formatting
and include the formatted file in this PR.

…in bench tuning

Avoid overriding Nemotron hybrid mamba_ssm_cache_dtype with auto during benchmark settings generation, so heuristics keep the HF-configured float32 cache dtype unless the user explicitly overrides it.

Signed-off-by: Yukun He <23156053+hyukn@users.noreply.github.com>
@hyukn
Copy link
Copy Markdown
Collaborator Author

hyukn commented Apr 8, 2026

/bot run --disable-fail-fast

@tensorrt-cicd
Copy link
Copy Markdown
Collaborator

PR_Github #42291 [ run ] triggered by Bot. Commit: f13736f Link to invocation

@tensorrt-cicd
Copy link
Copy Markdown
Collaborator

PR_Github #42291 [ run ] completed with state SUCCESS. Commit: f13736f
/LLM/main/L0_MergeRequest_PR pipeline #33087 completed with status: 'FAILURE'

CI Report

⚠️ Action Required:

  • Please check the failed tests and fix your PR
  • If you cannot view the failures, ask the CI triggerer to share details
  • Once fixed, request an NVIDIA team member to trigger CI again

Link to invocation

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.

3 participants