Skip to content

Fix GQA shape inference for shape-derived int32 sequence lengths - #33884

Open
Tianlei Wu (tianleiwu) with Copilot wants to merge 8 commits into
mainfrom
copilot/fix-group-query-attention-load
Open

Tianlei Wu (tianleiwu) with Copilot wants to merge 8 commits into
mainfrom
copilot/fix-group-query-attention-load

Conversation

Copilot AI commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Description

  • Parsing: Read total_sequence_length using the supplied TensorProto’s type, accepting propagated int64 values and int32 constants while preserving single-element validation.
  • Coverage: Add scalar and one-element regressions for constants and static-shape expressions, with and without past caches; verify inferred output dimensions.

Motivation and Context

Shape → Cast(INT32) → Squeeze produces a valid int32 sequence length, but shape propagation exposes its value as an int64 TensorProto. GQA’s unconditional int32 parsing rejects it during session creation with ParseData type mismatch, even when graph optimizations are disabled.

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

Copilot AI and others added 2 commits November 4, 2026 19:45
Co-authored-by: tianleiwu <30328909+tianleiwu@users.noreply.github.com>
Co-authored-by: tianleiwu <30328909+tianleiwu@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix GroupQueryAttention loading issue with static shapes Fix GQA shape inference for shape-derived int32 sequence lengths Nov 4, 2026
@tianleiwu
Tianlei Wu (tianleiwu) marked this pull request as ready for review November 6, 2026 17:35
Copilot AI balanced review requested due to automatic review settings November 6, 2026 17:35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No active review issues are identified for the GroupQueryAttention sequence-length shape-inference change.

No open issues 🎉

What changed in this PR

Fixes GroupQueryAttention shape inference for int32 sequence lengths derived from static shapes, addressing the session-creation failure reported in #33865.

Changes:

  • Parses sequence-length values using the supplied TensorProto type, supporting propagated int64 values and int32 constants while retaining single-element validation.
  • Adds regression coverage for scalar and one-element inputs, with and without past caches, and checks inferred output dimensions.
File Description
onnxruntime/​test/​contrib_ops/​group_query_attention_shape_inference_test.cc Adds constant and shape-derived sequence-length regression cases.
onnxruntime/​core/​graph/​contrib_ops/​bert_defs.cc Selects sequence-length parsing based on the TensorProto data type.

Review effort: Balanced

Retain shape-derived INT64 data handling for INT32 total_sequence_length inputs and its static-shape regressions; incorporate the current-main CI fixes.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@tianleiwu

Tianlei Wu (tianleiwu) commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

Maintenance update at 85f9bf12132941743ed1a75e6d5fb83bd31d95e9: the sequence-length changes, main refresh and reviewer fixes remain intact.

The separately reviewable native/navigation repairs are retained. e27bfcf3e9 completes AsyncWorker before resolve/reject dispatch can release its last reference, preserving once-only completion if dispatch throws. The original header's post-dispatch UAF was reproduced with real pinned Hermes/JSI; the fixed header passed 320 synchronized resolve/reject/cancel/Env-invalidation/dispatch-exception scenarios in each of two independent ASan+LSan processes, 640 total executions. f3b45608a9 gives the actual native Button a unique testID and uses by.id on both platforms without changing accessibility or the full14 assertion. All eight component/navigation cases passed, with all scoped linters passing and no introduced App type diagnostics; the pre-existing MNIST diagnostics were explicitly unchanged.

The old f3 iOS attempt 3 ran zero device cases because cold boot exceeded the original setup deadline, then cleanup/hanging Jest reached the job limit. 85f9bf1213 backports only the verified selected-simulator preparation workflow/docs repair: exact configured simulator selection, targeted reset, boot and app-service readiness. The 240-second Detox deadline, uninstall/reinstall policy, assertions and cleanup policy remain unchanged. All nine protocol cases executed against this branch's actual YAML passed.

The genuine current-head React Native workflow 42604665463 completed successfully. Fresh iOS artifact 13670441653 and Android artifact 13668343373 each contain exactly three real passing MNIST, Basic Types and EPContext cases, no failures/errors/skips, and the unchanged 14/14 checks passed assertion in device logs/traces. These are current85f9 executed simulator/device cases, not superseded f3 retries or zero-case setup reports.

The fresh complete all-20 audit at 2026-11-09T13:20:03Z verified this exact current85f9 head open, non-draft, mergeable, with zero unresolved threads and all current-head checks green. The current device and retained640-execution native proof remain valid. This PR's CI preparation is complete; normal human approval remains external. No tests, tolerances, runtime checks or hardware gates were weakened.

Target the accessible pressable instead of the wrong child text view selected on iOS. Exercise the actual Detox scenario on both platforms and retain the aggregate success assertion.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Run the native completion hook before dispatch can release the final worker reference. Preserve exactly-once completion when dispatch throws, and assert completion at promise settlement in the EPContext bridge regressions.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The iOS Button accessibility label is duplicated on native wrappers. Keep its accessibility label but identify the pressable with a unique testID. Reproduce the duplicate-label failure, verify the actual rendered native identifier and navigation, and preserve the complete 14/14 device assertion.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
iOS device runs have stalled during cold simulator setup before app installation. Reset only the simulator selected by the Detox configuration, then wait for boot and app services without restarting the global CoreSimulator service. Keep app reinstallation, device assertions, and the existing Detox setup deadline unchanged.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

GroupQueryAttention fails to load when total_sequence_length is computed from static shapes (ParseData type mismatch)

3 participants