fix: fix several nightly tests that were flaky

[terrykong]

• dpo loss increase was due to a num_workers change that changed dataset seed. needed to update these test metrics
• increased time on some tests to account for ckpt download or ones that finished too closely to the time limit
• some perf benchmarks were missing TB logs and failed
• some benchmarks had memory increases, but are already close to the 80G limit, so i removed those and we can detect regressions simply if the test ooms now

Summary by CodeRabbit

• Tests

  • Adjusted training loss metric thresholds across DPO and SFT test suites.
  • Increased allowed execution times for multiple tests.
  • Updated GPU hour consumption benchmarks for nightly test validation.
  • Removed memory usage assertion from specific test metrics.
  • Increased memory threshold constraints.

• Chores

  • Enabled TensorBoard logging in performance test configurations.
  • Modified model save format configuration setting.

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

[coderabbitai]

📝 Walkthrough

Walkthrough

Configuration and test parameter adjustments across multiple LLM test suites and recipes. Changes include updating training loss thresholds, increasing time limits (NUM_MINUTES), adjusting memory thresholds, enabling TensorBoard logging, and updating overall compute hour limits in unit tests.

Changes

Cohort / File(s)	Summary
Model Configuration examples/configs/recipes/llm/dapo-qwen2.5-7b.yaml	Changed model_save_format from "dcp" to null, removing explicit checkpoint format specification.
DPO Test Suites tests/test_suites/llm/dpo-llama3.1-8b-instruct-4n8g-fsdp2tp4.sh, tests/test_suites/llm/dpo-llama3.1-8b-instruct-4n8g-megatron.v2.sh	Updated train/loss metric threshold from 3.6 to 3.65 at step 1, relaxing passing criteria for initial training loss validation.
Performance Test Suites tests/test_suites/llm/performance/grpo-qwen3-235b-16n8g.sh, tests/test_suites/llm/performance/grpo-qwen3-235b-32n8g-async-1off.sh	Increased NUM_MINUTES from 100 to 115 and enabled TensorBoard logging via logger.tensorboard_enabled=True alongside existing Weights & Biases configuration.
SFT Test Suites (Timeout) tests/test_suites/llm/sft-llama3.2-1b-1n8g-fsdp2tp1.v3.sh, tests/test_suites/llm/sft-nanov3-30BA3B-2n8g-fsdp2-lora.sh, tests/test_suites/llm/sft-nanov3-30BA3B-2n8g-fsdp2.sh	Updated NUM_MINUTES from 15 to 30 to allow longer run durations, with updated comments noting larger buffer for checkpoint downloads.
SFT Test Suites (Memory & Assertions) tests/test_suites/llm/sft-llama3.1-70b-8n8g-tp4pp2-long-megatron.sh, tests/test_suites/llm/sft-llama3.1-8b-1n8g-fsdp2tp1-dynamicbatch.sh	Removed memory usage assertion (max(data["ray/node.0.gpu.0.mem_gb"]) < 70) from one test; increased memory threshold from 70 to 75 GB in another with observational note of ~72.6 GB usage.
Unit Tests tests/unit/test_recipes_and_test_suites.py	Renamed test function from test_nightly_compute_stays_below_1140_hours to test_nightly_compute_stays_below_1180_hours and updated assertion upper bound from 1140 to 1180 GPU hours.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

• fix: dpo mistral nightly needs more time #1225: Both PRs adjust test timeouts by increasing NUM_MINUTES to accommodate longer test execution durations.
• feat: Add Nemotron‑3 Nano 30B A3B BF16 SFT nightly tests (FSDP2, +LoRA) #1648: This PR directly modifies the same sft-nanov3 test scripts that were introduced in the referenced PR, updating their runtime parameters.
• cp: feat: Add Nemotron‑3 Nano 30B A3B BF16 SFT nightly tests (FSDP2, +LoRA) (1648) into r0.5.0 #1697: Both PRs touch the sft-nanov3-30BA3B test suite scripts, with this PR adjusting parameters added in the referenced PR.

Suggested reviewers

• yfw
• terrykong
• chtruong814

🚥 Pre-merge checks | ✅ 2 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.
Test Results For Major Changes	⚠️ Warning	PR modifies test thresholds affecting numerics/convergence but provides only rationale for changes, not validation demonstrating no regression.	Include test execution results, before/after convergence data, and evidence that the 1.39% threshold relaxation appropriately accounts for changes without indicating model quality regression.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix: fix several nightly tests that were flaky' directly and accurately summarizes the main change—fixing flaky nightly tests across multiple test suites.

_{✏️ Tip: You can configure your own custom Pre-merge Checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

examples/configs/recipes/llm/dapo-qwen2.5-7b.yaml (1)

32-32: Clarify the rationale for changing checkpoint format from DCP to default (safetensors).

Setting model_save_format to null will cause the system to use the default "safetensors" format instead of the distributed checkpoint (DCP) format. While this aligns with the PR's goal of fixing flaky nightly tests (safetensors is a more standard, potentially more reliable format for downloads), this behavioral change is not documented in the PR description.

Please add a note to the PR description explaining why DCP was replaced with the safetensors default, particularly how this change addresses the checkpoint download delays mentioned in the PR objectives.

📜 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 1720466 and 3f7d520.

📒 Files selected for processing (11)

• examples/configs/recipes/llm/dapo-qwen2.5-7b.yaml
• tests/test_suites/llm/dpo-llama3.1-8b-instruct-4n8g-fsdp2tp4.sh
• tests/test_suites/llm/dpo-llama3.1-8b-instruct-4n8g-megatron.v2.sh
• tests/test_suites/llm/performance/grpo-qwen3-235b-16n8g.sh
• tests/test_suites/llm/performance/grpo-qwen3-235b-32n8g-async-1off.sh
• tests/test_suites/llm/sft-llama3.1-70b-8n8g-tp4pp2-long-megatron.sh
• tests/test_suites/llm/sft-llama3.1-8b-1n8g-fsdp2tp1-dynamicbatch.sh
• tests/test_suites/llm/sft-llama3.2-1b-1n8g-fsdp2tp1.v3.sh
• tests/test_suites/llm/sft-nanov3-30BA3B-2n8g-fsdp2-lora.sh
• tests/test_suites/llm/sft-nanov3-30BA3B-2n8g-fsdp2.sh
• tests/unit/test_recipes_and_test_suites.py

💤 Files with no reviewable changes (1)

• tests/test_suites/llm/sft-llama3.1-70b-8n8g-tp4pp2-long-megatron.sh

🧰 Additional context used

📓 Path-based instructions (7)

**/*.sh

📄 CodeRabbit inference engine (CODING_GUIDELINES.md)

**/*.sh: Use uv run instead of python to execute scripts
Follow the Google Shell Style Guide for shell scripts

Files:

• tests/test_suites/llm/performance/grpo-qwen3-235b-16n8g.sh
• tests/test_suites/llm/sft-llama3.2-1b-1n8g-fsdp2tp1.v3.sh
• tests/test_suites/llm/sft-nanov3-30BA3B-2n8g-fsdp2-lora.sh
• tests/test_suites/llm/dpo-llama3.1-8b-instruct-4n8g-fsdp2tp4.sh
• tests/test_suites/llm/sft-nanov3-30BA3B-2n8g-fsdp2.sh
• tests/test_suites/llm/performance/grpo-qwen3-235b-32n8g-async-1off.sh
• tests/test_suites/llm/sft-llama3.1-8b-1n8g-fsdp2tp1-dynamicbatch.sh
• tests/test_suites/llm/dpo-llama3.1-8b-instruct-4n8g-megatron.v2.sh

tests/test_suites/**/*.sh

📄 CodeRabbit inference engine (CODING_GUIDELINES.md)

tests/test_suites/**/*.sh: When adding support for a new model, create a corresponding driver shell script under tests/test_suites/ in the matching domain
Driver shell scripts should match the YAML base name with .sh extension and invoke training entrypoint with uv run

Files:

• tests/test_suites/llm/performance/grpo-qwen3-235b-16n8g.sh
• tests/test_suites/llm/sft-llama3.2-1b-1n8g-fsdp2tp1.v3.sh
• tests/test_suites/llm/sft-nanov3-30BA3B-2n8g-fsdp2-lora.sh
• tests/test_suites/llm/dpo-llama3.1-8b-instruct-4n8g-fsdp2tp4.sh
• tests/test_suites/llm/sft-nanov3-30BA3B-2n8g-fsdp2.sh
• tests/test_suites/llm/performance/grpo-qwen3-235b-32n8g-async-1off.sh
• tests/test_suites/llm/sft-llama3.1-8b-1n8g-fsdp2tp1-dynamicbatch.sh
• tests/test_suites/llm/dpo-llama3.1-8b-instruct-4n8g-megatron.v2.sh

!(**/tests/**|**/test_*.py|**/test_*.sh)

📄 CodeRabbit inference engine (CODING_GUIDELINES.md)

Add the NVIDIA copyright header to all Python files and shell scripts (excluding tests). The header should include the current year

Files:

• tests/test_suites/llm/performance/grpo-qwen3-235b-16n8g.sh
• tests/test_suites/llm/sft-llama3.2-1b-1n8g-fsdp2tp1.v3.sh
• examples/configs/recipes/llm/dapo-qwen2.5-7b.yaml
• tests/test_suites/llm/sft-nanov3-30BA3B-2n8g-fsdp2-lora.sh
• tests/unit/test_recipes_and_test_suites.py
• tests/test_suites/llm/dpo-llama3.1-8b-instruct-4n8g-fsdp2tp4.sh
• tests/test_suites/llm/sft-nanov3-30BA3B-2n8g-fsdp2.sh
• tests/test_suites/llm/performance/grpo-qwen3-235b-32n8g-async-1off.sh
• tests/test_suites/llm/sft-llama3.1-8b-1n8g-fsdp2tp1-dynamicbatch.sh
• tests/test_suites/llm/dpo-llama3.1-8b-instruct-4n8g-megatron.v2.sh

**/*.{py,sh}

📄 CodeRabbit inference engine (CODING_GUIDELINES.md)

The NVIDIA copyright header should appear at the top of all Python files and shell scripts (excluding tests)

Files:

• tests/test_suites/llm/performance/grpo-qwen3-235b-16n8g.sh
• tests/test_suites/llm/sft-llama3.2-1b-1n8g-fsdp2tp1.v3.sh
• tests/test_suites/llm/sft-nanov3-30BA3B-2n8g-fsdp2-lora.sh
• tests/unit/test_recipes_and_test_suites.py
• tests/test_suites/llm/dpo-llama3.1-8b-instruct-4n8g-fsdp2tp4.sh
• tests/test_suites/llm/sft-nanov3-30BA3B-2n8g-fsdp2.sh
• tests/test_suites/llm/performance/grpo-qwen3-235b-32n8g-async-1off.sh
• tests/test_suites/llm/sft-llama3.1-8b-1n8g-fsdp2tp1-dynamicbatch.sh
• tests/test_suites/llm/dpo-llama3.1-8b-instruct-4n8g-megatron.v2.sh

examples/configs/recipes/**/*.yaml

📄 CodeRabbit inference engine (CODING_GUIDELINES.md)

When adding support for a new model, create a recipe YAML under examples/configs/recipes/ in the appropriate domain subdirectory (llm, vlm, etc.)

Files:

• examples/configs/recipes/llm/dapo-qwen2.5-7b.yaml

examples/configs/recipes/llm/*.yaml

📄 CodeRabbit inference engine (CODING_GUIDELINES.md)

Recipe YAML files should follow the naming pattern: --ng-[-modifiers][-long][.vN].yaml for LLM recipes

Files:

• examples/configs/recipes/llm/dapo-qwen2.5-7b.yaml

**/*.py

📄 CodeRabbit inference engine (CODING_GUIDELINES.md)

**/*.py: Conform code to Python 3.12+
Indent code with 4 spaces. Do not use tabs
Use snake_case for file names
Use PascalCase for class names
Use snake_case for function and method names
Use snake_case for local variables
Prefix variable names that start with a number with 'k' (e.g., k_99th_percentile)
Use upper snake_case with 'G' prefix for global variables (e.g., G_MY_GLOBAL)
Use upper snake_case for constants
Avoid shadowing variables declared in an outer scope
Initialize all externally visible members of a class in the constructor
Prefer docstrings over comments for interfaces that may be used outside a file
Reserve comments for code within a function or interfaces that are local to a file
If a piece of code is commented out, include a comment describing its usage and why it's commented out. Remove debug comments before merging
Use Google style docstrings for classes and functions in Python, which can be parsed by Sphinx
Avoid using reflection when functionality can be easily achieved without reflection
When using try-except blocks, limit the except clause to the smallest set of specific errors possible
When using try-except blocks for duck-typing, keep the body of the try as small as possible and use the else block for logic
YAML is the single source of truth for configuration defaults. Do not set non-None defaults in code for configuration values
For required configuration attributes, access config directly and expect presence (e.g., policy_cfg['precision']) without hidden defaults
Use typing.NotRequired to mark optional attributes in TypedDict for configuration
When adding a new config key to a TypedDict subclass, document the key's purpose, valid values/types, and recommended default, and reflect the default in exemplar YAMLs under examples/configs/*.yaml
Follow the Google Python Style Guide for Python code

Files:

• tests/unit/test_recipes_and_test_suites.py

🧠 Learnings (2)

📓 Common learnings

Learnt from: zpqiu
Repo: NVIDIA-NeMo/RL PR: 1324
File: tests/test_suites/llm/distillation-qwen3-32b-to-1.7b-base-1n8g-megatron-tp2pp2cp2-pack.sh:6-11
Timestamp: 2025-10-12T14:46:57.171Z
Learning: Test scripts in tests/test_suites/llm/ follow a standard configuration pattern that includes NUM_NODES, STEPS_PER_RUN, MAX_STEPS, NUM_RUNS (calculated as `$(( (MAX_STEPS + STEPS_PER_RUN - 1) / STEPS_PER_RUN ))`), and NUM_MINUTES. These variables are part of the test infrastructure's standard interface and should not be flagged as unused even if not directly referenced within the individual script, as they are consumed by external launch tooling or common.env.

📚 Learning: 2025-10-12T14:46:57.171Z

Learnt from: zpqiu
Repo: NVIDIA-NeMo/RL PR: 1324
File: tests/test_suites/llm/distillation-qwen3-32b-to-1.7b-base-1n8g-megatron-tp2pp2cp2-pack.sh:6-11
Timestamp: 2025-10-12T14:46:57.171Z
Learning: Test scripts in tests/test_suites/llm/ follow a standard configuration pattern that includes NUM_NODES, STEPS_PER_RUN, MAX_STEPS, NUM_RUNS (calculated as `$(( (MAX_STEPS + STEPS_PER_RUN - 1) / STEPS_PER_RUN ))`), and NUM_MINUTES. These variables are part of the test infrastructure's standard interface and should not be flagged as unused even if not directly referenced within the individual script, as they are consumed by external launch tooling or common.env.

Applied to files:

• tests/test_suites/llm/performance/grpo-qwen3-235b-16n8g.sh
• tests/test_suites/llm/sft-llama3.2-1b-1n8g-fsdp2tp1.v3.sh
• tests/test_suites/llm/sft-nanov3-30BA3B-2n8g-fsdp2-lora.sh
• tests/test_suites/llm/sft-nanov3-30BA3B-2n8g-fsdp2.sh
• tests/test_suites/llm/sft-llama3.1-8b-1n8g-fsdp2tp1-dynamicbatch.sh
• tests/test_suites/llm/dpo-llama3.1-8b-instruct-4n8g-megatron.v2.sh

🧬 Code graph analysis (1)

tests/unit/test_recipes_and_test_suites.py (1)

tests/unit/conftest.py (1)

• tracker (265-296)

🪛 Shellcheck (0.11.0)

tests/test_suites/llm/performance/grpo-qwen3-235b-16n8g.sh

[warning] 12-12: NUM_MINUTES appears unused. Verify use (or export if used externally).

(SC2034)

tests/test_suites/llm/sft-llama3.2-1b-1n8g-fsdp2tp1.v3.sh

[warning] 10-10: NUM_MINUTES appears unused. Verify use (or export if used externally).

(SC2034)

tests/test_suites/llm/sft-nanov3-30BA3B-2n8g-fsdp2-lora.sh

[warning] 10-10: NUM_MINUTES appears unused. Verify use (or export if used externally).

(SC2034)

tests/test_suites/llm/sft-nanov3-30BA3B-2n8g-fsdp2.sh

[warning] 10-10: NUM_MINUTES appears unused. Verify use (or export if used externally).

(SC2034)

⏰ 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). (6)

• GitHub Check: sphinx-build / Build docs
• GitHub Check: build-container / main
• GitHub Check: Lint check
• GitHub Check: Lint check
• GitHub Check: Post submodule check comment / Comment on PR
• GitHub Check: Post automodel integration comment / Comment on PR

🔇 Additional comments (10)

tests/test_suites/llm/dpo-llama3.1-8b-instruct-4n8g-megatron.v2.sh (1)

37-37: LGTM! Threshold adjustment aligns with documented root cause.

The relaxation of the initial loss threshold from 3.6 to 3.65 is consistent with the PR's explanation that a num_workers change altered the dataset seed, affecting training initialization. The fact that only the step-1 threshold is adjusted (while step-150 and other metrics remain unchanged) confirms that convergence behavior is unaffected, as expected from a seed change.

tests/test_suites/llm/dpo-llama3.1-8b-instruct-4n8g-fsdp2tp4.sh (1)

37-37: LGTM! Consistent threshold adjustment across the test suite.

The threshold relaxation from 3.6 to 3.65 matches the identical change in the megatron variant (dpo-llama3.1-8b-instruct-4n8g-megatron.v2.sh), ensuring consistent test criteria across different parallelization strategies for the same model. This consistency is appropriate given that the underlying cause (dataset seed change) affects both test configurations equally.

tests/test_suites/llm/sft-llama3.2-1b-1n8g-fsdp2tp1.v3.sh (1)

10-10: Timeout increase appropriately addresses checkpoint download delays.

Doubling NUM_MINUTES from 15 to 30 provides adequate buffer for initial checkpoint downloads, consistent with the PR's goal of reducing flakiness due to timing variability.

Based on learnings, NUM_MINUTES is consumed by external launch tooling—the Shellcheck warning can be safely ignored.

tests/test_suites/llm/sft-llama3.1-8b-1n8g-fsdp2tp1-dynamicbatch.sh (1)

37-41: Memory threshold adjustment accounts for observed variance.

Increasing the threshold from 70 GB to 75 GB provides reasonable headroom above the observed 72.6 GB (with noise), preventing spurious failures while maintaining protection against genuine memory regressions.

tests/test_suites/llm/sft-nanov3-30BA3B-2n8g-fsdp2.sh (1)

10-10: Timeout increase provides adequate buffer for checkpoint downloads.

With ~5 minutes of compute time for 20 steps (line 7: step_time ~ 15sec), the increased 30-minute limit provides 25 minutes for setup and checkpoint downloads, addressing the flakiness mentioned in the PR objectives.

Based on learnings, NUM_MINUTES is consumed by external launch tooling—the Shellcheck warning can be safely ignored.

tests/test_suites/llm/sft-nanov3-30BA3B-2n8g-fsdp2-lora.sh (1)

10-10: Timeout increase with clear documentation.

The updated comment clearly explains the rationale for the 30-minute buffer, making the intent transparent for future maintainers. The adjustment is consistent with other similar changes in this PR.

Based on learnings, NUM_MINUTES is consumed by external launch tooling—the Shellcheck warning can be safely ignored.

tests/unit/test_recipes_and_test_suites.py (1)

183-218: Compute hour limit adjustment reflects timeout increases across test suites.

The 40 GPU-hour increase (3.5%) appropriately accommodates the timeout extensions applied to multiple test suites in this PR, ensuring nightly tests have sufficient time to complete including checkpoint downloads.

tests/test_suites/llm/performance/grpo-qwen3-235b-16n8g.sh (2)

12-12: LGTM! Timeout increase addresses checkpoint download delays.

The 15% increase in NUM_MINUTES (100→115) aligns with the PR objective to handle checkpoint download delays and tests finishing near the time limit.

Note: The Shellcheck warning about NUM_MINUTES being unused is a false positive. Based on learnings, this variable is part of the test infrastructure's standard interface and is consumed by external launch tooling.

---

27-27: LGTM! TensorBoard enablement fixes missing logs issue.

Enabling TensorBoard logging ensures that logs are available when line 34 converts them to JSON, preventing test failures from missing TensorBoard logs as described in the PR objectives.

tests/test_suites/llm/performance/grpo-qwen3-235b-32n8g-async-1off.sh (1)

27-27: LGTM! TensorBoard enablement consistent with sibling test.

Enabling TensorBoard logging ensures that logs are available when line 34 converts them to JSON, preventing test failures. This change is consistent with the identical fix applied in grpo-qwen3-235b-16n8g.sh.