Skip to content

feat: add dapo recipe and test#1617

Merged
terrykong merged 4 commits intomainfrom
zhiyul/dapo_sv3_pr
Dec 22, 2025
Merged

feat: add dapo recipe and test#1617
terrykong merged 4 commits intomainfrom
zhiyul/dapo_sv3_pr

Conversation

@ZhiyuLi-Nvidia
Copy link
Contributor

@ZhiyuLi-Nvidia ZhiyuLi-Nvidia commented Dec 9, 2025

What does this PR do ?

Verified the test and recipe: https://wandb.ai/nvidia/nemo-rl/runs/ks1cpy6o/logs?nw=nwuserzhiyul
Update test and recipe: https://wandb.ai/nvidia/nemo-rl/runs/g5r4xwvf
Update test and recipe with 1.5k seq length: https://wandb.ai/nvidia/nemo-rl/runs/eobc39eb

Issues

List issues that this PR closes (syntax):

Usage

  • You can potentially add a usage example below
# Add a code snippet demonstrating how to use this

Before your PR is "Ready for review"

Pre checks:

  • Make sure you read and followed Contributor guidelines
  • Did you write any new necessary tests?
  • Did you run the unit tests and functional tests locally? Visit our Testing Guide for how to run tests
  • Did you add or update any necessary documentation? Visit our Document Development Guide for how to write, build and test the docs.

Additional Information

  • ...

Summary by CodeRabbit

  • New Features

    • Added comprehensive performance configuration and test suite for DAPO DeepSeek v3 with 64-node setup, including GRPO training parameters, resource allocation, data configuration, and automated metrics validation.
  • Chores

    • Updated internal submodule dependencies.

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

@ZhiyuLi-Nvidia ZhiyuLi-Nvidia requested review from a team as code owners December 9, 2025 17:24
@github-actions
Copy link

github-actions bot commented Dec 9, 2025

✅ Submodule Fast-Forward Check Results

Check based on commit: bbd6933 (PR #1617 from zhiyul/dapo_sv3_pr)

✅ Submodules that are properly updated:

Megatron-LM: ✅ PR branch is ahead of main branch (fast-forward)

All submodule changes look good! ✨

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 9, 2025

📝 Walkthrough

Walkthrough

Updates submodule tracking for 3rdparty/Megatron-LM to use a new branch variant with yarn fix support. Adds new DAPO DeepSeek v3 performance configuration file and corresponding test script with environment setup, execution, and metrics evaluation logic.

Changes

Cohort / File(s) Summary
Submodule Updates
.gitmodules, 3rdparty/Megatron-LM-workspace/Megatron-LM
Updated Megatron-LM submodule branch reference from yuya/nemo-rl-use-dev to yuya/nemo-rl-use-dev-w-yarn-fix and bumped commit pointer to 1d646249...
DAPO DeepSeek v3 Config & Tests
examples/configs/recipes/llm/performance/dapo-deepseek-v3-64n8g.yaml, tests/test_suites/llm/performance/dapo-deepseek-v3-64n8g.sh
Added comprehensive YAML configuration for DAPO DeepSeek v3 performance with grpo settings, policy/data/cluster parameters, and logging integrations. Added Bash test script setting up environment, executing experiment via run_grpo_math.py, converting TensorBoard logs to JSON, and conditionally evaluating metrics against thresholds.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • YAML configuration verification: Validate all grpo, policy, cluster, and logging settings align with intended performance tuning parameters and are compatible with the underlying infrastructure
  • Test script logic: Verify environment variable handling (NVLS disable, checkpoint override), parameter passing to Python scripts, and conditional metric evaluation logic
  • Submodule branch correctness: Confirm the new branch variant (...w-yarn-fix) is the intended tracking target and compatible with current codebase

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Test Results For Major Changes ⚠️ Warning PR introduces major changes (DAPO recipe and performance tests) but PR description is in draft status with unchecked pre-review items and no documented test results, performance benchmarks, or regression testing information. Run test suite locally and document results in PR description; provide before-and-after performance comparisons; validate numerics stability; complete all pre-review checklist items; update PR with comprehensive testing information.
✅ Passed checks (3 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check ✅ Passed The title clearly summarizes the main changes: adding a new DAPO (DeepSeek v3) recipe configuration and corresponding test script.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ 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 zhiyul/dapo_sv3_pr

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.

@guyueh1 guyueh1 self-requested a review December 9, 2025 18:17
@ZhiyuLi-Nvidia ZhiyuLi-Nvidia changed the title [Draft] Add dapo recipe and test feat: add dapo recipe and test Dec 11, 2025
@github-actions
Copy link

✅ Submodule Fast-Forward Check Results

Check based on commit: 679273f (PR #1617 from zhiyul/dapo_sv3_pr)

✅ Submodules that are properly updated:

Megatron-LM: ✅ PR branch is ahead of main branch (fast-forward)

All submodule changes look good! ✨

@ZhiyuLi-Nvidia ZhiyuLi-Nvidia added the CI:L1 Run doctests, unit tests, and functional tests label Dec 11, 2025
@ZhiyuLi-Nvidia
Copy link
Contributor Author

@guyueh1 could you take a review?

@guyueh1
Copy link
Contributor

guyueh1 commented Dec 20, 2025

@ZhiyuLi-Nvidia would you add this test to the tests/test_suites/performance_h100.txt?

Signed-off-by: Zhiyu Li <zhiyul@NVIDIA.com>
Signed-off-by: Zhiyu Li <zhiyul@NVIDIA.com>
Signed-off-by: Zhiyu Li <zhiyul@NVIDIA.com>
Signed-off-by: Zhiyu Li <zhiyul@NVIDIA.com>
@ZhiyuLi-Nvidia
Copy link
Contributor Author

@ZhiyuLi-Nvidia would you add this test to the tests/test_suites/performance_h100.txt?

Thank you @guyueh1, just added.

@terrykong
Copy link
Collaborator

@ZhiyuLi-Nvidia is this ready?

@guyueh1 to review

@terrykong terrykong added CI:L0 Run doctests and unit tests and removed CI:L1 Run doctests, unit tests, and functional tests labels Dec 22, 2025
@ZhiyuLi-Nvidia
Copy link
Contributor Author

@ZhiyuLi-Nvidia is this ready?

Yeap.

@guyueh1 I have address you comment to "add this test to the tests/test_suites/performance_h100.txt". Could you take another look?

@terrykong terrykong merged commit 56e8fcb into main Dec 22, 2025
41 of 42 checks passed
@terrykong terrykong deleted the zhiyul/dapo_sv3_pr branch December 22, 2025 18:07
chtruong814 pushed a commit that referenced this pull request Dec 22, 2025
Signed-off-by: Zhiyu Li <zhiyul@NVIDIA.com>
Signed-off-by: NeMo Bot <nemo-bot@nvidia.com>
DeL-TaiseiOzaki pushed a commit to DeL-TaiseiOzaki/RL that referenced this pull request Jan 8, 2026
Signed-off-by: Zhiyu Li <zhiyul@NVIDIA.com>
parthmannan pushed a commit to parthmannan/RL that referenced this pull request Jan 15, 2026
Signed-off-by: Zhiyu Li <zhiyul@NVIDIA.com>
Signed-off-by: Parth Mannan <pmannan@nvidia.com>
yuanhangsu1986 pushed a commit to yuanhangsu1986/RL-Nemontron-Edge-Omni that referenced this pull request Feb 12, 2026
Signed-off-by: Zhiyu Li <zhiyul@NVIDIA.com>
Signed-off-by: yuanhangs <yuanhangs@nvidia.com>
yuanhangsu1986 pushed a commit to yuanhangsu1986/RL-Nemontron-Edge-Omni that referenced this pull request Feb 21, 2026
Signed-off-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: Zhiyu Li <zhiyul@NVIDIA.com>
seonjinn pushed a commit that referenced this pull request Mar 8, 2026
Signed-off-by: Zhiyu Li <zhiyul@NVIDIA.com>
seonjinn pushed a commit that referenced this pull request Mar 9, 2026
Signed-off-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:L0 Run doctests and unit tests r0.5.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add performance scripts for DAPO algorithm

3 participants