[GGUF FE] Stateful conversion, op-translator fixes and test infrastructure - #37435
Open
mvafin wants to merge 3 commits into
Open
[GGUF FE] Stateful conversion, op-translator fixes and test infrastructure#37435mvafin wants to merge 3 commits into
mvafin wants to merge 3 commits into
Conversation
mvafin
force-pushed
the
mvafin/gguf/frontend-groundwork
branch
from
August 13, 2026 22:30
60b5d2a to
d1449d7
Compare
mvafin
force-pushed
the
mvafin/gguf/frontend-groundwork
branch
2 times, most recently
from
August 13, 2026 22:55
1b47fa0 to
31e09f7
Compare
mvafin
marked this pull request as ready for review
August 13, 2026 22:59
…cture Groundwork on the existing frontend, all of it reachable through the GgmlOvDecoder path that master already ships. The native .gguf builder is a separate change and does not appear here. MakeStateful. The frontend always converts to a stateless graph -- every KV cache an explicit Parameter/Result pair, as optimum-intel exports -- and being stateful is the consumer's choice, registered as a DecoderTransformationExtension so it runs ahead of the built-in stateless lowering. The pass also takes over beam_idx: it is a beam-search index into an OpenVINO state with no ggml counterpart, so declaring it in a decoder would leave a consumer-less input on the stateless graph. Op translators. Keep the output port when handing a value between translators (taking .get_node_shared_ptr() silently resolved to output 0, which throws for multi-output ops such as TopK); keep the static head layout in permute op_case 4; drop the builder-only op_case numbering from RESHAPE and VIEW; give each attention Transpose its own order constant; make the graph valid under both the SDPA and PagedAttention layouts by deriving the leading dims rather than pinning them; share the TopK-indices construction between ARGSORT and TOP_K, and let TOP_K tolerate a dynamic k instead of throwing. Quantization. Support the Q2_0 (ternary) type used by the Bonsai family. Decoder interface. Drop get_model_weights, which nothing calls. Tests. Add an op-coverage gate so a newly registered op cannot ship without a conversion test, and check the activation translators against captured output from real ggml rather than a numpy reimplementation of the formula -- a numpy oracle can only confirm the formula the author already guessed, which is how the GELU_QUICK error survived. CI. Add a GGUF_FE component so frontend changes scope their own jobs. ov_gguf_frontend_tests: 137/137. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR evolves the existing GGUF frontend conversion pipeline to support a “stateless-by-default” graph with an opt-in stateful lowering, improves correctness/robustness of multiple ggml op translators (multi-output handling, layout-polymorphic reshapes/permutes, shared TopK-indices helper, additional quant type), and significantly strengthens the GGUF FE test suite and CI scoping.
Changes:
- Introduces
ov::frontend::gguf::pass::MakeStatefulas aDecoderTransformationExtensionto convert KV-cacheSetRowspatterns into OpenVINO state (and managebeam_idx). - Fixes/extends multiple GGUF op translators (TopK/Argsort indices, VIEW/RESHAPE/PERMUTE layout handling, RoPE fusion-friendly decomposition, MoE
MUL_MAT_IDlowering) and adds Q2_0 quant support plus weight-path refactoring. - Expands GGUF FE tests (op-kernel vs real-ggml references, op-coverage gate) and adds a dedicated
GGUF_FECI component/test entry.
Reviewed changes
Copilot reviewed 40 out of 47 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/frontends/gguf/tests/test_ops.cpp | Extends op tests (unary inputs/domains, real-ggml oracle cases, additional op-case coverage for permute/view/reshape/topk). |
| src/frontends/gguf/tests/test_op_coverage.cpp | Adds a runtime op-coverage gate ensuring every registered op has a conversion test. |
| src/frontends/gguf/tests/test_extensions.cpp | Adds DecoderTransformationExtension coverage (stateless vs stateful cache behavior, IO contract checks). |
| src/frontends/gguf/tests/test_dequant_vs_ggml.cpp | Updates comments/formatting and adds Q2_0 dequant test coverage. |
| src/frontends/gguf/tests/op_test_utils.hpp | Tracks converted op types for coverage gating; includes cleanups. |
| src/frontends/gguf/tests/CMakeLists.txt | Updates GGUF test target sources/includes and installs reference data (includes an accidental duplicate source entry). |
| src/frontends/gguf/src/utils.hpp | Adds helpers (permute, make_topk_indices, etc.) and extends make_sin_cos signature. |
| src/frontends/gguf/src/utils.cpp | Implements new helpers, extends RoPE sin/cos building, and clarifies VIEW offset units. |
| src/frontends/gguf/src/translate_session.cpp | Updates translate flow to use model decoder, support extra inputs, weight detection, transformation ordering, and post-passes. |
| src/frontends/gguf/src/quant/weights.hpp | Adds zero-point type selector and extracted-weight split helpers for builder interop. |
| src/frontends/gguf/src/quant/weights.cpp | Implements zero-point type selection and weight extraction/splitting utilities; refines error strings. |
| src/frontends/gguf/src/quant/gguf.hpp | Adjusts quant API comments and adds get_shape helper declaration. |
| src/frontends/gguf/src/quant/gguf_quants.cpp | Documents/implements Q2_0 fill behavior details. |
| src/frontends/gguf/src/pass/make_stateful.cpp | Implements MakeStateful model pass converting cache SetRows into Variable/ReadValue/Assign state. |
| src/frontends/gguf/src/op/weight.cpp | Supports weight leaves from both ingest paths (pre-extracted tensors vs raw ggml bytes). |
| src/frontends/gguf/src/op/view.cpp | Unifies VIEW case handling across ingest paths and adds builder-only case 104 reshape-to-reference behavior. |
| src/frontends/gguf/src/op/top_k.cpp | Makes TOP_K tolerate dynamic k and shares TopK-indices construction helper. |
| src/frontends/gguf/src/op/set_rows.cpp | Uses context.get_output_type() and reshapes with special_zero for layout polymorphism. |
| src/frontends/gguf/src/op/rope.cpp | Improves layout-polymorphic reshapes and switches NEOX to decomposition pattern suitable for RoPE fusion. |
| src/frontends/gguf/src/op/rms_norm.cpp | Include/order adjustments; no semantic change intended. |
| src/frontends/gguf/src/op/reshape.cpp | Unifies op_case numbering and makes key reshapes layout-polymorphic via special_zero. |
| src/frontends/gguf/src/op/permute.cpp | Uses get_op_case(), preserves static head dims under dynamic reshape patterns, clarifies view offset units. |
| src/frontends/gguf/src/op/mul_mat_id.cpp | Adds GatherMatmul-based lowering for constant expert weights with generic fallback path. |
| src/frontends/gguf/src/op/glu_geglu.cpp | Ensures GEGLU uses TANH GELU approximation to match ggml semantics. |
| src/frontends/gguf/src/op/get_rows.cpp | Adds MoE gating gather case via GatherElements and uses context.get_output_type(). |
| src/frontends/gguf/src/op/flash_attn_ext.cpp | Supports ingest-path layout differences, optional attention sinks, and soft-cap decomposition. |
| src/frontends/gguf/src/op/argsort.cpp | Reuses shared TopK-indices helper. |
| src/frontends/gguf/src/op_table.hpp | Reorganizes translator declarations and adds missing unary/math ops in the table header. |
| src/frontends/gguf/src/op_table.cpp | Clarifies GGML_OP_NONE weight semantics for both ingest paths and minor ordering tweaks. |
| src/frontends/gguf/src/node_context.hpp | Adds accessors (get_input_names, get_op_case, get_output_type) and clarifies view offsets in elements. |
| src/frontends/gguf/src/input_model.hpp | Exposes underlying model decoder via get_model_decoder(). |
| src/frontends/gguf/src/input_model.cpp | Implements get_model_decoder(). |
| src/frontends/gguf/src/frontend.cpp | Updates discoverability docs and supported_impl behavior for decoder-based loading. |
| src/frontends/gguf/src/CMakeLists.txt | Updates GGUF frontend build description/comments. |
| src/frontends/gguf/include/openvino/frontend/gguf/make_stateful.hpp | Adds public header for MakeStateful pass. |
| src/frontends/gguf/include/openvino/frontend/gguf/frontend.hpp | Documents DecoderTransformationExtension + (currently mismatched) file-path ingest support. |
| src/frontends/gguf/include/openvino/frontend/gguf/decoder.hpp | Adds optional model-scope accessors (extra inputs) and clarifies decoder contract. |
| .github/workflows/ubuntu_24.yml | Enables gguf fixture generation in Ubuntu 24 workflow inputs. |
| .github/workflows/job_cxx_unit_tests.yml | Gates GGUF frontend tests on GGUF_FE affected-components flag. |
| .github/coverage/tests_cpp.yml | Adds ov_gguf_frontend_tests to coverage test list. |
| .github/components.yml | Adds GGUF_FE component definition and CPU build/revalidate mapping. |
Suppressed comments (1)
src/frontends/gguf/include/openvino/frontend/gguf/frontend.hpp:63
- [MEDIUM] The
load_impldocstring describes a second ingest path via a.gguffile path, butload_implcurrently errors out unlessvariants[0]is astd::shared_ptr<GgufDecoder>. Please update the documentation (or add the file-path ingest in the same PR) so the public header matches behavior.
/// \brief Load the input model, from either of the frontend's two ingest paths.
/// \param variants A single element, holding either:
/// - a `std::shared_ptr<GgufDecoder>` — a decoder supplied by a direct linker, wrapping
/// an already-built ggml graph (the llama.cpp cgraph path); or
/// - a path to a `.gguf` file — parsed here, with the transformer graph built
/// per-architecture by the native builder.
/// Both yield a GgufDecoder, so conversion past this point is identical.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Windows CI (MSVC, CMAKE_COMPILE_WARNING_AS_ERROR=ON) failed with warnings-as-errors: - utils.cpp: non_cont_dim() narrowed size_t to int on initialization (C4267). Cast explicitly instead. - test_ops.cpp: SoftMaxAlibi's slope computation narrowed std::pow's double result to float on initialization (C4244). Cast explicitly. Also fixed two correctness bugs found while reviewing the PR: - flash_attn_ext.cpp: the attention-sink reshape hardcoded axis 2 for the sink logit's target shape, but the head axis depends on op_case: it's 1 for the (currently the only reachable) llama.cpp cgraph layout and 2 for the ggml-natural layout. Use head_axis instead, and add a regression test (FlashAttnExtWithSinksCgraphLayout) that fails with a shape-mismatch exception without the fix. - translate_session.cpp: translate_graph pushed every get_model_inputs() entry's dynamic_pointer_cast<Parameter> into params unconditionally, dropping the previous null guard. decoder.hpp explicitly still allows a decoder to fold non-Parameter auxiliary inputs into get_model_inputs() (rather than splitting them into the new get_model_extra_inputs()), which is what the currently-shipping llama.cpp cgraph decoder does; that shape of input crashed conversion. Restored the guard and added a regression test (GetModelInputsToleratesNonParameterEntries). Verified locally: built openvino_gguf_frontend + ov_gguf_frontend_tests against the CPU plugin; all 139/139 tests pass (137 existing + 2 new). Confirmed each new test fails without its corresponding fix. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- tests/CMakeLists.txt: remove the duplicate op/top_k.cpp entry in FRONTEND_SRCS. - frontend.hpp: supported_impl/load_impl docstrings claimed .gguf file-path loading (magic-byte sniffing, native builder dispatch), but this PR's frontend.cpp only recognizes a std::shared_ptr<GgufDecoder> -- file-path loading is part of the separate, stacked native .gguf builder PR. Reworded both docstrings to match the actual implementation. Verified locally: rebuilt ov_gguf_frontend_tests against the CPU plugin after both changes; all 139/139 tests still pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Details:
GgmlOvDecoderpath master already ships; the native.ggufbuilder is a separate, stacked change (see below) and does not appear in this PR.MakeStatefulpass. The frontend always converts to a stateless graph — every KV cache an explicit Parameter/Result pair, mirroring how optimum-intel exports — and statefulness becomes the consumer's choice, registered as aDecoderTransformationExtensionso it runs ahead of the built-in stateless lowering. The pass also takes ownership ofbeam_idx: it is a beam-search index into an OpenVINO state with no ggml counterpart, so declaring it in a decoder would leave a consumer-less input on the stateless graph..get_node_shared_ptr()silently resolves to output 0, which throws for multi-output ops such asTopK. Keep the static head layout inpermuteop_case 4; drop builder-only op_case numbering fromRESHAPE/VIEW; give each attentionTransposeits own order constant; make the graph valid under both the SDPA and PagedAttention layouts by deriving the leading dims instead of pinning them; share the TopK-indices construction betweenARGSORTandTOP_K, and letTOP_Ktolerate a dynamickrather than throwing.Q2_0(ternary) type used by the Bonsai family.get_model_weights, which nothing calls.GELU_QUICKerror survived.ov_gguf_frontend_tests: 137/137.GGUF_FEcomponent so frontend changes scope their own jobs.Note
This is part 1 of 2. Part 2 (#37421) adds the native
.ggufgraph builder and stacks on this branch. Splitting it this way keeps the cross-cutting pieces — the decoder interface change and the pass architecture that the llama.cppggml-openvinobackend integrates against — reviewable without ~4.8k lines of builder alongside.Tickets:
AI Assistance: