Skip to content

docs: Create performance-summary.md for NeMo RL#1560

Merged
terrykong merged 20 commits intomainfrom
snowmanwwg-patch-4
Dec 1, 2025
Merged

docs: Create performance-summary.md for NeMo RL#1560
terrykong merged 20 commits intomainfrom
snowmanwwg-patch-4

Conversation

@snowmanwwg
Copy link
Copy Markdown
Contributor

@snowmanwwg snowmanwwg commented Nov 24, 2025

Added performance summary documentation for NeMo RL, including benchmarks, nomenclature, and performance metrics for large language models.

Summary by CodeRabbit

  • Documentation
    • Added comprehensive performance benchmarks documentation for NeMo RL framework, detailing performance metrics, training configurations, and generation performance across different systems and models.

✏️ Tip: You can customize this high-level summary in your review settings.

@snowmanwwg snowmanwwg requested a review from a team as a code owner November 24, 2025 07:23
@github-actions github-actions Bot added the Documentation Improvements or additions to documentation label Nov 24, 2025
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Nov 24, 2025

📝 Walkthrough

Walkthrough

A new documentation file is added to provide benchmarking information for NVIDIA NeMo Framework's NeMo RL training. The document includes performance metrics nomenclature, summary structure, and placeholder tables with TODO items for future updates.

Changes

Cohort / File(s) Summary
Performance documentation
docs/performance-summary.md
New file added documenting NeMo RL performance benchmarks, including metrics definitions (Step time, Tokens/sec/GPU, Training MFU), scaffold structure for performance data tables, and placeholder entries for GRPO Performance benchmarks

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

  • Review is straightforward since this is a documentation-only addition with placeholder content and TODO markers

Suggested labels

CI:docs

Suggested reviewers

  • terrykong

Pre-merge checks and finishing touches

✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Test Results For Major Changes ✅ Passed PR adds documentation file with performance benchmarks; documentation-only changes without code modifications are minor and do not require formal test documentation per custom check criteria.
Title check ✅ Passed The title 'docs: Create performance-summary.md for NeMo RL' directly and clearly describes the main change—adding a new documentation file for NeMo RL performance benchmarks.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch snowmanwwg-patch-4

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

  • Provide your own instructions using the high_level_summary_instructions setting.
  • Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
  • Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

  1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
  2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
  3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
  4. 📊 Contributor Summary — Include a Markdown table showing contributions:
    | Contributor | Lines Added | Lines Removed | Files Changed |
  5. ✔️ Additional Notes — Add any extra reviewer context.
    Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.


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

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

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4a59436 and 76b258b.

📒 Files selected for processing (1)
  • docs/performance-summary.md (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-20T14:59:08.052Z
Learnt from: CR
Repo: NVIDIA-NeMo/RL PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2025-09-20T14:59:08.052Z
Learning: If a change could affect performance, include before-and-after performance numbers in the PR description, along with configuration and context.

Applied to files:

  • docs/performance-summary.md
⏰ 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). (4)
  • GitHub Check: build-container / main
  • GitHub Check: Lint check
  • GitHub Check: Post submodule check comment / Comment on PR
  • GitHub Check: Post automodel integration comment / Comment on PR
🔇 Additional comments (2)
docs/performance-summary.md (2)

1-60: Add performance context to the PR description.

Per learnings, when a change involves performance documentation or impacts performance, the PR description should include before-and-after performance numbers, along with configuration and context. This helps reviewers understand the significance of the changes and any performance implications.

Please update the PR description to include relevant performance context or benchmarking goals for this documentation.


54-57: The table structure is correctly aligned and will render properly.

Verification shows all four rows (header, separator, and both data rows) contain exactly 19 pipes, which means 18 columns across the entire table. While the spacing around pipes is inconsistent for visual readability, the underlying table structure is sound and will parse correctly in Markdown. No fixes are required.

Likely an incorrect or invalid review comment.

Comment thread docs/performance-summary.md Outdated
Comment thread docs/performance-summary.md Outdated
Comment thread docs/performance-summary.md Outdated
Copy link
Copy Markdown
Collaborator

@terrykong terrykong left a comment

Choose a reason for hiding this comment

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

@snowmanwwg did you want to put this in the sidebar of our docs?

image

If so, you'll need to add this document into docs/index.md

@snowmanwwg
Copy link
Copy Markdown
Contributor Author

@snowmanwwg did you want to put this in the sidebar of our docs?

image If so, you'll need to add this document into `docs/index.md`

Yes I want it to be an item in the side bar. How to add to index.md?

Comment thread docs/performance-summary.md Outdated
Copy link
Copy Markdown
Contributor

@ZhiyuLi-Nvidia ZhiyuLi-Nvidia left a comment

Choose a reason for hiding this comment

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

Added the formula for tokens/sec/GPU

Comment thread docs/performance-summary.md Outdated
@snowmanwwg snowmanwwg requested a review from a team as a code owner November 24, 2025 19:46
Comment thread docs/about/performance-summary.md Outdated
Comment thread docs/about/performance-summary.md Outdated
Comment thread docs/about/performance-summary.md Outdated
Comment thread docs/about/performance-summary.md Outdated
Comment thread docs/about/performance-summary.md Outdated
Comment thread docs/about/performance-summary.md Outdated
Comment thread docs/about/performance-summary.md Outdated
Comment thread docs/about/performance-summary.md Outdated
@ZhiyuLi-Nvidia
Copy link
Copy Markdown
Contributor

Can we also replace MOE with MoE? I felt like the latter is more common.

@guyueh1 guyueh1 changed the title Create performance-summary.md for NeMo RL docs: Create performance-summary.md for NeMo RL Nov 25, 2025
Comment thread docs/about/performance-summary.md Outdated
terrykong and others added 3 commits December 1, 2025 20:53
Co-authored-by: Guyue Huang <140554423+guyueh1@users.noreply.github.com>
Signed-off-by: Terry Kong <terrycurtiskong@gmail.com>
Signed-off-by: Terry Kong <terryk@nvidia.com>
Co-authored-by: Zhiyu Li <zhiyul@NVIDIA.com>
Signed-off-by: Terry Kong <terrycurtiskong@gmail.com>
Signed-off-by: Terry Kong <terryk@nvidia.com>
Signed-off-by: Terry Kong <terryk@nvidia.com>
@terrykong terrykong enabled auto-merge (squash) December 1, 2025 21:11
@terrykong terrykong added CI:docs Run doctest and removed CI:docs Run doctest labels Dec 1, 2025
@terrykong terrykong merged commit cff17f8 into main Dec 1, 2025
40 checks passed
@terrykong terrykong deleted the snowmanwwg-patch-4 branch December 1, 2025 23:38
DeL-TaiseiOzaki pushed a commit to DeL-TaiseiOzaki/RL that referenced this pull request Jan 8, 2026
Signed-off-by: Wenwen Gao <94138584+snowmanwwg@users.noreply.github.com>
Signed-off-by: Lawrence Lane <llane@nvidia.com>
Signed-off-by: Youngeun Kwon <youngeunk@nvidia.com>
Signed-off-by: Terry Kong <terrycurtiskong@gmail.com>
Signed-off-by: Terry Kong <terryk@nvidia.com>
Co-authored-by: L.B. <llane@nvidia.com>
Co-authored-by: Youngeun Kwon <youngeunk@nvidia.com>
Co-authored-by: Guyue Huang <140554423+guyueh1@users.noreply.github.com>
Co-authored-by: Terry Kong <terrycurtiskong@gmail.com>
Co-authored-by: Terry Kong <terryk@nvidia.com>
Co-authored-by: Zhiyu Li <zhiyul@NVIDIA.com>
yuanhangsu1986 pushed a commit to yuanhangsu1986/RL-Nemontron-Edge-Omni that referenced this pull request Feb 21, 2026
Signed-off-by: Wenwen Gao <94138584+snowmanwwg@users.noreply.github.com>
Signed-off-by: Lawrence Lane <llane@nvidia.com>
Signed-off-by: Youngeun Kwon <youngeunk@nvidia.com>
Signed-off-by: Terry Kong <terrycurtiskong@gmail.com>
Signed-off-by: Terry Kong <terryk@nvidia.com>
Co-authored-by: L.B. <llane@nvidia.com>
Co-authored-by: Youngeun Kwon <youngeunk@nvidia.com>
Co-authored-by: Guyue Huang <140554423+guyueh1@users.noreply.github.com>
Co-authored-by: Terry Kong <terrycurtiskong@gmail.com>
Co-authored-by: Terry Kong <terryk@nvidia.com>
Co-authored-by: Zhiyu Li <zhiyul@NVIDIA.com>
Signed-off-by: yuanhangs <yuanhangs@nvidia.com>
seonjinn pushed a commit that referenced this pull request Mar 8, 2026
Signed-off-by: Wenwen Gao <94138584+snowmanwwg@users.noreply.github.com>
Signed-off-by: Lawrence Lane <llane@nvidia.com>
Signed-off-by: Youngeun Kwon <youngeunk@nvidia.com>
Signed-off-by: Terry Kong <terrycurtiskong@gmail.com>
Signed-off-by: Terry Kong <terryk@nvidia.com>
Co-authored-by: L.B. <llane@nvidia.com>
Co-authored-by: Youngeun Kwon <youngeunk@nvidia.com>
Co-authored-by: Guyue Huang <140554423+guyueh1@users.noreply.github.com>
Co-authored-by: Terry Kong <terrycurtiskong@gmail.com>
Co-authored-by: Terry Kong <terryk@nvidia.com>
Co-authored-by: Zhiyu Li <zhiyul@NVIDIA.com>
seonjinn pushed a commit that referenced this pull request Mar 8, 2026
Signed-off-by: Wenwen Gao <94138584+snowmanwwg@users.noreply.github.com>
Signed-off-by: Lawrence Lane <llane@nvidia.com>
Signed-off-by: Youngeun Kwon <youngeunk@nvidia.com>
Signed-off-by: Terry Kong <terrycurtiskong@gmail.com>
Signed-off-by: Terry Kong <terryk@nvidia.com>
Co-authored-by: L.B. <llane@nvidia.com>
Co-authored-by: Youngeun Kwon <youngeunk@nvidia.com>
Co-authored-by: Guyue Huang <140554423+guyueh1@users.noreply.github.com>
Co-authored-by: Terry Kong <terrycurtiskong@gmail.com>
Co-authored-by: Terry Kong <terryk@nvidia.com>
Co-authored-by: Zhiyu Li <zhiyul@NVIDIA.com>
seonjinn pushed a commit that referenced this pull request Mar 9, 2026
Signed-off-by: Wenwen Gao <94138584+snowmanwwg@users.noreply.github.com>
Signed-off-by: Lawrence Lane <llane@nvidia.com>
Signed-off-by: Youngeun Kwon <youngeunk@nvidia.com>
Signed-off-by: Terry Kong <terrycurtiskong@gmail.com>
Signed-off-by: Terry Kong <terryk@nvidia.com>
Co-authored-by: L.B. <llane@nvidia.com>
Co-authored-by: Youngeun Kwon <youngeunk@nvidia.com>
Co-authored-by: Guyue Huang <140554423+guyueh1@users.noreply.github.com>
Co-authored-by: Terry Kong <terrycurtiskong@gmail.com>
Co-authored-by: Terry Kong <terryk@nvidia.com>
Co-authored-by: Zhiyu Li <zhiyul@NVIDIA.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI:docs Run doctest Documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants