Replace all the .sh launch scripts with .py launch script - #2356
Conversation
…ipts Their models are dropped, so the launchers, their command snapshots and the doc pages built around them go with them. The Megatron model definitions stay: e2e tests and the low-precision / p2p examples still reference them. deepseek.md was the only page covering a surviving model, so it is rewritten around scripts/run_deepseek.py instead of the deleted shell recipe.
Each of these ships a launcher that cannot do what it says, and the recorded
snapshots pinned the broken output rather than flagging it.
Shell launchers:
- run-qwen3-next-80B-A3B.sh built --runtime-env-json without the comma after
NCCL_NVLS_ENABLE, so ray received invalid JSON.
- run-gpt-oss-20b-bf16.sh read ${HAS_NVLINK} but never probed for NVLink, so it
passed NCCL_NVLS_ENABLE="" to NCCL.
- run-qwen3-4B-base-sft.sh, run-qwen3-235B-A22B-sft.sh and
run-gpt-oss-20b-bf16.sh expanded ${EVAL_ARGS[@]} without ever declaring it.
- run-kimi-k25.sh passed --filter-zero-reward-samples, which no longer exists.
- run-glm4.5-355B-A32B.sh dumped rollout data to a personal absolute path, and
probed NVLink into a variable it never read.
- run-qwen3-4B_4xgpu.sh left ${WANDB_KEY} unquoted inside the array, so an unset
key made --wandb-key swallow the next flag; and it colocates on 4 GPUs without
--num-gpus-per-node, which defaults to 8.
Python launchers:
- run_glm47_flash.py had --eval-interval commented out, and the flag defaults to
None, so the eval block it builds never ran.
- run_qwen3_30b_a3b.py and amd/run_qwen3_30b_a3b.py dropped `config=args`,
silently discarding cuda_core_dump and extra_env_vars.
- run_glm45_355b_a32b.py defaulted `hardware` to the one value _execute_train
asserts against, so the snapshot harness had to override it to run at all.
os.environ.get(name, default) evaluates the default eagerly, so execute_train shelled out to nvidia-smi on every launch even when the caller had already chosen a value -- including on ROCm boxes, where the probe is meaningless.
A launcher may legitimately export its own knobs -- run_inkling.py sets MODEL_ARGS_NUM_LAYERS for its pruned variants, for instance -- and the harness never undid that, so the value leaked into every later test in the session. Running test_py_launch_scripts.py before test_model_args.py was enough to fail eight model-args snapshots; pytest-randomly could hit it in either order.
scripts/ now contains no .sh launcher. Each recipe moved to a typer launcher built on
command_utils.execute_train, and near-duplicate recipes were merged behind a per-variant
table instead of being copied per file:
run_qwen3_dense.py <- run-qwen3-{4B,4B_4xgpu,32B}.sh, run-qwen3.5-{4B,9B,27B}.sh,
run-qwen3.6-27B.sh (6 recipes, --model-name)
run_qwen3_sft.py <- run-qwen3-4B-base-sft.sh, run-qwen3-235B-A22B-sft.sh
run_qwen3_235b_a22b.py <- run-qwen3-235B-A22B.sh
run_qwen3_next_80b_a3b.py <- run-qwen3-next-80B-A3B{,-8gpus}.sh (--topology)
run_kimi_k2.py <- run-kimi-k2-{Instruct,Thinking}.sh
run_nemotron_3_nano.py <- run-nemotron-3-nano-{4b,30b-a3b}.sh
run_nemotron_3_super_120b_a12b.py <- run-nemotron-3-super-120b-a12b.sh (train / worker)
run_gpt_oss_20b.py <- run-gpt-oss-20b-bf16.sh
run_glm45_355b_a32b_8node.py <- run-glm4.5-355B-A32B.sh
amd/run_qwen3_4b.py <- amd/run-qwen3-4B-amd.sh
run-qwen3.5-35B-A3B-mtp.sh, run-glm4.7-flash.sh and run-kimi-k25.sh were absorbed by the
python launchers that already shadowed them, which gained the fields their recipes needed;
run_qwen3_5_35b_a3b_mtp_cp2_ep8.py is renamed since its parallelism is now a flag.
run_qwen3_4b_npu.sh is deleted with no replacement: it exported SLIME_SCRIPT_* names
nothing reads, duplicated what execute_train_npu already sets, and invoked the CUDA
launcher rather than the NPU one.
Every converted recipe was checked by diffing the deleted launcher's recorded argv against
a recording of its replacement: no parallelism, batch, response-length, algorithm,
optimizer or sglang value differs anywhere. The intended differences are the shared
execute_train preamble, wandb via get_default_wandb_args, --model-dir / --data-dir /
--output-dir replacing hardcoded paths, and --num-gpus-per-node now being passed so the
GPU-count field is honoured under colocate.
command_utils gains ssh_start_ray_workers for the four recipes that fan out to an
MPI-style hostfile over ssh before submitting.
.sh launch scripts with .py launch script
.sh launch scripts with .py launch script.sh launch scripts with .py launch script (no semantic change)
98f568d to
b22c1c4
Compare
test_the_repo_has_ray_launchers_to_check guards against the discovery glob going empty, and 24 launchers left scripts/ in this branch, so the marker scan now finds 43 rather than 67. Same adjustment already made to the sh snapshot suite's bound. The new python launchers do not appear in that scan by design: they build no runtime env of their own, they delegate to command_utils.execute_train, which the scan does find.
406735d to
bb0144e
Compare
| hf download --repo-type dataset zhuzilin/aime-2024 --local-dir $BASE_DIR/rl_data/aime-2024 | ||
| hf download zai-org/GLM-4.5 --local-dir /root/models/GLM-4.5-355B-A32B | ||
| hf download --repo-type dataset zhuzilin/dapo-math-17k --local-dir /root/datasets/dapo-math-17k | ||
| hf download --repo-type dataset zhuzilin/aime-2024 --local-dir /root/datasets/rl_data |
There was a problem hiding this comment.
we should unify the /root/datasets/rl_data and /root/datasets/
| f"{args.extra_args} " | ||
| ) | ||
|
|
||
| start_workers = args.ssh_start_workers and args.recipe.actor_num_nodes > 1 |
There was a problem hiding this comment.
only in this script using this gate
| # the ray workers download the tokenizer / config, so the cache belongs in their env | ||
| extra_env_vars={"HF_HOME": "/workspace/hf_cache"}, |
The previous commit kept scripts/models/glm4-9B.py because two things referenced it. Neither turns out to be a reason to keep the model: - tests/e2e/megatron/test_quick_start_glm4_9B.py registers itself with disabled="Not worthy for testing, too naive.", and run_suite.py keeps only entries whose `disabled` is None, so it has never run. - the p2p weight-transfer example carried a GLM-Z1-9B-0414 profile, which is the last consumer of the model definition and goes with the model. So the model definition, its expanded-args snapshot, the disabled e2e test, the p2p prepare/run configs, the GLM-Z1-9B.sh wrapper and the two recorded p2p argv snapshots are all removed, along with the profile name in test_self_executing_launchers.py. The p2p doc loses three rows: the Glm4ForCausalLM support row (whose only example was this model, and no code in the repo names that class), the node-count row, and the measured RDMA-vs-NCCL row. Dropping a measurement is deliberate: the configuration behind it no longer exists, so nobody can reproduce the number.
Moonlight's launcher went in the first commit, but the model definition stayed because two examples still used it. Both go now, so nothing in the repo names the model: - examples/infra_features/low_precision/run-moonlight-16B-A3B-int4.sh and its recorded commands. Three INT4 examples remain (qwen3-30B-A3B, qwen3-235B-A22B, kimi-k2-Thinking), so INT4 coverage survives, including on an MLA MoE via kimi-k2-Thinking. What is lost is the cheapest INT4 smoke test: Moonlight needed 4 GPUs on one node, and the smallest remaining one needs 8. - the p2p weight-transfer profile, its Moonlight-16B.sh wrapper and both recorded argv snapshots, plus the profile name in test_self_executing_launchers.py. scripts/models/moonlight.py, its expanded-args snapshot, and the two README entries go with them. The p2p doc loses the DeepseekV2ForCausalLM support row, the node-count row and the measured row -- the last one recorded the largest RDMA win in the table (-27.6%), and dropping it is deliberate: the configuration behind it is gone.
# Conflicts: # docs/developer/contributor-guide.md # docs/getting-started/quick-start.md # docs/models/kimi/kimi-k2.5.md # docs/platforms/amd.md # scripts/run-kimi-k25.sh # scripts/run-qwen3-4B.sh # tests/snapshots/launch_scripts/sh/scripts/run-kimi-k2-Instruct.sh.txt # tests/snapshots/launch_scripts/sh/scripts/run-kimi-k2-Thinking.sh.txt # tests/snapshots/launch_scripts/sh/scripts/run-kimi-k25.sh.txt # tests/snapshots/launch_scripts/sh/scripts/run-qwen3-4B.sh.txt
#2300 enabled the Miles dashboard in scripts/run-qwen3-4B.sh after this branch had already converted that recipe, so run_qwen3_dense.py was missing it. The flags are a per-recipe knob rather than a launcher-wide default: #2300 scoped them to the quick-start launcher, and Qwen3-4B is the quick-start recipe here. --dump-details follows the --output-dir convention instead of the .sh's hardcoded /root/Qwen3-4B_miles.
…ming/script-rewrite # Conflicts: # docs/docs.json # docs/user-guide/usage.md
# Conflicts: # docs/models/glm/glm4.md # docs/models/kimi/moonlight.md
# Conflicts: # docs/getting-started/quick-start.md # docs/models/qwen/index.md # docs/models/qwen/qwen3.md # docs/user-guide/training-script-walkthrough.md
#2391 replaced this page with a DeepSeek-V3.2 recipe, but V3 is still trainable: scripts/run_deepseek.py is the only launcher for the `deepseek-v3` model type, and no other launcher covers it (run_deepseek_v32.py is `deepseek-v32`, run_deepseek_v4.py is V4). Without the page the launcher had no documentation at all. The page comes back at its original path, so #2391's redirect from it to the V3.2 page is dropped -- it would otherwise shadow the restored page. The nav entry and the V3 rows in the three model tables are restored alongside it.
run_deepseek.py's train and run_deepseek_v32.py's full_train chained their prepare steps ahead of execute_train, but _prepare_megatron_ckpt and _prepare_cp fan out with exec_command_multi_node, whose first act is ray.init(address="auto"). So on a clean host they died before the launcher ever reached its own `ray start --head`, and starting a cluster by hand to get past that did not help either: execute_train's preamble runs `ray stop --force` and then brings up a fresh single-node head, so a multi-node conversion was followed by single-node training. Both now pass those steps as execute_train's before_ray_job_submit hook, which runs after the head is up and before the job is submitted -- and still runs under MILES_SCRIPT_EXTERNAL_RAY=1, where the pre-joined cluster is left alone. That covers both intended flows without touching execute_train's contract. The snapshot diff is the fix itself: `ray stop` / `ray start --head` moved ahead of the conversion and the rsyncs. Reported by @guapisolo on #2354.
The restored V3 page told the reader to join the workers first and then run the launcher, which cannot work now that the conversion runs inside execute_train's hook: the head does not exist yet at the point the page said to join. Multi-node needs the cluster up front plus MILES_SCRIPT_EXTERNAL_RAY=1 so the launcher leaves it alone, and the page now says so, including what the single-node invocation relies on instead.
# Conflicts: # docs/models/deepseek/deepseek.md
The reason belongs in the commit message, not in four lines above the call.
The GLM-4.5 and Kimi-K2-Instruct recipes read the eval set from `<data-dir>/rl_data/`,
inherited from their shell ancestors, while every other recipe -- including
Kimi-K2-Thinking in the same launcher -- reads `<data-dir>/aime-2024/`. Nothing produces
the rl_data layout: `U.hf_download_dataset("zhuzilin/aime-2024")` writes
`<data-dir>/aime-2024`, so both docs had to tell the reader to override --local-dir.
All of them now use the standard layout, which makes run_kimi_k2's per-recipe `eval_data`
field identical for both variants, so it is inlined.
The four launchers that fan out to an MPI-style hostfile had three names for the same switch: join_ray_workers, start_ray_workers and ssh_start_workers. They are all join_ray_workers now, docstrings and docs included. No argv changes. The extra recipe gate on two of them stays: run_qwen3_sft and run_qwen3_next_80b_a3b each carry a single-node recipe that must never fan out, while the other two are multi-node only.
This was the only launcher in the repo setting HF_HOME, and it set it to a literal /workspace/hf_cache -- the kind of machine path the launcher rules say not to bake in. The shell script exported it for the submitting process only, so the ray workers never saw it anyway; putting it in their runtime env was a change of behaviour, not a port of one. Anyone wanting a shared cache exports HF_HOME themselves.
# Conflicts: # scripts/run-glm4.5-355B-A32B.sh # scripts/run-gpt-oss-20b-bf16.sh # scripts/run-kimi-k25.sh # scripts/run-qwen3-235B-A22B-sft.sh # scripts/run-qwen3-4B-base-sft.sh # scripts/run-qwen3-4B_4xgpu.sh # scripts/run-qwen3-next-80B-A3B.sh # tests/snapshots/launch_scripts/sh/scripts/run-glm4.5-355B-A32B.sh.txt # tests/snapshots/launch_scripts/sh/scripts/run-gpt-oss-20b-bf16.sh.txt # tests/snapshots/launch_scripts/sh/scripts/run-kimi-k25.sh.txt # tests/snapshots/launch_scripts/sh/scripts/run-qwen3-4B_4xgpu.sh.txt # tests/snapshots/launch_scripts/sh/scripts/run-qwen3-next-80B-A3B.sh.txt
guapisolo
left a comment
There was a problem hiding this comment.
One snapshot-isolation issue.
| "GLOO_SOCKET_IFNAME", | ||
| "KEEP_MOE_LORA", | ||
| "MILES_SCRIPT_EXTERNAL_RAY", | ||
| "MLP_SOCKET_IFNAME", |
There was a problem hiding this comment.
[P2] Clear MLP_WORKER_0_HOST when freezing launcher snapshots
freeze_environment() inherits the caller's environment, but this list only clears MLP_SOCKET_IFNAME; four new launchers also read MLP_WORKER_0_HOST when building the SSH worker command. With MLP_WORKER_0_HOST=10.9.8.7, the recorded command skips that host while the committed snapshot expects 127.0.0.1, so the snapshot test is host-dependent and fails in MLP environments. Please add MLP_WORKER_0_HOST to CLEARED_ENV.
Zhichenzzz
left a comment
There was a problem hiding this comment.
LGTM if they could smoothly run
The four ssh-fan-out launchers read MLP_WORKER_0_HOST for the head to skip, and ssh_start_ray_workers bakes that host into the command string the harness records, so the recording followed the developer's environment. CLEARED_ENV froze its sibling MLP_SOCKET_IFNAME but not this one. Confirmed both ways in a container: with MLP_WORKER_0_HOST=10.9.8.7 exported, three snapshots fail before this change and all 178 pass after. The committed snapshots are the unset-variable form, so they do not move. Reported by @guapisolo on #2356.
# Conflicts: # docs/docs.json # docs/getting-started/quick-start.md # docs/index.md # docs/user-guide/concepts.md # docs/user-guide/index.md # docs/user-guide/training-script-walkthrough.md # docs/user-guide/usage.md
docs/getting-started/quick-start.md carried `<<<<<<< HEAD` / `>>>>>>> yueming/script-bugfix` from an earlier merge on this branch: that resolution pass checked for stale references but not for the markers themselves. The hunk kept the wrong side of #2398 as well, so the file now downloads zhuzilin/dapo-math-17k -- the mirror that actually carries the .jsonl the launcher reads -- into the --data-dir layout. `git grep '^<<<<<<< ' HEAD` over the whole tree finds nothing else.
.sh launch scripts with .py launch script (no semantic change).sh launch scripts with .py launch script
Last of three chained PRs cleaning up
scripts/. Stack: #2354 → #2355 → this. Base isyueming/script-bugfix, review the other two first.After this,
scripts/contains no.shlauncher. Part of #1837, which already moved the model definitions to python (#1910); this finishes the job for the launchers themselves.What replaces what
Near-duplicate recipes are merged behind a per-variant table instead of being copied per file:
run_qwen3_dense.pyrun-qwen3-{4B,4B_4xgpu,32B}.sh,run-qwen3.5-{4B,9B,27B}.sh,run-qwen3.6-27B.sh— 6 recipes behind--model-namerun_qwen3_sft.pyrun-qwen3-4B-base-sft.sh,run-qwen3-235B-A22B-sft.shrun_qwen3_235b_a22b.pyrun-qwen3-235B-A22B.shrun_qwen3_next_80b_a3b.pyrun-qwen3-next-80B-A3B{,-8gpus}.sh—--topologyrun_kimi_k2.pyrun-kimi-k2-{Instruct,Thinking}.shrun_nemotron_3_nano.pyrun-nemotron-3-nano-{4b,30b-a3b}.shrun_nemotron_3_super_120b_a12b.pyrun-nemotron-3-super-120b-a12b.sh—train/workercommands for the head/worker role splitrun_gpt_oss_20b.pyrun-gpt-oss-20b-bf16.shrun_glm45_355b_a32b_8node.pyrun-glm4.5-355B-A32B.shamd/run_qwen3_4b.pyamd/run-qwen3-4B-amd.shThree shell recipes were absorbed by python launchers that already shadowed them, which gained the fields their recipes needed:
run-qwen3.5-35B-A3B-mtp.sh→run_qwen3_5_35b_a3b_mtp.py(renamed from..._cp2_ep8.py, since the parallelism is now--parallelism),run-glm4.7-flash.sh→run_glm47_flash.py,run-kimi-k25.sh→run_kimi_k25.py.How semantic equivalence was checked
Not by eye. For each deleted launcher, its recorded snapshot from the base branch was diffed against a fresh recording of the replacement at equivalent settings, comparing the ray runtime env plus the
train.pyargv as a flag → values multiset — so ordering is ignored and every flag is accounted for. Merged launchers were recorded once per variant, since the snapshot suite only records default args.Result: no parallelism, batch-size, response-length, algorithm-constant, optimizer or sglang value differs anywhere, for any of the 23 recipes.
The intended differences are uniform and deliberate:
execute_trainowns the preamble, so every script gainsPYTHONUNBUFFERED/no_proxy/MASTER_ADDRand a repo-root-prefixedPYTHONPATHin the runtime env, killsmiles/redisrather thanpython, drops--dashboard-host/--dashboard-portfromray start(submission still targets the default 8265, so only remote dashboard access changes), and uses an absolutetrain.pypath.--model-dir(default/root/models),--data-dir(default/root/datasets) and--output-dirreplace hardcoded/root/<Model>,${BASE_DIR},${BASE_FOLDER},${MODEL_DIR}. Every old path is reachable by setting the fields, and the msc miles flavor already injects the cluster values throughMILES_SCRIPT_*.get_default_wandb_args, so it engages only whenWANDB_API_KEYis set. Two scripts had hardcoded project/group names; those are gone.--num-gpus-per-nodeis now always passed, so the GPU-count field is actually honoured under--colocateon sub-8-GPU nodes.--rollout-num-gpusis dropped where--colocateis set, sincearguments.pydocuments and implements it as ignored there.Kimi-K2-Instruct recipes read the eval set from
<data-dir>/rl_data/, a layout nothingproduces (
hf_download_datasetwrites<data-dir>/aime-2024), so they now use the standardone; and the gpt-oss launcher no longer injects
HF_HOME=/workspace/hf_cacheinto the rayruntime env — the shell script exported it for the submitting process only, so the workers
never saw it.
Two per-script notes: the three Kimi-K2 recipes never ran
ray startat all, which is expressed withMILES_SCRIPT_EXTERNAL_RAY=1— that knob also skips theray stop --force/pkill -9 raythe shell scripts did perform, which is unavoidable today. Andrun-qwen3-4B_4xgpu.shomitted--sglang-mem-fraction-staticwhile its sibling set0.7; the merged launcher applies the family value.Judgement calls worth a look
run_glm45_355b_a32b_8node.pyis a new file rather than a mode of the existingrun_glm45_355b_a32b.py. The two are different experiments for the same model — gspo vs grpo, TP8/EP16 vs TP4/EP8, 8 GPUs per node vs 4, EAGLE MTP and DeepEP and a 20-var NCCL/IB tuning block on one side only, router-health and fault-tolerance flags on the other. A recorded diff between them shows 77 differences, and the existing script asserts against H100. Merging would have bloated it and risked a recipe in active use.run_qwen3_4b_npu.shis deleted with no replacement. It exportedSLIME_SCRIPT_*names nothing in the repo reads, duplicated env varsexecute_train_npualready sets, and invokedrun_qwen3_4b.py— the CUDA launcher — not the NPU one. Its allocator and PYTHONPATH exports therefore never reached the NPU launcher, so folding them intorun_qwen3_4b_npu.pywould mean guessing at a configuration that has never run.Also here
command_utils.ssh_start_ray_workers— the four recipes that fan out to an MPI-style hostfile over ssh before submitting shared that loop; it is now one helper wired in throughbefore_ray_job_submit.MLP_SOCKET_IFNAMEadded to the harness'sCLEARED_ENV, so a developer with it exported cannot unfreeze the glm4.5 recording.tests/fast/launch_scripts/test_sh_harness.pyusedscripts/run-qwen3-4B.shas its own fixture and is re-pointed at a survivingexamples/launcher; the sixscripts/entries in_SCRIPTS_REFUSING_TO_RUN_WITHOUT_EXPLICIT_INPUTSare removed and the discovery lower bound adjusted for the 39 remainingexamples/snapshots.--model-nameand updatinghf download --local-dirdestinations to the new directory defaults.Testing
pytest tests/manual/launch_scripts tests/fast/launch_scripts tests/fast/utils/test_command_utils.py— 570 passed under random ordering.pre-commit run --all-filesclean.Follow-ups not in scope
scripts/run_qwen3_4b_npu.pyhas its own defects (a/root/modelvs/root/modelstypo, a datasetprepare()downloads butexecute()never reads,TRAIN_BACKENDasserted then ignored) and cannot be imported without the NPU docker patch, so it needs someone who can verify on NPU hardware.docs/user-guide/training-script-walkthrough.mdis still framed around bashXXX_ARGSarrays, which remains accurate forexamples/; re-centring it on the python launchers is a separate docs pass.