feat(cpu): add Ling-3.0-tiny mobile CPU support - #699
Conversation
📝 WalkthroughWalkthroughThe PR adds Ling-3.0-tiny support for ARM64 CPU inference. It introduces the model, tokenizer, Kimi Delta Attention, causal depthwise convolution, checkpoint validation, runtime integration, examples, and CPU tests. ChangesLing-3 CPU runtime
Estimated code review effort: 5 (Critical) | ~120 minutes Mergeability Score: 🟡 Moderate · up to This PR adds Ling-3.0-tiny mobile support and new stateful CPU operations, but the current head still has concrete merge-readiness issues: a validation-passing routing configuration can trigger undefined behavior during expert selection, and checkpoint validation can accept inconsistent shard assignments that may load incorrect model data. Merge should wait for these correctness issues to be fixed or explicitly accepted. Sequence Diagram(s)sequenceDiagram
participant Runner
participant Ling3Tokenizer
participant Ling3ForCausalLM
participant CPUBackend
Runner->>Ling3Tokenizer: tokenize prompt
Runner->>Ling3ForCausalLM: start generation
Ling3ForCausalLM->>CPUBackend: execute MLA, KDA, and convolution operations
CPUBackend-->>Ling3ForCausalLM: outputs and updated states
Ling3ForCausalLM-->>Runner: streamed token output and run status
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (9)
tests/cpu/Ling3TokenizerTest.cpp (1)
30-41: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the
LING3_OFFICIAL_TOKENIZERrequirement so the target is not silently empty.Both tests in this file skip when
LING3_OFFICIAL_TOKENIZERis unset. In a default CI run,Mllm-Test-Ling3-Tokenizertherefore passes without asserting anything. Record the required environment variable in the Ling-3 documentation, so maintainers know how to enable real coverage.#!/bin/bash # Description: Check whether the tokenizer test environment variables are documented. rg -n 'LING3_OFFICIAL_TOKENIZER|LING3_RUNTIME_CONFIG' --iglob '*.md' --iglob '*.txt' --iglob '*.yml' --iglob '*.yaml'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/cpu/Ling3TokenizerTest.cpp` around lines 30 - 41, Document the required LING3_OFFICIAL_TOKENIZER environment variable in the Ling-3 documentation, including how maintainers should set it to enable MatchesOfficialByteBPEAndNFCVectors and RendersOfficialSingleTurnThinkingTemplates. Ensure the documentation is discoverable in a supported Markdown, text, or YAML file and mention any related LING3_RUNTIME_CONFIG requirement if applicable.mllm/compile/jit/interpreter/AopsFromJson.hpp (1)
45-46: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRename implementation-reserved helper names.
__kimiDeltaAttentionFromJsonand__causalDepthwiseConv1dFromJsonuse C++ implementation-reserved identifiers. Rename them without a leading double underscore and update their declarations, definitions, and dispatch calls.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mllm/compile/jit/interpreter/AopsFromJson.hpp` around lines 45 - 46, Rename __kimiDeltaAttentionFromJson and __causalDepthwiseConv1dFromJson to equivalent names without leading double underscores, updating their declarations, definitions, and all dispatch call sites consistently.Source: Coding guidelines
mllm/models/ling3/tokenization_ling3.hpp (3)
54-68: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument that cluster composition covers accent marks only.
The cluster scan extends only while
unicode_cpt_flags(...).is_accent_markis true. Combining marks that are not classified as accent marks, for example Devanagari or Hebrew marks, stay decomposed, so the result is not full NFC. State this limitation in the comment so a later reader does not assume complete NFC coverage.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mllm/models/ling3/tokenization_ling3.hpp` around lines 54 - 68, Update the comment above the cluster scan to explicitly state that composition is limited to combining marks classified as accent marks by unicode_cpt_flags, and that other marks such as Devanagari or Hebrew remain decomposed rather than receiving full NFC normalization.
195-204: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winThe constructor parses
tokenizer.jsontwice and does not handle parse failures.
bpe_.initFromSentencePieceJson(file_path)already reads the file, and lines 199-201 read and parse the whole document again. For a 157k-entry vocabulary this doubles peak memory during startup, which matters on mobile targets.nlohmann::json::parsealso throwsnlohmann::json::parse_errorhere, which escapes as a message without Ling-3 context.Consider parsing once and passing the parsed document to the BPE loader, or wrap the parse in a
tryblock and rethrowstd::invalid_argumentwith the file path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mllm/models/ling3/tokenization_ling3.hpp` around lines 195 - 204, Update the Ling-3 tokenizer constructor around bpe_.initFromSentencePieceJson and tokenizer_json to avoid parsing tokenizer.json twice by parsing once and reusing the parsed document with the BPE loader if supported. Handle nlohmann::json::parse_error and rethrow std::invalid_argument containing clear Ling-3 context and the file path, while preserving the NFC normalizer validation.
260-268: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReuse
convert2IdsinconvertMessage.Both functions allocate a
[1, N]int64 tensor and fill it withbpe_._lookup_vocab. The only difference issetMemType(kExtraInput)versussetMemType(kNormal). Extract one helper that takes the memory type, then call it from both places.Also applies to: 297-304
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mllm/models/ling3/tokenization_ling3.hpp` around lines 260 - 268, Refactor the duplicated tensor allocation and vocabulary lookup logic in convert2Ids and convertMessage into one helper that accepts the desired memory type. Have both methods delegate to this helper, preserving kExtraInput for convert2Ids and kNormal for convertMessage while keeping the existing tensor shape, type, name, and token-to-ID behavior.mllm/models/ling3/modeling_ling3.hpp (3)
466-473: 🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoffPadding values to
qk_diminflates the KV cache.
padLing3ValuesForCacheexpands each value vector from 128 to 192 elements so the value cache matches the key dimension innn::StaticCache. At 2,048 tokens, 6 MLA layers, and 16 heads in FP32, the padding costs about 50% extra value-cache memory. Ifnn::StaticCachecan accept separate key and value head dimensions, prefer that path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mllm/models/ling3/modeling_ling3.hpp` around lines 466 - 473, Update the KV-cache path around padLing3ValuesForCache and cache->updateKVCache so keys use qk_dim_ while values retain their native value dimension, avoiding padded value storage. Use separate key/value head dimensions if nn::StaticCache supports them, and adjust the subsequent attention matmul to consume the unpadded cached values.
316-334: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winClarify why
nn::Conv1Dlayers are registered but never executed.
q_conv1d_,k_conv1d_, andv_conv1d_exist only to own the depthwise weights thatq_causal_conv_,k_causal_conv_, andv_causal_conv_consume. A reader can assume thenn::Conv1Dlayers run. Add a short comment that states they are weight holders for the checkpoint parameter names.As per coding guidelines: "Add comments for complex algorithms or non-obvious logic."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mllm/models/ling3/modeling_ling3.hpp` around lines 316 - 334, Add a concise comment immediately before the q_conv1d_, k_conv1d_, and v_conv1d_ registrations explaining that these Conv1D modules only hold depthwise weights for the corresponding causal convolution modules and preserve checkpoint parameter names; do not alter their execution or registration.Source: Coding guidelines
283-293: 🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoffPer-token, per-expert dispatch limits prefill throughput.
moeInfercalls one expert MLP per (token, route) pair, so prefill performstokens * top_k_linear calls at M=1. The comment explains the reason, and the behavior is correct. If prefill latency becomes a problem, group tokens by expert id and run one call per expert with the gathered rows.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mllm/models/ling3/modeling_ling3.hpp` around lines 283 - 293, Optimize moeInfer’s per-token expert dispatch by grouping token rows by expert ID, gathering each expert’s assigned inputs, and invoking each expert once on the grouped batch instead of once per (token, route) pair. Scatter the resulting rows back into routed_output using the original token/route positions, preserving id_values ordering, topk_weights aggregation, and output dtype behavior.mllm/models/ling3/configuration_ling3.hpp (1)
127-139: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCache
numFullAttentionLayers()or document the linear cost.
numFullAttentionLayers()iterates all layers on every call.hasOfficialLing3TinyArchitecturecalls it twice per invocation, andLing3ForCausalLMplus the runner call it again. The cost is small at 24 layers, so this is only a clarity item. Consider computing the counts once in the constructor.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mllm/models/ling3/configuration_ling3.hpp` around lines 127 - 139, Cache the result of numFullAttentionLayers() during configuration initialization and have subsequent callers reuse that stored count, including numKDALayers() and architecture checks such as hasOfficialLing3TinyArchitecture. Alternatively, document that the method intentionally performs a linear scan if caching cannot fit the existing design.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@examples/ling3/README.md`:
- Around line 12-36: Update the Ling-3 README command sequence to establish the
required working directory before using relative paths: add an explicit cd
examples/ling3 step before the validation, conversion, and smoke-run commands,
or convert every referenced script, configuration, and runner path to
repository-relative paths.
In `@examples/ling3/validate_checkpoint.py`:
- Around line 200-209: Update read_safetensors_header to determine the file’s
remaining size after reading the 8-byte length, define or reuse a maximum
metadata-size limit, and reject header_length when it exceeds either the
remaining file size or that limit before calling file.read(header_length).
Preserve the existing truncated-header and JSON parsing behavior for valid
lengths.
- Around line 19-61: Add "hidden_act": "silu" to OFFICIAL_CONTRACT and extend
the negative validation tests to mutate hidden_act to a different activation,
asserting validate_config rejects the altered configuration.
- Around line 212-221: Update validate_shards to verify each tensor’s weight_map
assignment while scanning the header: require weight_map[name] to equal the
current shard_name before recording its descriptor, while preserving duplicate
detection. Add a regression test using two valid tensor names whose index
entries are swapped between shards and assert validation fails.
In `@mllm/core/aops/CausalDepthwiseConv1DOp.hpp`:
- Around line 11-25: Document the public causal-convolution contract at
mllm/core/aops/CausalDepthwiseConv1DOp.hpp:11-25, including [B,S,C], [C,1,K],
and [B,C,K-1] layouts, both outputs, state_inplace ownership/behavior, and
std::invalid_argument conditions. Document constructor behavior and the
two-output forward contract at mllm/nn/layers/CausalDepthwiseConv1D.hpp:11-17.
Document contiguous FP32 tensor requirements and the factory purpose at
mllm/backends/cpu/ops/CausalDepthwiseConv1DOp.hpp:10-23; these sites all require
comments or docstrings for their public APIs.
Apply the same fix in `@mllm/core/aops/KimiDeltaAttentionOp.hpp` around lines 11 -
30: Documents CPU restrictions and factory purpose.
Apply the same fix in `@mllm/nn/Functional.hpp` around lines 171 - 176: Documents
serialized keys, values, returns, and errors.
In `@mllm/core/aops/KimiDeltaAttentionOp.hpp`:
- Line 27: Update the const accessor KimiDeltaAttentionOp::options() to add the
[[nodiscard]] attribute, preserving its return type and existing behavior.
Apply the same fix in `@mllm/core/aops/CausalDepthwiseConv1DOp.hpp` at line 25:
Apply the same attribute to the causal-convolution accessor.
In `@mllm/models/ling3/configuration_ling3.hpp`:
- Around line 63-65: Update the Ling3 configuration constructor’s token-loading
logic alongside pad_token_id and eos_token_id to read end_of_text_token_id and
bos_token_id from the JSON config, preserving the existing defaults only when
the corresponding keys are absent if that matches the surrounding configuration
behavior. Ensure Ling3ForCausalLM uses the configured end_of_text_token_id
rather than a hardcoded default.
In `@mllm/models/ling3/modeling_ling3.hpp`:
- Around line 216-229: Update Ling3Config::validate() in configuration_ling3.hpp
to reject configurations where num_experts_per_tok exceeds topk_group *
(num_experts / n_group), preventing routed-expert selection from calling
std::partial_sort beyond ranked_experts. Add the validation alongside the
existing expert-count and group constraints, using the specified
invalid-argument error context.
In `@mllm/models/ling3/tokenization_ling3.hpp`:
- Around line 14-21: Add the direct standard-library includes <unordered_set>
and <cstdint> to the header containing the tokenization declarations, alongside
the existing includes. This must directly support
std::unordered_set<std::wstring> in added_tokens_ and uint32_t usages without
relying on transitive includes.
In `@tests/cpu/CMakeLists.txt`:
- Around line 13-28: Register Mllm-Test-Ling3-KDA, Mllm-Test-Ling3-Config,
Mllm-Test-Ling3-RoPE, and Mllm-Test-Ling3-Tokenizer with CTest using the
existing test-registration convention, then update the CI workflow to invoke
CTest so these registered tests run in CI.
---
Nitpick comments:
In `@mllm/compile/jit/interpreter/AopsFromJson.hpp`:
- Around line 45-46: Rename __kimiDeltaAttentionFromJson and
__causalDepthwiseConv1dFromJson to equivalent names without leading double
underscores, updating their declarations, definitions, and all dispatch call
sites consistently.
In `@mllm/models/ling3/configuration_ling3.hpp`:
- Around line 127-139: Cache the result of numFullAttentionLayers() during
configuration initialization and have subsequent callers reuse that stored
count, including numKDALayers() and architecture checks such as
hasOfficialLing3TinyArchitecture. Alternatively, document that the method
intentionally performs a linear scan if caching cannot fit the existing design.
In `@mllm/models/ling3/modeling_ling3.hpp`:
- Around line 466-473: Update the KV-cache path around padLing3ValuesForCache
and cache->updateKVCache so keys use qk_dim_ while values retain their native
value dimension, avoiding padded value storage. Use separate key/value head
dimensions if nn::StaticCache supports them, and adjust the subsequent attention
matmul to consume the unpadded cached values.
- Around line 316-334: Add a concise comment immediately before the q_conv1d_,
k_conv1d_, and v_conv1d_ registrations explaining that these Conv1D modules only
hold depthwise weights for the corresponding causal convolution modules and
preserve checkpoint parameter names; do not alter their execution or
registration.
- Around line 283-293: Optimize moeInfer’s per-token expert dispatch by grouping
token rows by expert ID, gathering each expert’s assigned inputs, and invoking
each expert once on the grouped batch instead of once per (token, route) pair.
Scatter the resulting rows back into routed_output using the original
token/route positions, preserving id_values ordering, topk_weights aggregation,
and output dtype behavior.
In `@mllm/models/ling3/tokenization_ling3.hpp`:
- Around line 54-68: Update the comment above the cluster scan to explicitly
state that composition is limited to combining marks classified as accent marks
by unicode_cpt_flags, and that other marks such as Devanagari or Hebrew remain
decomposed rather than receiving full NFC normalization.
- Around line 195-204: Update the Ling-3 tokenizer constructor around
bpe_.initFromSentencePieceJson and tokenizer_json to avoid parsing
tokenizer.json twice by parsing once and reusing the parsed document with the
BPE loader if supported. Handle nlohmann::json::parse_error and rethrow
std::invalid_argument containing clear Ling-3 context and the file path, while
preserving the NFC normalizer validation.
- Around line 260-268: Refactor the duplicated tensor allocation and vocabulary
lookup logic in convert2Ids and convertMessage into one helper that accepts the
desired memory type. Have both methods delegate to this helper, preserving
kExtraInput for convert2Ids and kNormal for convertMessage while keeping the
existing tensor shape, type, name, and token-to-ID behavior.
In `@tests/cpu/Ling3TokenizerTest.cpp`:
- Around line 30-41: Document the required LING3_OFFICIAL_TOKENIZER environment
variable in the Ling-3 documentation, including how maintainers should set it to
enable MatchesOfficialByteBPEAndNFCVectors and
RendersOfficialSingleTurnThinkingTemplates. Ensure the documentation is
discoverable in a supported Markdown, text, or YAML file and mention any related
LING3_RUNTIME_CONFIG requirement if applicable.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 64faf2a4-85de-4f5e-9a69-933bfaafff29
⛔ Files ignored due to path filters (1)
bench_assets/ling3_tiny_architecture.pngis excluded by!**/*.png
📒 Files selected for processing (49)
README-ZH.mdREADME.mdexamples/CMakeLists.txtexamples/ling3/CMakeLists.txtexamples/ling3/README.mdexamples/ling3/config_tiny_w4a32_kai.jsonexamples/ling3/main.cppexamples/ling3/quant_cfg_tiny_w4a32_kai.jsonexamples/ling3/test_validators.pyexamples/ling3/validate_checkpoint.pyexamples/ling3/validate_converted_model.pymllm/backends/cpu/CPUBackend.cppmllm/backends/cpu/kernels/common/kda/kimi_delta_attention.cppmllm/backends/cpu/kernels/common/kda/kimi_delta_attention.hppmllm/backends/cpu/kernels/common/paged_attn/arch.hppmllm/backends/cpu/ops/CausalDepthwiseConv1DOp.cppmllm/backends/cpu/ops/CausalDepthwiseConv1DOp.hppmllm/backends/cpu/ops/KimiDeltaAttentionOp.cppmllm/backends/cpu/ops/KimiDeltaAttentionOp.hppmllm/compile/ir/GeneratedRTTIKind.hppmllm/compile/ir/NodeRTTIClassOfImpl.hppmllm/compile/ir/linalg/Op.cppmllm/compile/ir/linalg/Op.hppmllm/compile/ir/rtti_kind_gen.pymllm/compile/jit/binary/LinalgIRSerialization.cppmllm/compile/jit/binary/LinalgIRSerialization.hppmllm/compile/jit/interpreter/AopsFromJson.cppmllm/compile/jit/interpreter/AopsFromJson.hppmllm/core/OpTypes.hppmllm/core/aops/CausalDepthwiseConv1DOp.cppmllm/core/aops/CausalDepthwiseConv1DOp.hppmllm/core/aops/KimiDeltaAttentionOp.cppmllm/core/aops/KimiDeltaAttentionOp.hppmllm/models/ling3/configuration_ling3.hppmllm/models/ling3/modeling_ling3.hppmllm/models/ling3/tokenization_ling3.hppmllm/nn/Functional.cppmllm/nn/Functional.hppmllm/nn/Nn.hppmllm/nn/layers/CausalDepthwiseConv1D.cppmllm/nn/layers/CausalDepthwiseConv1D.hppmllm/nn/layers/KimiDeltaAttention.cppmllm/nn/layers/KimiDeltaAttention.hpptests/cpu/CMakeLists.txttests/cpu/KaiW4A32PackTest.cpptests/cpu/Ling3ConfigTest.cpptests/cpu/Ling3KDATest.cpptests/cpu/Ling3RoPETest.cpptests/cpu/Ling3TokenizerTest.cpp
| Validate the source checkpoint before conversion: | ||
|
|
||
| ```bash | ||
| python3 validate_checkpoint.py /path/to/Ling-3.0-tiny \ | ||
| --observed-revision a2ee06c0f2de5b171701aee7f73f70a1da75483b | ||
| ``` | ||
|
|
||
| Convert with the repository V2 converter and | ||
| `quant_cfg_tiny_w4a32_kai.json`, using model name `Ling-3.0-tiny`, then seal | ||
| the output descriptor table: | ||
|
|
||
| ```bash | ||
| python3 validate_converted_model.py /path/to/Ling-3.0-tiny.mllm \ | ||
| /path/to/Ling-3.0-tiny | ||
| ``` | ||
|
|
||
| Run one deterministic smoke request: | ||
|
|
||
| ```bash | ||
| ./mllm-ling3-runner \ | ||
| --model_path /path/to/Ling-3.0-tiny.mllm \ | ||
| --tokenizer_path /path/to/Ling-3.0-tiny/tokenizer.json \ | ||
| --config_path config_tiny_w4a32_kai.json \ | ||
| --prompt '你好，请用一句话介绍你自己。' \ | ||
| --max_new_tokens 8 --print_token_ids |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
State the required working directory for these commands.
The commands reference validate_checkpoint.py, validate_converted_model.py, and config_tiny_w4a32_kai.json with relative paths. They fail when users run them from the repository root.
Add an explicit cd examples/ling3 step, or make all script, configuration, and runner paths repository-relative.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/ling3/README.md` around lines 12 - 36, Update the Ling-3 README
command sequence to establish the required working directory before using
relative paths: add an explicit cd examples/ling3 step before the validation,
conversion, and smoke-run commands, or convert every referenced script,
configuration, and runner path to repository-relative paths.
| OFFICIAL_CONTRACT = { | ||
| "architectures": ["BailingMoeV3ForCausalLM"], | ||
| "model_type": "bailing_hybrid", | ||
| "hidden_size": 1536, | ||
| "intermediate_size": 4608, | ||
| "num_hidden_layers": 24, | ||
| "num_attention_heads": 16, | ||
| "num_key_value_heads": 16, | ||
| "head_dim": 128, | ||
| "vocab_size": 157184, | ||
| "max_position_embeddings": 131072, | ||
| "rms_norm_eps": 1e-6, | ||
| "rope_theta": 6000000, | ||
| "layer_group_size": 4, | ||
| "short_conv_kernel_size": 4, | ||
| "no_kda_lora": True, | ||
| "kda_safe_gate": True, | ||
| "kda_lower_bound": -5, | ||
| "q_lora_rank": 256, | ||
| "kv_lora_rank": 512, | ||
| "qk_rope_head_dim": 64, | ||
| "qk_nope_head_dim": 128, | ||
| "qk_head_dim": 192, | ||
| "v_head_dim": 128, | ||
| "rope_interleave": True, | ||
| "gated_attention_proj_granularity_type": "head_wise", | ||
| "num_experts": 128, | ||
| "num_shared_experts": 1, | ||
| "num_experts_per_tok": 8, | ||
| "n_group": 8, | ||
| "topk_group": 4, | ||
| "moe_intermediate_size": 512, | ||
| "moe_shared_expert_intermediate_size": 512, | ||
| "first_k_dense_replace": 1, | ||
| "routed_scaling_factor": 2.5, | ||
| "scoring_func": "sigmoid", | ||
| "topk_method": "noaux_tc", | ||
| "moe_router_enable_expert_bias": True, | ||
| "tie_word_embeddings": False, | ||
| "use_qkv_bias": False, | ||
| "pad_token_id": 156892, | ||
| "eos_token_id": 156895, | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Add hidden_act to OFFICIAL_CONTRACT.
examples/ling3/config_tiny_w4a32_kai.json declares "hidden_act": "silu", but validate_config does not validate it. An altered source or runtime configuration can therefore pass this claimed contract validation with a different activation.
Add the field and add a negative test that changes hidden_act.
Proposed fix
"rope_theta": 6000000,
+ "hidden_act": "silu",
"layer_group_size": 4,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/ling3/validate_checkpoint.py` around lines 19 - 61, Add
"hidden_act": "silu" to OFFICIAL_CONTRACT and extend the negative validation
tests to mutate hidden_act to a different activation, asserting validate_config
rejects the altered configuration.
| def read_safetensors_header(path: Path) -> dict: | ||
| with path.open("rb") as file: | ||
| raw_length = file.read(8) | ||
| if len(raw_length) != 8: | ||
| raise AssertionError(f"Truncated safetensors header: {path}") | ||
| header_length = struct.unpack("<Q", raw_length)[0] | ||
| raw_header = file.read(header_length) | ||
| if len(raw_header) != header_length: | ||
| raise AssertionError(f"Truncated safetensors metadata: {path}") | ||
| return json.loads(raw_header) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Bound the safetensors header length before reading it.
Line 205 accepts an input-controlled unsigned 64-bit length. Line 206 can then request an excessive allocation for a malformed checkpoint. Reject lengths larger than the remaining file size and a defined maximum metadata size before the read.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/ling3/validate_checkpoint.py` around lines 200 - 209, Update
read_safetensors_header to determine the file’s remaining size after reading the
8-byte length, define or reuse a maximum metadata-size limit, and reject
header_length when it exceeds either the remaining file size or that limit
before calling file.read(header_length). Preserve the existing truncated-header
and JSON parsing behavior for valid lengths.
| def validate_shards(checkpoint: Path, weight_map: dict[str, str], shapes: dict[str, list[int]]) -> None: | ||
| actual: dict[str, tuple[str, list[int]]] = {} | ||
| for shard_name in sorted(set(weight_map.values())): | ||
| header = read_safetensors_header(checkpoint / shard_name) | ||
| for name, descriptor in header.items(): | ||
| if name == "__metadata__": | ||
| continue | ||
| if name in actual: | ||
| raise AssertionError(f"Duplicate tensor across shards: {name}") | ||
| actual[name] = (descriptor["dtype"], descriptor["shape"]) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Validate every weight_map shard assignment.
The validator stores descriptors only by tensor name. A checkpoint index that swaps two valid tensor names between shards passes the tensor-set and shape checks, even though its index points each loader to the wrong shard.
Require weight_map[name] == shard_name while scanning each shard. Add a regression test with two swapped index entries.
Proposed fix
header = read_safetensors_header(checkpoint / shard_name)
for name, descriptor in header.items():
if name == "__metadata__":
continue
+ if weight_map.get(name) != shard_name:
+ raise AssertionError(f"{name}: index maps to {weight_map.get(name)!r}, found in {shard_name!r}")
if name in actual:
raise AssertionError(f"Duplicate tensor across shards: {name}")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def validate_shards(checkpoint: Path, weight_map: dict[str, str], shapes: dict[str, list[int]]) -> None: | |
| actual: dict[str, tuple[str, list[int]]] = {} | |
| for shard_name in sorted(set(weight_map.values())): | |
| header = read_safetensors_header(checkpoint / shard_name) | |
| for name, descriptor in header.items(): | |
| if name == "__metadata__": | |
| continue | |
| if name in actual: | |
| raise AssertionError(f"Duplicate tensor across shards: {name}") | |
| actual[name] = (descriptor["dtype"], descriptor["shape"]) | |
| def validate_shards(checkpoint: Path, weight_map: dict[str, str], shapes: dict[str, list[int]]) -> None: | |
| actual: dict[str, tuple[str, list[int]]] = {} | |
| for shard_name in sorted(set(weight_map.values())): | |
| header = read_safetensors_header(checkpoint / shard_name) | |
| for name, descriptor in header.items(): | |
| if name == "__metadata__": | |
| continue | |
| if weight_map.get(name) != shard_name: | |
| raise AssertionError(f"{name}: index maps to {weight_map.get(name)!r}, found in {shard_name!r}") | |
| if name in actual: | |
| raise AssertionError(f"Duplicate tensor across shards: {name}") | |
| actual[name] = (descriptor["dtype"], descriptor["shape"]) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/ling3/validate_checkpoint.py` around lines 212 - 221, Update
validate_shards to verify each tensor’s weight_map assignment while scanning the
header: require weight_map[name] to equal the current shard_name before
recording its descriptor, while preserving duplicate detection. Add a regression
test using two valid tensor names whose index entries are swapped between shards
and assert validation fails.
| struct CausalDepthwiseConv1DOpOptions : public BaseOpOptions<CausalDepthwiseConv1DOpOptions> { | ||
| bool state_inplace = false; | ||
| }; | ||
|
|
||
| class CausalDepthwiseConv1DOp : public BaseOp { | ||
| public: | ||
| explicit CausalDepthwiseConv1DOp(const CausalDepthwiseConv1DOpOptions& options); | ||
|
|
||
| void load(const ParameterFile::ptr_t& ploader) override; | ||
| void trace(void* trace_context, const std::vector<Tensor>& inputs, std::vector<Tensor>& outputs) override; | ||
| void forward(const std::vector<Tensor>& inputs, std::vector<Tensor>& outputs) override; | ||
| void reshape(const std::vector<Tensor>& inputs, std::vector<Tensor>& outputs) override; | ||
| void setup(const std::vector<Tensor>& inputs, std::vector<Tensor>& outputs) override; | ||
|
|
||
| inline const CausalDepthwiseConv1DOpOptions& options() const { return options_; } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Document the public stateful-operation APIs. Add clear documentation for tensor layouts, input and output ordering, state ownership, state_inplace aliasing behavior, constructor options, CPU restrictions, serialization fields, valid ranges, and validation errors across the new causal-convolution and Kimi Delta Attention declarations. Cover the corresponding layer, functional, backend, kernel, and serialization declarations so callers can use the contracts without reading implementation code.
📍 Affects 3 files
mllm/core/aops/CausalDepthwiseConv1DOp.hpp#L11-L25(this comment)mllm/core/aops/KimiDeltaAttentionOp.hpp#L11-L30mllm/nn/Functional.hpp#L171-L176
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mllm/core/aops/CausalDepthwiseConv1DOp.hpp` around lines 11 - 25, Document
the public causal-convolution contract at
mllm/core/aops/CausalDepthwiseConv1DOp.hpp:11-25, including [B,S,C], [C,1,K],
and [B,C,K-1] layouts, both outputs, state_inplace ownership/behavior, and
std::invalid_argument conditions. Document constructor behavior and the
two-output forward contract at mllm/nn/layers/CausalDepthwiseConv1D.hpp:11-17.
Document contiguous FP32 tensor requirements and the factory purpose at
mllm/backends/cpu/ops/CausalDepthwiseConv1DOp.hpp:10-23; these sites all require
comments or docstrings for their public APIs.
Apply the same fix in `@mllm/core/aops/KimiDeltaAttentionOp.hpp` around lines 11 -
30: Documents CPU restrictions and factory purpose.
Apply the same fix in `@mllm/nn/Functional.hpp` around lines 171 - 176: Documents
serialized keys, values, returns, and errors.
Source: Coding guidelines
| void reshape(const std::vector<Tensor>& inputs, std::vector<Tensor>& outputs) override; | ||
| void setup(const std::vector<Tensor>& inputs, std::vector<Tensor>& outputs) override; | ||
|
|
||
| inline const KimiDeltaAttentionOpOptions& options() const { return options_; } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Mark both public options() accessors as [[nodiscard]]. Apply this to the Kimi Delta Attention and causal-convolution option accessors. The Kimi accessor is relevant to the repository's clang-tidy configuration, which treats warnings as errors, and applying the attribute consistently prevents silently discarded configuration reads.
📍 Affects 2 files
mllm/core/aops/KimiDeltaAttentionOp.hpp#L27-L27(this comment)mllm/core/aops/CausalDepthwiseConv1DOp.hpp#L25-L25
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mllm/core/aops/KimiDeltaAttentionOp.hpp` at line 27, Update the const
accessor KimiDeltaAttentionOp::options() to add the [[nodiscard]] attribute,
preserving its return type and existing behavior.
Apply the same fix in `@mllm/core/aops/CausalDepthwiseConv1DOp.hpp` at line 25:
Apply the same attribute to the causal-convolution accessor.
Source: Linters/SAST tools
| pad_token_id = config.at("pad_token_id"); | ||
| eos_token_id = config.at("eos_token_id"); | ||
| max_cache_length = config.value("max_cache_length", max_cache_length); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Load end_of_text_token_id and bos_token_id from the config file.
The constructor reads pad_token_id and eos_token_id from JSON, but leaves end_of_text_token_id and bos_token_id at their hardcoded defaults. Ling3ForCausalLM uses end_of_text_token_id as an additional stop token (modeling_ling3.hpp Line 616). hasOfficialLing3TinyArchitecture does not verify either value, so a config that declares different ids is accepted while generation still stops on the hardcoded id.
🛠️ Proposed fix
pad_token_id = config.at("pad_token_id");
eos_token_id = config.at("eos_token_id");
+ end_of_text_token_id = config.value("end_of_text_token_id", end_of_text_token_id);
+ bos_token_id = config.value("bos_token_id", bos_token_id);
max_cache_length = config.value("max_cache_length", max_cache_length);Also applies to: 119-123
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mllm/models/ling3/configuration_ling3.hpp` around lines 63 - 65, Update the
Ling3 configuration constructor’s token-loading logic alongside pad_token_id and
eos_token_id to read end_of_text_token_id and bos_token_id from the JSON config,
preserving the existing defaults only when the corresponding keys are absent if
that matches the surrounding configuration behavior. Ensure Ling3ForCausalLM
uses the configured end_of_text_token_id rather than a hardcoded default.
| ranked_experts.clear(); | ||
| for (int group_index = 0; group_index < top_groups_; ++group_index) { | ||
| const int group = ranked_groups[group_index].second; | ||
| for (int offset = 0; offset < experts_per_group; ++offset) { | ||
| const int expert = group * experts_per_group + offset; | ||
| ranked_experts.emplace_back(scores[expert] + bias_values[expert], expert); | ||
| } | ||
| } | ||
| std::partial_sort(ranked_experts.begin(), ranked_experts.begin() + top_k_, ranked_experts.end(), | ||
| [](const auto& lhs, const auto& rhs) { | ||
| return lhs.first != rhs.first ? lhs.first > rhs.first : lhs.second < rhs.second; | ||
| }); | ||
| float score_sum = 1.0e-20F; | ||
| for (int route = 0; route < top_k_; ++route) { score_sum += scores[ranked_experts[route].second]; } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Guard the routed-expert candidate count before std::partial_sort.
ranked_experts holds top_groups_ * experts_per_group entries. std::partial_sort(begin, begin + top_k_, end) has undefined behavior when top_k_ exceeds that count. Ling3Config::validate() checks num_experts_per_tok <= num_experts and topk_group <= n_group, but it does not check num_experts_per_tok <= topk_group * (num_experts / n_group). A config with n_group = 8, topk_group = 1, and num_experts_per_tok = 32 passes validation and then reads past the end of the vector.
Add the missing constraint in Ling3Config::validate() in mllm/models/ling3/configuration_ling3.hpp.
🛡️ Proposed fix in `configuration_ling3.hpp` validate()
if (num_experts_per_tok > topk_group * (num_experts / n_group)) {
throw std::invalid_argument("Ling-3 top-k exceeds the routable experts in the selected groups");
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mllm/models/ling3/modeling_ling3.hpp` around lines 216 - 229, Update
Ling3Config::validate() in configuration_ling3.hpp to reject configurations
where num_experts_per_tok exceeds topk_group * (num_experts / n_group),
preventing routed-expert selection from calling std::partial_sort beyond
ranked_experts. Add the validation alongside the existing expert-count and group
constraints, using the specified invalid-argument error context.
| #include <algorithm> | ||
| #include <cwctype> | ||
| #include <fstream> | ||
| #include <stdexcept> | ||
| #include <string> | ||
| #include <string_view> | ||
| #include <unordered_map> | ||
| #include <vector> |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Add the missing <unordered_set> and <cstdint> includes.
Line 311 declares std::unordered_set<std::wstring> added_tokens_, and lines 27 and 39 use uint32_t. Neither header is included, so the file compiles only when another header pulls them in transitively. This breaks on other standard libraries.
🛠️ Proposed fix
`#include` <algorithm>
+#include <cstdint>
`#include` <cwctype>
`#include` <fstream>
`#include` <stdexcept>
`#include` <string>
`#include` <string_view>
`#include` <unordered_map>
+#include <unordered_set>
`#include` <vector>As per coding guidelines: "Ensure code is portable across supported platforms (e.g., Linux, Windows) unless explicitly platform-specific."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #include <algorithm> | |
| #include <cwctype> | |
| #include <fstream> | |
| #include <stdexcept> | |
| #include <string> | |
| #include <string_view> | |
| #include <unordered_map> | |
| #include <vector> | |
| #include <algorithm> | |
| #include <cstdint> | |
| #include <cwctype> | |
| #include <fstream> | |
| #include <stdexcept> | |
| #include <string> | |
| #include <string_view> | |
| #include <unordered_map> | |
| #include <unordered_set> | |
| #include <vector> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mllm/models/ling3/tokenization_ling3.hpp` around lines 14 - 21, Add the
direct standard-library includes <unordered_set> and <cstdint> to the header
containing the tokenization declarations, alongside the existing includes. This
must directly support std::unordered_set<std::wstring> in added_tokens_ and
uint32_t usages without relying on transitive includes.
Source: Coding guidelines
| add_executable(Mllm-Test-Ling3-KDA Ling3KDATest.cpp) | ||
| target_link_libraries(Mllm-Test-Ling3-KDA PRIVATE gtest_main MllmCPUBackend) | ||
| target_include_directories(Mllm-Test-Ling3-KDA PRIVATE ${MLLM_INCLUDE_DIR}) | ||
|
|
||
| add_executable(Mllm-Test-Ling3-Config Ling3ConfigTest.cpp) | ||
| target_link_libraries(Mllm-Test-Ling3-Config PRIVATE gtest_main MllmCPUBackend) | ||
| target_include_directories(Mllm-Test-Ling3-Config PRIVATE ${MLLM_INCLUDE_DIR}) | ||
| target_compile_definitions(Mllm-Test-Ling3-Config PRIVATE LING3_EXAMPLE_DIR="${CMAKE_CURRENT_SOURCE_DIR}/../../examples/ling3") | ||
|
|
||
| add_executable(Mllm-Test-Ling3-RoPE Ling3RoPETest.cpp) | ||
| target_link_libraries(Mllm-Test-Ling3-RoPE PRIVATE gtest_main MllmCPUBackend) | ||
| target_include_directories(Mllm-Test-Ling3-RoPE PRIVATE ${MLLM_INCLUDE_DIR}) | ||
|
|
||
| add_executable(Mllm-Test-Ling3-Tokenizer Ling3TokenizerTest.cpp) | ||
| target_link_libraries(Mllm-Test-Ling3-Tokenizer PRIVATE gtest_main MllmCPUBackend) | ||
| target_include_directories(Mllm-Test-Ling3-Tokenizer PRIVATE ${MLLM_INCLUDE_DIR}) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check how CPU tests are registered with CTest.
fd -t f 'CMakeLists.txt' tests | xargs -I{} sh -c 'echo "== {} =="; cat -n {}'
rg -n 'add_test|gtest_discover_tests|enable_testing|include\(GoogleTest\)' --iglob '*CMakeLists.txt' --iglob '*.cmake'Repository: UbiquitousLearning/mllm
Length of output: 10968
🏁 Script executed:
#!/bin/bash
printf '%s\n' '== CMake test setup =='
fd -t f 'CMakeLists.txt' . | xargs rg -n -H 'enable_testing|include\(GoogleTest\)|gtest_discover_tests|add_test|add_subdirectory\(tests\)'
printf '%s\n' '== CI test commands =='
fd -t f .github | xargs -r rg -n -H 'ctest|CTest|Mllm-Test|tests|cmake --build|cmake --install'Repository: UbiquitousLearning/mllm
Length of output: 1050
🏁 Script executed:
#!/bin/bash
printf '%s\n' '== Top-level test configuration =='
sed -n '260,290p' CMakeLists.txt
printf '%s\n' '== GitHub workflow files =='
fd -t f .github | sort
printf '%s\n' '== Test-related workflow and script references =='
rg -n -H -i 'ctest|test$|tests|cmake' .github scripts 2>/dev/null | head -200Repository: UbiquitousLearning/mllm
Length of output: 4967
Make the Ling3 tests run in CI.
The four targets have no add_test or gtest_discover_tests registration. Only Mllm-Test-Qwen35-Multimodal is registered in tests/cpu/CMakeLists.txt. The repository workflows also do not invoke CTest. Register the four targets and add a CI test step that runs them.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/cpu/CMakeLists.txt` around lines 13 - 28, Register Mllm-Test-Ling3-KDA,
Mllm-Test-Ling3-Config, Mllm-Test-Ling3-RoPE, and Mllm-Test-Ling3-Tokenizer with
CTest using the existing test-registration convention, then update the CI
workflow to invoke CTest so these registered tests run in CI.
1 participant
Summary
Adds end-to-end, text-only mobile CPU support for the pinned official
inclusionAI/Ling-3.0-tinycheckpoint on ARM64 macOS and Android.
The central change is not just a model wrapper: Ling's two reusable stateful
primitives now follow mllm's complete operation stack instead of calling CPU
kernels from model code.
Ling-3.0-tiny architecture
Ling-3.0-tiny is a 24-layer hybrid decoder built from six identical attention
groups:
This 3:1 schedule is the key mobile trade-off:
KimiDeltaAttentionoperation plus a formal stateful causal-convolution operation. KDA adds a CPU kernel; convolution dispatches to the existing optimized GDN convolution kernel.noaux_tcrouting, expert bias, normalized routed weights, and existing KAI Linear/MLP execution.The supported mobile envelope intentionally bounds the official architecture
to batch 1 and a 2,048-token cache, with FP32 recurrent/KV state and KAI-packed
INT4 Linear weights with dynamic INT8 activation quantization.
Architecture diagram and exact tiny-checkpoint geometry
Standard mllm abstraction
Both new primitives use the same dependency direction:
The operation contract makes state transitions explicit: previous recurrent or
convolution state is an input and updated state is an output. The public
Functional default keeps the input state immutable. Ling opts into the
serialized
state_inplacemode only for module-owned request state, avoiding afull state copy while retaining an explicit output state. Every new request
resets KDA recurrence, convolution history, and MLA cache state.
The model graph no longer includes or calls CPU backend kernels. The remaining
raw tensor work is checkpoint-specific model orchestration: adjacent-pair RoPE
layout preparation, grouped top-k routing/sparse dispatch, position IDs, and
module-owned state allocation. Reusable convolution and recurrent-attention
semantics are below the formal operation boundary.
Validation
The high-signal results are:
arm64-v8a, ARMv8.2 + FP16/DotProd/I8MM, and no-ffast-math; AArch64 ELF/BuildID/NEEDED audits pass.The Mac completion begins:
The OnePlus 13T correctness run reported 7.45 tok/s prefill and 8.43 tok/s
decode (118.6 ms/token). This is a single-run diagnostic attached to the
correctness replay, not a controlled performance or regression benchmark.
Mac and Android generation are integration checks, not perplexity or task
quality measurements.
Focused test matrix
The 14 stateful-operation tests cover independent references, prefill versus
tokenwise execution, Ling production geometries, invalid contracts, immutable
and in-place state modes, request isolation/reset, existing optimized
convolution parity, and linalg IR serialization/interpreter reconstruction.
Why the GDN regression scope stays small
This PR does not modify the optimized
depthwiseCausalConvF32implementation or the Qwen3.5 model path. It adds astandard backend operation that calls that existing kernel, then validates the
unchanged incumbent with its 7 GDN and 6 convolution tests on both Mac and
Android. This isolates the abstraction repair from a risky GDN kernel rewrite.
Convert, audit, and run
The runner emits
LING3_RUN_START, one record per generated token ID, andLING3_RUN_OK prompt_tokens=49 generated_tokens=64.Correctness and performance boundaries
-ffast-math; relaxed gate math changes generated tokens.M=1contract.Grouped expert-prefill optimization requires a separate numerical and
performance campaign.
claimed to be bitwise identical to each other. Each platform is stable
against its own pre-refactor 64-token sequence.
percentile statistics. No performance promotion or regression claim is made.
Supported scope and limits
Supported: Ling-3.0-tiny text generation, batch 1, cache length up to 2,048,
ARM64 macOS and Android, W4A32 KAI Linear execution.
Not claimed: other Ling checkpoints, vision/audio/MTP heads, batch sizes
greater than 1, 128K/1M-token mobile execution, perplexity, task-quality,
sustained-performance, energy, or formal memory benchmarks.
Summary by CodeRabbit