Repository navigation
feat(kimi-k3): add SiTU activations (situ_gate, situ_linear) - #5547
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, implementing its configuration, parameter mapping, custom SiTU activation functions, and unit tests. The feedback suggests enhancing the robustness of parameter mapping and hook functions by safely retrieving attributes from config when it is a PretrainedConfig instance instead of a dictionary, thereby preventing potential runtime crashes. Additionally, using kwargs.setdefault is recommended for cleaner initialization in KimiK3Config.
| def _resolve_kimi_k3_full_attn_layers(config) -> set[int]: | ||
| """Returns the 0-indexed set of full-attention (MLA) layer indices. | ||
|
|
||
| Two sources are accepted, with an explicit index base for each rather than sniffing: | ||
| * `config["linear_attn_config"]["full_attn_layers"]` -- the Hugging Face field. It is | ||
| 1-indexed by definition (`KimiLinearConfig.is_kda_layer` tests `layer_idx + 1`) and | ||
| is authoritative when present. | ||
| * `config["full_attn_layers"]` at the top level -- the MaxText convention used by | ||
| `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) | ||
| linear_cfg = config.get("linear_attn_config") or {} | ||
| if linear_cfg.get("full_attn_layers") is not None: | ||
| full_attn_layers = {int(x) - 1 for x in linear_cfg["full_attn_layers"]} | ||
| elif config.get("full_attn_layers") is not None: | ||
| full_attn_layers = {int(x) for x in config["full_attn_layers"]} | ||
| else: | ||
| full_attn_layers = {3, 7, 11, 15, 19, 23, 27, 31, 35, 39, 43, 47, 51, 55, 59, 63, 67, 71, 75, 79, 83, 87, 91, 92} | ||
| bad = sorted(i for i in full_attn_layers if not 0 <= i < num_layers) | ||
| if bad: | ||
| raise ValueError(f"kimi-k3 full_attn_layers resolved to out-of-range 0-indexed layers {bad} for {num_layers} layers") | ||
| return full_attn_layers |
There was a problem hiding this comment.
Using .get() directly on config will raise an AttributeError if config is a PretrainedConfig instance (which is the case during actual checkpoint conversion) rather than a dictionary. Introducing a robust helper _get that handles both dictionaries and object attributes prevents potential runtime crashes.
def _resolve_kimi_k3_full_attn_layers(config) -> set[int]:
"""Returns the 0-indexed set of full-attention (MLA) layer indices.
Two sources are accepted, with an explicit index base for each rather than sniffing:
* 'config["linear_attn_config"]["full_attn_layers"]' -- the Hugging Face field. It is
1-indexed by definition ('KimiLinearConfig.is_kda_layer' tests 'layer_idx + 1') and
is authoritative when present.
* 'config["full_attn_layers"]' at the top level -- the MaxText convention used by
'kimi-k3.yml' and by test configs, 0-indexed.
Falls back to the production 93-layer layout if neither is present.
"""
def _get(cfg, key, default=None):
if isinstance(cfg, dict):
return cfg.get(key, default)
return getattr(cfg, key, default)
num_layers = _get(config, "num_hidden_layers", 93)
linear_cfg = _get(config, "linear_attn_config") or {}
linear_full_attn = _get(linear_cfg, "full_attn_layers")
if linear_full_attn is not None:
full_attn_layers = {int(x) - 1 for x in linear_full_attn}
elif _get(config, "full_attn_layers") is not None:
full_attn_layers = {int(x) for x in _get(config, "full_attn_layers")}
else:
full_attn_layers = {3, 7, 11, 15, 19, 23, 27, 31, 35, 39, 43, 47, 51, 55, 59, 63, 67, 71, 75, 79, 83, 87, 91, 92}
bad = sorted(i for i in full_attn_layers if not 0 <= i < num_layers)
if bad:
raise ValueError(f"kimi-k3 full_attn_layers resolved to out-of-range 0-indexed layers {bad} for {num_layers} layers")
return full_attn_layers| 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) | ||
| first_num_dense_layers = config.get("first_k_dense_replace", 1) | ||
| num_experts = config.get("num_experts", 896) | ||
| full_attn_layers = _resolve_kimi_k3_full_attn_layers(config) |
There was a problem hiding this comment.
Using .get() directly on config will raise an AttributeError if config is a PretrainedConfig instance. Introducing a robust helper _get that handles both dictionaries and object attributes prevents potential runtime crashes.
| 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) | |
| first_num_dense_layers = config.get("first_k_dense_replace", 1) | |
| num_experts = config.get("num_experts", 896) | |
| full_attn_layers = _resolve_kimi_k3_full_attn_layers(config) | |
| 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.""" | |
| def _get(cfg, key, default=None): | |
| if isinstance(cfg, dict): | |
| return cfg.get(key, default) | |
| return getattr(cfg, key, default) | |
| num_main_layers = _get(config, "num_hidden_layers", 93) | |
| first_num_dense_layers = _get(config, "first_k_dense_replace", 1) | |
| num_experts = _get(config, "num_experts", 896) | |
| full_attn_layers = _resolve_kimi_k3_full_attn_layers(config) |
| 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) | ||
| num_heads = config.get("num_attention_heads", 96) | ||
| q_lora_rank = config.get("q_lora_rank", 1536) | ||
| kv_lora_rank = config.get("kv_lora_rank", 512) | ||
| qk_nope_head_dim = config.get("qk_nope_head_dim", 128) | ||
| qk_rope_head_dim = config.get("qk_rope_head_dim", 64) | ||
| v_head_dim = config.get("v_head_dim", 128) | ||
| qk_head_dim = qk_nope_head_dim + qk_rope_head_dim |
There was a problem hiding this comment.
Using .get() directly on config will raise an AttributeError if config is a PretrainedConfig instance. Introducing a robust helper _get that handles both dictionaries and object attributes prevents potential runtime crashes.
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."""
def _get(cfg, key, default=None):
if isinstance(cfg, dict):
return cfg.get(key, default)
return getattr(cfg, key, default)
hidden_size = _get(config, "hidden_size", 7168)
num_heads = _get(config, "num_attention_heads", 96)
q_lora_rank = _get(config, "q_lora_rank", 1536)
kv_lora_rank = _get(config, "kv_lora_rank", 512)
qk_nope_head_dim = _get(config, "qk_nope_head_dim", 128)
qk_rope_head_dim = _get(config, "qk_rope_head_dim", 64)
v_head_dim = _get(config, "v_head_dim", 128)
qk_head_dim = qk_nope_head_dim + qk_rope_head_dim| num_main_layers = config.get("num_hidden_layers", 93) | ||
| first_num_dense_layers = config.get("first_k_dense_replace", 1) | ||
| full_attn_layers = _resolve_kimi_k3_full_attn_layers(config) |
There was a problem hiding this comment.
Using .get() directly on config will raise an AttributeError if config is a PretrainedConfig instance. Use the _get helper defined at the top of the function to safely retrieve these attributes.
| num_main_layers = config.get("num_hidden_layers", 93) | |
| first_num_dense_layers = config.get("first_k_dense_replace", 1) | |
| full_attn_layers = _resolve_kimi_k3_full_attn_layers(config) | |
| num_main_layers = _get(config, "num_hidden_layers", 93) | |
| first_num_dense_layers = _get(config, "first_k_dense_replace", 1) | |
| full_attn_layers = _resolve_kimi_k3_full_attn_layers(config) |
| def __init__(self, **kwargs): | ||
| self.max_position_embeddings = kwargs.get("max_position_embeddings", 1048576) | ||
| super().__init__(**kwargs) |
There was a problem hiding this comment.
Using kwargs.setdefault is a cleaner and more idiomatic way to set default configuration values before passing them to the parent class constructor. This ensures that the parent class's __init__ properly handles the attribute initialization.
| def __init__(self, **kwargs): | |
| self.max_position_embeddings = kwargs.get("max_position_embeddings", 1048576) | |
| super().__init__(**kwargs) | |
| def __init__(self, **kwargs): | |
| kwargs.setdefault("max_position_embeddings", 1048576) | |
| super().__init__(**kwargs) |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
9865408 to
dff98af
Compare
b332c56 to
f341ad9
Compare
dff98af to
b6b6fdb
Compare
f341ad9 to
1af3969
Compare
b6b6fdb to
a5e8c86
Compare
1af3969 to
38df8a0
Compare
|
Review notes on PR #5547: Blocking
Should-Fix
|
a5e8c86 to
e6dffa7
Compare
38df8a0 to
d8b471c
Compare
Implement SiTU (Shifted-Truncated Unit) gate and linear activation functions with registration in linears.py.
d8b471c to
ef04f65
Compare
e6dffa7 to
1526d64
Compare
What
Adds the Kimi-K3 SiTU-GLU activations to
maxtext.layers.linearsand wires them into the Kimi-K3 config:situ_gate(x, beta=4.0) = beta * tanh(x / beta) * sigmoid(x)(gate branch)situ_linear(x, beta=25.0) = beta * tanh(x / beta)(soft-capped up branch)_convert_to_activation_function("situ_gate" | "situ_linear").kimi-k3.yml:mlp_activations: ["situ_gate", "situ_linear"]. This replaces the unresolvable["situ", "linear"]from PR1.Why
These match HF
SituAndMul. The default betas come from Kimi-K3'sconfig.json(activation_situ_beta=4.0,activation_situ_linear_beta=25.0) and are stored inKIMI_K3_SITU_BETA/KIMI_K3_SITU_LINEAR_BETA. Like HF, both functions upcast to fp32 and cast the result back to the input dtype, so bf16 runs don't round inside the tanh/scale chain. Other models are not affected.Size: 70 src / 247 test lines (vs. its stack base).
How tested
tests/unit/kimi_k3_situ_activation_test.pycovers:mathSituAndMulin fp32 and bf16, with inputs scaled ×20 so the caps are exercisedThe parity test pulls only the
SituAndMulclass out of the reference viaast. Because it doesn't import the reference module, it leaves nosys.modules/sys.path/transformersside effects. It skips when torch or the reference checkout is missing. pyink and pylint are clean apart from existing R0917 warnings.Known limitations
Stack: merge order PR0, PR1, PR2..PR7, PR8, PR9, PR10 (#6014) (this is PR2).
Checklist
Before submitting this PR, please make sure (put X in square brackets):
gemini-reviewlabel.