qwen3 muon fork - #4779
Conversation
… for GDN (PR AI-Hypercomputer#4577 / cl/955390793)
…ckward (with shard_map sharding support)
…outedMoE and GatedDeltaNet)
There was a problem hiding this comment.
Code Review
This pull request introduces a hybrid Gated Delta Net (GDN) implementation using Tokamax GDN v3 forward and custom VJP backward, refactors Qwen3-Next decoder blocks to support scanned and remainder layers, and forks the optax Muon optimizer to allow custom modifications. The review feedback highlights several critical issues: Flax Linen auto-naming of remainder layers will cause checkpoint conversion mismatches unless explicit names are provided; remainder layers are missing parameter hooks when scanning is enabled; the pure_jax_fused_conv1d_gdn function ignores the provided conv_state; and the non-mesh branch of Qwen3NextGatedDeltaNet discards the convolution bias by hardcoding it to None.
| layer = qwen3.Qwen3NextDecoderLayerToLinen( | ||
| config=cfg, | ||
| mesh=mesh, | ||
| model_mode=model_mode, | ||
| quant=self.quant, | ||
| layer_idx=layer_id, | ||
| ) |
There was a problem hiding this comment.
When scan_layers is enabled and there are remainder layers, they are instantiated without an explicit name. Flax Linen will auto-name them (e.g., Qwen3NextDecoderLayerToLinen_0), which will mismatch the expected prefix params-decoder-layers_{layer_id} in param_mapping.py. Specifying name=f"layers_{layer_id}" ensures the parameter names are consistent and checkpoint conversion works correctly.
| layer = qwen3.Qwen3NextDecoderLayerToLinen( | |
| config=cfg, | |
| mesh=mesh, | |
| model_mode=model_mode, | |
| quant=self.quant, | |
| layer_idx=layer_id, | |
| ) | |
| layer = qwen3.Qwen3NextDecoderLayerToLinen( | |
| config=cfg, | |
| mesh=mesh, | |
| model_mode=model_mode, | |
| quant=self.quant, | |
| layer_idx=layer_id, | |
| name=f"layers_{layer_id}", | |
| ) |
| if scan_layers: | ||
| _attach_block_hooks("params-decoder-scanned_blocks-local_layers", is_global=False) | ||
| _attach_block_hooks("params-decoder-scanned_blocks-global_layer", is_global=True) |
There was a problem hiding this comment.
When scan_layers is enabled, the hooks are only attached to the scanned blocks (local_layers and global_layer). If there are remainder layers, their parameters (such as attention query/key/value/out kernels) will not have the required hooks attached, leading to shape or value mismatches during checkpoint conversion. We should also attach hooks to the remainder layers.
| if scan_layers: | |
| _attach_block_hooks("params-decoder-scanned_blocks-local_layers", is_global=False) | |
| _attach_block_hooks("params-decoder-scanned_blocks-global_layer", is_global=True) | |
| if scan_layers: | |
| _attach_block_hooks("params-decoder-scanned_blocks-local_layers", is_global=False) | |
| _attach_block_hooks("params-decoder-scanned_blocks-global_layer", is_global=True) | |
| num_blocks = config.base_num_decoder_layers // config.inhomogeneous_layer_cycle_interval | |
| num_scanned = num_blocks * config.inhomogeneous_layer_cycle_interval | |
| num_remaining = config.base_num_decoder_layers % config.inhomogeneous_layer_cycle_interval | |
| if num_remaining > 0: | |
| for rem_idx in range(num_remaining): | |
| hf_layer_idx = num_scanned + rem_idx | |
| prefix = f"params-decoder-layers_{hf_layer_idx}" | |
| layer_in_block = rem_idx % config.inhomogeneous_layer_cycle_interval | |
| is_full_attention_layer = (layer_in_block + 1) % config.inhomogeneous_layer_cycle_interval == 0 | |
| _attach_block_hooks(prefix, is_global=is_full_attention_layer) |
| key_dim = num_k_heads * head_k_dim | ||
|
|
||
| # --- Step B: Pure JAX 1D Convolution --- | ||
| conv_input = jnp.pad(qkv, ((0, 0), (conv_kernel_size - 1, 0), (0, 0))) |
There was a problem hiding this comment.
In pure_jax_fused_conv1d_gdn, conv_state is accepted as an argument but is completely ignored in the function body, which instead always pads with zeros. If conv_state is provided (e.g., during stateful chunk-by-chunk processing), it should be prepended to qkv along the sequence dimension to ensure correct causal convolution outputs across chunk boundaries.
if conv_state is not None:
conv_input = jnp.concatenate([conv_state, qkv], axis=1)
else:
conv_input = jnp.pad(qkv, ((0, 0), (conv_kernel_size - 1, 0), (0, 0)))| core_attn_out, (next_conv_state, next_recurrent_state) = hybrid_fused_conv1d_gdn( | ||
| qkv=qkv, | ||
| b=b, | ||
| a=a, | ||
| conv_weight=self.conv1d.kernel.value, | ||
| conv_bias=None, |
There was a problem hiding this comment.
In the non-mesh branch of Qwen3NextGatedDeltaNet, conv_bias is hardcoded to None. If the convolution layer actually has a bias, this will discard it and produce incorrect results. We should extract and pass the actual conv_bias if it exists.
| core_attn_out, (next_conv_state, next_recurrent_state) = hybrid_fused_conv1d_gdn( | |
| qkv=qkv, | |
| b=b, | |
| a=a, | |
| conv_weight=self.conv1d.kernel.value, | |
| conv_bias=None, | |
| conv_bias_arg = self.conv1d.bias.value if (hasattr(self.conv1d, "bias") and self.conv1d.bias is not None) else None | |
| core_attn_out, (next_conv_state, next_recurrent_state) = hybrid_fused_conv1d_gdn( | |
| qkv=qkv, | |
| b=b, | |
| a=a, | |
| conv_weight=self.conv1d.kernel.value, | |
| conv_bias=conv_bias_arg, |
Description
Start with a short description of what the PR does and how this is a change from
the past.
The rest of the description includes relevant details and context, examples:
If the change fixes a bug or a Github issue, please include a link, e.g.,:
FIXES: b/123456
FIXES: #123456
You can also provide a comma-separated list. If you don't want to close a bug but
simply to reference it, use BUGS, e.g.:
BUGS: b/123456
Notice 1: Once all tests pass, the "pull ready" label will automatically be assigned.
This label is used for administrative purposes. Please do not add it manually.
Notice 2: For external contributions, our settings currently require an approval from a MaxText maintainer to trigger CI tests.
Tests
Please describe how you tested this change, and include any instructions and/or
commands to reproduce.
Checklist
Before submitting this PR, please make sure (put X in square brackets):
gemini-reviewlabel.