[Sync][B] models/backends: MiniMax-M2.5 + rollout GPU-placement validation (slime #1929/#1992/#1934/#1944) - #143
Conversation
There was a problem hiding this comment.
Code Review
This pull request adds support for the MiniMax-M2.5 (229B MoE) model, including training run scripts, a Megatron-to-HF converter, a model bridge, and a custom SelfAttention implementation to handle full-dimension QK Norm. Feedback focuses on optimizing the custom attention mechanism to avoid expensive tensor-parallel gather/scatter operations by using an all-reduce on the sum of squares instead. Additionally, the reviewer identified a regex matching bug in the parameter converter, a typo in the Python buffering environment variable, and opportunities to simplify the code.
| expert_pattern = r"mlp.experts\.(.+)\.weight(\d+)" | ||
| match = re.match(expert_pattern, rest) | ||
| if match: | ||
| rest, expert_idx = match.groups() |
There was a problem hiding this comment.
The regular expression r"mlp.experts\.(.+)\.weight(\d+)" will fail to match standard Megatron MoE expert parameter names. In PyTorch/Megatron, expert weights are typically named either mlp.experts.{expert_idx}.linear_fc1.weight (where the index is in the middle) or mlp.experts.linear_fc1.weight.{expert_idx} (where the index is at the end with a dot separator). The current regex expects no dot before the index and places the index at the very end, which will cause the conversion script to fail with a ValueError for expert weights. We should support both standard formats robustly.
match_middle = re.match(r"mlp.experts\.(\d+)\.(.+)\.weight", rest)
match_end = re.match(r"mlp.experts\.(.+)\.weight\.(\d+)", rest)
if match_middle or match_end:
if match_middle:
expert_idx, rest = match_middle.groups()
else:
rest, expert_idx = match_end.groups()| from megatron.core import parallel_state | ||
| from megatron.core.models.gpt.gpt_layer_specs import get_gpt_decoder_block_spec | ||
| from megatron.core.tensor_parallel import ( | ||
| gather_from_tensor_model_parallel_region, | ||
| scatter_to_tensor_model_parallel_region, | ||
| ) |
There was a problem hiding this comment.
Import torch and reduce_from_tensor_model_parallel_region to support the optimized local RMSNorm calculation.
| from megatron.core import parallel_state | |
| from megatron.core.models.gpt.gpt_layer_specs import get_gpt_decoder_block_spec | |
| from megatron.core.tensor_parallel import ( | |
| gather_from_tensor_model_parallel_region, | |
| scatter_to_tensor_model_parallel_region, | |
| ) | |
| import torch | |
| from megatron.core import parallel_state | |
| from megatron.core.models.gpt.gpt_layer_specs import get_gpt_decoder_block_spec | |
| from megatron.core.tensor_parallel import ( | |
| gather_from_tensor_model_parallel_region, | |
| reduce_from_tensor_model_parallel_region, | |
| scatter_to_tensor_model_parallel_region, | |
| ) |
| # Create full-dimension norms | ||
| self.q_norm = build_module( | ||
| q_layernorm, | ||
| hidden_size=self.hidden_size_per_attention_head * config.num_attention_heads, | ||
| config=self.config, | ||
| eps=self.config.layernorm_epsilon, | ||
| ) | ||
| self.k_norm = build_module( | ||
| k_layernorm, | ||
| hidden_size=self.hidden_size_per_attention_head * config.num_query_groups, | ||
| config=self.config, | ||
| eps=self.config.layernorm_epsilon, | ||
| ) |
There was a problem hiding this comment.
Initialize the RMSNorm modules with the local hidden size (partitioned across TP ranks) instead of the global hidden size to enable local RMSNorm computation with a single all-reduce on the sum of squares.
| # Create full-dimension norms | |
| self.q_norm = build_module( | |
| q_layernorm, | |
| hidden_size=self.hidden_size_per_attention_head * config.num_attention_heads, | |
| config=self.config, | |
| eps=self.config.layernorm_epsilon, | |
| ) | |
| self.k_norm = build_module( | |
| k_layernorm, | |
| hidden_size=self.hidden_size_per_attention_head * config.num_query_groups, | |
| config=self.config, | |
| eps=self.config.layernorm_epsilon, | |
| ) | |
| # Create full-dimension norms partitioned across TP ranks | |
| tp_size = parallel_state.get_tensor_model_parallel_world_size() | |
| self.q_norm = build_module( | |
| q_layernorm, | |
| hidden_size=self.hidden_size_per_attention_head * config.num_attention_heads // tp_size, | |
| config=self.config, | |
| eps=self.config.layernorm_epsilon, | |
| ) | |
| self.k_norm = build_module( | |
| k_layernorm, | |
| hidden_size=self.hidden_size_per_attention_head * config.num_query_groups // tp_size, | |
| config=self.config, | |
| eps=self.config.layernorm_epsilon, | |
| ) |
| def get_query_key_value_tensors(self, hidden_states, key_value_states=None, *args, **kwargs): | ||
| query, key, value = super().get_query_key_value_tensors(hidden_states, key_value_states, *args, **kwargs) | ||
| # query: [sq, b, num_heads_local, head_dim] | ||
| # key: [sq, b, num_kv_heads_local, head_dim] | ||
|
|
||
| # Merge head dims: [sq, b, num_heads_local * head_dim] | ||
| query = query.reshape(*query.shape[:-2], -1) | ||
| key = key.reshape(*key.shape[:-2], -1) | ||
|
|
||
| # TP gather -> full-dimension norm -> TP scatter | ||
| tp_size = parallel_state.get_tensor_model_parallel_world_size() | ||
| if tp_size > 1: | ||
| query = gather_from_tensor_model_parallel_region(query) | ||
| key = gather_from_tensor_model_parallel_region(key) | ||
|
|
||
| query = self.q_norm(query) | ||
| key = self.k_norm(key) | ||
|
|
||
| if tp_size > 1: | ||
| query = scatter_to_tensor_model_parallel_region(query) | ||
| key = scatter_to_tensor_model_parallel_region(key) | ||
|
|
||
| # Reshape back: [sq, b, num_heads_local, head_dim] | ||
| query = query.view(*query.shape[:2], -1, self.hidden_size_per_attention_head) | ||
| key = key.view(*key.shape[:2], -1, self.hidden_size_per_attention_head) | ||
|
|
||
| return query, key, value |
There was a problem hiding this comment.
The current implementation of full-dimension QK Norm performs an all_gather and a scatter of the large activation tensors (query and key) across TP ranks on every single attention layer. This introduces massive communication overhead and will severely bottleneck training. Since RMSNorm only requires the global sum of squares to compute the normalization factor, we can optimize this by computing the local sum of squares on each rank, performing a single all_reduce (sum) of shape [sq, b, 1], and then applying the normalization and partitioned weights locally. This mathematically identical optimization reduces the communication volume by orders of magnitude (e.g., from ~200MB to ~32KB per layer).
def get_query_key_value_tensors(self, hidden_states, key_value_states=None, *args, **kwargs):
query, key, value = super().get_query_key_value_tensors(hidden_states, key_value_states, *args, **kwargs)
# query: [sq, b, num_heads_local, head_dim]
# key: [sq, b, num_kv_heads_local, head_dim]
tp_size = parallel_state.get_tensor_model_parallel_world_size()
if tp_size > 1:
# Compute global RMSNorm using all-reduce on sum of squares to avoid expensive all-gather/scatter of activations
query_sq = query.pow(2).sum(dim=[-2, -1], keepdim=True)
key_sq = key.pow(2).sum(dim=[-2, -1], keepdim=True)
query_sq = reduce_from_tensor_model_parallel_region(query_sq)
key_sq = reduce_from_tensor_model_parallel_region(key_sq)
global_dim_q = self.config.num_attention_heads * self.hidden_size_per_attention_head
global_dim_k = self.config.num_query_groups * self.hidden_size_per_attention_head
query_rms = torch.rsqrt(query_sq / global_dim_q + self.config.layernorm_epsilon)
key_rms = torch.rsqrt(key_sq / global_dim_k + self.config.layernorm_epsilon)
query = query * query_rms
key = key * key_rms
else:
# Single GPU / TP=1: standard local RMSNorm
query_sq = query.pow(2).mean(dim=[-2, -1], keepdim=True)
key_sq = key.pow(2).mean(dim=[-2, -1], keepdim=True)
query = query * torch.rsqrt(query_sq + self.config.layernorm_epsilon)
key = key * torch.rsqrt(key_sq + self.config.layernorm_epsilon)
# Apply weight scaling (weights are partitioned/local)
query = query * self.q_norm.weight.view(-1, self.hidden_size_per_attention_head)
key = key * self.k_norm.weight.view(-1, self.hidden_size_per_attention_head)
return query, key, value|
|
||
| set -ex | ||
|
|
||
| export PYTHONBUFFERED=16 |
There was a problem hiding this comment.
The standard Python environment variable to disable output buffering is PYTHONUNBUFFERED, not PYTHONBUFFERED. Using PYTHONBUFFERED will have no effect, and standard output/error might remain buffered, which can delay logs or cause them to be lost in the event of a crash.
| export PYTHONBUFFERED=16 | |
| export PYTHONUNBUFFERED=1 |
| try: | ||
| head_dim = args.kv_channels if args.kv_channels is not None else args.hidden_size // args.num_attention_heads | ||
| except AttributeError: | ||
| head_dim = args.hidden_size // args.num_attention_heads |
a262bd4 to
4c22695
Compare
Port of THUDM/slime#1934 (add GPU placement validation before starting rollout engines) + #1944 (register its test to CI). 🔧 PORT — slime/→vime/ rewrite. - vime/ray/rollout_validation.py (new): pure, engine-agnostic validate_server_group_gpu_indices() — raises a descriptive ValueError when the rollout server group's GPU slots (gpu_offset + num_engines*num_gpu_per_engine) exceed the available reordered GPU ids. The error message's arg hints were genericized for vime's vLLM rollout (dropped the sglang-specific `--sglang-config server_groups` reference). - vime/ray/rollout.py: call the validator in ServerGroup.start_engines right after unpacking the placement group, before creating VLLMEngine actors. - tests/test_rollout_validation.py (new): pytest unit tests (accept valid / allow empty / reports config context) with a __main__ pytest entrypoint. - .github/workflows/pr-test.yml.j2 (+regenerated pr-test.yml): register test_rollout_validation.py in the 0-GPU cpu test matrix. (black also wrapped one pre-existing long _start_router line in rollout.py, required for the changed-file format check to pass.) Refs: THUDM/slime#1934, THUDM/slime#1944, #107 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: aoshen02 <aoshen@inferact.ai>
…ert scripts) Port THUDM/slime #1929 MiniMax-M2.5: megatron layer spec (vime_plugins/models/minimax_m2.py), mbridge online-sync bridge (vime_plugins/mbridge/minimax_m2.py), megatron->hf converter, and the minimax-m2 / run-minimax-m2 launchers (rollout path translated sglang->vLLM: --sglang-* -> --vllm-*, pkill sglang -> pkill vllm serve). The two convert-minimax-m2-*.sh scripts added by #1929 are intentionally omitted (deleted by follow-up #1992). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: aoshen02 <aoshen@inferact.ai>
…anslate --sglang-config -> --vllm-config) slime's validate_server_group_gpu_indices error tells the user to align --rollout-num-gpus, --rollout-num-gpus-per-engine, AND --sglang-config server_groups. server_groups is a framework (slime/vime) concept that vime fully carries under --vllm-config (vime/backends/vllm_utils/vllm_config.py: ServerGroupConfig/VllmConfig). The mega-B port had dropped the third hint for a generic phrase; restore it as the faithful 1:1 translation --vllm-config server_groups.
6972a8b to
367bdfb
Compare
Scope — PR-B (models/backends mega-PR), part of RFC #107
This PR lands the non-image half of mega-PR B. The two image-gated pieces (#1947 FlashQLA, #1952 disable-param-backup) are deliberately held for follow-up commits because both modify the build (
Dockerfile/docker/patch/latest/megatron.patch) and require a gb200+h200 image rebuild before their code can run — see "Pending" below.Included (B-core, runs on the current image)
07877b8):vime/ray/rollout_validation.py+validate_server_group_gpu_indices, wired into rollout start + registered in CI (test_rollout_validation.py, 0-GPU). Reused as-is from the already-translated branch.a262bd4): megatron layer spec (vime_plugins/models/minimax_m2.py), mbridge online-sync bridge, megatron→hf converter, andminimax-m2/run-minimax-m2launchers. Rollout path translated sglang→vLLM (--sglang-* → --vllm-*,--sglang-ep-size N → --vllm-enable-expert-parallel,pkill sglang → pkill -f "vllm serve"). The twoconvert-minimax-m2-*.shscripts that #1929 added are omitted (deleted by follow-up #1992).Pending (B-docker — separate commits, image-rebuild-gated)
docker/Dockerfile(pip install FlashQLA) → needs arm+x86 image rebuild. This is the Megatron train-side GDN kernel for Qwen3.5/Qwen3-Next (vLLM rollout already providesqwen_gdn_attention_core).docker/patch/latest/megatron.patch(DDP_ParamAndGradBuffersurgery) + actor/arguments python that depends on that patch (disable_param_buffers_cpu_backup) → also rebuild-gated; train-memory, higher-risk, will get its own careful review.CI
test_rollout_validation.pyregistered).🤖 Generated with Claude Code
Fidelity ledger (audited 2026-06-04)
ray/rollout_validation.pyGPU-placement pre-flighttest_rollout_validationPASS on rebuilt stack (8914)