Repository navigation
feat(attention_mla): optional sigmoid output gate and NoPE support - #5543
snehalv2002 wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces support for the Kimi-K3 model in MaxText, adding the model configuration, parameter mapping, transformation hooks, and corresponding unit tests. The reviewer feedback highlights several critical issues: in param_mapping.py, the config parameter needs to be safely converted to a dictionary to prevent AttributeError when a PretrainedConfig object is passed. In the unit tests, there is duplicated code for convert_pytorch_module_to_maxtext_params that should be imported from the utility module, and the tests should pass self.hf_config["text_config"] instead of self.hf_config to avoid falling back to default configuration values.
| `kimi-k3.yml` and by test configs, 0-indexed. | ||
| Falls back to the production 93-layer layout if neither is present. | ||
| """ | ||
| num_layers = config.get("num_hidden_layers", 93) |
There was a problem hiding this comment.
The config parameter can be either a dictionary or a Hugging Face PretrainedConfig object (such as KimiK3Config). Since PretrainedConfig does not have a .get() method, calling config.get(...) directly will raise an AttributeError during actual checkpoint conversion. Converting config to a dictionary at the beginning of the function ensures compatibility with both dictionary and configuration object inputs.
if not isinstance(config, dict):\n config = config.to_dict() if hasattr(config, "to_dict") else getattr(config, "__dict__", {})\n num_layers = config.get("num_hidden_layers", 93)|
|
||
| def KIMI_K3_MAXTEXT_TO_HF_PARAM_MAPPING(config, maxtext_config, scan_layers=False): | ||
| """Generates parameter mapping from MaxText to Hugging Face for Kimi-K3.""" | ||
| num_main_layers = config.get("num_hidden_layers", 93) |
There was a problem hiding this comment.
The config parameter can be a Hugging Face PretrainedConfig object which does not support the .get() method. Converting config to a dictionary at the beginning of the function ensures compatibility with both dictionary and configuration object inputs.
if not isinstance(config, dict):\n config = config.to_dict() if hasattr(config, "to_dict") else getattr(config, "__dict__", {})\n num_main_layers = config.get("num_hidden_layers", 93)|
|
||
| def KIMI_K3_MAXTEXT_TO_HF_PARAM_HOOK_FN(config, maxtext_config, scan_layers=False, saving_to_hf=False): | ||
| """Transformation hooks for Kimi-K3 parameters.""" | ||
| hidden_size = config.get("hidden_size", 7168) |
There was a problem hiding this comment.
The config parameter can be a Hugging Face PretrainedConfig object which does not support the .get() method. Converting config to a dictionary at the beginning of the function ensures compatibility with both dictionary and configuration object inputs.
if not isinstance(config, dict):\n config = config.to_dict() if hasattr(config, "to_dict") else getattr(config, "__dict__", {})\n hidden_size = config.get("hidden_size", 7168)| def convert_pytorch_module_to_maxtext_params(pt_state_dict, hf_config, maxtext_config, prefix_filter=None): | ||
| """Converts PyTorch state dict tensors to MaxText param arrays using Kimi-K3 mappings and hooks.""" | ||
| mapping_fn = PARAM_MAPPING["kimi-k3"] | ||
| hook_fn_factory = HOOK_FNS["kimi-k3"] | ||
|
|
||
| param_map = mapping_fn(hf_config, maxtext_config, scan_layers=False) | ||
| hook_map = hook_fn_factory(hf_config, maxtext_config, scan_layers=False, saving_to_hf=False) | ||
|
|
||
| converted_params = {} | ||
| for mt_key, hf_target in param_map.items(): | ||
| if prefix_filter and not mt_key.startswith(prefix_filter): | ||
| continue | ||
|
|
||
| if isinstance(hf_target, str): | ||
| matching_key = None | ||
| for k in pt_state_dict.keys(): | ||
| if hf_target.endswith(k): | ||
| matching_key = k | ||
| break | ||
|
|
||
| if matching_key in pt_state_dict: | ||
| tensor_np = pt_state_dict[matching_key].detach().cpu().float().numpy() | ||
| if mt_key in hook_map: | ||
| tensor_np = hook_map[mt_key](tensor_np) | ||
| converted_params[mt_key] = tensor_np | ||
|
|
||
| return converted_params |
There was a problem hiding this comment.
The convert_pytorch_module_to_maxtext_params function is duplicated here and in tests/utils/kimi_k3_conversion_utils.py. To improve maintainability and avoid code duplication, import the canonical version from the utility module instead.
from tests.utils.kimi_k3_conversion_utils import convert_pytorch_module_to_maxtext_params| converted_params = convert_pytorch_module_to_maxtext_params( | ||
| pt_mla.state_dict(), | ||
| self.hf_config, | ||
| None, | ||
| prefix_filter="params-decoder-layers_3-self_attention", | ||
| ) |
There was a problem hiding this comment.
The PARAM_MAPPING functions expect the flat configuration dictionary (or configuration object). Passing self.hf_config directly (which contains "text_config" as a nested key) causes the mapping functions to fall back to default values (e.g., defaulting num_hidden_layers to 93 instead of 4). Pass self.hf_config["text_config"] instead.
converted_params = convert_pytorch_module_to_maxtext_params(\n pt_mla.state_dict(),\n self.hf_config["text_config"],\n None,\n prefix_filter="params-decoder-layers_3-self_attention",\n )|
|
||
| def test_param_mapping_shapes(self): | ||
| """Verifies that Group 2 parameter mappings and transformation hooks produce correct MaxText shapes.""" | ||
| hooks = HOOK_FNS["kimi-k3"](self.hf_config, None, scan_layers=False, saving_to_hf=False) |
There was a problem hiding this comment.
The HOOK_FNS factory expects the flat configuration dictionary (or configuration object). Passing self.hf_config directly (which contains "text_config" as a nested key) causes the hook functions to fall back to default values. Pass self.hf_config["text_config"] instead.
| hooks = HOOK_FNS["kimi-k3"](self.hf_config, None, scan_layers=False, saving_to_hf=False) | |
| hooks = HOOK_FNS["kimi-k3"](self.hf_config["text_config"], None, scan_layers=False, saving_to_hf=False) |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
b332c56 to
f341ad9
Compare
59b2dc6 to
d17c8e2
Compare
d17c8e2 to
5f04dee
Compare
f341ad9 to
1af3969
Compare
Add optional sigmoid output gate to MLA attention layer controlled by mla_use_output_gate configuration.
5f04dee to
90fd652
Compare
1af3969 to
38df8a0
Compare
MLA: optional sigmoid output gate and NoPE (skip-RoPE) support
What
Two opt-in changes to the shared MLA layer (
attention_mla.py), both needed by Kimi-K3 full-attention layers:MLA(mla_use_output_gate=False). When it is True, the layer createsg_proj(embed_attn → q_heads × v_head_dim) and computesout = out_proj(attn * sigmoid(g_proj(inputs_q))). The gate gets the same logical sharding as the attention output. As in the HF reference, the sigmoid runs in the activation dtype. The constructor arg is the only switch: there is no config field, which is the same pattern asis_nope_layer. The Kimi-K3 decoder layer (PR8) passesmla_use_output_gate=True.is_nope_layerflag. It skipsapply_rotary_embeddingon theq_pe/k_peslices inmla_query_projectionandmla_kv_projection. The slices are still projected and still take part in attention. Kimi-K3 sets HFmla_use_nope: true. Before this PR, MLA silently ignored the flag.Both features are off by default. No existing MLA caller (DeepSeek, Kimi-K2, …) passes either flag, so their behaviour and params are unchanged.
Tests
New
tests/unit/kimi_k3_mla_gate_nope_test.py:g_projweights the output is exactly 0.5× the ungated output.is_nope_layer=Trueat non-zero positions, the output doesn't depend on position and equals the RoPE layer's output at position 0. RoPE at non-zero positions gives a different output.KimiMLAAttentionat positionsarange(seq), with weights loaded through PR1'skimi-k3param mapping. Every expected key must convert. A negative check confirms that the RoPE variant diverges. This test skips unless torch, transformers, einops and the untrackedkimi-k3-hf-reference/checkout are all present, and it restoressys.path/sys.modulesafterwards.Size: 36 src / 289 test lines (vs. its stack base).
Known limitations
attention_outandout_projnames.Stack: Kimi-K3 series, this is PR4; merge order PR0, PR1, PR2..PR7, PR8, PR9, PR10 (#6014).
Checklist
Before submitting this PR, please make sure (put X in square brackets):
gemini-reviewlabel.