Skip to content

Allow float bias with Conv QDQ node group#481

Open
qti-ashimaj wants to merge 1 commit into
mainfrom
dev/qti-ashimaj/convnodeselector
Open

Allow float bias with Conv QDQ node group#481
qti-ashimaj wants to merge 1 commit into
mainfrom
dev/qti-ashimaj/convnodeselector

Conversation

@qti-ashimaj

@qti-ashimaj qti-ashimaj commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator

Description

Allow bias to be float when selecting QDQ node group for Conv Op

Motivation and Context

If the bias is float (non-quantized) while the weights and activations are quantized, the Q and DQ nodes aren't getting properly partitioned into 1 group along with the conv op. As a result, graph finalization error occurs. This change will allow the node group to be formed even when bias is float.

The bias quantization is handled in #483

@qti-chuteng

Copy link
Copy Markdown
Collaborator

🟠 Major

[M-1] onnxruntime/core/providers/qnn/qnn_ep_utils.cc → OrtConvNodeGroupSelector::Check()

The semantic change to allow dq_nodes.size() == 2 (float-bias Conv) is a functional bug fix with a clear regression risk, but the PR diff contains no unit test. Per team convention, a bug-fix PR must include a reproducing test (CI red), a post-fix verification (CI green), and a kill-test fence — temporarily reverting the fix to confirm the test fails. Without a test, a future accidental revert to the default num_dq_inputs = -1 would go undetected.

Please add two test cases in onnxruntime/test/providers/qnn/:

  1. Float bias: Conv + DQ(input) + DQ(weight), bias as a float initializer (no DQ node) → assert Check() returns true
  2. INT32 bias regression: Conv + DQ(input) + DQ(weight) + DQ(bias, INT32) → confirm existing behavior is unchanged

Record the kill-test result in the PR description's Validation section.


🔵 Minor

[N-1] onnxruntime/core/providers/qnn/qnn_ep_utils.cc:L853

After passing static_cast<int>(dq_nodes.size()) as num_dq_inputs, the "reject if DQ count mismatches" guard inside CheckQDQNodes becomes a no-op (the passed value always equals the actual value). If Check() is ever called with dq_nodes.size() < 2, CheckQDQNodes can no longer intercept it, and the subsequent dq_nodes[0] at L858 / dq_nodes[1] at L859 would be out-of-bounds.

  if (!CheckQDQNodes(graph, ort_api, node, redundant_clip_node, dq_nodes, q_nodes, static_cast<int>(dq_nodes.size()))) {
    return false;
  }

  if (dq_nodes.size() < 2) {
    return false;  // Conv requires at least DQ nodes for input and weight
  }

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.

2 participants