Qualcomm: give custom op args with no QNN mapping a real error (pytorch#21868)

str is Iterable, so a string argument fell into the tensor-param branch
and died on QNN_TENSOR_TYPE_MAP[type(arg[0])] with a bare KeyError:
<class 'str'>. A list whose elements have no mapping -- str[] among them
-- still does.

Name the argument and say what to do. Strings are not a QNN limitation:
QNN_DATATYPE_STRING exists and PyQnnManagerAdaptor.cpp already reads it,
but AddScalarParam has no case for it, so point at that gap rather than
implying the backend cannot represent strings at all.

Authored with Claude Code.


cc @cbilgin

Author: psiddh

backends/qualcomm/builders/op_custom_op.py

@@ -20,6 +20,26 @@
 from .node_visitor import NodeVisitor, QNN_TENSOR_TYPE_MAP
 
 
+def _resolve_qnn_data_type(
+    py_type: type, arg_name: str, target: str, *, of_element: bool = False
+):
+    """QNN data type for a custom-op arg type, or a ValueError naming the arg."""
+    what = "element type" if of_element else "type"
+    if py_type is str:
+        raise ValueError(
+            f"String {what} for argument '{arg_name}' of {target} is unsupported: "
+            "QNN_DATATYPE_STRING is not plumbed through AddScalarParam in "
+            "aot/python/PyQnnManagerAdaptor.h. Encode it as an int in the op "
+            "schema, or add the missing case."
+        )
+    if py_type not in QNN_TENSOR_TYPE_MAP:
+        raise ValueError(
+            f"Argument '{arg_name}' of {target} has unsupported {what} "
+            f"{py_type.__name__}: QNN_TENSOR_TYPE_MAP has no entry for it."
+        )
+    return QNN_TENSOR_TYPE_MAP[py_type]
+
+
 class CustomOp(NodeVisitor):
     target = ""
     op_package_info = QnnExecuTorchOpPackageInfo()
@@ -61,20 +81,31 @@ def define_node(
                     nodes_to_wrappers,
                 )
                 custom_input_tensors.append(input_tensor_wrapper)
+            elif isinstance(arg, str):
+                # str is Iterable, so it would otherwise be taken for a tensor param.
+                _resolve_qnn_data_type(str, arg_name, self.target)
             elif isinstance(arg, Iterable):
-                tensor_parm_shape = [len(arg)]
+                values = list(arg)
+                if not values:
+                    raise ValueError(
+                        f"Argument '{arg_name}' of {self.target} is empty: a QNN "
+                        "tensor param needs at least one element."
+                    )
+                tensor_parm_shape = [len(values)]
                 custom_op.AddTensorParam(
                     arg_name,
-                    QNN_TENSOR_TYPE_MAP[type(arg[0])],
+                    _resolve_qnn_data_type(
+                        type(values[0]), arg_name, self.target, of_element=True
+                    ),
                     len(tensor_parm_shape),
                     tensor_parm_shape,
-                    np.array(arg),
+                    np.array(values),
                     True,
                 )
             else:
                 custom_op.AddScalarParam(
                     arg_name,
-                    QNN_TENSOR_TYPE_MAP[type(arg)],
+                    _resolve_qnn_data_type(type(arg), arg_name, self.target),
                     {QCOM_DATA: arg},
                 )
 

backends/qualcomm/tests/test_passes.py

@@ -15,13 +15,18 @@
 from executorch.backends.qualcomm._passes.qnn_pass_manager import (
     get_qnn_pass_manager_cls,
 )
+from executorch.backends.qualcomm.builders.op_custom_op import (
+    _resolve_qnn_data_type,
+    CustomOp,
+)
 from executorch.backends.qualcomm.builders.qnn_constants import OpContextLoader
 from executorch.backends.qualcomm.partition.qnn_partitioner import QnnOperatorSupport
 from executorch.backends.qualcomm.qnn_preprocess import QnnBackend
 from executorch.backends.qualcomm.quantizer.quantizer import QnnQuantizer, QuantDtype
 from executorch.backends.qualcomm.serialization.qc_schema import (
     QcomChipset,
     QnnExecuTorchBackendType,
+    QnnExecuTorchOpPackageInfo,
 )
 from executorch.backends.qualcomm.tests.models import (
     BroadcastAndMutate,
@@ -718,6 +723,78 @@ def forward(self, a2d, b):
                     "dedupe must not rank-promote the USER_INPUT_MUTATION write-back",
                 )
 
+    def _custom_op_node(self, arg, arg_name):
+        """A CustomOp builder plus a single-arg node, driven through define_node so
+        the branch dispatch is exercised rather than the type helper in isolation.
+        Branch order matters: str must be matched before Iterable, since str is
+        itself Iterable and would otherwise take the tensor-param path."""
+
+        class _Arg:
+            name = arg_name
+
+        class _Schema:
+            arguments = [_Arg()]
+
+        class _Target:
+            _schema = _Schema()
+
+        node = MagicMock()
+        node.name = "my_ops_foo_default"
+        node.target = _Target()
+        node.args = (arg,)
+
+        info = QnnExecuTorchOpPackageInfo()
+        info.custom_op_name = "my_ops.foo.default"
+        info.op_package_name = "FooOpPackage"
+        info.qnn_op_type_name = "Foo"
+        return CustomOp(info, {}, None, False, True), node
+
+    def test_custom_op_rejects_str_arg(self):
+        """A str custom-op arg must name the argument and the missing binding.
+
+        str is Iterable, so before the guard it reached the tensor-param branch and
+        died on QNN_TENSOR_TYPE_MAP[type(arg[0])] with a bare KeyError. Strings are
+        not a QNN limitation: QNN_DATATYPE_STRING exists and PyQnnManagerAdaptor.cpp
+        already reads it; only AddScalarParam has no case for it.
+        """
+        builder, node = self._custom_op_node("bilinear", "soft_nms_method")
+        with self.assertRaises(ValueError) as ctx:
+            builder.define_node(node, {})
+        message = str(ctx.exception)
+        self.assertIn("soft_nms_method", message)
+        self.assertIn("my_ops.foo.default", message)
+        self.assertIn("QNN_DATATYPE_STRING", message)
+        # A str must not be reported as a tensor param element type.
+        self.assertNotIn("element type", message)
+
+    def test_custom_op_rejects_unmapped_element_type(self):
+        """A list whose element type has no QNN mapping -- str[] among them -- must
+        report the element type rather than KeyError."""
+        builder, node = self._custom_op_node(["linear", "gaussian"], "modes")
+        with self.assertRaises(ValueError) as ctx:
+            builder.define_node(node, {})
+        self.assertIn("element type", str(ctx.exception))
+        self.assertIn("modes", str(ctx.exception))
+
+    def test_custom_op_rejects_empty_sequence(self):
+        """An empty sequence arg used to IndexError on arg[0] inside define_node."""
+        builder, node = self._custom_op_node([], "sizes")
+        with self.assertRaises(ValueError) as ctx:
+            builder.define_node(node, {})
+        self.assertIn("sizes", str(ctx.exception))
+        self.assertIn("empty", str(ctx.exception))
+
+    def test_custom_op_resolves_supported_types(self):
+        """Guard against a vacuous suite: the mapped scalar types still resolve.
+
+        Checked at the helper rather than through define_node, because a valid arg
+        carries on into output-tensor handling, which needs real tensor meta.
+        """
+        for py_type in (int, float, bool):
+            self.assertIsNotNone(
+                _resolve_qnn_data_type(py_type, "arg", "my_ops.foo.default")
+            )
+
 
 if __name__ == "__main__":
     unittest.main()