scripts, examples: stop forcing the deprecated Miles router in launchers - #2015
Open
Shi-Dong wants to merge 2 commits into
Open
scripts, examples: stop forcing the deprecated Miles router in launchers#2015Shi-Dong wants to merge 2 commits into
Shi-Dong wants to merge 2 commits into
Conversation
…LoRA launchers Both launchers baked --use-miles-router into misc_args, so every run took the deprecated text-based router path instead of sgl-router. The flag is declared action="store_true" with no negated form, and neither launcher exposes a knob for it, so callers had no way to opt out short of editing the script. Dropping it lets the launchers use the sgl-router default like the rest of the scripts.
Contributor
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
The Miles router is deprecated in favour of sgl-router, but twelve example launchers still passed --use-miles-router unconditionally. Because the flag is store_true with no CLI opt-out, anyone copying these recipes was silently pinned to the deprecated path with no way to override it from the command line. Dropping the flag lets router_manager fall through to sgl-router. Also removes a dead commented-out occurrence in the p2p profiling script so the recipe does not read as if the router were an option worth re-enabling.
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.
Summary
The Miles router is deprecated in favour of sgl-router, but 14 launcher recipes still passed
--use-miles-routerunconditionally, so every run started from them took the deprecated text-based routing path.There was no way to opt out from the caller side:
--use-miles-routeris declaredaction="store_true"with no negated form, so it cannot be switched off by appending anything to--extra-args/ the trailing arg list.ScriptArgsfield for it.Editing the recipe was the only workaround. This PR drops the flag everywhere it was hard-coded, so
router_managerfalls through to the sgl-router branch.scripts/(2 files) —run_glm5_1_744b_a40b_lora.py,run_glm5_2_744b_a40b_lora.py.examples/(13 files) — the LoRA recipes (dev.sh,run-kimi-k25-megatron-lora.sh,run-qwen2.5-0.5B-megatron-lora.sh,run-qwen2.5-3B-megatron-lora-disaggregated.sh,run-qwen2.5-3B-megatron-lora-disaggregated-multi-node.sh,run-qwen3-4b-megatron-lora-result.sh), the multi-LoRA driver (run_multi_lora.py), the four low-precision recipes, and the reproducibility recipe. The p2p profiling recipe also had a dead commented-out occurrence, removed so the flag does not read as an option worth re-enabling.Deliberately out of scope
miles/utils/arguments.pyand all router plumbing (router_manager.py,sglang_engine.py,dashboard/,router/) are untouched. Retiring the flag is a separate decision from unpinning the recipes.tests/e2e/sglang/test_r3_router_equivalence.pyexists specifically to compare Miles router against sgl-router, and the router/dashboard unit tests exercise the same code — removing the flag there would delete the coverage rather than modernise it. Three e2e tests pass it incidentally (tests/e2e/lora/test_lora_qwen2.5_0.5B.py,tests/e2e/megatron/test_quick_start_glm4_9B.py,tests/e2e/megatron/test_glm5_2_744b_a40b_5layer_nvfp4.py); happy to clean those up here if reviewers prefer tests in scope./advanced/miles-routerpage documents R3 rollout routing replay, not this flag.Test plan
grep -rn -- "use-miles-router" scripts/ examples/returns no matches.bash -npasses on all 12 edited shell recipes; the two edited Python launchers and the multi-LoRA driver still compile.python scripts/run_glm5_2_744b_a40b_lora.py train --model-name GLM-5.2_5layer --task gsm8k --num-gpus-per-node 4 --num-rollout 5Both complete all 5 rollouts and save adapters. After the change the Megatron arg dump reports
use_miles_router ... Falseandrouter_managerlogsRouter launched at <host>:<port>from the sgl-router branch. Train/rollout logprob parity is unchanged (max abs diff 2.2e-4 against a mean logprob of -10.2).