Skip to content

cp: fix: qwen32 nightly metric check more stable (1271) into r0.4.0#1308

Merged
terrykong merged 1 commit intor0.4.0from
cherry-pick-1271-r0.4.0
Oct 8, 2025
Merged

cp: fix: qwen32 nightly metric check more stable (1271) into r0.4.0#1308
terrykong merged 1 commit intor0.4.0from
cherry-pick-1271-r0.4.0

Conversation

@chtruong814
Copy link
Copy Markdown
Contributor

@chtruong814 chtruong814 commented Oct 8, 2025

beep boop [🤖]: Hi @terrykong 👋,

we've cherry picked #1271 into  for you! 🚀

Please review and approve this cherry pick by your convenience!

Summary by CodeRabbit

  • Tests
    • Updated loss metric validation to use a rolling average over recent steps, improving stability and reducing false negatives during CI runs.
    • Maintains existing checks for other metrics while making the loss evaluation less sensitive to single-step variance.
    • Enhances reliability across runs without altering application behavior or performance.
    • No changes to user-facing features; test coverage and overall verification goals remain consistent.

Signed-off-by: Terry Kong <terryk@nvidia.com>
Signed-off-by: NeMo Bot <nemo-bot@nvidia.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Oct 8, 2025

📝 Walkthrough

Walkthrough

Updates a test script to change the train/loss assertion from a single-step threshold at step 20 to a windowed mean over the last 16 steps with a slightly adjusted threshold. No other files or declarations are modified.

Changes

Cohort / File(s) Summary
LLM SFT test metric logic
tests/test_suites/llm/sft-qwen2.5-32b-4n8g-fsdp2tp8sp-actckpt.v3.sh
Replace data["train/loss"]["20"] < 0.3 check with mean(data["train/loss"], 16) < 0.31; other checks unchanged.

Sequence Diagram(s)

sequenceDiagram
  participant CI as CI Runner
  participant Test as sft-qwen2.5-32b-4n8g-fsdp2tp8sp-actckpt.v3.sh
  participant Train as Training Job
  participant Metrics as Metrics Store

  CI->>Test: Execute test script
  Test->>Train: Launch training
  Train-->>Metrics: Emit train/loss over steps
  Test->>Metrics: Fetch train/loss series
  Note over Test: New logic: compute mean over last 16 steps
  Test->>Test: mean(train/loss, 16) < 0.31 ?
  alt Pass
    Test-->>CI: Exit 0
  else Fail
    Test-->>CI: Exit non-zero
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested labels

r0.4.0, CI:L1

Suggested reviewers

  • terrykong
  • ashors1

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Title Check ⚠️ Warning The title mostly reflects the change to stabilize the Qwen32 nightly metric check but includes extraneous cherry-pick and branch details, backticks, and numbering that obscure a concise summary. Please simplify the title to a single clear sentence summarizing the main change, such as “Fix Qwen32 nightly metric stability check,” without mentioning the cherry-pick syntax, commit number, or target branch.
✅ Passed checks (3 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 changes. Docstring coverage check skipped.
Test Results For Major Changes ✅ Passed This PR only tweaks a shell-based metric validation in a test suite by changing a single-step loss threshold to a short window mean, which is a minor stability adjustment rather than a major feature, refactor, or performance-sensitive change. Because the instructions only require documented testing for major changes, the absence of explicit test results in the PR description does not violate the check criteria. Therefore, the custom check passes.
✨ 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 cherry-pick-1271-r0.4.0

📜 Recent 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 1860a52 and 9d78d63.

📒 Files selected for processing (1)
  • tests/test_suites/llm/sft-qwen2.5-32b-4n8g-fsdp2tp8sp-actckpt.v3.sh (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.sh

📄 CodeRabbit inference engine (CODING_GUIDELINES.md)

**/*.sh: Follow the Google Shell Style Guide for all shell scripts
Use uv run to execute Python scripts in shell/driver scripts instead of activating virtualenvs and calling python directly
Add the NVIDIA copyright header (with current year) at the top of all shell scripts, excluding tests/ and test-only scripts

Files:

  • tests/test_suites/llm/sft-qwen2.5-32b-4n8g-fsdp2tp8sp-actckpt.v3.sh
tests/test_suites/llm/*.sh

📄 CodeRabbit inference engine (CODING_GUIDELINES.md)

LLM driver script filenames must mirror the YAML base name and follow the same pattern with .sh extension

Files:

  • tests/test_suites/llm/sft-qwen2.5-32b-4n8g-fsdp2tp8sp-actckpt.v3.sh
tests/test_suites/**

📄 CodeRabbit inference engine (CODING_GUIDELINES.md)

Place driver shell scripts and common.env under tests/test_suites// and list nightly tests in tests/test_suites/nightly.txt

Files:

  • tests/test_suites/llm/sft-qwen2.5-32b-4n8g-fsdp2tp8sp-actckpt.v3.sh
⏰ 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: Lint check
  • GitHub Check: Lint check
  • GitHub Check: Post automodel integration comment / Comment on PR
  • GitHub Check: Post submodule check comment / Comment on 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.

@terrykong terrykong added the CI:docs Run doctest label Oct 8, 2025
@terrykong terrykong enabled auto-merge (squash) October 8, 2025 16:06
@terrykong terrykong merged commit e4c0103 into r0.4.0 Oct 8, 2025
66 of 69 checks passed
@terrykong terrykong deleted the cherry-pick-1271-r0.4.0 branch October 8, 2025 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants