Support GLM-5.2 744B-A40B - #1376
Conversation
GLM-5.2 reuses GLM-5's glm_moe_dsa architecture but adds DSA cross-layer index sharing (index_topk_freq=4): only "computing" layers carry indexer weights and run the sparse top-k; "skip" layers reuse the most recent computing layer's indices. - miles_plugins/models/glm5/glm5.py: port the index-sharing logic from slime (#2072) -- is_skip_topk_layer/source_compute_layer, per-layer skip_topk flags, a per-microbatch top-k holder on packed_seq_params, drop indexer modules on skip layers, get_glm5_spec schedule + PP-split guard. Plain DSA (freq=1) is unchanged. - scripts/models/glm5.2-744B-A40B.sh (+_5layer): megatron model args (rotary-base 8000000). - scripts/run_glm5_2_744b_a40b.py: launcher mirroring slime's recipe (BF16 train + FP8 rollout, PD disaggregation, NSA + EAGLE); single-node 5-layer minimal path converts on 1 GPU (PP=1) so the stage starts on a computing layer. - tests/e2e/megatron/test_glm5_2_744b_a40b_5layer_ci.py: model-scripts smoke test on Pinaster/GLM-5.2_5layer. Validated end-to-end on 8xH100: convert -> rollout -> train step all pass.
There was a problem hiding this comment.
Code Review
This pull request introduces support for GLM-5.2 (744B-A40B) with DSA cross-layer index sharing, adding model architecture modifications, training scripts, and end-to-end tests. The reviewer feedback focuses on optimizing and cleaning up the implementation: calculating the computing layer index in O(1) time instead of using a loop, refactoring duplicated topk_indices logic in the forward pass, removing an unnecessary loop when validating pipeline splits, and adding a descriptive error message to a NotImplementedError in the training script.
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.
| layer = layer_number | ||
| while is_skip_topk_layer(layer, skip_topk_offset, topk_freq): | ||
| layer -= 1 | ||
| return layer |
There was a problem hiding this comment.
The computing layer index can be calculated directly using an O(1) mathematical formula instead of a while loop. This is more efficient and completely avoids any risk of infinite loops.
| layer = layer_number | |
| while is_skip_topk_layer(layer, skip_topk_offset, topk_freq): | |
| layer -= 1 | |
| return layer | |
| if layer_number <= skip_topk_offset: | |
| return layer_number | |
| return skip_topk_offset + ((layer_number - skip_topk_offset) // topk_freq) * topk_freq |
| if self.index_share: | ||
| # Cross-layer index sharing. The top-k holder lives on the per-microbatch | ||
| # ``packed_seq_params`` object: it is closure-captured by Megatron's | ||
| # activation-checkpoint ``custom_forward``, so the same instance is reused at | ||
| # recompute time. That gives per-microbatch isolation (no cross-microbatch | ||
| # clobber under PP 1F1B) AND recompute safety. A stage always starts on a | ||
| # computing layer (asserted in ``get_glm5_spec``), so a skip layer's source is | ||
| # always in-stage. | ||
| holder = getattr(packed_seq_params, self._HOLDER_ATTR, None) | ||
| if holder is None: | ||
| holder = {} | ||
| setattr(packed_seq_params, self._HOLDER_ATTR, holder) | ||
|
|
||
| if self.skip_topk: | ||
| if self._source_layer not in holder: | ||
| raise AssertionError( | ||
| "DSA index-share: skip layer " | ||
| f"(layer_number={self.layer_number}) needs the top-k of its source " | ||
| f"computing layer (layer_number={self._source_layer}), but that layer " | ||
| "did not run in this pipeline stage's forward. Cross-PP top-k sharing " | ||
| "is not supported; ensure every pipeline stage starts on a computing " | ||
| f"layer (index_topk_freq={self.index_topk_freq}, " | ||
| f"index_skip_topk_offset={self.skip_topk_offset}). " | ||
| f"Holder has layers {sorted(holder)}." | ||
| ) | ||
| topk_indices = holder[self._source_layer] | ||
| else: | ||
| starts, ends = generate_varlen_mask_params(packed_seq_params.cu_seqlens_q) | ||
| index_key = index_key.squeeze(1) | ||
| head_weights = head_weights.unsqueeze(-1) | ||
| starts = scatter_to_sequence_parallel_region(starts, group=parallel_state.get_context_parallel_group()) | ||
| ends = scatter_to_sequence_parallel_region(ends, group=parallel_state.get_context_parallel_group()) | ||
| _, topk_indices = fused_select_topk(index_query, index_key, head_weights, starts, ends) | ||
| holder[self.layer_number] = topk_indices | ||
| else: | ||
| starts, ends = generate_varlen_mask_params(packed_seq_params.cu_seqlens_q) | ||
| index_key = index_key.squeeze(1) | ||
| head_weights = head_weights.unsqueeze(-1) | ||
| starts = scatter_to_sequence_parallel_region(starts, group=parallel_state.get_context_parallel_group()) | ||
| ends = scatter_to_sequence_parallel_region(ends, group=parallel_state.get_context_parallel_group()) | ||
| _, topk_indices = fused_select_topk(index_query, index_key, head_weights, starts, ends) |
There was a problem hiding this comment.
The logic for computing topk_indices is duplicated three times in this method. We can refactor this to compute topk_indices once in the else block, and then conditionally store it in the holder if self.index_share is enabled. This significantly improves maintainability and readability.
if self.skip_topk:
# Cross-layer index sharing. The top-k holder lives on the per-microbatch
# ``packed_seq_params`` object: it is closure-captured by Megatron's
# activation-checkpoint ``custom_forward``, so the same instance is reused at
# recompute time. That gives per-microbatch isolation (no cross-microbatch
# clobber under PP 1F1B) AND recompute safety. A stage always starts on a
# computing layer (asserted in ``get_glm5_spec``), so a skip layer's source is
# always in-stage.
holder = getattr(packed_seq_params, self._HOLDER_ATTR, None)
if holder is None or self._source_layer not in holder:
raise AssertionError(
"DSA index-share: skip layer "
f"(layer_number={self.layer_number}) needs the top-k of its source "
f"computing layer (layer_number={self._source_layer}), but that layer "
"did not run in this pipeline stage's forward. Cross-PP top-k sharing "
"is not supported; ensure every pipeline stage starts on a computing "
f"layer (index_topk_freq={self.index_topk_freq}, "
f"index_skip_topk_offset={self.skip_topk_offset}). "
f"Holder has layers {sorted(holder) if holder is not None else []}."
)
topk_indices = holder[self._source_layer]
else:
starts, ends = generate_varlen_mask_params(packed_seq_params.cu_seqlens_q)
index_key = index_key.squeeze(1)
head_weights = head_weights.unsqueeze(-1)
starts = scatter_to_sequence_parallel_region(starts, group=parallel_state.get_context_parallel_group())
ends = scatter_to_sequence_parallel_region(ends, group=parallel_state.get_context_parallel_group())
_, topk_indices = fused_select_topk(index_query, index_key, head_weights, starts, ends)
if self.index_share:
holder = getattr(packed_seq_params, self._HOLDER_ATTR, None)
if holder is None:
holder = {}
setattr(packed_seq_params, self._HOLDER_ATTR, holder)
holder[self.layer_number] = topk_indices| for local_id in range(num_layers_to_build): | ||
| layer_number = local_id + layer_offset + 1 # Megatron layer_number is 1-indexed | ||
| if local_id == 0 and is_skip_topk_layer( | ||
| layer_number, config.index_skip_topk_offset, config.index_topk_freq | ||
| ): | ||
| src = source_compute_layer(layer_number, config.index_skip_topk_offset, config.index_topk_freq) | ||
| raise AssertionError( | ||
| "DSA index-share pipeline split is invalid: this stage starts at global " | ||
| f"layer_number={layer_number} which is a skip layer whose source computing " | ||
| f"layer={src} is on a previous pipeline stage. Cross-layer top-k sharing does " | ||
| "not cross PP boundaries. Choose a pipeline layout where every stage begins on " | ||
| "a computing layer (index_topk_freq=" | ||
| f"{config.index_topk_freq}, index_skip_topk_offset={config.index_skip_topk_offset})." | ||
| ) |
There was a problem hiding this comment.
Since the check is only performed for local_id == 0, there is no need to loop over all layers. We can directly check the first layer of the stage, which is cleaner and more efficient.
| for local_id in range(num_layers_to_build): | |
| layer_number = local_id + layer_offset + 1 # Megatron layer_number is 1-indexed | |
| if local_id == 0 and is_skip_topk_layer( | |
| layer_number, config.index_skip_topk_offset, config.index_topk_freq | |
| ): | |
| src = source_compute_layer(layer_number, config.index_skip_topk_offset, config.index_topk_freq) | |
| raise AssertionError( | |
| "DSA index-share pipeline split is invalid: this stage starts at global " | |
| f"layer_number={layer_number} which is a skip layer whose source computing " | |
| f"layer={src} is on a previous pipeline stage. Cross-layer top-k sharing does " | |
| "not cross PP boundaries. Choose a pipeline layout where every stage begins on " | |
| "a computing layer (index_topk_freq=" | |
| f"{config.index_topk_freq}, index_skip_topk_offset={config.index_skip_topk_offset})." | |
| ) | |
| if num_layers_to_build > 0: | |
| layer_number = layer_offset + 1 # Megatron layer_number is 1-indexed | |
| if is_skip_topk_layer( | |
| layer_number, config.index_skip_topk_offset, config.index_topk_freq | |
| ): | |
| src = source_compute_layer(layer_number, config.index_skip_topk_offset, config.index_topk_freq) | |
| raise AssertionError( | |
| "DSA index-share pipeline split is invalid: this stage starts at global " | |
| f"layer_number={layer_number} which is a skip layer whose source computing " | |
| f"layer={src} is on a previous pipeline stage. Cross-layer top-k sharing does " | |
| "not cross PP boundaries. Choose a pipeline layout where every stage begins on " | |
| "a computing layer (index_topk_freq=" | |
| f"{config.index_topk_freq}, index_skip_topk_offset={config.index_skip_topk_offset})." | |
| ) |
| "--expert-tensor-parallel-size 1 " | ||
| ) | ||
| else: | ||
| raise NotImplementedError |
|
cc catches a potential issue: Full-model checkpoint conversion will crash on the new assertion run_glm5_2_744b_a40b.py → _prepare_megatron_ckpt (the non-pruned branch) keeps the GLM-5 conversion layout verbatim: extra_args += ( Conversion runs tools/convert_hf_to_torch_dist.py, which builds the model via get_model(get_model_provider_func(args), ...) (line 110) → get_glm5_spec per pipeline stage. get_glm5_spec loads the GLM-5.2 HF config With PP=4 and last=18 over 78 layers, Megatron splits [20, 20, 20, 18] → stages start at 1-indexed layers 1, 21, 41, 61. For freq=4, offset=3:
So pp_ranks 1/2/3 each start on a skip layer and raise AssertionError("DSA index-share pipeline split is invalid…"). The documented full-model prepare step aborts. This is almost certainly an oversight rather than intent: the author did account for the constraint everywhere else — the train layout (PP=8), and even the 5-layer convert path, whose comment explicitly says "nproc=1 Why it slips through CI: the 5-layer smoke test converts via the <= 5 branch (PP=1, single GPU), so the multi-stage split is never exercised. Fix: use an index-share-valid conversion split. Either reuse the training layout (PP=8, first=14/last=16), or a tuned PP=4 split such as first=14/last=24 → [14,20,20,24] → starts 1,15,35,55 (all ≡ computing). Verify the |
Add scripts/models/glm5.2-744B-A40B.sh (= GLM-5.1 dims, --rotary-base 8e6; converges with PR radixark#1376's registry) and glm5.2-744B-A40B_7layer.sh (7-layer prune: 3 dense + 4 MoE). Extend scripts/run_glm5_lora.py to map GLM-5.2 / GLM-5.2-7layer model-names to those registries, add an _HF_REPO download map, and default --hf-checkpoint to a local {model_dir}/{model_name} path. GLM-5.2's DSA cross-layer index sharing (index_topk_freq) is read from the HF config by the Megatron-Bridge GLM5 provider (CrossLayerDSAttention) -- no extra CLI args here. Verified e2e: run_glm5_lora.py GLM-5.2-7layer train-only (replay of a GLM-5.1 rollout dump) -> TRAIN EXIT 0 + PEFT adapter. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add scripts/models/glm5.2-744B-A40B.sh (= GLM-5.1 dims, --rotary-base 8e6; converges with PR radixark#1376's registry) and glm5.2-744B-A40B_7layer.sh (7-layer prune: 3 dense + 4 MoE). Extend scripts/run_glm5_lora.py to map GLM-5.2 / GLM-5.2-7layer model-names to those registries, add an _HF_REPO download map, and default --hf-checkpoint to a local {model_dir}/{model_name} path. GLM-5.2's DSA cross-layer index sharing (index_topk_freq) is read from the HF config by the Megatron-Bridge GLM5 provider (CrossLayerDSAttention) -- no extra CLI args here. Verified e2e: run_glm5_lora.py GLM-5.2-7layer train-only (replay of a GLM-5.1 rollout dump) -> TRAIN EXIT 0 + PEFT adapter.
df3fc36 to
df778e6
Compare
|
@yushengsu-thu good catch! fixed |
Add scripts/models/glm5.2-744B-A40B.sh (= GLM-5.1 dims, --rotary-base 8e6; converges with PR radixark#1376's registry) and glm5.2-744B-A40B_7layer.sh (7-layer prune: 3 dense + 4 MoE). Extend scripts/run_glm5_lora.py to map GLM-5.2 / GLM-5.2-7layer model-names to those registries, add an _HF_REPO download map, and default --hf-checkpoint to a local {model_dir}/{model_name} path. GLM-5.2's DSA cross-layer index sharing (index_topk_freq) is read from the HF config by the Megatron-Bridge GLM5 provider (CrossLayerDSAttention) -- no extra CLI args here. Verified e2e: run_glm5_lora.py GLM-5.2-7layer train-only (replay of a GLM-5.1 rollout dump) -> TRAIN EXIT 0 + PEFT adapter.
…cripts Follow the run_glm5_2_744b_a40b.py convention (radixark#1376): one run script + one scripts/models registry per model family, LoRA variants suffixed _lora: scripts/run_glm5_2_744b_a40b_lora.py GLM-5.2 (full 744B-A40B + 5layer toy) scripts/run_glm5_1_744b_a40b_lora.py GLM-5.1 (full 744B-A40B + 6layer toy) scripts/models/glm5.2-744B-A40B_lora.sh scripts/models/glm5.2-744B-A40B_5layer_lora.sh scripts/models/glm5.1-744B-A40B_lora.sh scripts/models/glm5.1-744B-A40B_6layer_lora.sh The _lora.sh registries carry the architecture MODEL_ARGS (verbatim from their full-FT counterparts; 5.1 vs 5.2 differ only in rotary-base 1e6 vs 8e6) plus a comments-only section documenting the LoRA flags the runner emits; putting LoRA flags in MODEL_ARGS was rejected because argparse last-wins means the .py could never turn a .sh-emitted boolean off (e.g. KEEP_MOE_LORA=0 must be able to drop --experts-shared-outer-loras). Emitted train args are token-identical to run_glm5_lora.py for the covered models (verified by comment-stripped diff); the deleted GLM-5.1-4layer/-20layer entries remain reachable via the surviving glm5-744B-A40B_{4,20}layer.sh. CI: test_glm5_1_lora_6layer_ci.py renamed to the new convention and repointed; test_glm5_2_lora_5layer_ci.py replaced by test_glm5_2_744b_a40b_5layer_lora_ci.py which drops the obsolete GLM-5.1-dump/train-only replay dance (sglang serves GLM-5.2 live now) for a real 2-step rollout->train->save-adapter loop. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HFoyYjC55SRsNRXUzWMWKH Signed-off-by: Yusheng Su <yushengsu.thu@gmail.com>
This PR is migrated from (THUDM/slime#2093).
Thanks @zhuzilin!