Repository navigation
fix(nightly): fix flow-grpo and sft-gpt-oss nightlies - #4788
Merged
Merged
Conversation
Signed-off-by: Yuki Huang <yukih@nvidia.com>
… grad_norm checks Signed-off-by: Yuki Huang <yukih@nvidia.com>
yuki-97
commented
Oct 19, 2026
yuki-97
left a comment
Contributor
Author
There was a problem hiding this comment.
Self-review via /review-pr-team: 2 agents (reviewer, devil's-advocate pass).
Verdict — no findings above the confidence bar. LGTM, no action needed.
Verified clean
- The per-node install cannot race node registration: ray.sub:1141 only starts the driver once all
NUM_ACTORSworkers are online, soray.nodes()already lists every node. ray.init(address="auto")attaches to the running cluster, the same call the driver path makes at virtual_cluster.py:483, andrayis importable because the install now runs afteruv sync.- A failed install propagates:
check=Trueraises in the task,ray.getre-raises in the driver, and common.env:5 setsset -eou pipefailwith the recipe scripts callingrun_flow_grpo_recipeas a plain statement. - The new SFT bounds are not vacuous: step-50
grad_normof 9.5-10.1 sits inside8.0-14.0, and loss of ~0.255-0.30 is under0.35.
Lower-priority notes that are not inline
- main still has the old SFT bounds, and its
research/flow_grpo/tests/test_suites/common.envhas no cv2 apt install; #4789 (the main cp of #4696) still carries the driver-only version. This PR has nocherry-pick-to-mainlabel, so nothing will land it on main automatically. grad_norm["50"] < 14.0is the thinner of the two grad_norm bounds against the 12.0-13.6 seen at steps 48/49;< 15.0would be an optional loosening.
Generated by Claude Code
Contributor
Author
|
/ok to test 00824a0 |
yuki-97
marked this pull request as ready for review
October 19, 2026 16:00
terrykong
approved these changes
Oct 19, 2026
terrykong
pushed a commit
that referenced
this pull request
Oct 20, 2026
…ightlies (#4817) Signed-off-by: Yuki Huang <yukih@nvidia.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do ?
Fixes two failing nightlies:
flow_grpo-qwen-image-ocr-2n8g-dp16-lorasft-gpt-oss-20b-1n8g-automodel-ep8What changed
1. Run the apt install as a Ray task pinned to each alive node in
common.env. #4696 ranapt-getonce in the driver shell, so on 2n8g only the head node gotlibxcb1, and_RewardWorkeron the other node still failed withImportError: libxcb.so.1. The #4696 validation passed because that container already shipped the libs (libxcb1 is already the newest version), so the install was a no-op. The install moves afteruv syncbecauseuv run --no-sync pythonneeds the venv.2. Re-baseline
sft-gpt-oss-20b-1n8g-automodel-ep8.sh.train/loss["50"] < 0.4becomes< 0.35,train/grad_norm["50"] < 17.5becomes< 14.0, and> 10.0becomes> 8.0. The old band was set in #1708 and every nightly since the Automodel r0.6.0 bump (#3498) fails the lower bound. The shift is a benign change from the bump, not a regression:cp_gradient_fanoutis 1, and thescale_grads_and_clip_grad_normcall is unchanged.max_grad_normis 1.0, so the update is clipped on both sides of the bump.8-11and< 0.3would fail on step-50 placement alone.Tests
Validated both recipes. All checks pass.
🤖 Generated with Claude Code