[fix] fix the bugs/outdated commands in .sh scripts and the corresponding snapshots - #2355
Merged
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.
yueming-yuan
requested review from
Shi-Dong,
Zhichenzzz,
fzyzcjy,
guapisolo,
jybsuper and
maocheng23
as code owners
August 10, 2026 19:56
This was referenced Aug 10, 2026
yueming-yuan
changed the title
.sh scripts and the corresponding snapshots
yueming-yuan
changed the title
.sh scripts and the corresponding snapshots.sh scripts and the corresponding snapshots
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.
guapisolo
approved these changes
Aug 11, 2026
# Conflicts: # docs/index.md # docs/models/deepseek/deepseek.md # docs/models/deepseek/index.md # docs/models/glm/index.md # docs/models/index.md
# Conflicts: # docs/models/glm/glm4.md # docs/models/kimi/moonlight.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.
Arist12
added a commit
to Arist12/miles
that referenced
this pull request
Aug 13, 2026
…uncher
No parser defines the flag and no other file references it, so
examples/lora/run-kimi-k25-megatron-lora.sh exits in argument parsing before it reaches a GPU,
on any platform:
train.py: error: unrecognized arguments: --filter-zero-reward-samples
git log -S shows the flag arrived with the example itself, so this recipe has never started.
radixark#2355 already removed the same line from the sibling scripts/run-kimi-k25.sh by deleting it, and
its Python port keeps no dynamic-sampling filter, so deleting is what matches the settled intent
for this recipe rather than substituting a filter it has never run with.
docs/models/kimi/kimi-k2.5.md quotes the same block; the launch-script snapshot is regenerated.
The snapshot harness records argv and exits, so it never runs the parser -- which is why nothing
caught this.
Arist12
added a commit
to Arist12/miles
that referenced
this pull request
Aug 13, 2026
…uncher
No parser defines the flag and no other file references it, so
examples/lora/run-kimi-k25-megatron-lora.sh exits in argument parsing before it reaches a GPU,
on any platform:
train.py: error: unrecognized arguments: --filter-zero-reward-samples
git log -S shows the flag arrived with the example itself, so this recipe has never started.
radixark#2355 already removed the same line from the sibling scripts/run-kimi-k25.sh by deleting it, and
its Python port keeps no dynamic-sampling filter, so deleting is what matches the settled intent
for this recipe rather than substituting a filter it has never run with.
docs/models/kimi/kimi-k2.5.md quotes the same block; the launch-script snapshot is regenerated.
The snapshot harness records argv and exits, so it never runs the parser -- which is why nothing
caught this.
Arist12
added a commit
to Arist12/miles
that referenced
this pull request
Aug 13, 2026
…uncher
No parser defines the flag and no other file references it, so
examples/lora/run-kimi-k25-megatron-lora.sh exits in argument parsing before it reaches a GPU,
on any platform:
train.py: error: unrecognized arguments: --filter-zero-reward-samples
git log -S shows the flag arrived with the example itself, so this recipe has never started.
radixark#2355 already removed the same line from the sibling scripts/run-kimi-k25.sh by deleting it, and
its Python port keeps no dynamic-sampling filter, so deleting is what matches the settled intent
for this recipe rather than substituting a filter it has never run with.
docs/models/kimi/kimi-k2.5.md quotes the same block; the launch-script snapshot is regenerated.
The snapshot harness records argv and exits, so it never runs the parser -- which is why nothing
caught this.
Arist12
added a commit
to Arist12/miles
that referenced
this pull request
Aug 13, 2026
…uncher
No parser defines the flag and no other file references it, so
examples/lora/run-kimi-k25-megatron-lora.sh exits in argument parsing before it reaches a GPU,
on any platform:
train.py: error: unrecognized arguments: --filter-zero-reward-samples
git log -S shows the flag arrived with the example itself, so this recipe has never started.
radixark#2355 already removed the same line from the sibling scripts/run-kimi-k25.sh by deleting it, and
its Python port keeps no dynamic-sampling filter, so deleting is what matches the settled intent
for this recipe rather than substituting a filter it has never run with.
docs/models/kimi/kimi-k2.5.md quotes the same block; the launch-script snapshot is regenerated.
The snapshot harness records argv and exits, so it never runs the parser -- which is why nothing
caught this.
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
2 participants
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.
Second of three chained PRs cleaning up
scripts/. Stack: #2354 → #2355 (this) → #2356. Base isyueming/script-delete, review that one first.Each of these ships a launcher that cannot do what it says. The snapshot suite added in #1899/#1901 recorded the broken output rather than flagging it, so the fixes are visible here as snapshot diffs — one diff per fix, nothing else.
Doing this before the python rewrite keeps that rewrite honest: the argv comparison in the next PR is against launchers that already work, so a difference there means the port drifted, not that it inherited a bug.
Shell launchers
run-qwen3-next-80B-A3B.sh--runtime-env-jsonwas built without the comma afterNCCL_NVLS_ENABLE, so ray received invalid JSONrun-gpt-oss-20b-bf16.sh${HAS_NVLINK}but never probed for NVLink, passingNCCL_NVLS_ENABLE=""to NCCLrun-qwen3-4B-base-sft.sh,run-qwen3-235B-A22B-sft.sh,run-gpt-oss-20b-bf16.sh${EVAL_ARGS[@]}without ever declaring itrun-kimi-k25.sh--filter-zero-reward-samples, which no longer exists anywhere in milesrun-glm4.5-355B-A32B.shrun-qwen3-4B_4xgpu.sh${WANDB_KEY}unquoted inside the array, so an unset key made--wandb-keyswallow the next flag; and it colocates on 4 GPUs without--num-gpus-per-node, whose default is 8Python launchers
run_glm47_flash.pyhad--eval-intervalcommented out. The flag defaults toNone, so the eval block it builds never ran.run_qwen3_30b_a3b.pyandamd/run_qwen3_30b_a3b.pydroppedconfig=args, silently discardingcuda_core_dumpandextra_env_vars.run_glm45_355b_a32b.pydefaultedhardwaretoH100— the one value_execute_trainasserts against — so the snapshot harness had to override it to run the script at all. That override is now unnecessary and removed.Two more found while working
execute_traincomputedos.environ.get("NCCL_NVLS_ENABLE", str(int(check_has_nvlink()))). Python evaluates aget()default eagerly, so every launch shelled out tonvidia-smieven when the caller had already chosen a value — including on ROCm boxes, where the probe is meaningless.run_inkling.pysetsMODEL_ARGS_NUM_LAYERSfor its pruned variants, and that leaked into every later test in the session. Reproduced on the unmodified baseline: runningtest_py_launch_scripts.pybeforetest_model_args.pyfails eight model-args snapshots.pytest-randomlyis installed, so this could fire in either order.Testing
pytest tests/manual/launch_scripts tests/fast/launch_scripts tests/fast/utils/test_command_utils.py— 594 passed, and now 524 passed under random ordering too, which failed before the harness fix. Every snapshot diff in this PR was read line by line to confirm it contains only the intended fix.