Repository navigation
QVAC-25280: add Ternary Bonsai core and CPU support - #283
Conversation
Review StatusCurrent Status: ❌ PENDING Pending reviews: Needs 1 Management or Team Lead, and 1 more from Management, Team Lead, or Member. |
2d2f4f5 to
74e530e
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
github-actions
Bot
added
documentation
f90c72d to
2afd46e
Compare
amangupta-tether
left a comment
amangupta-tether
left a comment
There was a problem hiding this comment.
I found three issues in the Hadamard metadata and embedding paths.
| std::vector<std::string> inverse_names; | ||
| ml.get_arr("prism.hadamard.inverse_weight_names", inverse_names, false); | ||
| for (const auto & name : inverse_names) { | ||
| // the graph applies the inverse only to the token-embedding lookup; any |
There was a problem hiding this comment.
[P1] Handle tied output weights when the token embedding is folded. When output.weight is absent, Qwen3 and Qwen3.5 reuse token_embd.weight as the output matrix (see src/models/qwen35.cpp:44-52). This branch restores only the looked-up input rows; the output matmul still uses the folded matrix without a matching activation transform, so logits are incorrect for a tied model. Please reject this metadata combination or transform the output path as well.
| @@ -2325,6 +2377,17 @@ ggml_tensor * llm_graph_context::build_inp_embd(ggml_tensor * tok_embd) const { | |||
|
|
|||
| cur = ggml_get_rows(ctx0, tok_embd, inp->tokens); | |||
|
|
|||
| // Restore folded embedding rows to the primal basis. | |||
| if (hadamard_inverses) { | |||
There was a problem hiding this comment.
[P2] Apply the inverse transform to MTP embedding lookups. Qwen3.5 MTP calls ggml_get_rows directly on model.tok_embd when its own embedding table is absent (src/models/qwen35.cpp:520-524; the same pattern appears in qwen35moe and qwen3next). That path skips this inverse, so MTP receives folded coordinates and the graph verifier can reject the lookup. Please apply the inverse on those paths or reject this combination.
| f"Hadamard tensor {record['name']!r} maps to {mapped!r}, which is not on a " | ||
| "verified Hadamard-aware matmul path" | ||
| ) | ||
| weight_names.append(mapped) |
There was a problem hiding this comment.
[P2] Resolve Hadamard names after gate/up expert fusion. With --fuse-gate-up-exps, modify_tensors emits only blk.*.ffn_gate_up_exps.weight, but the manifest still records separate blk.*.ffn_gate_exps.weight and blk.*.ffn_up_exps.weight here. The loader then fails to find those named tensors. Please record the fused tensor name or reject the flag when a manifest requests these weights.
yingying0906
left a comment
yingying0906
left a comment
There was a problem hiding this comment.
Found a few issues on the Hadamard load path and the CPU kernels, details inline.
9866576 to
55cc12a
Compare
d288a5f to
268acd2
Compare
* QVAC-25280 ggml: add PQ2_0 and PTQ1_0 types * QVAC-25280 llama: apply Bonsai Hadamard metadata * QVAC-25280 tests: cover Bonsai backend operations * QVAC-25280 conversion: narrow Hadamard manifest types * QVAC-25280 conversion: satisfy nested function style * tests : relax WebGPU signed FWHT tolerance * ggml-cpu : optimize PTQ1_0 with x86 VNNI * tests : cover wide FWHT kernels * ggml-cpu : use ARM32 table lookup wrapper * bonsai : address review feedback * conversion : fix flake8 spacing * convert : reject duplicate fused Hadamard records * metal : reject unsupported core matmuls * model : defer Hadamard init for split loads * ggml : add AVX2 ternary dot products (cherry picked from commit e45fc65)
Overview
Adds the core runtime and CPU support for the published
prism-ml/Ternary-Bonsai-2-27B-ggufformats:PQ2_0: tensor type 142, 34 bytes per 128 weights, file type 141PTQ1_0: tensor type 143, 28 bytes per 128 weights, file type 143This is PR 1 of 4 in the Ternary Bonsai stack.
Verification
126on Apple M5 CPU and Ryzen CPURequirements