Repository navigation
feat(kimi-k3): add KimiK3DecoderLayer and unscanned kimi_linear decoder - #5550
snehalv2002 wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Code Review
This pull request adds support for the Kimi-K3 model in MaxText, implementing Kimi Delta Attention (KDA), Gated Multi-Head Latent Attention (MLA), the Attention Residuals (AttnRes) highway subsystem, and Latent MoE layers, along with configuration files, parameter mappings, and unit tests. The review feedback identifies critical issues: a missing is_nope_layer parameter and initialization in the MLA layer causing runtime errors, a severe memory bottleneck in the Latent MoE layer that could trigger Out-Of-Memory (OOM) errors, and an initialization order issue in KimiK3Config where configuration values might be overwritten.
| rope_factor: float = 40.0, # rotary embedding factor | ||
| name: str | None = None, | ||
| rngs: Optional[nnx.Rngs] = None, | ||
| mla_use_output_gate: bool = False, | ||
| ): |
There was a problem hiding this comment.
The is_nope_layer parameter is passed to attention_mla.MLA in kimi_k3.py but is missing from the __init__ signature. This will raise a TypeError at runtime. Please add is_nope_layer: bool = False to the signature.
rope_factor: float = 40.0, # rotary embedding factor
name: str | None = None,
rngs: Optional[nnx.Rngs] = None,
mla_use_output_gate: bool = False,
is_nope_layer: bool = False,
):| self.original_max_position_embeddings = original_max_position_embeddings | ||
| self.mscale = mscale | ||
| self.rope_factor = rope_factor | ||
| self.use_output_gate = mla_use_output_gate or getattr(config, "mla_use_output_gate", False) | ||
|
|
There was a problem hiding this comment.
Initialize self.is_nope_layer in __init__ so it can be accessed in mla_query_projection and mla_kv_projection without raising an AttributeError.
| self.original_max_position_embeddings = original_max_position_embeddings | |
| self.mscale = mscale | |
| self.rope_factor = rope_factor | |
| self.use_output_gate = mla_use_output_gate or getattr(config, "mla_use_output_gate", False) | |
| self.original_max_position_embeddings = original_max_position_embeddings | |
| self.mscale = mscale | |
| self.rope_factor = rope_factor | |
| self.use_output_gate = mla_use_output_gate or getattr(config, "mla_use_output_gate", False) | |
| self.is_nope_layer = is_nope_layer |
| wi_0_selected = self._selected("wi_0", topk_idx) # [N, top_k, in_features, intermediate_dim] | ||
| wi_1_selected = self._selected("wi_1", topk_idx) # [N, top_k, in_features, intermediate_dim] | ||
| wo_selected = self._selected("wo", topk_idx) # [N, top_k, intermediate_dim, in_features] |
There was a problem hiding this comment.
Gathering the expert weights for each token individually via self._selected materializes extremely large intermediate tensors of shape [N, top_k, in_features, intermediate_dim]. For a standard sequence length of wi_0_selected alone in bfloat16 requires over 720 GB of memory, causing immediate Out-Of-Memory (OOM) errors on any accelerator. Avoid materializing the gathered weights per token. Instead, group/sort the tokens by their selected expert indices and perform batched matrix multiplications on the grouped tokens, or leverage MaxText's existing highly-optimized RoutedMoE implementation from maxtext/layers/moe.py which handles sharded expert computation efficiently without weight replication.
| def __init__(self, **kwargs): | ||
| self.max_position_embeddings = kwargs.get("max_position_embeddings", 1048576) | ||
| super().__init__(**kwargs) |
There was a problem hiding this comment.
Setting self.max_position_embeddings before calling super().__init__(**kwargs) can lead to it being overwritten by default values defined in the base class constructor if it is not passed in kwargs. It is safer to update kwargs directly before calling super().__init__.
| def __init__(self, **kwargs): | |
| self.max_position_embeddings = kwargs.get("max_position_embeddings", 1048576) | |
| super().__init__(**kwargs) | |
| def __init__(self, **kwargs): | |
| kwargs["max_position_embeddings"] = kwargs.get("max_position_embeddings", 1048576) | |
| super().__init__(**kwargs) |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
36ab6da to
46c7b07
Compare
ec046d4 to
7772cd2
Compare
46c7b07 to
d8e44f2
Compare
7772cd2 to
bd7ac57
Compare
bd7ac57 to
9caa785
Compare
d8e44f2 to
2120147
Compare
…uite Add native KimiK3DecoderLayer, Kimi-K3 full model architecture wiring, and end-to-end layer/model parity test suite.
2120147 to
ac24906
Compare
9caa785 to
e7de5b9
Compare
Description
Combines the Kimi-K3 layers from PR3–PR6 into a native
KimiK3DecoderLayerand wires updecoder_block: kimi_linear. With this PR, Kimi-K3 trains in MaxText (bf16).models/kimi_k3.py:KimiK3DecoderLayer. Each layer uses KDA or gated NoPE MLA attention (chosen byfull_attn_layers) and a dense MLP (layer 0) or the latent MoE, joined by the AttnRes highway.layer_idx,is_linear_attnandis_moeare required keyword arguments.layers/nnx_decoders.py: an unscannedkimi_linearlayer loop. After the last layer comes the output AttnRes pooling, thendecoder_norm.NotImplementedErroron KDA layers, because KDA state isn't cached yet;scan_layersand pipeline parallelism raiseValueError;configs/types.pyrejectspacking=Trueforkimi_linear, because KDA ignores segment ids;kimi-k3.ymltherefore setspacking: false.tests/utils/kimi_k3_parity_utils.py: loads the HF reference with a CPU-faithful KDA kernel, and undoes itssys.path/sys.modules/kernel patches inunpatch_hf_reference(). Also adds shared layer helpers for building and running a layer.assert_logit_parityis used only by PR9.Size: 504 src / 1540 test lines (vs. its stack base).
q_norm/kv_norm) use eps 1e-6 to match HFKimiRMSNorm(new optionalMLA(lora_norm_epsilon=...); default unchanged for other models).kimi_linear(KDA's conv/recurrence can't run on load-balanced CP shards).Tests
kimi_k3_decoder_test.py(runs in CI, needs no torch): config pipeline, released 93-layer layout, layer composition, NoPE, forward andjax.gradfiniteness, remat full vs none equivalence, param-tree contract, and all four guards.kimi_k3_decoder_vs_reference_test.pyandkimi_k3_full_depth_vs_reference_test.py: compare against Moonshot's PyTorch code (8-layer toy model, and the 93-layer layout at toy width). They skip unless torch, transformers and einops are installed andkimi-k3-hf-reference/is cloned, so CI doesn't run them.End to end: the full 93-layer model ran 3 training steps on C4 with lm_loss < 2.5.
Known limitations
packing=False) are supported.Stack: merge order PR0, PR1, PR2..PR7, PR8 (this PR), PR9, PR10 (#6014). This PR depends on PR3–PR6; its base is
kimi-k3-stack/pr8-baseuntil they merge.Checklist
Before submitting this PR, please make sure (put X in square brackets):
gemini-reviewlabel.