Skip to content

feat: support swanlab logger#923

Merged
terrykong merged 2 commits intomainfrom
tk/pr-716
Sep 25, 2025
Merged

feat: support swanlab logger#923
terrykong merged 2 commits intomainfrom
tk/pr-716

Conversation

@terrykong
Copy link
Collaborator

Replacement of #716 (just resolving issues in the PR to help it along)

Thanks to @tpoisonooo for driving

@github-actions github-actions bot added the documentation Improvements or additions to documentation label Aug 14, 2025
@terrykong terrykong mentioned this pull request Aug 14, 2025
4 tasks
@terrykong terrykong requested review from parthchadha and yfw August 14, 2025 18:11
@terrykong terrykong marked this pull request as ready for review August 14, 2025 18:11
@terrykong terrykong enabled auto-merge August 14, 2025 18:17
@terrykong terrykong disabled auto-merge August 14, 2025 18:31
@terrykong terrykong enabled auto-merge August 14, 2025 18:31
yfw
yfw previously approved these changes Aug 14, 2025
@terrykong terrykong added this pull request to the merge queue Aug 14, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to a conflict with the base branch Aug 14, 2025
@terrykong terrykong enabled auto-merge August 15, 2025 06:45
@terrykong terrykong added this pull request to the merge queue Aug 15, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 15, 2025
@terrykong
Copy link
Collaborator Author

the doctest CI test failing is odd. still working on reproducing it locally

@terrykong
Copy link
Collaborator Author

ok, was able to pull the image that the CI used, for some reason the image is not correct. need to follow up with someone from @NVIDIA-NeMo/automation

@terrykong terrykong added CI:docs Run doctest and removed CI:docs Run doctest labels Aug 20, 2025
@terrykong terrykong added CI:docs Run doctest and removed CI:docs Run doctest labels Aug 20, 2025
@terrykong
Copy link
Collaborator Author

Thanks to @chtruong814 for helping solve this in #942

Retrying the CI. @yfw could you re-approve?

@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 28, 2025
@terrykong terrykong enabled auto-merge August 28, 2025 16:02
@yfw yfw self-requested a review August 28, 2025 16:22
yfw
yfw previously approved these changes Aug 28, 2025
@terrykong terrykong added this pull request to the merge queue Aug 28, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 28, 2025
@github-actions
Copy link

github-actions bot commented Sep 4, 2025

❌ Submodule Fast-Forward Check Failed

Check based on commit: cdd5837 (PR #923 from tk/pr-716)

❌ Submodules that need attention:

Megatron-LM: ❌ PR branch is BEHIND main branch
TARGET (main branch): https://github.com/terrykong/Megatron-LM/commits/e2d5bcd605108e2cf64fdb91fdfc669f10a57f56/
CURRENT (PR #923 from tk/pr-716): https://github.com/terrykong/Megatron-LM/commits/2ff0f099ffc30ffd152e3e29e921a1609d00855c/

Please ensure all submodule commits are fast-forwards of the main branch before merging.

@terrykong terrykong enabled auto-merge September 4, 2025 04:44
@github-actions
Copy link

github-actions bot commented Sep 4, 2025

❌ Submodule Fast-Forward Check Failed

Check based on commit: 397e420 (PR #923 from tk/pr-716)

❌ Submodules that need attention:

Megatron-LM: ❌ PR branch is BEHIND main branch
TARGET (main branch): https://github.com/terrykong/Megatron-LM/commits/e2d5bcd605108e2cf64fdb91fdfc669f10a57f56/
CURRENT (PR #923 from tk/pr-716): https://github.com/terrykong/Megatron-LM/commits/2ff0f099ffc30ffd152e3e29e921a1609d00855c/

Please ensure all submodule commits are fast-forwards of the main branch before merging.

@terrykong terrykong disabled auto-merge September 4, 2025 04:45
@terrykong terrykong enabled auto-merge September 4, 2025 04:46
yfw
yfw previously approved these changes Sep 4, 2025
@terrykong terrykong added this pull request to the merge queue Sep 4, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Sep 4, 2025
@terrykong terrykong added this pull request to the merge queue Sep 4, 2025
github-merge-queue bot pushed a commit that referenced this pull request Sep 4, 2025
Signed-off-by: Terry Kong <terryk@nvidia.com>
Signed-off-by: Terry Kong <terrycurtiskong@gmail.com>
Co-authored-by: tpoisonooo <khj.application@aliyun.com>
Co-authored-by: Yi-Fu Wu <yifu.wu@gmail.com>
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Sep 5, 2025
@terrykong terrykong requested review from a team as code owners September 25, 2025 17:03
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 25, 2025

Warning

Rate limit exceeded

@terrykong has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 6 minutes and 9 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between e60c4d9 and 0f7fd8f.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (12)
  • docs/design-docs/logger.md (9 hunks)
  • examples/configs/dpo.yaml (1 hunks)
  • examples/configs/grpo_math_1B.yaml (1 hunks)
  • examples/configs/grpo_math_1B_megatron.yaml (1 hunks)
  • examples/configs/grpo_sliding_puzzle.yaml (1 hunks)
  • examples/configs/rm.yaml (1 hunks)
  • examples/configs/sft.yaml (1 hunks)
  • examples/configs/sft_openmathinstruct2.yaml (1 hunks)
  • examples/configs/vlm_grpo_3B.yaml (1 hunks)
  • nemo_rl/utils/logger.py (7 hunks)
  • pyproject.toml (1 hunks)
  • tests/unit/utils/test_logger.py (22 hunks)
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch tk/pr-716

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.

Signed-off-by: Terry Kong <terryk@nvidia.com>

revert recipes

Signed-off-by: Terry Kong <terryk@nvidia.com>
Signed-off-by: Terry Kong <terryk@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI:L1 Run doctests, unit tests, and functional tests documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants