KEMBAR78
[None][doc] fix example in docstring by tomeras91 · Pull Request #7410 · NVIDIA/TensorRT-LLM · GitHub
Skip to content

Conversation

@tomeras91
Copy link
Collaborator

@tomeras91 tomeras91 commented Aug 31, 2025

Summary by CodeRabbit

  • Documentation
    • Corrected a misleading example in the docs to accurately reflect computed results for sequence-to-chunk indexing and offsets.
    • Clarifies expected outputs to reduce confusion when interpreting chunk index calculations.
    • No functional, performance, or behavioral changes to the software; this update affects documentation only.

Description

Fix a wrong expected output in the docstring of cu_seqlens_to_chunk_indices_offsets. The explanation was correct, but the expected output had a typo and didn't match

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

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

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

Signed-off-by: Tomer Asida <57313761+tomeras91@users.noreply.github.com>
@tomeras91
Copy link
Collaborator Author

/bot run

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Aug 31, 2025

📝 Walkthrough

Walkthrough

Docstring example in cu_seqlens_to_chunk_indices_offsets updated: for cu_seqlens = [0, 5, 10] and chunk_size = 8, chunk_indices corrected from [0, 1, 0] to [0, 0, 1]. No code or behavior changes; chunk_offsets remains [0, 5, 0].

Changes

Cohort / File(s) Summary
Docstring correction
tensorrt_llm/_torch/modules/mamba/mamba2_metadata.py
Fixed example output for chunk_indices in cu_seqlens_to_chunk_indices_offsets docstring; no logic or API changes.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

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.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share
🪧 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.
    • 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.
  • 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 the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore or @coderabbit 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.

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • 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.

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: 0

🧹 Nitpick comments (2)
tensorrt_llm/_torch/modules/mamba/mamba2_metadata.py (2)

1-2: Consider updating the header year range to include 2025.

Repo guideline says to prepend the current year; this header ends at 2024. If policy permits, update to 2022–2025.

-# SPDX-FileCopyrightText: Copyright (c) 2022-2024 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+# SPDX-FileCopyrightText: Copyright (c) 2022-2025 NVIDIA CORPORATION & AFFILIATES. All rights reserved.

81-84: Typo in comment: “inidces” → “indices”.

Minor readability nit.

-        # adjust inidces and offsets
+        # adjust indices and offsets
📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between ec595a8 and b444ff9.

📒 Files selected for processing (1)
  • tensorrt_llm/_torch/modules/mamba/mamba2_metadata.py (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{cpp,cc,cxx,cu,h,hpp,hh,hxx,cuh,py}

📄 CodeRabbit inference engine (CODING_GUIDELINES.md)

**/*.{cpp,cc,cxx,cu,h,hpp,hh,hxx,cuh,py}: Use spaces only; no tabs; indent with 4 spaces
Prepend NVIDIA copyright header (current year) to all source files (.cpp, .h, .cu, .py, etc.)

Files:

  • tensorrt_llm/_torch/modules/mamba/mamba2_metadata.py
**/*.py

📄 CodeRabbit inference engine (CODING_GUIDELINES.md)

**/*.py: Python code must target Python 3.8+
Indent Python with 4 spaces; no tabs
Preserve module namespaces when importing: from package.subpackage import foo; then call foo.SomeClass() instead of importing the class directly
Python naming: files snake_case; classes PascalCase; functions/methods snake_case; locals snake_case (prefix k_ when starting with a number); globals UPPER_SNAKE_CASE with G_ prefix; constants UPPER_SNAKE_CASE
Avoid shadowing outer-scope variables; initialize all externally visible members in init
Prefer docstrings for interfaces used outside a file; limit comments to function-internal or file-local interfaces
Use Google-style docstrings for classes and functions; document attributes/variables inline so Sphinx can render them
Avoid reflection when simpler alternatives exist; prefer explicit parameters and return dicts over locals()/dynamic tricks
In try/except, catch the narrowest exceptions possible; keep try bodies minimal and use else for the main logic when doing duck-typing checks

Files:

  • tensorrt_llm/_torch/modules/mamba/mamba2_metadata.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
🔇 Additional comments (1)
tensorrt_llm/_torch/modules/mamba/mamba2_metadata.py (1)

48-49: Docstring example fix is correct.

For cu_seqlens = [0, 5, 10] and chunk_size = 8, the logical chunks map to physical chunks [0, 0, 1] with offsets [0, 5, 0]. Change aligns with the explanation.

Signed-off-by: Tomer Asida <57313761+tomeras91@users.noreply.github.com>
@tomeras91
Copy link
Collaborator Author

/bot run

@tensorrt-cicd
Copy link
Collaborator

PR_Github #17121 [ run ] triggered by Bot

@tensorrt-cicd
Copy link
Collaborator

PR_Github #17122 [ run ] triggered by Bot

@tensorrt-cicd
Copy link
Collaborator

PR_Github #17121 [ run ] completed with state ABORTED

@chzblych
Copy link
Collaborator

chzblych commented Sep 1, 2025

/bot run

@tensorrt-cicd
Copy link
Collaborator

PR_Github #17125 [ run ] triggered by Bot

@tensorrt-cicd
Copy link
Collaborator

PR_Github #17122 [ run ] completed with state ABORTED
/LLM/main/L0_MergeRequest_PR pipeline #12876 completed with status: 'FAILURE'

@tensorrt-cicd
Copy link
Collaborator

PR_Github #17125 [ run ] completed with state FAILURE
/LLM/main/L0_MergeRequest_PR pipeline #12878 completed with status: 'FAILURE'

@chzblych
Copy link
Collaborator

chzblych commented Sep 1, 2025

/bot run

@tensorrt-cicd
Copy link
Collaborator

PR_Github #17129 [ run ] triggered by Bot

@tensorrt-cicd
Copy link
Collaborator

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

Copy link
Collaborator

@shaharmor98 shaharmor98 left a comment

Choose a reason for hiding this comment

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

LGTM

@tomeras91
Copy link
Collaborator Author

/bot run

@tensorrt-cicd
Copy link
Collaborator

PR_Github #17156 [ run ] triggered by Bot

@tensorrt-cicd
Copy link
Collaborator

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

@tomeras91
Copy link
Collaborator Author

/bot run

@tensorrt-cicd
Copy link
Collaborator

PR_Github #17304 [ run ] triggered by Bot

@tensorrt-cicd
Copy link
Collaborator

PR_Github #17304 [ run ] completed with state SUCCESS
/LLM/main/L0_MergeRequest_PR pipeline #13006 completed with status: 'SUCCESS'

@tomeras91 tomeras91 merged commit 9c8d216 into NVIDIA:main Sep 2, 2025
5 checks passed
@tomeras91 tomeras91 deleted the fix-mamba-chunk-indices-docstring branch September 2, 2025 08:59
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.

4 participants