[ONNX] Enhance GroupQueryAttention to handle null inputs and improve … by sgbihu · Pull Request #36842 · openvinotoolkit/openvino

sgbihu · 2026-07-13T08:35:14Z

Details:

The ONNX's default unused input is NullNode to take the input position and let the user to get input node with index.
But the OV doesn't support NullNode, that makes the compile error.

Solution:

Add the missing input index in the operator
Check the index before using it.

Tickets:

CVS-190171

AI Assistance:

AI assistance used: no / yes
If yes, summarize how AI was used and what human validation was performed (build/tests/manual checks).

Copilot

Pull request overview

Note

Copilot couldn't run its full agentic review because no GitHub Actions runner was available. Make sure your repository has a runner available to run Copilot's review, or add a copilot-setup-steps.yml file specifying one with the runs-on attribute. See the docs for more details.

This PR updates the ONNX com.microsoft.GroupQueryAttention frontend import and the internal OpenVINO GroupQueryAttention op to handle ONNX NullNode optional inputs by filtering them out while preserving enough positional information to map ONNX input positions to actual OpenVINO input indices.

Changes:

Filter NullNode inputs during ONNX import and record their original ONNX positions.
Extend internal::GroupQueryAttention to store null_onnx_input_positions and expose helpers to map ONNX positions to OV indices.
Update the GQA decomposition pass to use mapped indices for several optional inputs (scales, RoPE caches, position ids, mask).

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
src/frontends/onnx/frontend/src/op/com.microsoft/group_query_attention.cpp	Filters out `NullNode` inputs and passes null-position metadata into the internal op.
src/core/src/op/group_query_attention.cpp	Stores null-position metadata and uses it to validate quantization scale inputs.
src/core/dev_api/openvino/op/group_query_attention.hpp	Adds ONNX input position enum + helpers (`has_input`, `get_input_index`) and persists null-position metadata.
src/common/transformations/src/transformations/op_conversions/group_query_attention_decomposition.cpp	Uses mapped indices for some optional inputs rather than hard-coded indices.

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

src/common/transformations/src/transformations/op_conversions/group_query_attention_decomposition.cpp:115

[BLOCKER] is_null lambda is now unused (no remaining call sites), which is likely to trigger -Wunused-variable under the project’s warning settings and break the build. Please remove it (or use it).

    auto is_null = [](const ov::Output<ov::Node>& output) {
        return output.get_node_shared_ptr()->description() == "NullNode";
    };

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

…index mapping

…g and improve clarity

…nput validation

…l input positions and improve type checks

…d error handling for missing inputs

…nd enhance input checks for non-null tensors

…index type conversion

…o validate_and_infer_types for improved consistency and error handling

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

CuriousPanCake · 2026-07-14T12:05:09Z

I guess, it would've been good to add some tests

CuriousPanCake · 2026-07-14T12:28:32Z

        return m_kv_cache_bit_width != 0 && m_k_quant_type != "NONE";
    }

+    // Get original input positions that are null/missing and therefore omitted from OV inputs.


Remove this comment.

Suggested change

// Get original input positions that are null/missing and therefore omitted from OV inputs.

CuriousPanCake · 2026-07-14T12:28:39Z

+        return static_cast<int64_t>(get_input_size() + m_null_input_positions.size());
+    }
+
+    // Check if a specific original input position exists and was not filtered as NullNode.


Remove this comment.

Suggested change

// Check if a specific original input position exists and was not filtered as NullNode.

CuriousPanCake · 2026-07-14T13:29:13Z

    }

+    // Get original input positions that are null/missing and therefore omitted from OV inputs.
+    const std::vector<int64_t>& get_null_input_positions() const {


I'm a afraid the new functionality introduces a risk of getting silent and non-silent errors when using the traditional ov::Node's API for inputs acquisition.

Say, we've compacted from a following ONNX representation:
0:Q 1:K 2:V 3:past_key 4:past_value 5:seqlens_k 6:total_seq_len 7:NULL 8:NULL 9:position_ids 10:attention_mask into
0:Q 1:K 2:V 3:past_key 4:past_value 5:seqlens_k 6:total_seq_len 7:position_ids 8:attention_mask
It's okay, if we use new functions for obtaining indices and getters for inputs as indices will be translated, but it's not clear what input is at what index. In the example above attention_mask has input 10, but for ONNX's
0:Q 1:K 2:V 3:past_key 4:past_value 5:seqlens_k 6:total_seq_len 7:cos_cache 8:sin_cache 9:NULL 10:attention_mask
translated into OV's
0:Q 1:K 2:V 3:past_key 4:past_value 5:seqlens_k 6:total_seq_len 7:cos_cache 8:sin_cache 9:attention_mask
it's index 9 now. So there's no clarity when we use ov::Node::input_value() and I'm pretty sure it's gonna cause a bug at some point. More than that, handling ops. with variable number of inputs is really complicated.

@mitruska @praasz what do you think?

I understand your concern. But we don't have an operator to keep the index aligned. The NullNode only for frontend. The OV core doesn't has it. If we want to keep the order, we need to add an operator like NullNode. I can change the code if OV has a better way to do this.

I wanna hear other people opinions. I don't really like the suggested solution, but I cannot come with a better one myself.

ankitm3k · 2026-07-14T13:32:45Z

@CuriousPanCake fyi for the latest GQA spec doc - https://github.com/microsoft/onnxruntime/blob/main/docs/ContribOperators.md#com.microsoft.GroupQueryAttention

sgbihu requested a review from Copilot July 13, 2026 08:35

Copilot AI reviewed Jul 13, 2026

View reviewed changes

Comment thread ...n/transformations/src/transformations/op_conversions/group_query_attention_decomposition.cpp Outdated

Comment thread src/core/dev_api/openvino/op/group_query_attention.hpp Outdated

Copilot started reviewing on behalf of sgbihu July 13, 2026 09:02 View session

github-actions Bot added category: Core OpenVINO Core (aka ngraph) category: transformations OpenVINO Runtime library - Transformations category: ONNX FE OpenVINO ONNX FrontEnd labels Jul 13, 2026

sgbihu requested a review from Copilot July 14, 2026 01:17

Copilot started reviewing on behalf of sgbihu July 14, 2026 01:17 View session

sgbihu marked this pull request as ready for review July 14, 2026 01:20

sgbihu requested review from a team as code owners July 14, 2026 01:20

Copilot AI reviewed Jul 14, 2026

View reviewed changes

Comment thread src/core/dev_api/openvino/op/group_query_attention.hpp

Comment thread ...n/transformations/src/transformations/op_conversions/group_query_attention_decomposition.cpp Outdated

Comment thread src/frontends/onnx/frontend/src/op/com.microsoft/group_query_attention.cpp

sgbihu requested a review from Copilot July 14, 2026 02:07

Copilot started reviewing on behalf of sgbihu July 14, 2026 02:08 View session

Copilot AI reviewed Jul 14, 2026

View reviewed changes

Comment thread src/frontends/onnx/frontend/src/op/com.microsoft/group_query_attention.cpp

sgbihu requested a review from Copilot July 14, 2026 02:14

Copilot started reviewing on behalf of sgbihu July 14, 2026 02:15 View session

sgbihu requested a review from ankitm3k July 14, 2026 02:16

Copilot AI reviewed Jul 14, 2026

View reviewed changes

Comment thread src/frontends/onnx/frontend/src/op/com.microsoft/group_query_attention.cpp

sgbihu requested a review from Copilot July 14, 2026 02:22

Copilot started reviewing on behalf of sgbihu July 14, 2026 02:23 View session

Copilot AI reviewed Jul 14, 2026

View reviewed changes

Comment thread src/frontends/onnx/frontend/src/op/com.microsoft/group_query_attention.cpp

Comment thread ...n/transformations/src/transformations/op_conversions/group_query_attention_decomposition.cpp Outdated

sgbihu requested a review from Copilot July 14, 2026 02:53

Copilot started reviewing on behalf of sgbihu July 14, 2026 02:54 View session

Copilot AI reviewed Jul 14, 2026

View reviewed changes

Comment thread src/core/src/op/group_query_attention.cpp

sgbihu requested a review from Copilot July 14, 2026 03:54

Copilot started reviewing on behalf of sgbihu July 14, 2026 03:54 View session

Copilot AI reviewed Jul 14, 2026

View reviewed changes

Comment thread src/core/dev_api/openvino/op/group_query_attention.hpp Outdated

Comment thread src/frontends/onnx/frontend/src/op/com.microsoft/group_query_attention.cpp Outdated

Comment thread src/core/src/op/group_query_attention.cpp Outdated

sgbihu requested a review from a team as a code owner July 14, 2026 04:55

Copilot AI reviewed Jul 14, 2026

View reviewed changes

Comment thread src/plugins/intel_npu/src/plugin/npuw/npuw_transformations/decompose_gqa.cpp Outdated

sgbihu requested a review from Copilot July 14, 2026 05:25

Copilot started reviewing on behalf of sgbihu July 14, 2026 05:25 View session

Copilot AI reviewed Jul 14, 2026

View reviewed changes

Comment thread src/core/dev_api/openvino/op/group_query_attention.hpp

Comment thread src/frontends/onnx/frontend/src/op/com.microsoft/group_query_attention.cpp

sgbihu requested a review from Copilot July 14, 2026 05:42

Copilot started reviewing on behalf of sgbihu July 14, 2026 05:43 View session

Copilot AI reviewed Jul 14, 2026

View reviewed changes

Comment thread src/core/src/op/group_query_attention.cpp

sgbihu added 9 commits July 14, 2026 14:11

[ONNX] Enhance GroupQueryAttention to handle null inputs and improve …

38bd9f1

…index mapping

Refactor GroupQueryAttention to standardize null input position namin…

25f7ae5

…g and improve clarity

Update based on review input

d6b23c2

Refactor GroupQueryAttention to remove null node checks and enhance i…

5abb099

…nput validation

Enhance GroupQueryAttention constructor with input validation for nul…

e9788f8

…l input positions and improve type checks

Refactor GroupQueryAttention input retrieval to enhance validation an…

b0dd271

…d error handling for missing inputs

Refactor GroupQueryAttention validation messages to improve clarity a…

2d86684

…nd enhance input checks for non-null tensors

Fix input value retrieval in GroupQueryAttention by ensuring correct …

76da671

…index type conversion

Refactor GroupQueryAttention to move null input position validation t…

7d6698a

…o validate_and_infer_types for improved consistency and error handling

sgbihu force-pushed the gqa_avoid_nullnode branch from 3551fc7 to 7d6698a Compare July 14, 2026 06:13

sgbihu requested a review from Copilot July 14, 2026 06:16

Copilot started reviewing on behalf of sgbihu July 14, 2026 06:16 View session

Copilot AI reviewed Jul 14, 2026

View reviewed changes

praasz assigned mitruska Jul 14, 2026

CuriousPanCake requested a review from mvafin July 14, 2026 11:46

CuriousPanCake reviewed Jul 14, 2026

View reviewed changes

CuriousPanCake requested changes Jul 14, 2026

View reviewed changes

Delete the comments

2a7d17a

sgbihu requested a review from CuriousPanCake July 16, 2026 02:25

Uh oh!

Conversation

sgbihu commented Jul 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Details:

Solution:

Tickets:

AI Assistance:

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

CuriousPanCake commented Jul 14, 2026

Uh oh!

CuriousPanCake Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

CuriousPanCake Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

CuriousPanCake Jul 14, 2026

sgbihu commented Jul 13, 2026 •

edited

Loading