Skip to content

fsdp(scripts): RL scripts for representative dense + MoE models#1486

Draft
Zhichenzzz wants to merge 5 commits into
zhichen/fsdp-refactorfrom
zhichen/fsdp-rl-scripts
Draft

fsdp(scripts): RL scripts for representative dense + MoE models#1486
Zhichenzzz wants to merge 5 commits into
zhichen/fsdp-refactorfrom
zhichen/fsdp-rl-scripts

Conversation

@Zhichenzzz

@Zhichenzzz Zhichenzzz commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Stacked on #1469 (the FSDP adaptations/ refactor) — base is zhichen/fsdp-refactor, so the diff is just the scripts.

FSDP-backend GRPO RL launch scripts, one per model family (one dense + one MoE). Shared policy for all:

  • train: DAPO-math-17k · eval: gsm8k · seq: 8k response len
  • GRPO, AdamW, colocate sglang rollout, no checkpointing

scripts/fsdp_rl/common.sh holds the shared launcher; each per-model script just sets MODEL + GPU layout and sources it. GPU counts are sized per model (4/8 GPU, CPU-offload for big ones; multi-node for the huge MoEs). See scripts/fsdp_rl/README.md for the table.

Dense: Qwen3-4B, Qwen3.5-4B, Nemotron-3-Nano-4B, Gemma-4-31B
MoE: Qwen3-30B-A3B, Qwen3.5-35B-A3B (GatedDeltaNet), GLM-4.7-Flash (glm4_moe_lite), Gemma-4-26B-A4B, Nemotron-3-Nano-30B-A3B (nemotron_h), gpt-oss-20B, Qwen3-Next-80B-A3B, DeepSeek-V3 671B, Kimi-K2.5 ~1T

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces FSDP-backend RL reference scripts for various dense and MoE models, featuring a shared launcher script (common.sh) and model-specific configurations. Feedback on the shared launcher focuses on improving robustness and portability: replacing the aggressive pkill -9 python with targeted process termination, adding a validation check to prevent multi-node runs from hanging on localhost, dynamically resolving the repository root directory instead of hardcoding /root/miles, and allowing the SGLang attention backend to be configurable to support non-Hooper GPU architectures.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread scripts/fsdp_rl/common.sh
DAPO="$DATA_DIR/dapo-math-17k/dapo-math-17k.jsonl"
GSM8K_TEST="$DATA_DIR/gsm8k/test.parquet"

pkill -9 sglang 2>/dev/null; ray stop --force 2>/dev/null; pkill -9 ray 2>/dev/null; pkill -9 python 2>/dev/null; sleep 2

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Using pkill -9 python is extremely aggressive and dangerous. It will abruptly terminate all Python processes owned by the current user on the machine. This has several severe consequences:

  1. On shared multi-user clusters, it will kill other unrelated training jobs, Jupyter notebooks, or background tasks the user is running.
  2. If this script is invoked or orchestrated by a Python-based wrapper (e.g., a custom launcher, a benchmarking harness, or a CI/CD runner), it will kill its own parent process, causing the run to terminate prematurely and fail to report results.

Instead, you should target only the specific processes related to this run (e.g., train.py and sglang) using the -f flag to match the command line pattern.

Suggested change
pkill -9 sglang 2>/dev/null; ray stop --force 2>/dev/null; pkill -9 ray 2>/dev/null; pkill -9 python 2>/dev/null; sleep 2
pkill -9 -f sglang 2>/dev/null; ray stop --force 2>/dev/null; pkill -9 -f ray 2>/dev/null; pkill -9 -f train.py 2>/dev/null; sleep 2

Comment thread scripts/fsdp_rl/common.sh
Comment on lines +71 to +76
ray start --head --node-ip-address "$MASTER_ADDR" --num-gpus "$GPUS_PER_NODE" --disable-usage-stats
if [ "$NNODES" -gt 1 ]; then
echo ">> NNODES=$NNODES: start ray on the other $((NNODES-1)) node(s) with"
echo " ray start --address=$MASTER_ADDR:6379 --num-gpus=$GPUS_PER_NODE"
echo " (or launch via your multi-node cluster tooling) so all $((NNODES*GPUS_PER_NODE)) GPUs join before training."
fi

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

When running a multi-node setup (NNODES > 1), if MASTER_ADDR is left at its default value of 127.0.0.1 (localhost), the worker nodes on other machines will not be able to connect to the Ray head node. This will cause the multi-node training run to fail or hang indefinitely.

We should add a validation check to ensure that MASTER_ADDR is set to a non-loopback IP address when NNODES > 1.

Suggested change
ray start --head --node-ip-address "$MASTER_ADDR" --num-gpus "$GPUS_PER_NODE" --disable-usage-stats
if [ "$NNODES" -gt 1 ]; then
echo ">> NNODES=$NNODES: start ray on the other $((NNODES-1)) node(s) with"
echo " ray start --address=$MASTER_ADDR:6379 --num-gpus=$GPUS_PER_NODE"
echo " (or launch via your multi-node cluster tooling) so all $((NNODES*GPUS_PER_NODE)) GPUs join before training."
fi
if [ "$NNODES" -gt 1 ] && [ "$MASTER_ADDR" = "127.0.0.1" ]; then
echo "ERROR: NNODES=$NNODES (> 1) but MASTER_ADDR is 127.0.0.1 (localhost)."
echo "Please set MASTER_ADDR to the head node's reachable network IP address so other nodes can connect."
exit 1
fi
ray start --head --node-ip-address "$MASTER_ADDR" --num-gpus "$GPUS_PER_NODE" --disable-usage-stats
if [ "$NNODES" -gt 1 ]; then
echo ">> NNODES=$NNODES: start ray on the other $((NNODES-1)) node(s) with"
echo " ray start --address=$MASTER_ADDR:6379 --num-gpus=$GPUS_PER_NODE"
echo " (or launch via your multi-node cluster tooling) so all $((NNODES*GPUS_PER_NODE)) GPUs join before training."
fi

Comment thread scripts/fsdp_rl/common.sh

: "${MODELS_DIR:=/cluster_public/miles_data/models}"
: "${DATA_DIR:=/cluster_public/miles_data/datasets}"
: "${MILES_DIR:=/root/miles}"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The default value for MILES_DIR is hardcoded to /root/miles, which is typically only accessible by the root user and assumes a specific directory structure. If a non-root user runs this script, or if the repository is cloned to a different location, the script will fail at cd "$MILES_DIR".

We can dynamically resolve the repository root directory relative to the script's location using BASH_SOURCE. This makes the script work out-of-the-box in any environment without requiring manual environment variable overrides.

Suggested change
: "${MILES_DIR:=/root/miles}"
: "${MILES_DIR:=$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)}"

Comment thread scripts/fsdp_rl/common.sh Outdated
)
GRPO_ARGS=( --advantage-estimator grpo --kl-loss-coef 0.0 --kl-coef 0.0 --entropy-coef 0.0 --eps-clip 0.2 --eps-clip-high 0.28 )
OPTIMIZER_ARGS=( --optimizer adam --lr 1e-6 --lr-decay-style constant --weight-decay 0.1 --adam-beta1 0.9 --adam-beta2 0.98 )
SGLANG_ARGS=( --rollout-num-gpus-per-engine 1 --sglang-mem-fraction-static "$SGLANG_MEM" --sglang-decode-log-interval 1000 --sglang-chunked-prefill-size 4096 --sglang-attention-backend fa3 )

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Hardcoding --sglang-attention-backend fa3 restricts these scripts to Hopper GPUs (H100/H200). If a user attempts to run these scripts on Ampere (A100) or other GPU architectures, SGLang will fail to start because FlashAttention-3 is not supported.

To make the scripts more portable and robust across different GPU architectures, we should allow overriding the attention backend via an environment variable (e.g., SGLANG_ATTN_BACKEND), defaulting to fa3.

Suggested change
SGLANG_ARGS=( --rollout-num-gpus-per-engine 1 --sglang-mem-fraction-static "$SGLANG_MEM" --sglang-decode-log-interval 1000 --sglang-chunked-prefill-size 4096 --sglang-attention-backend fa3 )
SGLANG_ARGS=( --rollout-num-gpus-per-engine 1 --sglang-mem-fraction-static "$SGLANG_MEM" --sglang-decode-log-interval 1000 --sglang-chunked-prefill-size 4096 --sglang-attention-backend "${SGLANG_ATTN_BACKEND:-fa3}" )

@Zhichenzzz Zhichenzzz marked this pull request as draft June 26, 2026 04:26
One GRPO RL script per model family (one dense + one MoE), on the FSDP backend:
DAPO-math train, gsm8k eval, 8k response len, no checkpointing. Shared launcher
(common.sh) + thin per-model configs; GPU counts sized per model (4/8 GPU, CPU
offload for big ones; deepseek-v3 multi-node). See scripts/fsdp_rl/README.md.
@Zhichenzzz Zhichenzzz force-pushed the zhichen/fsdp-rl-scripts branch from ce6847e to 9d001d8 Compare June 26, 2026 04:28
Two fixes found by running them: (1) the FSDP backend is gated behind --ci-test
(arguments.py raises otherwise), so pass --ci-test --ci-disable-logprobs-checker;
(2) gsm8k eval messages are in the 'messages' column while DAPO train uses 'prompt',
so set --eval-input-key messages. Also lower default sglang-mem-fraction to 0.45 to
leave room for the colocated training step (raise CPU_OFFLOAD / lower this on OOM).
Validated on qwen3-4b (trains end-to-end, 8k seq, gsm8k eval -> wandb). Defaults now
CPU_OFFLOAD=1, SGLANG_MEM=0.4, MAX_TOKENS_PER_GPU=10240 (the 0.6/16384 combo OOM'd in the
colocated training step). Add ROLLOUT_GPUS_PER_ENGINE (sglang TP/engine): 1 for <=24B, 2
for 26B+ which don't fit one GPU at the sglang fraction. Per-model scripts simplified to
MODEL + GPU layout + per-engine, inheriting the safe defaults.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant