[GGUF FE] Support Q2_0 quantization and add missing op translators - #37380
Conversation
33089b9 to
c858847
Compare
dcf4df6 to
735dc4f
Compare
There was a problem hiding this comment.
Pull request overview
This PR extends the GGUF frontend to support the Q2_0 (ternary) weight quantization format and adds missing ggml op translators to reduce graph fallbacks, while also simplifying GGML_OP_NORM translation by emitting MVN directly and improving output naming for a view case.
Changes:
- Add
GGUF_TYPE_Q2_0support end-to-end (type enum, parsing, fill/dequant paths, and test coverage). - Add translators + unit tests for
LOG,SIN,COS,TOP_K, and unaryRELU/ELU/GELU_QUICK. - Translate
GGML_OP_NORMtoMVNand fix naming consistency intranslate_viewop_case 3.
Reviewed changes
Copilot reviewed 14 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/frontends/gguf/tests/test_weights.cpp | Adds Q2_0 to the parameterized weight suite and a token-embedding requant regression test. |
| src/frontends/gguf/tests/test_ops.cpp | Adds unary op coverage (relu/elu/gelu_quick/sin/cos) and dedicated tests for LOG and TOP_K; adds dynamic-shape coverage for NORM. |
| src/frontends/gguf/tests/test_dequant_vs_ggml.cpp | Adds Q2_0 type name handling and a bit-exact tolerance for dequant-vs-ggml validation. |
| src/frontends/gguf/tests/CMakeLists.txt | Ensures new op translation sources are built into the test target. |
| src/frontends/gguf/src/quant/weights.cpp | Wires Q2_0 through weight node creation, quant-type name mapping, and zero-point element-type selection. |
| src/frontends/gguf/src/quant/gguf.hpp | Introduces GGUF_TYPE_Q2_0 and declares gguf_fill_q2_0. |
| src/frontends/gguf/src/quant/gguf_quants.cpp | Implements gguf_fill_q2_0 (extract scale + packed u2 codes + constant zp=1). |
| src/frontends/gguf/src/op/view.cpp | Adjusts op_case 3 to apply rename_outputs_with_suffix for consistent output naming. |
| src/frontends/gguf/src/op/unary_gelu.cpp | Adds GGML_UNARY_OP_GELU_QUICK translation via a sigmoid-based approximation. |
| src/frontends/gguf/src/op/unary_elu.cpp | Adds GGML_UNARY_OP_ELU translation as ELU(alpha=1). |
| src/frontends/gguf/src/op/top_k.cpp | Adds GGML_OP_TOP_K translation using ov::op::v11::TopK and returns indices. |
| src/frontends/gguf/src/op/norm.cpp | Replaces manual mean/variance decomposition with a single MVN op. |
| src/frontends/gguf/src/op_table.hpp | Declares new converter entry points (top_k, gelu_quick, elu). |
| src/frontends/gguf/src/op_table.cpp | Registers new ops (log/sin/cos/top_k and unary relu/elu/gelu_quick) in the supported-op map. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
735dc4f to
ec6b161
Compare
GGML_TYPE_Q2_0 (type id 42) is upstream ggml's ternary weight format, used by
the Bonsai family (prism-ml/Ternary-Bonsai-*-gguf) for every matmul weight.
Without it make_weight_node throws "unsupported weight quant type".
The block is |f16 d|u2 qs[64]| (18 bytes / 64 weights) and dequantizes as
(code - 1) * d, so it reuses the existing asymmetric-u2 subgraph already used by
Q2_K -- Multiply(Subtract(Convert(u2, f16), zp), scale) -- with a group size of
64 and a zero-point that is the constant 1. ggml packs the codes 4 per byte
LSB-first, exactly the order an OpenVINO u2 Constant reads, so the code bytes
are copied verbatim instead of being repacked.
The zero-point follows zp_type like the other asymmetric types: u8 on the MatMul
path (it is the exact integer 1, so the CPU plugin can fold the dequant in), f16
on the token_embd / output requant path, whose dequant reads zp as f16.
Tested against real ggml: tests/test_data/q2_0_{qbytes,deq}.npy were produced by
linking llama.cpp's dequantize_row_q2_0. Unlike every other type in that suite
Q2_0 is asserted bit-exact (tolerance 0) -- both sides compute (code - 1) * d
from the same f16 scale and the zero-point of 1 is exact, so no dequant noise is
introduced. GGUFWeightRequant.Q2_0AsTokenEmbd covers the requant path.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ec6b161 to
8dac869
Compare
8dac869 to
b0dcd4e
Compare
…IEW outputs
The llama.cpp ggml-openvino backend gates its op list on what this frontend can
translate -- its supported_unary_ops is exactly {GELU, SILU, TANH, SOFTPLUS,
SIGMOID, EXP, NEG}, mirroring the table here. Anything missing is filtered onto
the CPU, which splits the graph. Add the translators that were already proven in
the .gguf builder branch:
GGML_OP_LOG / SIN / COS -> v0::Log / Sin / Cos via the 1-to-1 helper
GGML_UNARY_OP_RELU -> v0::Relu via the 1-to-1 helper
GGML_UNARY_OP_ELU -> v0::Elu, alpha fixed at 1 (ggml's op_elu is
`x > 0 ? x : expm1(x)` and takes no parameter)
GGML_UNARY_OP_GELU_QUICK -> x * sigmoid(1.702x), matching
ggml_gelu_quick_f32; a different approximation
from GGML_UNARY_OP_GELU, not interchangeable
GGML_OP_TOP_K -> v11::TopK indices, descending, i32, along the
last axis (ggml's cmp_top_k sorts by value >)
TOP_K takes its index element type from the decoder's output_type attribute, as
ARGSORT already does, so an i64 TOP_K output produces an i64 tensor rather than
an i32 one that contradicts the model signature.
Also fix translate_view's op_case 3, the only case that returned its node
without rename_outputs_with_suffix: when such a view is a model output the
Result was fed by a bare "Slice_N" with no trace of the ggml tensor it came
from. Naming only, no structural or numerical change. The rename is skipped when the
view is a pure pass-through (no rank restore, no slice, no reshape): the value is
then still the producer's output, and renaming it would rename a node owned by
another ggml tensor -- a model input, in the reachable case -- with the suffix
compounding, since the helper appends.
Each new op has a unit test against a scalar reference (LOG gets its own inputs
since it is only defined for x > 0, TOP_K checks the index values), plus
TopKIndexTypeFollowsOutput and ViewPassThroughKeepsProducerName for the two
cases above. ov_gguf_frontend_tests: 117/117 pass.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
GGML_OP_NORM is (x - mean) / sqrt(var + eps) over the last axis, which is exactly MVN with normalize_variance and the epsilon inside the sqrt. It was built by hand from ReduceMean/Subtract/Multiply/ReduceMean/Add/Sqrt/Divide. ov::pass::MVNFusion does currently match that decomposition and collapses it back to a single MVN, so today the two forms compile to the same CPU runtime graph (Reduce + Subgraph + RMS). Emitting the op directly removes that dependency: the graph no longer relies on a fusion pattern continuing to match, which any later change to the decomposition or to the pass could silently break, leaving the slow form behind. It also drops 7 nodes to 1, so there is less for the transformation pipeline to walk and fuse. The epsilon is still read without a default, so a decoder that fails to provide it keeps failing loudly rather than silently normalizing with a made-up value, and the axis stays the literal -1 rather than being computed from the rank, so the translator never queries a shape that may be dynamic. Covered by the existing GGUFOps.Norm value test plus a new GGUFOps. NormDynamicShape that runs the same reference through a dynamic token dimension. ov_gguf_frontend_tests: 114/114 pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Several translators took an ov::Output, called get_node_shared_ptr() on it, and
then used the node where an Output was expected. That compiles, because a node
implicitly converts to its output(0), but it silently discards the port: if the
value came from any other output of a multi-output producer, the consumer is
wired to the wrong tensor.
That is reachable today. ARGSORT returns output(1) of a TopK (the indices), and
GGML_OP_TOP_K now does the same, so a translator consuming either one and
round-tripping it through the node would read the values instead.
Affected:
- get_dimensions() took a node, so it measured the shape of output(0). It now
takes an Output. The ShapeOf overload is unchanged.
- translate_soft_max multiplied and measured the node instead of the input.
- translate_argsort measured the node instead of the input.
- translate_rope held its data as a node; it is now an Output (and renamed from
data_node to data), which also makes the element type it reads that of the
actual port.
No functional change for the models exercised today (their SOFT_MAX/ROPE inputs
are single-output producers), so this removes a latent trap rather than fixing an
observed miscompare.
GGUFOps.GetDimensionsKeepsOutputPort pins the utility: it builds a TopK, asks for
the dimensions of output(1), and asserts the ShapeOf is connected to port 1. It
fails if get_dimensions goes back through the node.
ov_gguf_frontend_tests: 115/115 pass.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| num_inputs_check(context, 1, 1); | ||
|
|
||
| auto input = context.get_input(0); | ||
| const int64_t k = context.get_output_shape()[context.get_output_shape().size() - 1].get_length(); |
There was a problem hiding this comment.
What: context.get_output_shape() is constructed twice on one line. More importantly, Dimension::get_length() throws if the output's last (k) dimension is dynamic. This differs from translate_argsort, which derives its length from the input shape at runtime via get_dimensions(...) and tolerates dynamic rank.
Why it matters: If a model ever produces a TOP_K whose extent is not statically known, conversion aborts with a hard throw rather than degrading. It's also a minor readability/efficiency nit.
Fix: Cache the shape in a local; if a dynamic k is reachable, derive it dynamically as ARGSORT does:
| const int64_t k = context.get_output_shape()[context.get_output_shape().size() - 1].get_length(); | |
| const auto& out_ps = context.get_output_shape(); | |
| const int64_t k = out_ps[out_ps.size() - 1].get_length(); |
| auto input = context.get_input(0); | ||
| const int64_t k = context.get_output_shape()[context.get_output_shape().size() - 1].get_length(); | ||
| auto k_node = ov::op::v0::Constant::create(ov::element::i64, ov::Shape{}, {k}); | ||
| auto topk = std::make_shared<ov::op::v11::TopK>(input, |
There was a problem hiding this comment.
[nitpick] Near-duplicate v11::TopK construction across top_k.cpp and argsort.cpp.
translate_top_k (top_k.cpp:25–32) and translate_argsort (argsort.cpp:47–55) both
build a v11::TopK, thread the output_type attribute through as the index element
type, and return topk->output(1) via rename_outputs_with_suffix. Only the axis/k
derivation and the sort Mode differ.
Why it matters: the "indices come from output(1), index type follows the decoder's
output_type" contract is now duplicated in two places. If that contract changes (e.g.
a different port or a default index type), both sites must be kept in sync, and one can
silently drift.
Fix (optional): extract a small shared helper in utils, e.g.
// returns the indices port (output(1)) of a TopK over `axis`
ov::Output<ov::Node> make_topk_indices(const ov::Output<ov::Node>& input,
const ov::Output<ov::Node>& k,
int64_t axis,
ov::op::v11::TopK::Mode mode,
const ov::element::Type& index_type);
and call it from both translators. Non-blocking — the two ops are distinct enough that
keeping them separate is also defensible.
Two review comments from openvinotoolkit#37380 that arrived after it was merged, so they are applied here instead. translate_top_k built context.get_output_shape() twice on one line and called Dimension::get_length() on the last axis unconditionally, which throws if that extent is dynamic. Cache the shape and fall back to reading the extent off the input at runtime, so such a model converts rather than aborting -- ARGSORT already derives its k this way. Both translators also built the same v11::TopK, threaded the decoder's "output_type" through as the index element type, and returned output(1). Move that into a make_topk_indices() helper in utils so the "indices are output(1), index type follows output_type" contract lives in one place; only the axis/k derivation and the sort mode stay at the call sites. ov_gguf_frontend_tests: 138/138, including GGUFOps.TopK, GGUFOps.Argsort and GGUFOps.TopKIndexTypeFollowsOutput. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Details:
GGML_TYPE_Q2_0(type id 42), upstream ggml's ternary weight format, used by the Bonsai family (prism-ml/Ternary-Bonsai-*-gguf) for every matmul weight. Without itmake_weight_nodethrowsunsupported weight quant type. The block is|f16 d|u2 qs[64]|(18 bytes / 64 weights), dequantized as(code - 1) * d, so it reuses the asymmetric-u2subgraph already used by Q2_K with group size 64 and a zero-point of 1. ggml packs the codes 4 per byte LSB-first, the same order an OpenVINOu2Constant reads, so the code bytes are copied verbatim.GGML_OP_LOG/SIN/COS/TOP_KandGGML_UNARY_OP_RELU/ELU/GELU_QUICK. The llama.cpp ggml-openvino backend gates its op list on what this frontend can translate — itssupported_unary_opsis exactly{GELU, SILU, TANH, SOFTPLUS, SIGMOID, EXP, NEG}, mirroring the table here — so anything missing is filtered onto the CPU and splits the graph.MVNforGGML_OP_NORMinstead of the hand-builtReduceMean/Subtract/Multiply/ReduceMean/Add/Sqrt/Dividechain (7 nodes to 1).MVNFusiondoes currently collapse the decomposition back to the same thing, so both forms compile to the same CPU runtime graph today; emitting the op directly removes the dependency on that pattern continuing to match, which a later change could silently break and leave the slow form behind.ov::Output, calledget_node_shared_ptr(), and used the node where anOutputwas expected — which compiles via the implicit conversion tooutput(0)but silently drops the port.ARGSORTreturnsoutput(1)of a TopK and the newTOP_Kdoes too, so this is reachable. Fixed inget_dimensions(it measured output 0's shape),translate_soft_max,translate_argsort,translate_rope, and theviewcase below.translate_viewop_case 3, the only case that returned its node withoutrename_outputs_with_suffix: when such a view is a model output, the Result was fed by a bareSlice_Nwith no trace of the originating ggml tensor. Naming only, no structural or numerical change. The rename is skipped for a pure pass-through view, where the value is still the producer's output and renaming it would rename another ggml tensor's node — reachably a model input.dequantize_row_q2_0and is asserted bit-exact (tolerance0), unlike every other type in that suite; each new op has a unit test against a scalar reference;GGML_OP_NORMgains a dynamic-token-dimension case;GetDimensionsKeepsOutputPortpins the port fix;TopKIndexTypeFollowsOutputandViewPassThroughKeepsProducerNamepin the review fixes.ov_gguf_frontend_tests: 117/117 pass.ov-frontend-swap-llamacpp-syncbranch: Ternary-Bonsai-27B (Q2_0) and Muse-Glimmer-30B both run on the OpenVINO backend and match the CPU backend — perplexity 5.7488 vs 5.7400 and 4.8431 vs 4.8530 (0.15% / 0.20%, versus 0.16% for a Qwen3 control). Q2_0 additionally needsGGML_TYPE_Q2_0in that backend'ssupported_types, which is a llama.cpp-side change and not part of this PR.Tickets:
AI Assistance: