Skip to content

Add quantize/dequantize_per_channel_group support to QNN backend - #19629

Open
Hyungkeun-Park wants to merge 6 commits into
pytorch:mainfrom
Hyungkeun-Park:feat/qnn-mps-per-channel-group-quantization
Open

Add quantize/dequantize_per_channel_group support to QNN backend#19629
Hyungkeun-Park wants to merge 6 commits into
pytorch:mainfrom
Hyungkeun-Park:feat/qnn-mps-per-channel-group-quantization

Conversation

@Hyungkeun-Park

@Hyungkeun-Park Hyungkeun-Park commented May 18, 2026

Copy link
Copy Markdown
Contributor

Motivation

quantized_decomposed.quantize_per_channel_group and dequantize_per_channel_group are what group-wise (int4) LLM weight quantization lowers to, but the QNN backend did not recognize them, causing two distinct failures:

  1. The ops were decomposed away during torch.export instead of being preserved for the backend.
  2. InsertIOQDQ raised a KeyError when a pre-quantized weight annotated with a dequantize_per_channel_group encoding fed the graph output, because the op was absent from q_dq_map.

This change gives them the same treatment torchao.quantize_affine / dequantize_affine already receive.

Changes

backends/qualcomm/builders/node_visitor.py

  • Register both ops in q_ops / dq_ops.
  • Add both entries to the module-level q_dq_map (this is the dict InsertIOQDQ imports and looks up, so this is what fixes the KeyError).
  • Add the PER_CHANNEL_GROUP_ENCODING set and route it through make_qnn_per_block_config, since QNN models group-wise quantization as a per-block encoding.
  • make_qnn_per_block_config now rejects non-zero zero_points (zero_points / zero_point) with a ValueError: QNN blockwise expansion hardcodes a per-channel offset of 0, so an asymmetric weight would otherwise silently lower to a symmetric encoding.

backends/qualcomm/partition/utils.py

  • Add both ops to get_skip_decomp_table so they survive export and reach the backend unchanged.

backends/qualcomm/partition/qnn_partitioner.py

  • QnnOperatorSupport refuses to delegate per_channel_group q/dq when soc_info.htp_info.htp_arch < HtpArch.V69, mirroring validate_lpbq_support. Pre-quantized weights bypass the quantizer, so without this there is no arch check on this path; blockwise expansion hard-fails on V68. The op falls back to CPU with a warning.

backends/qualcomm/_passes/utils.py

  • Expose the block scale tensor as QCOM_SCALE in get_quant_attrs, which is what make_qnn_per_block_config reads. The remap is gated on the encoding rather than on the presence of a "scales" key, because quantize_per_channel.default also has a "scales" argument but is consumed as a per-channel (not per-block) config.
  • Validate that scales.shape[-1] * group_size matches the weight's input-feature count, so a mismatched checkpoint fails at annotation time instead of producing a silently wrong encoding (QCOM_NUM_BLOCKS_PER_AXIS is inferred from the scale shape downstream).

Testing

pytest backends/qualcomm/tests/test_passes.py -k "insert_io_qdq or per_block or q_dq_map"
  • test_q_dq_map_pins_per_channel_group_pairs pins the q_dq_map entries through to_q_op / to_dq_op: to_q_op(dequantize_per_channel_group) is quantize_per_channel_group, to_dq_op(quantize_per_channel_group) is dequantize_per_channel_group, plus the identity cases. to_dq_op short-circuits for targets already in dq_ops, so this is the only place the cross-pair mapping is exercised; it fails with a self-referential dequantize_per_channel_group entry.
  • test_insert_io_qdq_per_channel_group_dequantizes_output annotates a parameter with a dequantize_per_channel_group encoding and wires it to the graph output. That is the dequantize-before-output branch of InsertIOQDQ (the insert-quantize-after-input branch is skipped for parameters); it asserts a dequantize_per_channel_group node now feeds the output.
  • test_make_qnn_per_block_config_rejects_asymmetric covers the non-zero zero_points rejection.

The encoding gating in get_quant_attrs was checked directly: quantize_per_channel.default no longer gets a QCOM_SCALE entry written, while dequantize_per_channel_group.default does.

Not validated on device: no HTP device is available in this environment, so end-to-end numerics of the per-block lowering (scale reshaping, symmetric encoding) have not been verified on any HTP arch. The partitioner arch gate (< V69 falls back to CPU) and the asymmetric/group_size validations are there so the unverified paths fail loudly or fall back instead of lowering silently wrong.

cc @qti-horodnic @cbilgin @psiddh @cccclai @abhinaykukkadapu

@pytorch-bot

pytorch-bot Bot commented May 18, 2026

Copy link
Copy Markdown

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/19629

Note: Links to docs will display an error until the docs builds have been completed.

⚠️ 15 Awaiting Approval

As of commit c34cfa0 with merge base 4ce2ec2 (image):

AWAITING APPROVAL - The following workflows need approval before CI can run:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label May 18, 2026
@Hyungkeun-Park

Copy link
Copy Markdown
Contributor Author

@pytorchbot label "release notes: qualcomm"

@pytorch-bot pytorch-bot Bot added the release notes: qualcomm Changes to the Qualcomm backend delegate label May 18, 2026
@linux-foundation-easycla

linux-foundation-easycla Bot commented May 26, 2026

Copy link
Copy Markdown

CLA Signed
The committers listed above are authorized under a signed CLA.

  • ✅ login: Hyungkeun-Park-Nota / name: hyungkeun.park (79a29da, d2d5aab)

@nil-is-all nil-is-all added the module: qnn Issues related to Qualcomm's QNN delegate and code under backends/qualcomm/ label May 26, 2026
@nil-is-all

Copy link
Copy Markdown
Contributor

Thanks for the PR. @shewu-quic @haowhsu-quic @winskuo-quic @DannyYuyang-quic could any of you review this?

@github-actions

Copy link
Copy Markdown

Looks like this PR hasn't been updated in a while so we're going to go ahead and mark this as Stale.
Feel free to remove the Stale label if you feel this was a mistake.
If you are unable to remove the Stale label please contact a maintainer in order to do so.
If you want the bot to never mark this PR stale again, add the no-stale label.
Stale pull requests will automatically be closed after 30 days of inactivity.

@github-actions github-actions Bot added the Stale PRs inactive for over 60 days label Jul 26, 2026
@qti-horodnic

Copy link
Copy Markdown
Contributor

@Hyungkeun-Park-Nota

Thanks for this, sorry for the slow review turnaround. This pr is stale against main, a few things to note:

  1. The q_dq_map part targets code that's since changed. The dict at insert_io_qdq.py was moved into builders/node_visitor.py. The current insert_io_qdq.py imports it instead, so there's no class-level copy anymore.

  2. In the current main the two entries need to go in node_visitor.py as the lookup that raises the KeyError resolves against the module-level dict:

target=q_dq_map[n.meta[QCOM_QUANT_ATTRS][QCOM_ENCODING]]

So the rebased change would be adding these to q_dq_map in builders/node_visitor.py like:

exir_ops.edge.quantized_decomposed.quantize_per_channel_group.default: exir_ops.edge.quantized_decomposed.dequantize_per_channel_group.default,
exir_ops.edge.quantized_decomposed.dequantize_per_channel_group.default: exir_ops.edge.quantized_decomposed.dequantize_per_channel_group.default,

It's worth re-checking test_insert_io_qdq_per_channel_group_no_key_error after the rebase since the injected node is a placeholder, is_parameter() may skip the input-Q branch, I'd confirm the test actually exercises the q_dq_map lookup and fails without the fix.

  1. I currently have a pr in internal review which adds torchao affine-op / per-block quant support which touches two of the same spots your pr does:
  • node_visitor.q_dq_map: Adds the per-tensor and per-channel entries. If my pr lands first, your change here becomes exactly the 2-line addition above with no other edits needed to that file.
  • get_quant_attrs() in _passes/utils.py: Adds input_dtype/output_dtype arg-name aliasing adjacent to your scalesQCOM_SCALE remap, so there'll be a conflict. Separately, I'd suggest gating that remap on the encoding target rather than key presence, since quantize_per_channel.default also has a scales arg and would end up with QCOM_SCALE written unintentionally.

Either way works and I don't want to hold you up further. If it's convenient, rebasing my pr merges would shrink this diff and avoid the get_quant_attrs conflict entirely. Your call, but the issues I mentioned above would need addressing regardless.

@nil-is-all nil-is-all removed the Stale PRs inactive for over 60 days label Jul 28, 2026
quantized_decomposed.quantize_per_channel_group and its dequantize
counterpart are what group-wise (int4) LLM weight quantization lowers to,
but the QNN backend did not recognize them: the ops were decomposed away
during export instead of reaching the backend, and InsertIOQDQ raised a
KeyError when a pre-quantized weight annotated with a
dequantize_per_channel_group encoding fed the graph output.

Give them the same treatment torchao.quantize_affine / dequantize_affine
already receive:

- node_visitor: register both ops in q_ops / dq_ops and in q_dq_map, and
  route them through make_qnn_per_block_config via the new
  PER_CHANNEL_GROUP_ENCODING set (QNN models group-wise quantization as a
  per-block encoding).
- partition/utils: keep both ops out of the decomposition table so they
  survive export and reach the backend unchanged.
- _passes/utils: expose the block scale tensor as QCOM_SCALE, which
  make_qnn_per_block_config reads. The remap is gated on the encoding
  rather than on the presence of a "scales" key, because
  quantize_per_channel.default also has a "scales" argument but is
  consumed as a per-channel (not per-block) config.
- qnn_pass_manager: register both ops in node_visitor.q_ops / dq_ops in
  get_to_edge_transform_passes, mirroring the torchao workaround.

Test: test_insert_io_qdq_per_channel_group_resolves_through_q_dq_map
annotates a parameter with a dequantize_per_channel_group encoding and
wires it to the graph output, which is the branch of InsertIOQDQ that
resolves the encoding through q_dq_map (the insert-quantize-after-input
branch is skipped for parameters). Without the q_dq_map entries the test
fails with KeyError at insert_io_qdq.py's q_dq_map lookup; with them it
asserts a dequantize_per_channel_group node now feeds the output.
@Hyungkeun-Park
Hyungkeun-Park force-pushed the feat/qnn-mps-per-channel-group-quantization branch from d2d5aab to 78c58b8 Compare August 4, 2026 02:29
@Hyungkeun-Park

Hyungkeun-Park commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

@qti-horodnic Thanks for the pointers. Rebuilt the branch on top of main (4b4df96) so it's no longer stale, and updated the PR description.

You were right about q_dq_map. I dropped the insert_io_qdq.py change entirely and added the two entries to the module-level dict in node_visitor.py instead.

On the test: the KeyError comes from the dequantize-before-output branch, which has no is_parameter() guard. That guard only affects the insert-quantize-after-input branch above it, so for a parameter node the output branch is the only thing that resolves the encoding. I did check that it fails without the fix. Reverting just the two q_dq_map entries gives:

File "backends/qualcomm/_passes/insert_io_qdq.py", line 79, in _insert
    target=q_dq_map[n.meta[QCOM_QUANT_ATTRS][QCOM_ENCODING]],
KeyError: <EdgeOpOverload: quantized_decomposed.dequantize_per_channel_group.default>

The test now picks the parameter placeholder via is_parameter() and asserts the inserted node's target instead of just "no KeyError". Renamed it to test_insert_io_qdq_per_channel_group_resolves_through_q_dq_map.

Good catch on get_quant_attrs, quantize_per_channel.default would indeed have picked up QCOM_SCALE. It's gated on the encoding now:

if quant_node.target in PER_CHANNEL_GROUP_ENCODING:
    quant_attrs[QCOM_SCALE] = quant_attrs[QCOM_SCALES]

PER_CHANNEL_GROUP_ENCODING is a new set in node_visitor.py next to PER_CHANNEL_ENCODING / PER_TENSOR_ENCODING, also used for the per_block_encoding routing.

That should leave the overlap with your PR at the q_dq_map dict and those two lines. I can rebase on top of yours once it lands if that's easier, otherwise this is ready.

Comment on lines +372 to +374
node_visitor.q_ops.add(
exir_ops.edge.quantized_decomposed.quantize_per_channel_group.default
)

@qti-horodnic qti-horodnic Aug 4, 2026

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.

Minor: These lines are redundant since the ops are already in the literal sets in node_visitor.py. This block is a workaround that exists because the torchao namespace isn't resolvable at node_visitor import time. quantized_decomposed ops resolve fine which is why the node_visitor.py change works. Keeping both makes it look like these depend on the workaround, which will confuse whoever eventually removes the TODO.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removed. You're right — quantized_decomposed resolves at node_visitor import time, so the literal sets are sufficient and piggybacking on the torchao workaround was misleading.

per_block_encoding = {
exir_ops.edge.torchao.quantize_affine.default,
exir_ops.edge.torchao.dequantize_affine.default,
*PER_CHANNEL_GROUP_ENCODING,

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.

This routes to make_qnn_per_block_config, which hardcodes offset = 0. But dequantize_per_channel_group accepts zero_points (your test even sets it to None). An asymmetric group-quantized weight currently lowers to a symmetric encoding silently, which is a wrong-answer path rather than an error. Could we reject it explicitly? Somewhere near the top of make_qnn_per_block_config like:

zps = quant_attrs.get("zero_points")
if zps is not None and torch.any(zps != 0):
    raise ValueError(
        "per_channel_group with non-zero zero_points is not supported; "
        "QNN blockwise expansion requires symmetric quantization"
    )

Also, this lowers to QNN_QUANTIZATION_ENCODING_BLOCKWISE_EXPANSION, the same encoding LPBQ uses, and validate_lpbq_support gates that at >= HtpArch.V69. Since pre-quantized weights bypass the quantizer, there's no arch check on this path at all. We've separately confirmed block-wise encodings hard-fail on V68 and have accuracy issues at V69 for some shapes, only becoming reliable around V79. Which archs have you validated this on? If it has the same V69+ constraint it needs an equivalent gate.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Both points addressed in d67952a:

  1. Asymmetric rejection: added at the top of make_qnn_per_block_config, checking both zero_points and zero_point so the torchao affine path gets the same guard. Covered by a new test (test_make_qnn_per_block_config_rejects_asymmetric).

  2. Arch gate: I haven't validated this on any HTP device — this environment has no device attached, and the PR's scope was making the ops survive lowering. So per your point that the constraint applies structurally (same BLOCKWISE_EXPANSION encoding as LPBQ), I added a gate rather than claiming validation: QnnOperatorSupport.is_node_supported now refuses to delegate per_channel_group q/dq when soc_info.htp_info.htp_arch < HtpArch.V69, mirroring validate_lpbq_support. The op then falls back to CPU with a warning instead of producing wrong numerics on V68. I put it in the partitioner because the pre-quantized path never reaches the quantizer's check, and NodeVisitor has no access to soc_info — happy to move it if you'd prefer it plumbed differently. If V69's shape-dependent accuracy issues warrant gating at V79 instead, I'd take your guidance on the threshold since I can't measure it here.

Comment thread backends/qualcomm/tests/test_passes.py Outdated
# one quantize (input) and one dequantize (output) = +2 nodes.
self.assertEqual(node_count_after, node_count_before + 2)

def test_insert_io_qdq_per_channel_group_resolves_through_q_dq_map(self):

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.

Good test. One gap: this covers the pass-level fix but nothing exercises make_qnn_per_block_config end-to-end with real group-quantized weights, so the scale-reshaping and the symmetry assumption are untested. Not asking you to add a device test here, but if you've validated numerics locally on an int4 model, could you mention it in the PR description?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I have not validated numerics on a real int4 model on device — no HTP device is available in this environment. Updated the PR description to state that explicitly, and added the partitioner arch gate + asymmetric rejection so the untested paths fail loudly (or fall back) rather than lowering silently wrong.

Comment thread backends/qualcomm/_passes/utils.py Outdated
# on the presence of the key: quantize_per_channel.default also has a
# "scales" argument but is consumed as a per-channel (not per-block) config.
if quant_node.target in PER_CHANNEL_GROUP_ENCODING:
quant_attrs[QCOM_SCALE] = quant_attrs[QCOM_SCALES]

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.

Minor: group_size gets read into quant_attrs here and then never used, QCOM_NUM_BLOCKS_PER_AXIS is inferred from q_scales.shape[1] in make_qnn_per_block_config instead. An assert that they match would catch a mismatched checkpoint early rather than producing a silently wrong encoding.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added in d67952a: get_quant_attrs now raises ValueError when scales.shape[-1] * group_size doesn't match the weight's input-feature count, so a mismatched checkpoint fails at annotation time instead of producing a silently wrong encoding.

@qti-horodnic

Copy link
Copy Markdown
Contributor

@qti-horodnic Thanks for the pointers. Rebuilt the branch on top of main (4b4df96) so it's no longer stale, and updated the PR description.

You were right about q_dq_map. I dropped the insert_io_qdq.py change entirely and added the two entries to the module-level dict in node_visitor.py instead.

On the test: the KeyError comes from the dequantize-before-output branch, which has no is_parameter() guard. That guard only affects the insert-quantize-after-input branch above it, so for a parameter node the output branch is the only thing that resolves the encoding. I did check that it fails without the fix. Reverting just the two q_dq_map entries gives:

File "backends/qualcomm/_passes/insert_io_qdq.py", line 79, in _insert
    target=q_dq_map[n.meta[QCOM_QUANT_ATTRS][QCOM_ENCODING]],
KeyError: <EdgeOpOverload: quantized_decomposed.dequantize_per_channel_group.default>

The test now picks the parameter placeholder via is_parameter() and asserts the inserted node's target instead of just "no KeyError". Renamed it to test_insert_io_qdq_per_channel_group_resolves_through_q_dq_map.

Good catch on get_quant_attrs, quantize_per_channel.default would indeed have picked up QCOM_SCALE. It's gated on the encoding now:

if quant_node.target in PER_CHANNEL_GROUP_ENCODING:
    quant_attrs[QCOM_SCALE] = quant_attrs[QCOM_SCALES]

PER_CHANNEL_GROUP_ENCODING is a new set in node_visitor.py next to PER_CHANNEL_ENCODING / PER_TENSOR_ENCODING, also used for the per_block_encoding routing.

That should leave the overlap with your PR at the q_dq_map dict and those two lines. I can rebase on top of yours once it lands if that's easier, otherwise this is ready.

@Hyungkeun-Park-Nota

Thanks for the detailed writeup, you're right about is_parameter. The guard only gates the input-Q branch, so the output-DQ path resolves the encoding unconditionally, the KeyError repro settles it. The rename plus asserting the inserted node's target is a good improvement.

PER_CHANNEL_GROUP_ENCODING and the gated get_quant_attrs remap both look right.

I've left a few comments, only one is blocking (on node_visitor.py), the others are minor. Overall, the change is solid.

Regarding the merge order: yes, please don't block on my pr. If your change gets approved first it can be merged, I'll rebase accordingly, no worries.

psiddh and others added 2 commits August 4, 2026 16:32
- Reject non-zero zero_points in make_qnn_per_block_config instead of
  silently lowering to a symmetric encoding
- Skip delegating per_channel_group q/dq on HTP < V69 in QnnOperatorSupport,
  mirroring validate_lpbq_support for the pre-quantized path that bypasses
  the quantizer
- Drop redundant dynamic q_ops/dq_ops registration in qnn_pass_manager
  (quantized_decomposed resolves at import time, unlike the torchao ops)
- Validate group_size against the scale shape in get_quant_attrs
@Hyungkeun-Park

Copy link
Copy Markdown
Contributor Author

@qti-horodnic All four comments addressed in d67952a, replies inline. Summary:

  • Blocking (per-block config): non-zero zero_points now raises ValueError (covered by test_make_qnn_per_block_config_rejects_asymmetric). On the arch question: I have not validated on any HTP device — this environment has none attached — so instead of claiming validation I added a delegation gate in QnnOperatorSupport that skips per_channel_group q/dq below HtpArch.V69 (mirroring validate_lpbq_support), falling back to CPU with a warning. Open to raising the threshold or relocating the gate per your guidance.
  • Removed the redundant dynamic registration in qnn_pass_manager.py.
  • get_quant_attrs now validates group_size against the scale shape.
  • PR description updated to state explicitly that on-device numerics are unverified.

pytest backends/qualcomm/tests/test_passes.py passes locally apart from two pre-existing environment failures (test_mha_to_sha: old torchao in the venv; test_resolve_debug_handle: missing .fbs when running from source) that fail identically without this change.

@qti-horodnic qti-horodnic 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.

@Hyungkeun-Park

Please rebase the changes, I think your main is stale again. I've left some comments mostly around keeping the scope contained, please take a look when you get a chance.

exir_ops.edge.quantized_decomposed.quantize_per_tensor.default: exir_ops.edge.quantized_decomposed.dequantize_per_tensor.default,
exir_ops.edge.quantized_decomposed.quantize_per_tensor.tensor: exir_ops.edge.quantized_decomposed.dequantize_per_tensor.tensor,
exir_ops.edge.quantized_decomposed.quantize_per_channel.default: exir_ops.edge.quantized_decomposed.dequantize_per_channel.default,
exir_ops.edge.quantized_decomposed.dequantize_per_channel.default: exir_ops.edge.quantized_decomposed.dequantize_per_channel.default,

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.

This key already exists on line 118 right? I think this is something you might have forgotten to remove, seems unrelated to the PR.

Comment thread backends/qualcomm/tests/test_passes.py Outdated
is resolved, and it raised ``KeyError`` before per_channel_group was
added to the map.
"""
gm, ep = self._build_quantized_graph()

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.

Minor: Let's give variables full meaningful names which we can derive their purpose from. I realize you probably followed the example of the other tests here, but I think we should improve upon those practices if we can. Applies in multiple places in this file, names like n, ep, u etc.

exir_ops.edge.quantized_decomposed.quantize_per_channel.default: exir_ops.edge.quantized_decomposed.dequantize_per_channel.default,
exir_ops.edge.quantized_decomposed.dequantize_per_channel.default: exir_ops.edge.quantized_decomposed.dequantize_per_channel.default,
exir_ops.edge.quantized_decomposed.quantize_per_channel_group.default: exir_ops.edge.quantized_decomposed.dequantize_per_channel_group.default,
exir_ops.edge.quantized_decomposed.dequantize_per_channel_group.default: exir_ops.edge.quantized_decomposed.dequantize_per_channel_group.default,

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.

Isn't this dead code? Since to_dq_op returns early for anything in dq_ops

}


def to_q_op(target):

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.

This returns the dq op, which then trips assert target in q_ops in insert_quant_node. We should mirror the per-channel/per-tensor pairs like:

exir_ops.edge.quantized_decomposed.quantize_per_channel_group.default: exir_ops.edge.quantized_decomposed.dequantize_per_channel_group.default,
exir_ops.edge.quantized_decomposed.dequantize_per_channel_group.default: exir_ops.edge.quantized_decomposed.quantize_per_channel_group.default,

Comment thread backends/qualcomm/tests/test_passes.py Outdated
# one quantize (input) and one dequantize (output) = +2 nodes.
self.assertEqual(node_count_after, node_count_before + 2)

def test_insert_io_qdq_per_channel_group_resolves_through_q_dq_map(self):

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.

I think you may need to rebase again, since this test no longer tests q_dq_map. insert_io_qdq is now to_dq_op, which short-circuits for anything in dq_ops and this PR adds dequantize_per_channel_group there, so the lookup never happens. The test passes with all the new q_dq_map entries reverted, and the KeyError repro in the description is against an old main.

The test is still valuable so please rename it accordingly and add a case that actually pins the mappingm. E.g. asserting to_q_op(dq_pcg) is q_pcg and to_dq_op(q_pcg) is dq_pcg.

# Per-channel-group lowers to blockwise expansion (the encoding LPBQ
# uses), which requires HTP >= V69. Pre-quantized weights bypass the
# quantizer's validate_lpbq_support check, so gate the delegation here.
if (

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.

QnnOperatorSupport is used by other backends like GPU and LPAI, but this reads soc_info.htp_info.htp_arch unconditionally. Let's add a check here for backend_options.backend_type == QnnExecuTorchBackendType.kHtpBackend to keep the scope intentionally on HTP.

…test

- q_dq_map: map dequantize_per_channel_group -> quantize_per_channel_group
  (it pointed back at itself, which trips `assert target in q_ops` in
  insert_quant_node) and drop a stray duplicate dequantize_per_channel entry.
- QnnOperatorSupport: only apply the < V69 blockwise gate when the backend
  is HTP; the checker is reused by GPU/LPAI which have no htp_arch.
- tests: rename the InsertIOQDQ test (to_dq_op short-circuits for dq_ops,
  so it no longer exercises q_dq_map), add a test pinning to_q_op/to_dq_op
  for the per_channel_group pair, use descriptive variable names.

Claude-Session: https://claude.ai/code/session_01BjGb82xs4F5EDYGnYtP8Gp
@Hyungkeun-Park

Hyungkeun-Park commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

@qti-horodnic Thanks for the second pass. All six comments are addressed in 79d51be, replies inline.

  • q_dq_map: dequantize_per_channel_group now maps to quantize_per_channel_group (it pointed at itself, which would trip assert target in q_ops in insert_quant_node), and the stray duplicate dequantize_per_channel entry from the last main sync is removed.
  • Partitioner gate is scoped to backend_type == kHtpBackend so GPU/LPAI users of QnnOperatorSupport never read htp_arch.
  • Tests: renamed the InsertIOQDQ test (it covers the pass path, not the map, now that to_dq_op short-circuits for dq_ops), added test_q_dq_map_pins_per_channel_group_pairs asserting both to_q_op/to_dq_op directions, and used descriptive variable names in the tests this PR adds. PR description updated accordingly.

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

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. module: qnn Issues related to Qualcomm's QNN delegate and code under backends/qualcomm/ release notes: qualcomm Changes to the Qualcomm backend delegate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants