🔗 Helpful Links

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

• 📄 Preview Python docs built from this PR

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

❌ 3 New Failures, 6 Pending, 2 Unrelated Failures

As of commit a7fca32 with merge base d76bbe3 ():

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

This PR needs a release notes: label

If your change should be included in the release notes (i.e. would users of this library care about this change?), please use a label starting with release notes:. This helps us keep track and include your important work in the next release notes.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "release notes: none"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

Hi @psiddh,
Thank you for reviewing the PR. The following PRs are pending review and merge. Could you please take a look?

Bug Fix PR:

• Qualcomm AI Engine Direct - Minor qnn_config fix #19388
• Qualcomm AI Engine Direct - Fix Full #19359

Op Enablement PR:

• Qualcomm AI Engine Direct - Adding QNN backend support for randn core ATen op #19377
• Qualcomm AI Engine Direct - Adding QNN backend support for tan core ATen op #19301
• Qualcomm AI Engine Direct - Adding QNN backend support for scatter.src core ATen op #19283

Claude Skill PR:

• Qualcomm AI Engine Direct - Updating Claude skill for new op development #19302

LLM Related PR:

• Qualcomm AI Engine Direct - Decouple quantization and compile graphs for faster VLM/LLM PTQ #19220
• Qualcomm AI Engine Direct - Refactor llama runner for dynamic IO dtypes #19146

Debugging Related PR:

• Qualcomm AI Engine Direct - heap profiling at runtime with HTP backend #19224
• Qualcomm AI Engine Direct - Debugger Convergence Phase 2: Migrating to official numeric discrepancy evaluator #18834

Others:

• Qualcomm AI Engine Direct - Remove AIHub Example Scripts #19144
• Qualcomm AI Engine Direct - Refactor QnnDlcManager #19105
• Qualcomm AI Engine Direct - Add fp16a8w quantization config #19537

Hi @psiddh, Thank you for reviewing the PR. The following PRs are pending review and merge. Could you please take a look?

Bug Fix PR:

• Qualcomm AI Engine Direct - Minor qnn_config fix #19388
• Qualcomm AI Engine Direct - Fix Full #19359

Op Enablement PR:

• Qualcomm AI Engine Direct - Adding QNN backend support for randn core ATen op #19377
• Qualcomm AI Engine Direct - Adding QNN backend support for tan core ATen op #19301
• Qualcomm AI Engine Direct - Adding QNN backend support for scatter.src core ATen op #19283

Claude Skill PR:

• Qualcomm AI Engine Direct - Updating Claude skill for new op development #19302

LLM Related PR:

• Qualcomm AI Engine Direct - Decouple quantization and compile graphs for faster VLM/LLM PTQ #19220
• Qualcomm AI Engine Direct - Refactor llama runner for dynamic IO dtypes #19146

Debugging Related PR:

• Qualcomm AI Engine Direct - heap profiling at runtime with HTP backend #19224
• Qualcomm AI Engine Direct - Debugger Convergence Phase 2: Migrating to official numeric discrepancy evaluator #18834

Others:

• Qualcomm AI Engine Direct - Remove AIHub Example Scripts #19144
• Qualcomm AI Engine Direct - Refactor QnnDlcManager #19105
• Qualcomm AI Engine Direct - Add fp16a8w quantization config #19537

Thanks for listing all the PRs I will start looking at them

@claude review this pr

Claude finished @psiddh's task in 2m 48s —— View job

Reviewing PR #19537: Qualcomm AI Engine Direct - Add fp16a8w quantization config

• Read changed files and gather context
• Review new pass: insert_cast_for_fp_act_quantized_weight.py
• Review quantizer changes (qconfig.py, quantizer.py, rules.py, annotators/htp_rules.py)
• Review pass manager and utils changes
• Review test changes
• Post review feedback

Summary

This PR adds support for fp16a8w quantization (FP16 activation with INT8 quantized weight) for QNN HTP backend, limited to Conv2d (kernel_size=1) and Linear ops. The implementation is well-structured across several components:

1. New pass (InsertCastForFpActQuantizedWeight) to insert fp32→fp16 casts around qualifying ops
2. New qconfig functions for PTQ, QAT, and per-channel variants
3. Annotator updates in htp_rules.py to handle None activation specs gracefully
4. Tests covering Conv2d, Linear, QAT, and composite model scenarios

Feedback

Design — Looks Good

The approach of keeping activations unquantized (input_activation=None, output_activation=None) and only quantizing weights, then inserting dtype casts in a later pass, is clean. It avoids entangling the quantization annotation logic with dtype casting and keeps the constraint (QNN HTP requires fp16 activations with quantized weights) isolated in a single pass.

The pass dependency ordering (InsertCastForFpActQuantizedWeight: [FoldQDQ, LayoutTransform]) is correct — it needs QCOM_QUANT_ATTRS metadata from FoldQDQ and final layout from LayoutTransform.

Issues / Suggestions

1. Potential issue with SimpleModel(kernel_size=1) and padding=1 — backends/qualcomm/tests/models.py:2257

When kernel_size=1 is passed, the convolutions are created with padding=1. A 1x1 conv with padding=1 on a 28x28 input produces a 30x30 output. After 4 convolutions: 28→30→32→34→36. The add(x5, y5) still works since both paths have identical shape, and the downstream reshape arithmetic works out coincidentally (elements = 32 * 36 * 36 = 41472; reshaped to (8, 5184); but Linear expects input dim = 4).

Actually, looking more carefully at the forward pass: z2 = torch.mean(z1, [1, 2], True) reduces dims 1 and 2, keeping them as size 1. So z2 shape = (1, 1, 1, 32) regardless of spatial size. Then reshape(z2, (8, -1)) = (8, 4). This works. No issue here — the test is correct, though using padding=0 would be more natural for 1x1 convs.

2. Bias metadata mutation in the pass is in-place — backends/qualcomm/_passes/insert_cast_for_fp_act_quantized_weight.py:103-104

if bias_node.meta["val"].dtype == torch.float32:
    bias_node.meta["val"] = bias_node.meta["val"].to(torch.float16)

This mutates the bias node's metadata. If the same bias node is shared between multiple ops (unlikely but possible in weight-sharing scenarios), this could cause unexpected behavior. Since QNN typically doesn't have shared biases and the pass runs late in the pipeline, this is likely fine in practice.

3. act_observer parameter is unused in fp16a8w configs — backends/qualcomm/quantizer/qconfig.py:115

def get_fp16a8w_qnn_ptq_config(
    act_symmetric: bool = False,
    act_observer=MovingAverageMinMaxObserver,  # ← unused
    eps: float = None,
) -> QuantizationConfig:

The act_observer parameter is accepted but never used since input_activation=None and output_activation=None. This is harmless (matches the interface expected by ModuleQConfig.__post_init__), but could be misleading to users. Consider adding a brief note that this parameter is ignored for fp16a8w configs, or simply remove it from the signature and handle the kwarg-passing in __post_init__ differently.

4. htp_rules.py changes are defensive and correct but verbose

The pattern of guarding every annotation with if quantization_config.input_activation is not None: is applied consistently across ~15 annotators (Cat, Chunk, Div, HardSigmoid, IndexCopy, IndexPut, Linear, MaskedFill, MatMul, Sigmoid, Stack, Tanh, Topk, Unbind, Where). This is the right approach to support fp16a8w where activations aren't quantized, but it adds significant code volume.

A potential future improvement could be to add a helper like:

def _should_annotate_activations(config: QuantizationConfig) -> bool:
    return config.input_activation is not None

and use early-return patterns. But this is a style suggestion, not a blocking concern for this PR.

5. annotate_single_in_share_out and annotate_single_in in rules.py already handle None input specs

The existing utility functions in rules.py (lines 93-134) already check if input_act_qspec is not None before adding to the map. This means ops that delegate to these utilities (via the default GeneralOpDef.annotate) don't need changes. The per-op changes in htp_rules.py are only needed for ops with custom annotators — this is correct.

6. Missing __all__ update for new fp16a8w configs — backends/qualcomm/quantizer/quantizer.py:69-80

The __all__ list doesn't include the new fp16a8w config functions:

__all__ = [
    "QnnQuantizer",
    "QuantDtype",
    "get_16a4w_qnn_ptq_config",
    ...
]

Consider adding get_fp16a8w_qnn_ptq_config, get_fp16a8w_qnn_qat_config, etc. if they're intended to be part of the public API. Fix this →

No Issues Found

• The pass correctly handles both biased and unbiased conv/linear ops
• Dead code elimination is properly called after the pass
• The copy_meta usage preserves metadata correctly
• The test coverage (PTQ, QAT, per-channel, with/without bias, composite model, fp16 input) is thorough
• The QuantDtype.use_fp16a8w = 6 enum value doesn't conflict with existing values

Verdict

Approve with minor suggestions. The implementation is clean, well-tested, and correctly integrates with the existing QNN quantization pipeline. The annotator changes in htp_rules.py are mechanical but necessary. The main code suggestion is the missing __all__ export (item 6), which is minor.