You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2020/09/22 16:51:45 UTC

[GitHub] [incubator-tvm] d-smirnov opened a new pull request #6532: ACL support: "add" operation

d-smirnov opened a new pull request #6532:
URL: https://github.com/apache/incubator-tvm/pull/6532


   Added support for an "add" operation implemented via ACL + unit test


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-tvm] d-smirnov commented on a change in pull request #6532: [BYOC][ACL] Support add operation

Posted by GitBox <gi...@apache.org>.
d-smirnov commented on a change in pull request #6532:
URL: https://github.com/apache/incubator-tvm/pull/6532#discussion_r496093843



##########
File path: src/runtime/contrib/arm_compute_lib/acl_runtime.cc
##########
@@ -140,8 +141,13 @@ class ACLRuntime : public JSONRuntimeBase {
           CreateGlobalPoolingLayer(&layer_, node);
         } else if ("reshape" == op_name) {
           CreateReshapeLayer(&layer_, node);
+<<<<<<< HEAD
         } else if ("maximum" == op_name) {
           CreateMaximumLayer(&layer_, node);
+=======
+        } else if ("add" == op_name || "qnn.add" == op_name) {
+          CreateAddLayer(&layer_, node);
+>>>>>>> a7fa43daf... ACL: add operation

Review comment:
       Done




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-tvm] d-smirnov commented on a change in pull request #6532: [BYOC][ACL] Support add operation

Posted by GitBox <gi...@apache.org>.
d-smirnov commented on a change in pull request #6532:
URL: https://github.com/apache/incubator-tvm/pull/6532#discussion_r495921373



##########
File path: tests/python/contrib/test_arm_compute_lib/test_add.py
##########
@@ -0,0 +1,142 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Arm Compute Library integration reshape tests."""
+
+import numpy as np
+
+import tvm
+import tvm.testing
+from tvm import relay
+
+from test_arm_compute_lib.infrastructure import (
+    skip_runtime_test,
+    skip_codegen_test,
+    build_and_run,
+    verify,
+    verify_codegen,
+)
+from test_arm_compute_lib.infrastructure import Device
+
+
+def _get_model(shape, dtype, var_names, op, op_params):
+    a = relay.var(next(var_names), shape=shape, dtype=dtype)
+    b = relay.var(next(var_names), shape=shape, dtype=dtype)
+    return op(a, b, **op_params)
+
+
+def _get_expected_codegen(shape, dtype):
+    node = {
+        "op": "kernel",
+        "name": "add",
+        "inputs": [[0, 0, 0], [1, 0, 0]],
+        "attrs": {
+            "num_inputs": "2",
+            "num_outputs": "1",
+            "shape": [[list(shape)]],
+            "dtype": [[dtype]],
+        },
+    }
+
+    input_a = {"op": "input", "name": "", "attrs": {"shape": [[list(shape)]], "dtype": [[dtype]]}}
+    input_b = {"op": "input", "name": "", "attrs": {"shape": [[list(shape)]], "dtype": [[dtype]]}}
+    return [input_a, input_b, node]
+
+
+def test_add():
+    Device.load("test_config.json")
+
+    if skip_runtime_test():
+        return
+
+    device = Device()
+    np.random.seed(0)
+
+    for dtype, low, high, atol, rtol, op, op_params in [
+        ("float32", -127, 128, 1e-7, 1e-7, relay.add, {}),
+        # different qnn params
+        (
+            "uint8",
+            0,
+            255,
+            0.0,
+            1.0,
+            relay.qnn.op.add,
+            {
+                "lhs_scale": relay.const(0.0156863, "float32"),
+                "lhs_zero_point": relay.const(127, "int32"),
+                "rhs_scale": relay.const(0.0117647, "float32"),
+                "rhs_zero_point": relay.const(85, "int32"),
+                "output_scale": relay.const(0.0235294, "float32"),
+                "output_zero_point": relay.const(128, "int32"),
+            },
+        ),
+        # same qnn params
+        (
+            "uint8",
+            0,
+            255,
+            0.0,
+            1.0,
+            relay.qnn.op.add,
+            {
+                "lhs_scale": relay.const(0.0126863, "float32"),
+                "lhs_zero_point": relay.const(127, "int32"),
+                "rhs_scale": relay.const(0.0126863, "float32"),
+                "rhs_zero_point": relay.const(127, "int32"),
+                "output_scale": relay.const(0.0126863, "float32"),
+                "output_zero_point": relay.const(127, "int32"),
+            },
+        ),
+    ]:

Review comment:
       1. Done.
   2. Removed




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-tvm] d-smirnov commented on a change in pull request #6532: [BYOC][ACL] Support add operation

Posted by GitBox <gi...@apache.org>.
d-smirnov commented on a change in pull request #6532:
URL: https://github.com/apache/incubator-tvm/pull/6532#discussion_r495926886



##########
File path: src/runtime/contrib/arm_compute_lib/acl_runtime.cc
##########
@@ -401,6 +406,50 @@ class ACLRuntime : public JSONRuntimeBase {
     layer->function = function;
   }
 
+  /*!
+   * \brief Creates an add layer
+   *
+   * \param layer The ACL layer to build. Containing inputs, outputs and the ACL function.
+   * \param node  The JSON representation of the operator.
+   */
+  void CreateAddLayer(CachedLayer* layer, const JSONGraphNode& node) {
+    layer->inputs.push_back(MakeACLTensorFromJSONEntry(node.GetInputs()[0]));
+    layer->inputs.push_back(MakeACLTensorFromJSONEntry(node.GetInputs()[1]));
+    layer->outputs.push_back(MakeACLTensorFromJSONNode(node));
+    AppendAddOp(layer);
+  }
+
+  /*!
+   * \brief Creates a qnn.add layer
+   *
+   * \param layer The ACL layer to build. Containing inputs, outputs and the ACL function.
+   * \param node  The JSON representation of the operator.
+   */
+  void CreateQnnAddLayer(CachedLayer* layer, const JSONGraphNode& node) {
+    layer->inputs.push_back(MakeACLTensorFromJSONEntry(node.GetInputs()[0], &node.GetInputs()[2],
+                                                       &node.GetInputs()[3]));
+    layer->inputs.push_back(MakeACLTensorFromJSONEntry(node.GetInputs()[1], &node.GetInputs()[4],
+                                                       &node.GetInputs()[5]));
+    layer->outputs.push_back(
+        MakeACLTensorFromJSONNode(node, &node.GetInputs()[6], &node.GetInputs()[7]));
+    AppendAddOp(layer);
+  }
+
+  void AppendAddOp(CachedLayer* layer) {
+    /** Initialise the kernel's inputs, output and conversion policy.
+     *
+     * @param[in]  input1 First tensor input. Data types supported: U8/QASYMM8/S16/F16/F32
+     * @param[in]  input2 Second tensor input. Data types supported: U8/QASYMM8/S16/F16/F32
+     * @param[out] output Output tensor. Data types supported: U8/QASYMM8/S16/F16/F32
+     * @param[in]  policy Policy to use to handle overflow.
+     * void configure(ITensor *input1, ITensor *input2, ITensor *output, ConvertPolicy policy);
+     */
+    auto f = std::make_shared<arm_compute::NEArithmeticAddition>();
+    f->configure(&layer->inputs[0], &layer->inputs[1], &layer->outputs[0],
+                 arm_compute::ConvertPolicy::WRAP);
+    layer->function = f;
+  }

Review comment:
       updated




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-tvm] lhutton1 commented on a change in pull request #6532: [BYOC][ACL] Support add operation

Posted by GitBox <gi...@apache.org>.
lhutton1 commented on a change in pull request #6532:
URL: https://github.com/apache/incubator-tvm/pull/6532#discussion_r496634223



##########
File path: tests/python/contrib/test_arm_compute_lib/test_add.py
##########
@@ -0,0 +1,135 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Arm Compute Library integration reshape tests."""
+
+import numpy as np
+
+import tvm
+import tvm.testing
+from tvm import relay
+
+from test_arm_compute_lib.infrastructure import (
+    skip_runtime_test,
+    skip_codegen_test,
+    build_and_run,
+    verify,
+    verify_codegen,
+)
+from test_arm_compute_lib.infrastructure import Device
+
+_qnn_params = {
+    "lhs_scale": relay.const(0.0156863, "float32"),
+    "lhs_zero_point": relay.const(127, "int32"),
+    "rhs_scale": relay.const(0.0117647, "float32"),
+    "rhs_zero_point": relay.const(85, "int32"),
+    "output_scale": relay.const(0.0235294, "float32"),
+    "output_zero_point": relay.const(128, "int32"),
+}
+
+
+def _get_model(shape, dtype, var_names, op, op_params):
+    a = relay.var(next(var_names), shape=shape, dtype=dtype)
+    b = relay.var(next(var_names), shape=shape, dtype=dtype)
+    return op(a, b, **op_params)
+
+
+def _get_expected_codegen(shape, dtype, op_name, qnn_params):
+    input_a = {"op": "input", "name": "", "attrs": {"shape": [[list(shape)]], "dtype": [[dtype]]}}
+    input_b = {"op": "input", "name": "", "attrs": {"shape": [[list(shape)]], "dtype": [[dtype]]}}
+    input_qnn = [
+        {
+            "op": "const",
+            "name": "",
+            "attrs": {
+                "shape": [[list(qnn_params[_].data.shape)]],
+                "dtype": [[qnn_params[_].data.dtype]],
+            },
+        }
+        for _ in qnn_params
+    ]
+    inputs = [input_a, input_b, *input_qnn]
+    node = {
+        "op": "kernel",
+        "name": op_name,
+        "inputs": [[_, 0, 0] for _ in range(len(inputs))],
+        "attrs": {
+            "num_inputs": str(len(inputs)),
+            "num_outputs": "1",
+            "shape": [[list(shape)]],
+            "dtype": [[dtype]],
+        },
+    }
+
+    return [*inputs, node]
+
+
+def test_runtime_add():
+    Device.load("test_config.json")
+
+    if skip_runtime_test():
+        return
+
+    device = Device()
+    np.random.seed(0)
+
+    for dtype, low, high, atol, rtol, op, op_params in [
+        ("float32", -127, 128, 1e-7, 1e-7, relay.add, {}),
+        ("uint8", 0, 255, 0.0, 1.0, relay.qnn.op.add, _qnn_params),
+    ]:
+        shape = (2, 2)
+        for inputs in [
+            {
+                "a": tvm.nd.array(np.random.uniform(low, high, shape).astype(dtype)),
+                "b": tvm.nd.array(np.random.uniform(low, high, shape).astype(dtype)),
+            }
+        ]:
+            outputs = []
+            func = _get_model(shape, dtype, iter(inputs), op, op_params)
+            for acl in [True, False]:
+                outputs.append(build_and_run(func, inputs, 1, None, device, enable_acl=acl)[0])
+
+            config = {
+                "shape": shape,
+                "dtype": dtype,
+                "inputs": inputs,
+                "operation": op,
+                "op_params": op_params,
+            }
+
+            # verify_saturation=False as the result of add_QASYMM8_QASYMM8_QASYMM8
+            # is always saturated currently.
+            verify(outputs, atol=atol, rtol=rtol, config=config, verify_saturation=False)
+
+
+def test_runtime_codegen_add():

Review comment:
       To avoid confusion I think we should keep codegen tests named `test_codegen_add`

##########
File path: tests/python/contrib/test_arm_compute_lib/test_add.py
##########
@@ -0,0 +1,135 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Arm Compute Library integration reshape tests."""
+
+import numpy as np
+
+import tvm
+import tvm.testing
+from tvm import relay
+
+from test_arm_compute_lib.infrastructure import (
+    skip_runtime_test,
+    skip_codegen_test,
+    build_and_run,
+    verify,
+    verify_codegen,
+)
+from test_arm_compute_lib.infrastructure import Device
+
+_qnn_params = {
+    "lhs_scale": relay.const(0.0156863, "float32"),
+    "lhs_zero_point": relay.const(127, "int32"),
+    "rhs_scale": relay.const(0.0117647, "float32"),
+    "rhs_zero_point": relay.const(85, "int32"),
+    "output_scale": relay.const(0.0235294, "float32"),
+    "output_zero_point": relay.const(128, "int32"),
+}
+
+
+def _get_model(shape, dtype, var_names, op, op_params):
+    a = relay.var(next(var_names), shape=shape, dtype=dtype)
+    b = relay.var(next(var_names), shape=shape, dtype=dtype)
+    return op(a, b, **op_params)
+
+
+def _get_expected_codegen(shape, dtype, op_name, qnn_params):
+    input_a = {"op": "input", "name": "", "attrs": {"shape": [[list(shape)]], "dtype": [[dtype]]}}
+    input_b = {"op": "input", "name": "", "attrs": {"shape": [[list(shape)]], "dtype": [[dtype]]}}
+    input_qnn = [
+        {
+            "op": "const",
+            "name": "",
+            "attrs": {
+                "shape": [[list(qnn_params[_].data.shape)]],
+                "dtype": [[qnn_params[_].data.dtype]],
+            },
+        }
+        for _ in qnn_params
+    ]
+    inputs = [input_a, input_b, *input_qnn]
+    node = {
+        "op": "kernel",
+        "name": op_name,
+        "inputs": [[_, 0, 0] for _ in range(len(inputs))],
+        "attrs": {
+            "num_inputs": str(len(inputs)),
+            "num_outputs": "1",
+            "shape": [[list(shape)]],
+            "dtype": [[dtype]],
+        },
+    }
+
+    return [*inputs, node]
+
+
+def test_runtime_add():
+    Device.load("test_config.json")
+
+    if skip_runtime_test():
+        return
+
+    device = Device()
+    np.random.seed(0)
+
+    for dtype, low, high, atol, rtol, op, op_params in [
+        ("float32", -127, 128, 1e-7, 1e-7, relay.add, {}),
+        ("uint8", 0, 255, 0.0, 1.0, relay.qnn.op.add, _qnn_params),
+    ]:
+        shape = (2, 2)
+        for inputs in [
+            {
+                "a": tvm.nd.array(np.random.uniform(low, high, shape).astype(dtype)),
+                "b": tvm.nd.array(np.random.uniform(low, high, shape).astype(dtype)),
+            }
+        ]:
+            outputs = []
+            func = _get_model(shape, dtype, iter(inputs), op, op_params)
+            for acl in [True, False]:
+                outputs.append(build_and_run(func, inputs, 1, None, device, enable_acl=acl)[0])
+
+            config = {
+                "shape": shape,
+                "dtype": dtype,
+                "inputs": inputs,
+                "operation": op,
+                "op_params": op_params,
+            }
+
+            # verify_saturation=False as the result of add_QASYMM8_QASYMM8_QASYMM8
+            # is always saturated currently.

Review comment:
       Could this be because of the qnn params being used in the test? We can generate qnn params that won't saturate by looking at this example https://github.com/apache/incubator-tvm/blob/master/tests/python/contrib/test_ethosn/test_addition.py#L45. I think we should try and fix this as we won't checking the result is accurate if it is saturated.
   
   If this can be enabled for `qnn.add`, then we will need to make sure it's disabled for `add`.

##########
File path: src/runtime/contrib/arm_compute_lib/acl_runtime.cc
##########
@@ -417,6 +420,45 @@ class ACLRuntime : public JSONRuntimeBase {
     function->configure(&layer->inputs[0], &layer->inputs[1], &layer->outputs[0]);
     layer->function = function;
   }
+  /*!
+   * \brief Creates an add/qnn.add layer
+   *
+   * \param layer The ACL layer to build. Containing inputs, outputs and the ACL function.
+   * \param node  The JSON representation of the operator.
+   */
+  void CreateAddLayer(CachedLayer* layer, const JSONGraphNode& node) {
+    auto op_name = node.GetOpName();
+    if ("add" == op_name) {
+      layer->inputs.push_back(MakeACLTensorFromJSONEntry(node.GetInputs()[0]));
+      layer->inputs.push_back(MakeACLTensorFromJSONEntry(node.GetInputs()[1]));
+      layer->outputs.push_back(MakeACLTensorFromJSONNode(node));
+    } else if ("qnn.add" == op_name) {
+      layer->inputs.push_back(MakeACLTensorFromJSONEntry(node.GetInputs()[0], &node.GetInputs()[2],
+                                                         &node.GetInputs()[3]));
+      layer->inputs.push_back(MakeACLTensorFromJSONEntry(node.GetInputs()[1], &node.GetInputs()[4],
+                                                         &node.GetInputs()[5]));
+      layer->outputs.push_back(
+          MakeACLTensorFromJSONNode(node, &node.GetInputs()[6], &node.GetInputs()[7]));
+    } else {
+      throw std::runtime_error("Unsupported form of add op: " + op_name);
+    }
+
+    /** Initialise the kernel's inputs, output and conversion policy.

Review comment:
       Personally I don't think we need this doc string, feel free to ignore though




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-tvm] d-smirnov commented on a change in pull request #6532: [BYOC][ACL] Support add operation

Posted by GitBox <gi...@apache.org>.
d-smirnov commented on a change in pull request #6532:
URL: https://github.com/apache/incubator-tvm/pull/6532#discussion_r494834251



##########
File path: python/tvm/relay/op/contrib/arm_compute_lib.py
##########
@@ -161,6 +161,17 @@ def l2_pool2d_pattern():
         pattern = is_op("sqrt")(pattern)
         return pattern
 
+    def add_pattern():
+        """Create an add pattern.
+
+        Returns
+        -------
+        pattern : dataflow_pattern.AltPattern
+            Denotes the convolution pattern.

Review comment:
       removed

##########
File path: python/tvm/relay/op/contrib/arm_compute_lib.py
##########
@@ -161,6 +161,17 @@ def l2_pool2d_pattern():
         pattern = is_op("sqrt")(pattern)
         return pattern
 
+    def add_pattern():
+        """Create an add pattern.
+
+        Returns
+        -------
+        pattern : dataflow_pattern.AltPattern
+            Denotes the convolution pattern.
+        """
+        pattern = is_op("add")(wildcard(), is_constant(), is_constant())

Review comment:
       removed

##########
File path: python/tvm/relay/op/contrib/arm_compute_lib.py
##########
@@ -337,3 +354,17 @@ def global_avg_pool2d(attrs, args):
     if attrs.layout != "NHWC":
         return False
     return True
+
+
+@tvm.ir.register_op_attr("add", "target.arm_compute_lib")
+@tvm.ir.register_op_attr("qnn.add", "target.arm_compute_lib")
+def add(attrs, args):
+    """Check if the external ACL codegen for add should be used."""
+    for typ in [args[0].checked_type, args[1].checked_type]:
+        if typ.dtype not in ["float32", "uint8"]:

Review comment:
       1. Yep. The pattern is not needed. Removed. 
   2. Yep. Has been split

##########
File path: tests/python/contrib/test_arm_compute_lib/test_add.py
##########
@@ -0,0 +1,142 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Arm Compute Library integration reshape tests."""
+
+import numpy as np
+
+import tvm
+import tvm.testing
+from tvm import relay
+
+from test_arm_compute_lib.infrastructure import (
+    skip_runtime_test,
+    skip_codegen_test,
+    build_and_run,
+    verify,
+    verify_codegen,
+)
+from test_arm_compute_lib.infrastructure import Device
+
+
+def _get_model(shape, dtype, var_names, op, op_params):
+    a = relay.var(next(var_names), shape=shape, dtype=dtype)
+    b = relay.var(next(var_names), shape=shape, dtype=dtype)
+    return op(a, b, **op_params)
+
+
+def _get_expected_codegen(shape, dtype):
+    node = {
+        "op": "kernel",
+        "name": "add",
+        "inputs": [[0, 0, 0], [1, 0, 0]],
+        "attrs": {
+            "num_inputs": "2",
+            "num_outputs": "1",
+            "shape": [[list(shape)]],
+            "dtype": [[dtype]],
+        },
+    }
+
+    input_a = {"op": "input", "name": "", "attrs": {"shape": [[list(shape)]], "dtype": [[dtype]]}}
+    input_b = {"op": "input", "name": "", "attrs": {"shape": [[list(shape)]], "dtype": [[dtype]]}}
+    return [input_a, input_b, node]
+
+
+def test_add():
+    Device.load("test_config.json")
+
+    if skip_runtime_test():
+        return
+
+    device = Device()
+    np.random.seed(0)
+
+    for dtype, low, high, atol, rtol, op, op_params in [
+        ("float32", -127, 128, 1e-7, 1e-7, relay.add, {}),
+        # different qnn params
+        (
+            "uint8",
+            0,
+            255,
+            0.0,
+            1.0,
+            relay.qnn.op.add,
+            {
+                "lhs_scale": relay.const(0.0156863, "float32"),
+                "lhs_zero_point": relay.const(127, "int32"),
+                "rhs_scale": relay.const(0.0117647, "float32"),
+                "rhs_zero_point": relay.const(85, "int32"),
+                "output_scale": relay.const(0.0235294, "float32"),
+                "output_zero_point": relay.const(128, "int32"),
+            },
+        ),
+        # same qnn params
+        (
+            "uint8",
+            0,
+            255,
+            0.0,
+            1.0,
+            relay.qnn.op.add,
+            {
+                "lhs_scale": relay.const(0.0126863, "float32"),
+                "lhs_zero_point": relay.const(127, "int32"),
+                "rhs_scale": relay.const(0.0126863, "float32"),
+                "rhs_zero_point": relay.const(127, "int32"),
+                "output_scale": relay.const(0.0126863, "float32"),
+                "output_zero_point": relay.const(127, "int32"),
+            },
+        ),
+    ]:
+        shape = (2, 2)
+        for inputs in [
+            {
+                "a": tvm.nd.array(np.random.uniform(low, high, shape).astype(dtype)),
+                "b": tvm.nd.array(np.random.uniform(low, high, shape).astype(dtype)),
+            }
+        ]:
+            outputs = []
+            func = _get_model(shape, dtype, iter(inputs), op, op_params)
+            for acl in [True, False]:
+                outputs.append(build_and_run(func, inputs, 1, None, device, enable_acl=acl)[0])
+
+            config = {
+                "shape": shape,
+                "dtype": dtype,
+                "inputs": inputs,
+                "operation": op,
+                "op_params": op_params,
+            }
+            verify(outputs, atol=atol, rtol=rtol, config=config)
+
+
+def test_codegen_add():
+    if skip_codegen_test():
+        return
+
+    shape = (1, 1, 1, 1000)
+    inputs = {"a", "b"}
+    for dtype in ["float32", "uint8"]:
+        for new_shape in [(1, 1000), (10, 10, 10)]:
+            func = _get_model(shape, dtype, iter(inputs), relay.add, {})

Review comment:
       Added

##########
File path: tests/python/contrib/test_arm_compute_lib/test_add.py
##########
@@ -0,0 +1,142 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Arm Compute Library integration reshape tests."""
+
+import numpy as np
+
+import tvm
+import tvm.testing
+from tvm import relay
+
+from test_arm_compute_lib.infrastructure import (
+    skip_runtime_test,
+    skip_codegen_test,
+    build_and_run,
+    verify,
+    verify_codegen,
+)
+from test_arm_compute_lib.infrastructure import Device
+
+
+def _get_model(shape, dtype, var_names, op, op_params):
+    a = relay.var(next(var_names), shape=shape, dtype=dtype)
+    b = relay.var(next(var_names), shape=shape, dtype=dtype)
+    return op(a, b, **op_params)
+
+
+def _get_expected_codegen(shape, dtype):
+    node = {
+        "op": "kernel",
+        "name": "add",
+        "inputs": [[0, 0, 0], [1, 0, 0]],
+        "attrs": {
+            "num_inputs": "2",
+            "num_outputs": "1",
+            "shape": [[list(shape)]],
+            "dtype": [[dtype]],
+        },
+    }
+
+    input_a = {"op": "input", "name": "", "attrs": {"shape": [[list(shape)]], "dtype": [[dtype]]}}
+    input_b = {"op": "input", "name": "", "attrs": {"shape": [[list(shape)]], "dtype": [[dtype]]}}
+    return [input_a, input_b, node]
+
+
+def test_add():
+    Device.load("test_config.json")
+
+    if skip_runtime_test():
+        return
+
+    device = Device()
+    np.random.seed(0)
+
+    for dtype, low, high, atol, rtol, op, op_params in [
+        ("float32", -127, 128, 1e-7, 1e-7, relay.add, {}),
+        # different qnn params
+        (
+            "uint8",
+            0,
+            255,
+            0.0,
+            1.0,
+            relay.qnn.op.add,
+            {
+                "lhs_scale": relay.const(0.0156863, "float32"),
+                "lhs_zero_point": relay.const(127, "int32"),
+                "rhs_scale": relay.const(0.0117647, "float32"),
+                "rhs_zero_point": relay.const(85, "int32"),
+                "output_scale": relay.const(0.0235294, "float32"),
+                "output_zero_point": relay.const(128, "int32"),
+            },
+        ),
+        # same qnn params
+        (
+            "uint8",
+            0,
+            255,
+            0.0,
+            1.0,
+            relay.qnn.op.add,
+            {
+                "lhs_scale": relay.const(0.0126863, "float32"),
+                "lhs_zero_point": relay.const(127, "int32"),
+                "rhs_scale": relay.const(0.0126863, "float32"),
+                "rhs_zero_point": relay.const(127, "int32"),
+                "output_scale": relay.const(0.0126863, "float32"),
+                "output_zero_point": relay.const(127, "int32"),
+            },
+        ),
+    ]:
+        shape = (2, 2)
+        for inputs in [
+            {
+                "a": tvm.nd.array(np.random.uniform(low, high, shape).astype(dtype)),
+                "b": tvm.nd.array(np.random.uniform(low, high, shape).astype(dtype)),
+            }
+        ]:
+            outputs = []
+            func = _get_model(shape, dtype, iter(inputs), op, op_params)
+            for acl in [True, False]:
+                outputs.append(build_and_run(func, inputs, 1, None, device, enable_acl=acl)[0])
+
+            config = {
+                "shape": shape,
+                "dtype": dtype,
+                "inputs": inputs,
+                "operation": op,
+                "op_params": op_params,
+            }
+            verify(outputs, atol=atol, rtol=rtol, config=config)

Review comment:
       Could you elaborate the assertion of "0.25 * outs[0].asnumpy().size" for small sized (e.g. (2,2) or similar ) test cases, please? 
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-tvm] d-smirnov commented on a change in pull request #6532: [BYOC][ACL] Support add operation

Posted by GitBox <gi...@apache.org>.
d-smirnov commented on a change in pull request #6532:
URL: https://github.com/apache/incubator-tvm/pull/6532#discussion_r502667277



##########
File path: tests/python/contrib/test_arm_compute_lib/test_add.py
##########
@@ -0,0 +1,135 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Arm Compute Library integration reshape tests."""
+
+import numpy as np
+
+import tvm
+import tvm.testing
+from tvm import relay
+
+from test_arm_compute_lib.infrastructure import (
+    skip_runtime_test,
+    skip_codegen_test,
+    build_and_run,
+    verify,
+    verify_codegen,
+)
+from test_arm_compute_lib.infrastructure import Device
+
+_qnn_params = {
+    "lhs_scale": relay.const(0.0156863, "float32"),
+    "lhs_zero_point": relay.const(127, "int32"),
+    "rhs_scale": relay.const(0.0117647, "float32"),
+    "rhs_zero_point": relay.const(85, "int32"),
+    "output_scale": relay.const(0.0235294, "float32"),
+    "output_zero_point": relay.const(128, "int32"),
+}
+
+
+def _get_model(shape, dtype, var_names, op, op_params):
+    a = relay.var(next(var_names), shape=shape, dtype=dtype)
+    b = relay.var(next(var_names), shape=shape, dtype=dtype)
+    return op(a, b, **op_params)
+
+
+def _get_expected_codegen(shape, dtype, op_name, qnn_params):
+    input_a = {"op": "input", "name": "", "attrs": {"shape": [[list(shape)]], "dtype": [[dtype]]}}
+    input_b = {"op": "input", "name": "", "attrs": {"shape": [[list(shape)]], "dtype": [[dtype]]}}
+    input_qnn = [
+        {
+            "op": "const",
+            "name": "",
+            "attrs": {
+                "shape": [[list(qnn_params[_].data.shape)]],
+                "dtype": [[qnn_params[_].data.dtype]],
+            },
+        }
+        for _ in qnn_params
+    ]
+    inputs = [input_a, input_b, *input_qnn]
+    node = {
+        "op": "kernel",
+        "name": op_name,
+        "inputs": [[_, 0, 0] for _ in range(len(inputs))],
+        "attrs": {
+            "num_inputs": str(len(inputs)),
+            "num_outputs": "1",
+            "shape": [[list(shape)]],
+            "dtype": [[dtype]],
+        },
+    }
+
+    return [*inputs, node]
+
+
+def test_runtime_add():
+    Device.load("test_config.json")
+
+    if skip_runtime_test():
+        return
+
+    device = Device()
+    np.random.seed(0)
+
+    for dtype, low, high, atol, rtol, op, op_params in [
+        ("float32", -127, 128, 1e-7, 1e-7, relay.add, {}),
+        ("uint8", 0, 255, 0.0, 1.0, relay.qnn.op.add, _qnn_params),
+    ]:
+        shape = (2, 2)
+        for inputs in [
+            {
+                "a": tvm.nd.array(np.random.uniform(low, high, shape).astype(dtype)),
+                "b": tvm.nd.array(np.random.uniform(low, high, shape).astype(dtype)),
+            }
+        ]:
+            outputs = []
+            func = _get_model(shape, dtype, iter(inputs), op, op_params)
+            for acl in [True, False]:
+                outputs.append(build_and_run(func, inputs, 1, None, device, enable_acl=acl)[0])
+
+            config = {
+                "shape": shape,
+                "dtype": dtype,
+                "inputs": inputs,
+                "operation": op,
+                "op_params": op_params,
+            }
+
+            # verify_saturation=False as the result of add_QASYMM8_QASYMM8_QASYMM8
+            # is always saturated currently.

Review comment:
       ACL's _add_QASYMM8_QASYMM8_QASYMM8_ implemented in a way that it always saturates result. It just cannot be verified as uint8 always cast to QASYMM8. As for the parameter combination which does not cause saturation for qnn.add could you elaborate why this is needed?

##########
File path: src/runtime/contrib/arm_compute_lib/acl_runtime.cc
##########
@@ -417,6 +420,45 @@ class ACLRuntime : public JSONRuntimeBase {
     function->configure(&layer->inputs[0], &layer->inputs[1], &layer->outputs[0]);
     layer->function = function;
   }
+  /*!
+   * \brief Creates an add/qnn.add layer
+   *
+   * \param layer The ACL layer to build. Containing inputs, outputs and the ACL function.
+   * \param node  The JSON representation of the operator.
+   */
+  void CreateAddLayer(CachedLayer* layer, const JSONGraphNode& node) {
+    auto op_name = node.GetOpName();
+    if ("add" == op_name) {
+      layer->inputs.push_back(MakeACLTensorFromJSONEntry(node.GetInputs()[0]));
+      layer->inputs.push_back(MakeACLTensorFromJSONEntry(node.GetInputs()[1]));
+      layer->outputs.push_back(MakeACLTensorFromJSONNode(node));
+    } else if ("qnn.add" == op_name) {
+      layer->inputs.push_back(MakeACLTensorFromJSONEntry(node.GetInputs()[0], &node.GetInputs()[2],
+                                                         &node.GetInputs()[3]));
+      layer->inputs.push_back(MakeACLTensorFromJSONEntry(node.GetInputs()[1], &node.GetInputs()[4],
+                                                         &node.GetInputs()[5]));
+      layer->outputs.push_back(
+          MakeACLTensorFromJSONNode(node, &node.GetInputs()[6], &node.GetInputs()[7]));
+    } else {
+      throw std::runtime_error("Unsupported form of add op: " + op_name);
+    }
+
+    /** Initialise the kernel's inputs, output and conversion policy.

Review comment:
       removed

##########
File path: tests/python/contrib/test_arm_compute_lib/test_add.py
##########
@@ -0,0 +1,135 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Arm Compute Library integration reshape tests."""
+
+import numpy as np
+
+import tvm
+import tvm.testing
+from tvm import relay
+
+from test_arm_compute_lib.infrastructure import (
+    skip_runtime_test,
+    skip_codegen_test,
+    build_and_run,
+    verify,
+    verify_codegen,
+)
+from test_arm_compute_lib.infrastructure import Device
+
+_qnn_params = {
+    "lhs_scale": relay.const(0.0156863, "float32"),
+    "lhs_zero_point": relay.const(127, "int32"),
+    "rhs_scale": relay.const(0.0117647, "float32"),
+    "rhs_zero_point": relay.const(85, "int32"),
+    "output_scale": relay.const(0.0235294, "float32"),
+    "output_zero_point": relay.const(128, "int32"),
+}
+
+
+def _get_model(shape, dtype, var_names, op, op_params):
+    a = relay.var(next(var_names), shape=shape, dtype=dtype)
+    b = relay.var(next(var_names), shape=shape, dtype=dtype)
+    return op(a, b, **op_params)
+
+
+def _get_expected_codegen(shape, dtype, op_name, qnn_params):
+    input_a = {"op": "input", "name": "", "attrs": {"shape": [[list(shape)]], "dtype": [[dtype]]}}
+    input_b = {"op": "input", "name": "", "attrs": {"shape": [[list(shape)]], "dtype": [[dtype]]}}
+    input_qnn = [
+        {
+            "op": "const",
+            "name": "",
+            "attrs": {
+                "shape": [[list(qnn_params[_].data.shape)]],
+                "dtype": [[qnn_params[_].data.dtype]],
+            },
+        }
+        for _ in qnn_params
+    ]
+    inputs = [input_a, input_b, *input_qnn]
+    node = {
+        "op": "kernel",
+        "name": op_name,
+        "inputs": [[_, 0, 0] for _ in range(len(inputs))],
+        "attrs": {
+            "num_inputs": str(len(inputs)),
+            "num_outputs": "1",
+            "shape": [[list(shape)]],
+            "dtype": [[dtype]],
+        },
+    }
+
+    return [*inputs, node]
+
+
+def test_runtime_add():
+    Device.load("test_config.json")
+
+    if skip_runtime_test():
+        return
+
+    device = Device()
+    np.random.seed(0)
+
+    for dtype, low, high, atol, rtol, op, op_params in [
+        ("float32", -127, 128, 1e-7, 1e-7, relay.add, {}),
+        ("uint8", 0, 255, 0.0, 1.0, relay.qnn.op.add, _qnn_params),
+    ]:
+        shape = (2, 2)
+        for inputs in [
+            {
+                "a": tvm.nd.array(np.random.uniform(low, high, shape).astype(dtype)),
+                "b": tvm.nd.array(np.random.uniform(low, high, shape).astype(dtype)),
+            }
+        ]:
+            outputs = []
+            func = _get_model(shape, dtype, iter(inputs), op, op_params)
+            for acl in [True, False]:
+                outputs.append(build_and_run(func, inputs, 1, None, device, enable_acl=acl)[0])
+
+            config = {
+                "shape": shape,
+                "dtype": dtype,
+                "inputs": inputs,
+                "operation": op,
+                "op_params": op_params,
+            }
+
+            # verify_saturation=False as the result of add_QASYMM8_QASYMM8_QASYMM8
+            # is always saturated currently.
+            verify(outputs, atol=atol, rtol=rtol, config=config, verify_saturation=False)
+
+
+def test_runtime_codegen_add():

Review comment:
       operations added to arm_compute_lib.rst
   method renamed




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-tvm] d-smirnov commented on a change in pull request #6532: [BYOC][ACL] Support add operation

Posted by GitBox <gi...@apache.org>.
d-smirnov commented on a change in pull request #6532:
URL: https://github.com/apache/incubator-tvm/pull/6532#discussion_r496104995



##########
File path: python/tvm/relay/op/contrib/arm_compute_lib.py
##########
@@ -345,3 +345,23 @@ def maximum(attrs, args):
     type_a = args[0].checked_type
     type_b = args[0].checked_type
     return (type_a.dtype == "float32") and (type_b.dtype == "float32")
+
+
+@tvm.ir.register_op_attr("add", "target.arm_compute_lib")
+def add(attrs, args):
+    """Check if the external ACL codegen for add should be used."""
+    for typ in [args[0].checked_type, args[1].checked_type]:
+        if typ.dtype not in ["float32"]:

Review comment:
       done




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-tvm] comaniac commented on pull request #6532: [BYOC][ACL] Support add operation

Posted by GitBox <gi...@apache.org>.
comaniac commented on pull request #6532:
URL: https://github.com/apache/incubator-tvm/pull/6532#issuecomment-704622967


   @d-smirnov would you address the comments from @lhutton1 or?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-tvm] d-smirnov commented on a change in pull request #6532: [BYOC][ACL] Support add operation

Posted by GitBox <gi...@apache.org>.
d-smirnov commented on a change in pull request #6532:
URL: https://github.com/apache/incubator-tvm/pull/6532#discussion_r495926658



##########
File path: tests/python/contrib/test_arm_compute_lib/test_add.py
##########
@@ -0,0 +1,142 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Arm Compute Library integration reshape tests."""
+
+import numpy as np
+
+import tvm
+import tvm.testing
+from tvm import relay
+
+from test_arm_compute_lib.infrastructure import (
+    skip_runtime_test,
+    skip_codegen_test,
+    build_and_run,
+    verify,
+    verify_codegen,
+)
+from test_arm_compute_lib.infrastructure import Device
+
+
+def _get_model(shape, dtype, var_names, op, op_params):
+    a = relay.var(next(var_names), shape=shape, dtype=dtype)
+    b = relay.var(next(var_names), shape=shape, dtype=dtype)
+    return op(a, b, **op_params)
+
+
+def _get_expected_codegen(shape, dtype):
+    node = {
+        "op": "kernel",
+        "name": "add",
+        "inputs": [[0, 0, 0], [1, 0, 0]],
+        "attrs": {
+            "num_inputs": "2",
+            "num_outputs": "1",
+            "shape": [[list(shape)]],
+            "dtype": [[dtype]],
+        },
+    }
+
+    input_a = {"op": "input", "name": "", "attrs": {"shape": [[list(shape)]], "dtype": [[dtype]]}}
+    input_b = {"op": "input", "name": "", "attrs": {"shape": [[list(shape)]], "dtype": [[dtype]]}}
+    return [input_a, input_b, node]
+
+
+def test_add():
+    Device.load("test_config.json")
+
+    if skip_runtime_test():
+        return
+
+    device = Device()
+    np.random.seed(0)
+
+    for dtype, low, high, atol, rtol, op, op_params in [
+        ("float32", -127, 128, 1e-7, 1e-7, relay.add, {}),
+        # different qnn params
+        (
+            "uint8",
+            0,
+            255,
+            0.0,
+            1.0,
+            relay.qnn.op.add,
+            {
+                "lhs_scale": relay.const(0.0156863, "float32"),
+                "lhs_zero_point": relay.const(127, "int32"),
+                "rhs_scale": relay.const(0.0117647, "float32"),
+                "rhs_zero_point": relay.const(85, "int32"),
+                "output_scale": relay.const(0.0235294, "float32"),
+                "output_zero_point": relay.const(128, "int32"),
+            },
+        ),
+        # same qnn params
+        (
+            "uint8",
+            0,
+            255,
+            0.0,
+            1.0,
+            relay.qnn.op.add,
+            {
+                "lhs_scale": relay.const(0.0126863, "float32"),
+                "lhs_zero_point": relay.const(127, "int32"),
+                "rhs_scale": relay.const(0.0126863, "float32"),
+                "rhs_zero_point": relay.const(127, "int32"),
+                "output_scale": relay.const(0.0126863, "float32"),
+                "output_zero_point": relay.const(127, "int32"),
+            },
+        ),
+    ]:
+        shape = (2, 2)
+        for inputs in [
+            {
+                "a": tvm.nd.array(np.random.uniform(low, high, shape).astype(dtype)),
+                "b": tvm.nd.array(np.random.uniform(low, high, shape).astype(dtype)),
+            }
+        ]:
+            outputs = []
+            func = _get_model(shape, dtype, iter(inputs), op, op_params)
+            for acl in [True, False]:
+                outputs.append(build_and_run(func, inputs, 1, None, device, enable_acl=acl)[0])
+
+            config = {
+                "shape": shape,
+                "dtype": dtype,
+                "inputs": inputs,
+                "operation": op,
+                "op_params": op_params,
+            }
+            verify(outputs, atol=atol, rtol=rtol, config=config)
+
+
+def test_codegen_add():
+    if skip_codegen_test():
+        return
+
+    shape = (1, 1, 1, 1000)
+    inputs = {"a", "b"}
+    for dtype in ["float32", "uint8"]:
+        for new_shape in [(1, 1000), (10, 10, 10)]:

Review comment:
       fixed




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-tvm] d-smirnov commented on a change in pull request #6532: [BYOC][ACL] Support add operation

Posted by GitBox <gi...@apache.org>.
d-smirnov commented on a change in pull request #6532:
URL: https://github.com/apache/incubator-tvm/pull/6532#discussion_r496104903



##########
File path: python/tvm/relay/op/contrib/arm_compute_lib.py
##########
@@ -345,3 +345,23 @@ def maximum(attrs, args):
     type_a = args[0].checked_type
     type_b = args[0].checked_type
     return (type_a.dtype == "float32") and (type_b.dtype == "float32")
+
+
+@tvm.ir.register_op_attr("add", "target.arm_compute_lib")
+def add(attrs, args):
+    """Check if the external ACL codegen for add should be used."""
+    for typ in [args[0].checked_type, args[1].checked_type]:
+        if typ.dtype not in ["float32"]:
+            return False
+
+    return True
+
+
+@tvm.ir.register_op_attr("qnn.add", "target.arm_compute_lib")
+def qnn_add(attrs, args):
+    """Check if the external ACL codegen for add should be used."""
+    for typ in [args[0].checked_type, args[1].checked_type]:
+        if typ.dtype not in ["uint8"]:

Review comment:
       done




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-tvm] comaniac commented on a change in pull request #6532: ACL support: "add" operation

Posted by GitBox <gi...@apache.org>.
comaniac commented on a change in pull request #6532:
URL: https://github.com/apache/incubator-tvm/pull/6532#discussion_r492903028



##########
File path: python/tvm/relay/op/contrib/arm_compute_lib.py
##########
@@ -161,6 +161,17 @@ def l2_pool2d_pattern():
         pattern = is_op("sqrt")(pattern)
         return pattern
 
+    def add_pattern():
+        """Create an add pattern.
+
+        Returns
+        -------
+        pattern : dataflow_pattern.AltPattern
+            Denotes the convolution pattern.
+        """
+        pattern = is_op("add")(wildcard(), is_constant(), is_constant())

Review comment:
       Why add has 3 arguments? Did I miss something?

##########
File path: python/tvm/relay/op/contrib/arm_compute_lib.py
##########
@@ -161,6 +161,17 @@ def l2_pool2d_pattern():
         pattern = is_op("sqrt")(pattern)
         return pattern
 
+    def add_pattern():
+        """Create an add pattern.
+
+        Returns
+        -------
+        pattern : dataflow_pattern.AltPattern
+            Denotes the convolution pattern.

Review comment:
       ```suggestion
               Denotes the add pattern.
   ```

##########
File path: tests/python/contrib/test_arm_compute_lib/test_add.py
##########
@@ -0,0 +1,142 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Arm Compute Library integration reshape tests."""
+
+import numpy as np
+
+import tvm
+import tvm.testing
+from tvm import relay
+
+from test_arm_compute_lib.infrastructure import (
+    skip_runtime_test,
+    skip_codegen_test,
+    build_and_run,
+    verify,
+    verify_codegen,
+)
+from test_arm_compute_lib.infrastructure import Device
+
+
+def _get_model(shape, dtype, var_names, op, op_params):
+    a = relay.var(next(var_names), shape=shape, dtype=dtype)
+    b = relay.var(next(var_names), shape=shape, dtype=dtype)
+    return op(a, b, **op_params)
+
+
+def _get_expected_codegen(shape, dtype):
+    node = {
+        "op": "kernel",
+        "name": "add",
+        "inputs": [[0, 0, 0], [1, 0, 0]],
+        "attrs": {
+            "num_inputs": "2",
+            "num_outputs": "1",
+            "shape": [[list(shape)]],
+            "dtype": [[dtype]],
+        },
+    }
+
+    input_a = {"op": "input", "name": "", "attrs": {"shape": [[list(shape)]], "dtype": [[dtype]]}}
+    input_b = {"op": "input", "name": "", "attrs": {"shape": [[list(shape)]], "dtype": [[dtype]]}}
+    return [input_a, input_b, node]
+
+
+def test_add():

Review comment:
       Better to be "test_runtime_add".

##########
File path: tests/python/contrib/test_arm_compute_lib/test_add.py
##########
@@ -0,0 +1,142 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Arm Compute Library integration reshape tests."""
+
+import numpy as np
+
+import tvm
+import tvm.testing
+from tvm import relay
+
+from test_arm_compute_lib.infrastructure import (
+    skip_runtime_test,
+    skip_codegen_test,
+    build_and_run,
+    verify,
+    verify_codegen,
+)
+from test_arm_compute_lib.infrastructure import Device
+
+
+def _get_model(shape, dtype, var_names, op, op_params):
+    a = relay.var(next(var_names), shape=shape, dtype=dtype)
+    b = relay.var(next(var_names), shape=shape, dtype=dtype)
+    return op(a, b, **op_params)
+
+
+def _get_expected_codegen(shape, dtype):
+    node = {
+        "op": "kernel",
+        "name": "add",
+        "inputs": [[0, 0, 0], [1, 0, 0]],
+        "attrs": {
+            "num_inputs": "2",
+            "num_outputs": "1",
+            "shape": [[list(shape)]],
+            "dtype": [[dtype]],
+        },
+    }
+
+    input_a = {"op": "input", "name": "", "attrs": {"shape": [[list(shape)]], "dtype": [[dtype]]}}
+    input_b = {"op": "input", "name": "", "attrs": {"shape": [[list(shape)]], "dtype": [[dtype]]}}
+    return [input_a, input_b, node]
+
+
+def test_add():
+    Device.load("test_config.json")
+
+    if skip_runtime_test():
+        return
+
+    device = Device()
+    np.random.seed(0)
+
+    for dtype, low, high, atol, rtol, op, op_params in [
+        ("float32", -127, 128, 1e-7, 1e-7, relay.add, {}),
+        # different qnn params
+        (
+            "uint8",
+            0,
+            255,
+            0.0,
+            1.0,
+            relay.qnn.op.add,
+            {
+                "lhs_scale": relay.const(0.0156863, "float32"),
+                "lhs_zero_point": relay.const(127, "int32"),
+                "rhs_scale": relay.const(0.0117647, "float32"),
+                "rhs_zero_point": relay.const(85, "int32"),
+                "output_scale": relay.const(0.0235294, "float32"),
+                "output_zero_point": relay.const(128, "int32"),
+            },
+        ),
+        # same qnn params
+        (
+            "uint8",
+            0,
+            255,
+            0.0,
+            1.0,
+            relay.qnn.op.add,
+            {
+                "lhs_scale": relay.const(0.0126863, "float32"),
+                "lhs_zero_point": relay.const(127, "int32"),
+                "rhs_scale": relay.const(0.0126863, "float32"),
+                "rhs_zero_point": relay.const(127, "int32"),
+                "output_scale": relay.const(0.0126863, "float32"),
+                "output_zero_point": relay.const(127, "int32"),
+            },
+        ),
+    ]:
+        shape = (2, 2)
+        for inputs in [
+            {
+                "a": tvm.nd.array(np.random.uniform(low, high, shape).astype(dtype)),
+                "b": tvm.nd.array(np.random.uniform(low, high, shape).astype(dtype)),
+            }
+        ]:
+            outputs = []
+            func = _get_model(shape, dtype, iter(inputs), op, op_params)
+            for acl in [True, False]:
+                outputs.append(build_and_run(func, inputs, 1, None, device, enable_acl=acl)[0])
+
+            config = {
+                "shape": shape,
+                "dtype": dtype,
+                "inputs": inputs,
+                "operation": op,
+                "op_params": op_params,
+            }
+            verify(outputs, atol=atol, rtol=rtol, config=config)
+
+
+def test_codegen_add():
+    if skip_codegen_test():
+        return
+
+    shape = (1, 1, 1, 1000)
+    inputs = {"a", "b"}
+    for dtype in ["float32", "uint8"]:
+        for new_shape in [(1, 1000), (10, 10, 10)]:

Review comment:
       `new_shape` is never used...

##########
File path: tests/python/contrib/test_arm_compute_lib/test_add.py
##########
@@ -0,0 +1,142 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Arm Compute Library integration reshape tests."""
+
+import numpy as np
+
+import tvm
+import tvm.testing
+from tvm import relay
+
+from test_arm_compute_lib.infrastructure import (
+    skip_runtime_test,
+    skip_codegen_test,
+    build_and_run,
+    verify,
+    verify_codegen,
+)
+from test_arm_compute_lib.infrastructure import Device
+
+
+def _get_model(shape, dtype, var_names, op, op_params):
+    a = relay.var(next(var_names), shape=shape, dtype=dtype)
+    b = relay.var(next(var_names), shape=shape, dtype=dtype)
+    return op(a, b, **op_params)
+
+
+def _get_expected_codegen(shape, dtype):
+    node = {
+        "op": "kernel",
+        "name": "add",
+        "inputs": [[0, 0, 0], [1, 0, 0]],
+        "attrs": {
+            "num_inputs": "2",
+            "num_outputs": "1",
+            "shape": [[list(shape)]],
+            "dtype": [[dtype]],
+        },
+    }
+
+    input_a = {"op": "input", "name": "", "attrs": {"shape": [[list(shape)]], "dtype": [[dtype]]}}
+    input_b = {"op": "input", "name": "", "attrs": {"shape": [[list(shape)]], "dtype": [[dtype]]}}
+    return [input_a, input_b, node]
+
+
+def test_add():
+    Device.load("test_config.json")
+
+    if skip_runtime_test():
+        return
+
+    device = Device()
+    np.random.seed(0)
+
+    for dtype, low, high, atol, rtol, op, op_params in [
+        ("float32", -127, 128, 1e-7, 1e-7, relay.add, {}),
+        # different qnn params
+        (
+            "uint8",
+            0,
+            255,
+            0.0,
+            1.0,
+            relay.qnn.op.add,
+            {
+                "lhs_scale": relay.const(0.0156863, "float32"),
+                "lhs_zero_point": relay.const(127, "int32"),
+                "rhs_scale": relay.const(0.0117647, "float32"),
+                "rhs_zero_point": relay.const(85, "int32"),
+                "output_scale": relay.const(0.0235294, "float32"),
+                "output_zero_point": relay.const(128, "int32"),
+            },
+        ),
+        # same qnn params
+        (
+            "uint8",
+            0,
+            255,
+            0.0,
+            1.0,
+            relay.qnn.op.add,
+            {
+                "lhs_scale": relay.const(0.0126863, "float32"),
+                "lhs_zero_point": relay.const(127, "int32"),
+                "rhs_scale": relay.const(0.0126863, "float32"),
+                "rhs_zero_point": relay.const(127, "int32"),
+                "output_scale": relay.const(0.0126863, "float32"),
+                "output_zero_point": relay.const(127, "int32"),
+            },
+        ),
+    ]:

Review comment:
       1. This is hard to follow. If possible, could you make a list of 3 elements and iterate the list here?
   2. Could you elaborate the purpose of testing 2 QNN adds?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-tvm] lhutton1 commented on a change in pull request #6532: [BYOC][ACL] Support add operation

Posted by GitBox <gi...@apache.org>.
lhutton1 commented on a change in pull request #6532:
URL: https://github.com/apache/incubator-tvm/pull/6532#discussion_r494901379



##########
File path: tests/python/contrib/test_arm_compute_lib/test_add.py
##########
@@ -0,0 +1,142 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Arm Compute Library integration reshape tests."""
+
+import numpy as np
+
+import tvm
+import tvm.testing
+from tvm import relay
+
+from test_arm_compute_lib.infrastructure import (
+    skip_runtime_test,
+    skip_codegen_test,
+    build_and_run,
+    verify,
+    verify_codegen,
+)
+from test_arm_compute_lib.infrastructure import Device
+
+
+def _get_model(shape, dtype, var_names, op, op_params):
+    a = relay.var(next(var_names), shape=shape, dtype=dtype)
+    b = relay.var(next(var_names), shape=shape, dtype=dtype)
+    return op(a, b, **op_params)
+
+
+def _get_expected_codegen(shape, dtype):
+    node = {
+        "op": "kernel",
+        "name": "add",
+        "inputs": [[0, 0, 0], [1, 0, 0]],
+        "attrs": {
+            "num_inputs": "2",
+            "num_outputs": "1",
+            "shape": [[list(shape)]],
+            "dtype": [[dtype]],
+        },
+    }
+
+    input_a = {"op": "input", "name": "", "attrs": {"shape": [[list(shape)]], "dtype": [[dtype]]}}
+    input_b = {"op": "input", "name": "", "attrs": {"shape": [[list(shape)]], "dtype": [[dtype]]}}
+    return [input_a, input_b, node]
+
+
+def test_add():
+    Device.load("test_config.json")
+
+    if skip_runtime_test():
+        return
+
+    device = Device()
+    np.random.seed(0)
+
+    for dtype, low, high, atol, rtol, op, op_params in [
+        ("float32", -127, 128, 1e-7, 1e-7, relay.add, {}),
+        # different qnn params
+        (
+            "uint8",
+            0,
+            255,
+            0.0,
+            1.0,
+            relay.qnn.op.add,
+            {
+                "lhs_scale": relay.const(0.0156863, "float32"),
+                "lhs_zero_point": relay.const(127, "int32"),
+                "rhs_scale": relay.const(0.0117647, "float32"),
+                "rhs_zero_point": relay.const(85, "int32"),
+                "output_scale": relay.const(0.0235294, "float32"),
+                "output_zero_point": relay.const(128, "int32"),
+            },
+        ),
+        # same qnn params
+        (
+            "uint8",
+            0,
+            255,
+            0.0,
+            1.0,
+            relay.qnn.op.add,
+            {
+                "lhs_scale": relay.const(0.0126863, "float32"),
+                "lhs_zero_point": relay.const(127, "int32"),
+                "rhs_scale": relay.const(0.0126863, "float32"),
+                "rhs_zero_point": relay.const(127, "int32"),
+                "output_scale": relay.const(0.0126863, "float32"),
+                "output_zero_point": relay.const(127, "int32"),
+            },
+        ),
+    ]:
+        shape = (2, 2)
+        for inputs in [
+            {
+                "a": tvm.nd.array(np.random.uniform(low, high, shape).astype(dtype)),
+                "b": tvm.nd.array(np.random.uniform(low, high, shape).astype(dtype)),
+            }
+        ]:
+            outputs = []
+            func = _get_model(shape, dtype, iter(inputs), op, op_params)
+            for acl in [True, False]:
+                outputs.append(build_and_run(func, inputs, 1, None, device, enable_acl=acl)[0])
+
+            config = {
+                "shape": shape,
+                "dtype": dtype,
+                "inputs": inputs,
+                "operation": op,
+                "op_params": op_params,
+            }
+            verify(outputs, atol=atol, rtol=rtol, config=config)

Review comment:
       The intention was to ensure that most of the values output are not saturated i.e. most values are not 255 or 0. I can see why this would cause an issue with test cases of 2x2 as a single value that is 255 or 0 would trigger the asserts. I think the best approach here would be to increase the size of the test case, or to reduce 0.25 down to a value less than 25%?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-tvm] comaniac merged pull request #6532: [BYOC][ACL] Support add operation

Posted by GitBox <gi...@apache.org>.
comaniac merged pull request #6532:
URL: https://github.com/apache/incubator-tvm/pull/6532


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-tvm] comaniac commented on pull request #6532: [BYOC][ACL] Support add operation

Posted by GitBox <gi...@apache.org>.
comaniac commented on pull request #6532:
URL: https://github.com/apache/incubator-tvm/pull/6532#issuecomment-706727726


   Thanks @d-smirnov @lhutton1 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-tvm] lhutton1 commented on a change in pull request #6532: [BYOC][ACL] Support add operation

Posted by GitBox <gi...@apache.org>.
lhutton1 commented on a change in pull request #6532:
URL: https://github.com/apache/incubator-tvm/pull/6532#discussion_r493373422



##########
File path: src/runtime/contrib/arm_compute_lib/acl_runtime.cc
##########
@@ -401,6 +406,50 @@ class ACLRuntime : public JSONRuntimeBase {
     layer->function = function;
   }
 
+  /*!
+   * \brief Creates an add layer
+   *
+   * \param layer The ACL layer to build. Containing inputs, outputs and the ACL function.
+   * \param node  The JSON representation of the operator.
+   */
+  void CreateAddLayer(CachedLayer* layer, const JSONGraphNode& node) {
+    layer->inputs.push_back(MakeACLTensorFromJSONEntry(node.GetInputs()[0]));
+    layer->inputs.push_back(MakeACLTensorFromJSONEntry(node.GetInputs()[1]));
+    layer->outputs.push_back(MakeACLTensorFromJSONNode(node));
+    AppendAddOp(layer);
+  }
+
+  /*!
+   * \brief Creates a qnn.add layer
+   *
+   * \param layer The ACL layer to build. Containing inputs, outputs and the ACL function.
+   * \param node  The JSON representation of the operator.
+   */
+  void CreateQnnAddLayer(CachedLayer* layer, const JSONGraphNode& node) {
+    layer->inputs.push_back(MakeACLTensorFromJSONEntry(node.GetInputs()[0], &node.GetInputs()[2],
+                                                       &node.GetInputs()[3]));
+    layer->inputs.push_back(MakeACLTensorFromJSONEntry(node.GetInputs()[1], &node.GetInputs()[4],
+                                                       &node.GetInputs()[5]));
+    layer->outputs.push_back(
+        MakeACLTensorFromJSONNode(node, &node.GetInputs()[6], &node.GetInputs()[7]));
+    AppendAddOp(layer);
+  }
+
+  void AppendAddOp(CachedLayer* layer) {
+    /** Initialise the kernel's inputs, output and conversion policy.
+     *
+     * @param[in]  input1 First tensor input. Data types supported: U8/QASYMM8/S16/F16/F32
+     * @param[in]  input2 Second tensor input. Data types supported: U8/QASYMM8/S16/F16/F32
+     * @param[out] output Output tensor. Data types supported: U8/QASYMM8/S16/F16/F32
+     * @param[in]  policy Policy to use to handle overflow.
+     * void configure(ITensor *input1, ITensor *input2, ITensor *output, ConvertPolicy policy);
+     */
+    auto f = std::make_shared<arm_compute::NEArithmeticAddition>();
+    f->configure(&layer->inputs[0], &layer->inputs[1], &layer->outputs[0],
+                 arm_compute::ConvertPolicy::WRAP);
+    layer->function = f;
+  }

Review comment:
       I think it would be simpler to cover qnn.add in the same function as add, a bit like the CreateConvolution2DLayer function

##########
File path: tests/python/contrib/test_arm_compute_lib/test_add.py
##########
@@ -0,0 +1,142 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Arm Compute Library integration reshape tests."""
+
+import numpy as np
+
+import tvm
+import tvm.testing
+from tvm import relay
+
+from test_arm_compute_lib.infrastructure import (
+    skip_runtime_test,
+    skip_codegen_test,
+    build_and_run,
+    verify,
+    verify_codegen,
+)
+from test_arm_compute_lib.infrastructure import Device
+
+
+def _get_model(shape, dtype, var_names, op, op_params):
+    a = relay.var(next(var_names), shape=shape, dtype=dtype)
+    b = relay.var(next(var_names), shape=shape, dtype=dtype)
+    return op(a, b, **op_params)
+
+
+def _get_expected_codegen(shape, dtype):
+    node = {
+        "op": "kernel",
+        "name": "add",
+        "inputs": [[0, 0, 0], [1, 0, 0]],
+        "attrs": {
+            "num_inputs": "2",
+            "num_outputs": "1",
+            "shape": [[list(shape)]],
+            "dtype": [[dtype]],
+        },
+    }
+
+    input_a = {"op": "input", "name": "", "attrs": {"shape": [[list(shape)]], "dtype": [[dtype]]}}
+    input_b = {"op": "input", "name": "", "attrs": {"shape": [[list(shape)]], "dtype": [[dtype]]}}
+    return [input_a, input_b, node]
+
+
+def test_add():
+    Device.load("test_config.json")
+
+    if skip_runtime_test():
+        return
+
+    device = Device()
+    np.random.seed(0)
+
+    for dtype, low, high, atol, rtol, op, op_params in [
+        ("float32", -127, 128, 1e-7, 1e-7, relay.add, {}),
+        # different qnn params
+        (
+            "uint8",
+            0,
+            255,
+            0.0,
+            1.0,
+            relay.qnn.op.add,
+            {
+                "lhs_scale": relay.const(0.0156863, "float32"),
+                "lhs_zero_point": relay.const(127, "int32"),
+                "rhs_scale": relay.const(0.0117647, "float32"),
+                "rhs_zero_point": relay.const(85, "int32"),
+                "output_scale": relay.const(0.0235294, "float32"),
+                "output_zero_point": relay.const(128, "int32"),
+            },
+        ),
+        # same qnn params
+        (
+            "uint8",
+            0,
+            255,
+            0.0,
+            1.0,
+            relay.qnn.op.add,
+            {
+                "lhs_scale": relay.const(0.0126863, "float32"),
+                "lhs_zero_point": relay.const(127, "int32"),
+                "rhs_scale": relay.const(0.0126863, "float32"),
+                "rhs_zero_point": relay.const(127, "int32"),
+                "output_scale": relay.const(0.0126863, "float32"),
+                "output_zero_point": relay.const(127, "int32"),
+            },
+        ),
+    ]:
+        shape = (2, 2)
+        for inputs in [
+            {
+                "a": tvm.nd.array(np.random.uniform(low, high, shape).astype(dtype)),
+                "b": tvm.nd.array(np.random.uniform(low, high, shape).astype(dtype)),
+            }
+        ]:
+            outputs = []
+            func = _get_model(shape, dtype, iter(inputs), op, op_params)
+            for acl in [True, False]:
+                outputs.append(build_and_run(func, inputs, 1, None, device, enable_acl=acl)[0])
+
+            config = {
+                "shape": shape,
+                "dtype": dtype,
+                "inputs": inputs,
+                "operation": op,
+                "op_params": op_params,
+            }
+            verify(outputs, atol=atol, rtol=rtol, config=config)

Review comment:
       When we test qnn.add we should also use the parameter `verify_saturation=True`

##########
File path: python/tvm/relay/op/contrib/arm_compute_lib.py
##########
@@ -161,6 +161,17 @@ def l2_pool2d_pattern():
         pattern = is_op("sqrt")(pattern)
         return pattern
 
+    def add_pattern():
+        """Create an add pattern.
+
+        Returns
+        -------
+        pattern : dataflow_pattern.AltPattern
+            Denotes the convolution pattern.
+        """
+        pattern = is_op("add")(wildcard(), is_constant(), is_constant())

Review comment:
       I'm not sure we need to define a pattern for add? as it is a single op we shouldn't need to wrap it within a composite function

##########
File path: tests/python/contrib/test_arm_compute_lib/test_add.py
##########
@@ -0,0 +1,142 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Arm Compute Library integration reshape tests."""
+
+import numpy as np
+
+import tvm
+import tvm.testing
+from tvm import relay
+
+from test_arm_compute_lib.infrastructure import (
+    skip_runtime_test,
+    skip_codegen_test,
+    build_and_run,
+    verify,
+    verify_codegen,
+)
+from test_arm_compute_lib.infrastructure import Device
+
+
+def _get_model(shape, dtype, var_names, op, op_params):
+    a = relay.var(next(var_names), shape=shape, dtype=dtype)
+    b = relay.var(next(var_names), shape=shape, dtype=dtype)
+    return op(a, b, **op_params)
+
+
+def _get_expected_codegen(shape, dtype):
+    node = {
+        "op": "kernel",
+        "name": "add",
+        "inputs": [[0, 0, 0], [1, 0, 0]],
+        "attrs": {
+            "num_inputs": "2",
+            "num_outputs": "1",
+            "shape": [[list(shape)]],
+            "dtype": [[dtype]],
+        },
+    }
+
+    input_a = {"op": "input", "name": "", "attrs": {"shape": [[list(shape)]], "dtype": [[dtype]]}}
+    input_b = {"op": "input", "name": "", "attrs": {"shape": [[list(shape)]], "dtype": [[dtype]]}}
+    return [input_a, input_b, node]
+
+
+def test_add():
+    Device.load("test_config.json")
+
+    if skip_runtime_test():
+        return
+
+    device = Device()
+    np.random.seed(0)
+
+    for dtype, low, high, atol, rtol, op, op_params in [
+        ("float32", -127, 128, 1e-7, 1e-7, relay.add, {}),
+        # different qnn params
+        (
+            "uint8",
+            0,
+            255,
+            0.0,
+            1.0,
+            relay.qnn.op.add,
+            {
+                "lhs_scale": relay.const(0.0156863, "float32"),
+                "lhs_zero_point": relay.const(127, "int32"),
+                "rhs_scale": relay.const(0.0117647, "float32"),
+                "rhs_zero_point": relay.const(85, "int32"),
+                "output_scale": relay.const(0.0235294, "float32"),
+                "output_zero_point": relay.const(128, "int32"),
+            },
+        ),
+        # same qnn params
+        (
+            "uint8",
+            0,
+            255,
+            0.0,
+            1.0,
+            relay.qnn.op.add,
+            {
+                "lhs_scale": relay.const(0.0126863, "float32"),
+                "lhs_zero_point": relay.const(127, "int32"),
+                "rhs_scale": relay.const(0.0126863, "float32"),
+                "rhs_zero_point": relay.const(127, "int32"),
+                "output_scale": relay.const(0.0126863, "float32"),
+                "output_zero_point": relay.const(127, "int32"),
+            },
+        ),
+    ]:

Review comment:
       Why not test `add`?

##########
File path: tests/python/contrib/test_arm_compute_lib/test_add.py
##########
@@ -0,0 +1,142 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Arm Compute Library integration reshape tests."""
+
+import numpy as np
+
+import tvm
+import tvm.testing
+from tvm import relay
+
+from test_arm_compute_lib.infrastructure import (
+    skip_runtime_test,
+    skip_codegen_test,
+    build_and_run,
+    verify,
+    verify_codegen,
+)
+from test_arm_compute_lib.infrastructure import Device
+
+
+def _get_model(shape, dtype, var_names, op, op_params):
+    a = relay.var(next(var_names), shape=shape, dtype=dtype)
+    b = relay.var(next(var_names), shape=shape, dtype=dtype)
+    return op(a, b, **op_params)
+
+
+def _get_expected_codegen(shape, dtype):
+    node = {
+        "op": "kernel",
+        "name": "add",
+        "inputs": [[0, 0, 0], [1, 0, 0]],
+        "attrs": {
+            "num_inputs": "2",
+            "num_outputs": "1",
+            "shape": [[list(shape)]],
+            "dtype": [[dtype]],
+        },
+    }
+
+    input_a = {"op": "input", "name": "", "attrs": {"shape": [[list(shape)]], "dtype": [[dtype]]}}
+    input_b = {"op": "input", "name": "", "attrs": {"shape": [[list(shape)]], "dtype": [[dtype]]}}
+    return [input_a, input_b, node]
+
+
+def test_add():
+    Device.load("test_config.json")
+
+    if skip_runtime_test():
+        return
+
+    device = Device()
+    np.random.seed(0)
+
+    for dtype, low, high, atol, rtol, op, op_params in [
+        ("float32", -127, 128, 1e-7, 1e-7, relay.add, {}),
+        # different qnn params
+        (
+            "uint8",
+            0,
+            255,
+            0.0,
+            1.0,
+            relay.qnn.op.add,
+            {
+                "lhs_scale": relay.const(0.0156863, "float32"),
+                "lhs_zero_point": relay.const(127, "int32"),
+                "rhs_scale": relay.const(0.0117647, "float32"),
+                "rhs_zero_point": relay.const(85, "int32"),
+                "output_scale": relay.const(0.0235294, "float32"),
+                "output_zero_point": relay.const(128, "int32"),
+            },
+        ),
+        # same qnn params
+        (
+            "uint8",
+            0,
+            255,
+            0.0,
+            1.0,
+            relay.qnn.op.add,
+            {
+                "lhs_scale": relay.const(0.0126863, "float32"),
+                "lhs_zero_point": relay.const(127, "int32"),
+                "rhs_scale": relay.const(0.0126863, "float32"),
+                "rhs_zero_point": relay.const(127, "int32"),
+                "output_scale": relay.const(0.0126863, "float32"),
+                "output_zero_point": relay.const(127, "int32"),
+            },
+        ),
+    ]:
+        shape = (2, 2)
+        for inputs in [
+            {
+                "a": tvm.nd.array(np.random.uniform(low, high, shape).astype(dtype)),
+                "b": tvm.nd.array(np.random.uniform(low, high, shape).astype(dtype)),
+            }
+        ]:
+            outputs = []
+            func = _get_model(shape, dtype, iter(inputs), op, op_params)
+            for acl in [True, False]:
+                outputs.append(build_and_run(func, inputs, 1, None, device, enable_acl=acl)[0])
+
+            config = {
+                "shape": shape,
+                "dtype": dtype,
+                "inputs": inputs,
+                "operation": op,
+                "op_params": op_params,
+            }
+            verify(outputs, atol=atol, rtol=rtol, config=config)
+
+
+def test_codegen_add():
+    if skip_codegen_test():
+        return
+
+    shape = (1, 1, 1, 1000)
+    inputs = {"a", "b"}
+    for dtype in ["float32", "uint8"]:
+        for new_shape in [(1, 1000), (10, 10, 10)]:
+            func = _get_model(shape, dtype, iter(inputs), relay.add, {})

Review comment:
       Why not test `qnn.add`?

##########
File path: python/tvm/relay/op/contrib/arm_compute_lib.py
##########
@@ -337,3 +354,17 @@ def global_avg_pool2d(attrs, args):
     if attrs.layout != "NHWC":
         return False
     return True
+
+
+@tvm.ir.register_op_attr("add", "target.arm_compute_lib")
+@tvm.ir.register_op_attr("qnn.add", "target.arm_compute_lib")
+def add(attrs, args):
+    """Check if the external ACL codegen for add should be used."""
+    for typ in [args[0].checked_type, args[1].checked_type]:
+        if typ.dtype not in ["float32", "uint8"]:

Review comment:
       I think it would be better to write add and qnn.add separately. This way we can state add must have float32 input tensors and qnn.add must have uint8 inputs. Currently we allow a mixture of types.

##########
File path: tests/python/contrib/test_arm_compute_lib/test_add.py
##########
@@ -0,0 +1,142 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Arm Compute Library integration reshape tests."""
+
+import numpy as np
+
+import tvm
+import tvm.testing
+from tvm import relay
+
+from test_arm_compute_lib.infrastructure import (
+    skip_runtime_test,
+    skip_codegen_test,
+    build_and_run,
+    verify,
+    verify_codegen,
+)
+from test_arm_compute_lib.infrastructure import Device
+
+
+def _get_model(shape, dtype, var_names, op, op_params):
+    a = relay.var(next(var_names), shape=shape, dtype=dtype)
+    b = relay.var(next(var_names), shape=shape, dtype=dtype)
+    return op(a, b, **op_params)
+
+
+def _get_expected_codegen(shape, dtype):
+    node = {
+        "op": "kernel",
+        "name": "add",
+        "inputs": [[0, 0, 0], [1, 0, 0]],
+        "attrs": {
+            "num_inputs": "2",
+            "num_outputs": "1",
+            "shape": [[list(shape)]],
+            "dtype": [[dtype]],
+        },
+    }
+
+    input_a = {"op": "input", "name": "", "attrs": {"shape": [[list(shape)]], "dtype": [[dtype]]}}
+    input_b = {"op": "input", "name": "", "attrs": {"shape": [[list(shape)]], "dtype": [[dtype]]}}
+    return [input_a, input_b, node]
+
+
+def test_add():

Review comment:
       Probably best to do this for the rest of the tests also




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-tvm] d-smirnov commented on a change in pull request #6532: [BYOC][ACL] Support add operation

Posted by GitBox <gi...@apache.org>.
d-smirnov commented on a change in pull request #6532:
URL: https://github.com/apache/incubator-tvm/pull/6532#discussion_r494835086



##########
File path: python/tvm/relay/op/contrib/arm_compute_lib.py
##########
@@ -161,6 +161,17 @@ def l2_pool2d_pattern():
         pattern = is_op("sqrt")(pattern)
         return pattern
 
+    def add_pattern():
+        """Create an add pattern.
+
+        Returns
+        -------
+        pattern : dataflow_pattern.AltPattern
+            Denotes the convolution pattern.
+        """
+        pattern = is_op("add")(wildcard(), is_constant(), is_constant())

Review comment:
       removed




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-tvm] d-smirnov commented on a change in pull request #6532: [BYOC][ACL] Support add operation

Posted by GitBox <gi...@apache.org>.
d-smirnov commented on a change in pull request #6532:
URL: https://github.com/apache/incubator-tvm/pull/6532#discussion_r494853126



##########
File path: tests/python/contrib/test_arm_compute_lib/test_add.py
##########
@@ -0,0 +1,142 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Arm Compute Library integration reshape tests."""
+
+import numpy as np
+
+import tvm
+import tvm.testing
+from tvm import relay
+
+from test_arm_compute_lib.infrastructure import (
+    skip_runtime_test,
+    skip_codegen_test,
+    build_and_run,
+    verify,
+    verify_codegen,
+)
+from test_arm_compute_lib.infrastructure import Device
+
+
+def _get_model(shape, dtype, var_names, op, op_params):
+    a = relay.var(next(var_names), shape=shape, dtype=dtype)
+    b = relay.var(next(var_names), shape=shape, dtype=dtype)
+    return op(a, b, **op_params)
+
+
+def _get_expected_codegen(shape, dtype):
+    node = {
+        "op": "kernel",
+        "name": "add",
+        "inputs": [[0, 0, 0], [1, 0, 0]],
+        "attrs": {
+            "num_inputs": "2",
+            "num_outputs": "1",
+            "shape": [[list(shape)]],
+            "dtype": [[dtype]],
+        },
+    }
+
+    input_a = {"op": "input", "name": "", "attrs": {"shape": [[list(shape)]], "dtype": [[dtype]]}}
+    input_b = {"op": "input", "name": "", "attrs": {"shape": [[list(shape)]], "dtype": [[dtype]]}}
+    return [input_a, input_b, node]
+
+
+def test_add():
+    Device.load("test_config.json")
+
+    if skip_runtime_test():
+        return
+
+    device = Device()
+    np.random.seed(0)
+
+    for dtype, low, high, atol, rtol, op, op_params in [
+        ("float32", -127, 128, 1e-7, 1e-7, relay.add, {}),
+        # different qnn params
+        (
+            "uint8",
+            0,
+            255,
+            0.0,
+            1.0,
+            relay.qnn.op.add,
+            {
+                "lhs_scale": relay.const(0.0156863, "float32"),
+                "lhs_zero_point": relay.const(127, "int32"),
+                "rhs_scale": relay.const(0.0117647, "float32"),
+                "rhs_zero_point": relay.const(85, "int32"),
+                "output_scale": relay.const(0.0235294, "float32"),
+                "output_zero_point": relay.const(128, "int32"),
+            },
+        ),
+        # same qnn params
+        (
+            "uint8",
+            0,
+            255,
+            0.0,
+            1.0,
+            relay.qnn.op.add,
+            {
+                "lhs_scale": relay.const(0.0126863, "float32"),
+                "lhs_zero_point": relay.const(127, "int32"),
+                "rhs_scale": relay.const(0.0126863, "float32"),
+                "rhs_zero_point": relay.const(127, "int32"),
+                "output_scale": relay.const(0.0126863, "float32"),
+                "output_zero_point": relay.const(127, "int32"),
+            },
+        ),
+    ]:
+        shape = (2, 2)
+        for inputs in [
+            {
+                "a": tvm.nd.array(np.random.uniform(low, high, shape).astype(dtype)),
+                "b": tvm.nd.array(np.random.uniform(low, high, shape).astype(dtype)),
+            }
+        ]:
+            outputs = []
+            func = _get_model(shape, dtype, iter(inputs), op, op_params)
+            for acl in [True, False]:
+                outputs.append(build_and_run(func, inputs, 1, None, device, enable_acl=acl)[0])
+
+            config = {
+                "shape": shape,
+                "dtype": dtype,
+                "inputs": inputs,
+                "operation": op,
+                "op_params": op_params,
+            }
+            verify(outputs, atol=atol, rtol=rtol, config=config)

Review comment:
       Could you elaborate the assertion of "0.25 * outs[0].asnumpy().size" for small sized (e.g. (2,2) or similar ) test cases, please? 
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-tvm] comaniac commented on a change in pull request #6532: ACL support: "add" operation

Posted by GitBox <gi...@apache.org>.
comaniac commented on a change in pull request #6532:
URL: https://github.com/apache/incubator-tvm/pull/6532#discussion_r492903028



##########
File path: python/tvm/relay/op/contrib/arm_compute_lib.py
##########
@@ -161,6 +161,17 @@ def l2_pool2d_pattern():
         pattern = is_op("sqrt")(pattern)
         return pattern
 
+    def add_pattern():
+        """Create an add pattern.
+
+        Returns
+        -------
+        pattern : dataflow_pattern.AltPattern
+            Denotes the convolution pattern.
+        """
+        pattern = is_op("add")(wildcard(), is_constant(), is_constant())

Review comment:
       Why add has 3 arguments? Did I miss something?

##########
File path: python/tvm/relay/op/contrib/arm_compute_lib.py
##########
@@ -161,6 +161,17 @@ def l2_pool2d_pattern():
         pattern = is_op("sqrt")(pattern)
         return pattern
 
+    def add_pattern():
+        """Create an add pattern.
+
+        Returns
+        -------
+        pattern : dataflow_pattern.AltPattern
+            Denotes the convolution pattern.

Review comment:
       ```suggestion
               Denotes the add pattern.
   ```

##########
File path: tests/python/contrib/test_arm_compute_lib/test_add.py
##########
@@ -0,0 +1,142 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Arm Compute Library integration reshape tests."""
+
+import numpy as np
+
+import tvm
+import tvm.testing
+from tvm import relay
+
+from test_arm_compute_lib.infrastructure import (
+    skip_runtime_test,
+    skip_codegen_test,
+    build_and_run,
+    verify,
+    verify_codegen,
+)
+from test_arm_compute_lib.infrastructure import Device
+
+
+def _get_model(shape, dtype, var_names, op, op_params):
+    a = relay.var(next(var_names), shape=shape, dtype=dtype)
+    b = relay.var(next(var_names), shape=shape, dtype=dtype)
+    return op(a, b, **op_params)
+
+
+def _get_expected_codegen(shape, dtype):
+    node = {
+        "op": "kernel",
+        "name": "add",
+        "inputs": [[0, 0, 0], [1, 0, 0]],
+        "attrs": {
+            "num_inputs": "2",
+            "num_outputs": "1",
+            "shape": [[list(shape)]],
+            "dtype": [[dtype]],
+        },
+    }
+
+    input_a = {"op": "input", "name": "", "attrs": {"shape": [[list(shape)]], "dtype": [[dtype]]}}
+    input_b = {"op": "input", "name": "", "attrs": {"shape": [[list(shape)]], "dtype": [[dtype]]}}
+    return [input_a, input_b, node]
+
+
+def test_add():

Review comment:
       Better to be "test_runtime_add".

##########
File path: tests/python/contrib/test_arm_compute_lib/test_add.py
##########
@@ -0,0 +1,142 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Arm Compute Library integration reshape tests."""
+
+import numpy as np
+
+import tvm
+import tvm.testing
+from tvm import relay
+
+from test_arm_compute_lib.infrastructure import (
+    skip_runtime_test,
+    skip_codegen_test,
+    build_and_run,
+    verify,
+    verify_codegen,
+)
+from test_arm_compute_lib.infrastructure import Device
+
+
+def _get_model(shape, dtype, var_names, op, op_params):
+    a = relay.var(next(var_names), shape=shape, dtype=dtype)
+    b = relay.var(next(var_names), shape=shape, dtype=dtype)
+    return op(a, b, **op_params)
+
+
+def _get_expected_codegen(shape, dtype):
+    node = {
+        "op": "kernel",
+        "name": "add",
+        "inputs": [[0, 0, 0], [1, 0, 0]],
+        "attrs": {
+            "num_inputs": "2",
+            "num_outputs": "1",
+            "shape": [[list(shape)]],
+            "dtype": [[dtype]],
+        },
+    }
+
+    input_a = {"op": "input", "name": "", "attrs": {"shape": [[list(shape)]], "dtype": [[dtype]]}}
+    input_b = {"op": "input", "name": "", "attrs": {"shape": [[list(shape)]], "dtype": [[dtype]]}}
+    return [input_a, input_b, node]
+
+
+def test_add():
+    Device.load("test_config.json")
+
+    if skip_runtime_test():
+        return
+
+    device = Device()
+    np.random.seed(0)
+
+    for dtype, low, high, atol, rtol, op, op_params in [
+        ("float32", -127, 128, 1e-7, 1e-7, relay.add, {}),
+        # different qnn params
+        (
+            "uint8",
+            0,
+            255,
+            0.0,
+            1.0,
+            relay.qnn.op.add,
+            {
+                "lhs_scale": relay.const(0.0156863, "float32"),
+                "lhs_zero_point": relay.const(127, "int32"),
+                "rhs_scale": relay.const(0.0117647, "float32"),
+                "rhs_zero_point": relay.const(85, "int32"),
+                "output_scale": relay.const(0.0235294, "float32"),
+                "output_zero_point": relay.const(128, "int32"),
+            },
+        ),
+        # same qnn params
+        (
+            "uint8",
+            0,
+            255,
+            0.0,
+            1.0,
+            relay.qnn.op.add,
+            {
+                "lhs_scale": relay.const(0.0126863, "float32"),
+                "lhs_zero_point": relay.const(127, "int32"),
+                "rhs_scale": relay.const(0.0126863, "float32"),
+                "rhs_zero_point": relay.const(127, "int32"),
+                "output_scale": relay.const(0.0126863, "float32"),
+                "output_zero_point": relay.const(127, "int32"),
+            },
+        ),
+    ]:
+        shape = (2, 2)
+        for inputs in [
+            {
+                "a": tvm.nd.array(np.random.uniform(low, high, shape).astype(dtype)),
+                "b": tvm.nd.array(np.random.uniform(low, high, shape).astype(dtype)),
+            }
+        ]:
+            outputs = []
+            func = _get_model(shape, dtype, iter(inputs), op, op_params)
+            for acl in [True, False]:
+                outputs.append(build_and_run(func, inputs, 1, None, device, enable_acl=acl)[0])
+
+            config = {
+                "shape": shape,
+                "dtype": dtype,
+                "inputs": inputs,
+                "operation": op,
+                "op_params": op_params,
+            }
+            verify(outputs, atol=atol, rtol=rtol, config=config)
+
+
+def test_codegen_add():
+    if skip_codegen_test():
+        return
+
+    shape = (1, 1, 1, 1000)
+    inputs = {"a", "b"}
+    for dtype in ["float32", "uint8"]:
+        for new_shape in [(1, 1000), (10, 10, 10)]:

Review comment:
       `new_shape` is never used...

##########
File path: tests/python/contrib/test_arm_compute_lib/test_add.py
##########
@@ -0,0 +1,142 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Arm Compute Library integration reshape tests."""
+
+import numpy as np
+
+import tvm
+import tvm.testing
+from tvm import relay
+
+from test_arm_compute_lib.infrastructure import (
+    skip_runtime_test,
+    skip_codegen_test,
+    build_and_run,
+    verify,
+    verify_codegen,
+)
+from test_arm_compute_lib.infrastructure import Device
+
+
+def _get_model(shape, dtype, var_names, op, op_params):
+    a = relay.var(next(var_names), shape=shape, dtype=dtype)
+    b = relay.var(next(var_names), shape=shape, dtype=dtype)
+    return op(a, b, **op_params)
+
+
+def _get_expected_codegen(shape, dtype):
+    node = {
+        "op": "kernel",
+        "name": "add",
+        "inputs": [[0, 0, 0], [1, 0, 0]],
+        "attrs": {
+            "num_inputs": "2",
+            "num_outputs": "1",
+            "shape": [[list(shape)]],
+            "dtype": [[dtype]],
+        },
+    }
+
+    input_a = {"op": "input", "name": "", "attrs": {"shape": [[list(shape)]], "dtype": [[dtype]]}}
+    input_b = {"op": "input", "name": "", "attrs": {"shape": [[list(shape)]], "dtype": [[dtype]]}}
+    return [input_a, input_b, node]
+
+
+def test_add():
+    Device.load("test_config.json")
+
+    if skip_runtime_test():
+        return
+
+    device = Device()
+    np.random.seed(0)
+
+    for dtype, low, high, atol, rtol, op, op_params in [
+        ("float32", -127, 128, 1e-7, 1e-7, relay.add, {}),
+        # different qnn params
+        (
+            "uint8",
+            0,
+            255,
+            0.0,
+            1.0,
+            relay.qnn.op.add,
+            {
+                "lhs_scale": relay.const(0.0156863, "float32"),
+                "lhs_zero_point": relay.const(127, "int32"),
+                "rhs_scale": relay.const(0.0117647, "float32"),
+                "rhs_zero_point": relay.const(85, "int32"),
+                "output_scale": relay.const(0.0235294, "float32"),
+                "output_zero_point": relay.const(128, "int32"),
+            },
+        ),
+        # same qnn params
+        (
+            "uint8",
+            0,
+            255,
+            0.0,
+            1.0,
+            relay.qnn.op.add,
+            {
+                "lhs_scale": relay.const(0.0126863, "float32"),
+                "lhs_zero_point": relay.const(127, "int32"),
+                "rhs_scale": relay.const(0.0126863, "float32"),
+                "rhs_zero_point": relay.const(127, "int32"),
+                "output_scale": relay.const(0.0126863, "float32"),
+                "output_zero_point": relay.const(127, "int32"),
+            },
+        ),
+    ]:

Review comment:
       1. This is hard to follow. If possible, could you make a list of 3 elements and iterate the list here?
   2. Could you elaborate the purpose of testing 2 QNN adds?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-tvm] lhutton1 commented on a change in pull request #6532: [BYOC][ACL] Support add operation

Posted by GitBox <gi...@apache.org>.
lhutton1 commented on a change in pull request #6532:
URL: https://github.com/apache/incubator-tvm/pull/6532#discussion_r494901379



##########
File path: tests/python/contrib/test_arm_compute_lib/test_add.py
##########
@@ -0,0 +1,142 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Arm Compute Library integration reshape tests."""
+
+import numpy as np
+
+import tvm
+import tvm.testing
+from tvm import relay
+
+from test_arm_compute_lib.infrastructure import (
+    skip_runtime_test,
+    skip_codegen_test,
+    build_and_run,
+    verify,
+    verify_codegen,
+)
+from test_arm_compute_lib.infrastructure import Device
+
+
+def _get_model(shape, dtype, var_names, op, op_params):
+    a = relay.var(next(var_names), shape=shape, dtype=dtype)
+    b = relay.var(next(var_names), shape=shape, dtype=dtype)
+    return op(a, b, **op_params)
+
+
+def _get_expected_codegen(shape, dtype):
+    node = {
+        "op": "kernel",
+        "name": "add",
+        "inputs": [[0, 0, 0], [1, 0, 0]],
+        "attrs": {
+            "num_inputs": "2",
+            "num_outputs": "1",
+            "shape": [[list(shape)]],
+            "dtype": [[dtype]],
+        },
+    }
+
+    input_a = {"op": "input", "name": "", "attrs": {"shape": [[list(shape)]], "dtype": [[dtype]]}}
+    input_b = {"op": "input", "name": "", "attrs": {"shape": [[list(shape)]], "dtype": [[dtype]]}}
+    return [input_a, input_b, node]
+
+
+def test_add():
+    Device.load("test_config.json")
+
+    if skip_runtime_test():
+        return
+
+    device = Device()
+    np.random.seed(0)
+
+    for dtype, low, high, atol, rtol, op, op_params in [
+        ("float32", -127, 128, 1e-7, 1e-7, relay.add, {}),
+        # different qnn params
+        (
+            "uint8",
+            0,
+            255,
+            0.0,
+            1.0,
+            relay.qnn.op.add,
+            {
+                "lhs_scale": relay.const(0.0156863, "float32"),
+                "lhs_zero_point": relay.const(127, "int32"),
+                "rhs_scale": relay.const(0.0117647, "float32"),
+                "rhs_zero_point": relay.const(85, "int32"),
+                "output_scale": relay.const(0.0235294, "float32"),
+                "output_zero_point": relay.const(128, "int32"),
+            },
+        ),
+        # same qnn params
+        (
+            "uint8",
+            0,
+            255,
+            0.0,
+            1.0,
+            relay.qnn.op.add,
+            {
+                "lhs_scale": relay.const(0.0126863, "float32"),
+                "lhs_zero_point": relay.const(127, "int32"),
+                "rhs_scale": relay.const(0.0126863, "float32"),
+                "rhs_zero_point": relay.const(127, "int32"),
+                "output_scale": relay.const(0.0126863, "float32"),
+                "output_zero_point": relay.const(127, "int32"),
+            },
+        ),
+    ]:
+        shape = (2, 2)
+        for inputs in [
+            {
+                "a": tvm.nd.array(np.random.uniform(low, high, shape).astype(dtype)),
+                "b": tvm.nd.array(np.random.uniform(low, high, shape).astype(dtype)),
+            }
+        ]:
+            outputs = []
+            func = _get_model(shape, dtype, iter(inputs), op, op_params)
+            for acl in [True, False]:
+                outputs.append(build_and_run(func, inputs, 1, None, device, enable_acl=acl)[0])
+
+            config = {
+                "shape": shape,
+                "dtype": dtype,
+                "inputs": inputs,
+                "operation": op,
+                "op_params": op_params,
+            }
+            verify(outputs, atol=atol, rtol=rtol, config=config)

Review comment:
       The intention was to ensure that most of the values output are not saturated i.e. most values are not 255 or 0. I can see why this would cause an issue with test cases of 2x2 as a single value that is 255 or 0 would trigger the asserts. I think the best approach here would be to increase the size of the test case?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-tvm] d-smirnov commented on a change in pull request #6532: [BYOC][ACL] Support add operation

Posted by GitBox <gi...@apache.org>.
d-smirnov commented on a change in pull request #6532:
URL: https://github.com/apache/incubator-tvm/pull/6532#discussion_r494841239



##########
File path: tests/python/contrib/test_arm_compute_lib/test_add.py
##########
@@ -0,0 +1,142 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Arm Compute Library integration reshape tests."""
+
+import numpy as np
+
+import tvm
+import tvm.testing
+from tvm import relay
+
+from test_arm_compute_lib.infrastructure import (
+    skip_runtime_test,
+    skip_codegen_test,
+    build_and_run,
+    verify,
+    verify_codegen,
+)
+from test_arm_compute_lib.infrastructure import Device
+
+
+def _get_model(shape, dtype, var_names, op, op_params):
+    a = relay.var(next(var_names), shape=shape, dtype=dtype)
+    b = relay.var(next(var_names), shape=shape, dtype=dtype)
+    return op(a, b, **op_params)
+
+
+def _get_expected_codegen(shape, dtype):
+    node = {
+        "op": "kernel",
+        "name": "add",
+        "inputs": [[0, 0, 0], [1, 0, 0]],
+        "attrs": {
+            "num_inputs": "2",
+            "num_outputs": "1",
+            "shape": [[list(shape)]],
+            "dtype": [[dtype]],
+        },
+    }
+
+    input_a = {"op": "input", "name": "", "attrs": {"shape": [[list(shape)]], "dtype": [[dtype]]}}
+    input_b = {"op": "input", "name": "", "attrs": {"shape": [[list(shape)]], "dtype": [[dtype]]}}
+    return [input_a, input_b, node]
+
+
+def test_add():
+    Device.load("test_config.json")
+
+    if skip_runtime_test():
+        return
+
+    device = Device()
+    np.random.seed(0)
+
+    for dtype, low, high, atol, rtol, op, op_params in [
+        ("float32", -127, 128, 1e-7, 1e-7, relay.add, {}),
+        # different qnn params
+        (
+            "uint8",
+            0,
+            255,
+            0.0,
+            1.0,
+            relay.qnn.op.add,
+            {
+                "lhs_scale": relay.const(0.0156863, "float32"),
+                "lhs_zero_point": relay.const(127, "int32"),
+                "rhs_scale": relay.const(0.0117647, "float32"),
+                "rhs_zero_point": relay.const(85, "int32"),
+                "output_scale": relay.const(0.0235294, "float32"),
+                "output_zero_point": relay.const(128, "int32"),
+            },
+        ),
+        # same qnn params
+        (
+            "uint8",
+            0,
+            255,
+            0.0,
+            1.0,
+            relay.qnn.op.add,
+            {
+                "lhs_scale": relay.const(0.0126863, "float32"),
+                "lhs_zero_point": relay.const(127, "int32"),
+                "rhs_scale": relay.const(0.0126863, "float32"),
+                "rhs_zero_point": relay.const(127, "int32"),
+                "output_scale": relay.const(0.0126863, "float32"),
+                "output_zero_point": relay.const(127, "int32"),
+            },
+        ),
+    ]:
+        shape = (2, 2)
+        for inputs in [
+            {
+                "a": tvm.nd.array(np.random.uniform(low, high, shape).astype(dtype)),
+                "b": tvm.nd.array(np.random.uniform(low, high, shape).astype(dtype)),
+            }
+        ]:
+            outputs = []
+            func = _get_model(shape, dtype, iter(inputs), op, op_params)
+            for acl in [True, False]:
+                outputs.append(build_and_run(func, inputs, 1, None, device, enable_acl=acl)[0])
+
+            config = {
+                "shape": shape,
+                "dtype": dtype,
+                "inputs": inputs,
+                "operation": op,
+                "op_params": op_params,
+            }
+            verify(outputs, atol=atol, rtol=rtol, config=config)
+
+
+def test_codegen_add():
+    if skip_codegen_test():
+        return
+
+    shape = (1, 1, 1, 1000)
+    inputs = {"a", "b"}
+    for dtype in ["float32", "uint8"]:
+        for new_shape in [(1, 1000), (10, 10, 10)]:
+            func = _get_model(shape, dtype, iter(inputs), relay.add, {})

Review comment:
       Added




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-tvm] d-smirnov commented on a change in pull request #6532: [BYOC][ACL] Support add operation

Posted by GitBox <gi...@apache.org>.
d-smirnov commented on a change in pull request #6532:
URL: https://github.com/apache/incubator-tvm/pull/6532#discussion_r495930108



##########
File path: tests/python/contrib/test_arm_compute_lib/test_add.py
##########
@@ -0,0 +1,142 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Arm Compute Library integration reshape tests."""
+
+import numpy as np
+
+import tvm
+import tvm.testing
+from tvm import relay
+
+from test_arm_compute_lib.infrastructure import (
+    skip_runtime_test,
+    skip_codegen_test,
+    build_and_run,
+    verify,
+    verify_codegen,
+)
+from test_arm_compute_lib.infrastructure import Device
+
+
+def _get_model(shape, dtype, var_names, op, op_params):
+    a = relay.var(next(var_names), shape=shape, dtype=dtype)
+    b = relay.var(next(var_names), shape=shape, dtype=dtype)
+    return op(a, b, **op_params)
+
+
+def _get_expected_codegen(shape, dtype):
+    node = {
+        "op": "kernel",
+        "name": "add",
+        "inputs": [[0, 0, 0], [1, 0, 0]],
+        "attrs": {
+            "num_inputs": "2",
+            "num_outputs": "1",
+            "shape": [[list(shape)]],
+            "dtype": [[dtype]],
+        },
+    }
+
+    input_a = {"op": "input", "name": "", "attrs": {"shape": [[list(shape)]], "dtype": [[dtype]]}}
+    input_b = {"op": "input", "name": "", "attrs": {"shape": [[list(shape)]], "dtype": [[dtype]]}}
+    return [input_a, input_b, node]
+
+
+def test_add():
+    Device.load("test_config.json")
+
+    if skip_runtime_test():
+        return
+
+    device = Device()
+    np.random.seed(0)
+
+    for dtype, low, high, atol, rtol, op, op_params in [
+        ("float32", -127, 128, 1e-7, 1e-7, relay.add, {}),
+        # different qnn params
+        (
+            "uint8",
+            0,
+            255,
+            0.0,
+            1.0,
+            relay.qnn.op.add,
+            {
+                "lhs_scale": relay.const(0.0156863, "float32"),
+                "lhs_zero_point": relay.const(127, "int32"),
+                "rhs_scale": relay.const(0.0117647, "float32"),
+                "rhs_zero_point": relay.const(85, "int32"),
+                "output_scale": relay.const(0.0235294, "float32"),
+                "output_zero_point": relay.const(128, "int32"),
+            },
+        ),
+        # same qnn params
+        (
+            "uint8",
+            0,
+            255,
+            0.0,
+            1.0,
+            relay.qnn.op.add,
+            {
+                "lhs_scale": relay.const(0.0126863, "float32"),
+                "lhs_zero_point": relay.const(127, "int32"),
+                "rhs_scale": relay.const(0.0126863, "float32"),
+                "rhs_zero_point": relay.const(127, "int32"),
+                "output_scale": relay.const(0.0126863, "float32"),
+                "output_zero_point": relay.const(127, "int32"),
+            },
+        ),
+    ]:
+        shape = (2, 2)
+        for inputs in [
+            {
+                "a": tvm.nd.array(np.random.uniform(low, high, shape).astype(dtype)),
+                "b": tvm.nd.array(np.random.uniform(low, high, shape).astype(dtype)),
+            }
+        ]:
+            outputs = []
+            func = _get_model(shape, dtype, iter(inputs), op, op_params)
+            for acl in [True, False]:
+                outputs.append(build_and_run(func, inputs, 1, None, device, enable_acl=acl)[0])
+
+            config = {
+                "shape": shape,
+                "dtype": dtype,
+                "inputs": inputs,
+                "operation": op,
+                "op_params": op_params,
+            }
+            verify(outputs, atol=atol, rtol=rtol, config=config)

Review comment:
       add_QASYMM8_QASYMM8_QASYMM8 currently always saturate result. So verify_saturation=True cannot be used




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-tvm] d-smirnov commented on a change in pull request #6532: [BYOC][ACL] Support add operation

Posted by GitBox <gi...@apache.org>.
d-smirnov commented on a change in pull request #6532:
URL: https://github.com/apache/incubator-tvm/pull/6532#discussion_r494834251



##########
File path: python/tvm/relay/op/contrib/arm_compute_lib.py
##########
@@ -161,6 +161,17 @@ def l2_pool2d_pattern():
         pattern = is_op("sqrt")(pattern)
         return pattern
 
+    def add_pattern():
+        """Create an add pattern.
+
+        Returns
+        -------
+        pattern : dataflow_pattern.AltPattern
+            Denotes the convolution pattern.

Review comment:
       removed




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-tvm] comaniac commented on a change in pull request #6532: [BYOC][ACL] Support add operation

Posted by GitBox <gi...@apache.org>.
comaniac commented on a change in pull request #6532:
URL: https://github.com/apache/incubator-tvm/pull/6532#discussion_r496082766



##########
File path: python/tvm/relay/op/contrib/arm_compute_lib.py
##########
@@ -345,3 +345,23 @@ def maximum(attrs, args):
     type_a = args[0].checked_type
     type_b = args[0].checked_type
     return (type_a.dtype == "float32") and (type_b.dtype == "float32")
+
+
+@tvm.ir.register_op_attr("add", "target.arm_compute_lib")
+def add(attrs, args):
+    """Check if the external ACL codegen for add should be used."""
+    for typ in [args[0].checked_type, args[1].checked_type]:
+        if typ.dtype not in ["float32"]:

Review comment:
       ```suggestion
           if typ.dtype != "float32":
   ```

##########
File path: python/tvm/relay/op/contrib/arm_compute_lib.py
##########
@@ -345,3 +345,23 @@ def maximum(attrs, args):
     type_a = args[0].checked_type
     type_b = args[0].checked_type
     return (type_a.dtype == "float32") and (type_b.dtype == "float32")
+
+
+@tvm.ir.register_op_attr("add", "target.arm_compute_lib")
+def add(attrs, args):
+    """Check if the external ACL codegen for add should be used."""
+    for typ in [args[0].checked_type, args[1].checked_type]:
+        if typ.dtype not in ["float32"]:
+            return False
+
+    return True
+
+
+@tvm.ir.register_op_attr("qnn.add", "target.arm_compute_lib")
+def qnn_add(attrs, args):
+    """Check if the external ACL codegen for add should be used."""
+    for typ in [args[0].checked_type, args[1].checked_type]:
+        if typ.dtype not in ["uint8"]:

Review comment:
       ```suggestion
           if typ.dtype != "uint8":
   ```

##########
File path: src/runtime/contrib/arm_compute_lib/acl_runtime.cc
##########
@@ -140,8 +141,13 @@ class ACLRuntime : public JSONRuntimeBase {
           CreateGlobalPoolingLayer(&layer_, node);
         } else if ("reshape" == op_name) {
           CreateReshapeLayer(&layer_, node);
+<<<<<<< HEAD
         } else if ("maximum" == op_name) {
           CreateMaximumLayer(&layer_, node);
+=======
+        } else if ("add" == op_name || "qnn.add" == op_name) {
+          CreateAddLayer(&layer_, node);
+>>>>>>> a7fa43daf... ACL: add operation

Review comment:
       Fix conflict.

##########
File path: src/runtime/contrib/arm_compute_lib/acl_runtime.cc
##########
@@ -416,15 +422,54 @@ class ACLRuntime : public JSONRuntimeBase {
     auto function = std::make_shared<arm_compute::NEElementwiseMax>();
     function->configure(&layer->inputs[0], &layer->inputs[1], &layer->outputs[0]);
     layer->function = function;
-  }
 
-  /*! \brief Allow ACL functions to request auxiliary memory from TVM. */
-  ACLAllocator allocator_;
-  /*!
-   * \brief The network layers represented by acl functions.
-   * \note Currently only supports a single layer.
-   */
-  CachedLayer layer_;
+    /*!
+     * \brief Creates an add/qnn.add layer
+     *
+     * \param layer The ACL layer to build. Containing inputs, outputs and the ACL function.
+     * \param node  The JSON representation of the operator.
+     */
+    void CreateAddLayer(CachedLayer * layer, const JSONGraphNode& node) {
+      auto op_name = node.GetOpName();
+      if ("add" == op_name) {
+        layer->inputs.push_back(MakeACLTensorFromJSONEntry(node.GetInputs()[0]));
+        layer->inputs.push_back(MakeACLTensorFromJSONEntry(node.GetInputs()[1]));
+        layer->outputs.push_back(MakeACLTensorFromJSONNode(node));
+      } else if ("qnn.add" == op_name) {
+        layer->inputs.push_back(MakeACLTensorFromJSONEntry(
+            node.GetInputs()[0], &node.GetInputs()[2], &node.GetInputs()[3]));
+        layer->inputs.push_back(MakeACLTensorFromJSONEntry(
+            node.GetInputs()[1], &node.GetInputs()[4], &node.GetInputs()[5]));
+        layer->outputs.push_back(
+            MakeACLTensorFromJSONNode(node, &node.GetInputs()[6], &node.GetInputs()[7]));
+      } else {
+        LOG(FATAL) << "Unsupported op: " << op_name;

Review comment:
       IMO here can simply throw out assertion, since you have checked if `op_name` is `add` or `qnn.add` before invoking this function in `BuildEngine`.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-tvm] d-smirnov commented on a change in pull request #6532: [BYOC][ACL] Support add operation

Posted by GitBox <gi...@apache.org>.
d-smirnov commented on a change in pull request #6532:
URL: https://github.com/apache/incubator-tvm/pull/6532#discussion_r495926446



##########
File path: tests/python/contrib/test_arm_compute_lib/test_add.py
##########
@@ -0,0 +1,142 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Arm Compute Library integration reshape tests."""
+
+import numpy as np
+
+import tvm
+import tvm.testing
+from tvm import relay
+
+from test_arm_compute_lib.infrastructure import (
+    skip_runtime_test,
+    skip_codegen_test,
+    build_and_run,
+    verify,
+    verify_codegen,
+)
+from test_arm_compute_lib.infrastructure import Device
+
+
+def _get_model(shape, dtype, var_names, op, op_params):
+    a = relay.var(next(var_names), shape=shape, dtype=dtype)
+    b = relay.var(next(var_names), shape=shape, dtype=dtype)
+    return op(a, b, **op_params)
+
+
+def _get_expected_codegen(shape, dtype):
+    node = {
+        "op": "kernel",
+        "name": "add",
+        "inputs": [[0, 0, 0], [1, 0, 0]],
+        "attrs": {
+            "num_inputs": "2",
+            "num_outputs": "1",
+            "shape": [[list(shape)]],
+            "dtype": [[dtype]],
+        },
+    }
+
+    input_a = {"op": "input", "name": "", "attrs": {"shape": [[list(shape)]], "dtype": [[dtype]]}}
+    input_b = {"op": "input", "name": "", "attrs": {"shape": [[list(shape)]], "dtype": [[dtype]]}}
+    return [input_a, input_b, node]
+
+
+def test_add():

Review comment:
       added "runtime"




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-tvm] d-smirnov commented on a change in pull request #6532: [BYOC][ACL] Support add operation

Posted by GitBox <gi...@apache.org>.
d-smirnov commented on a change in pull request #6532:
URL: https://github.com/apache/incubator-tvm/pull/6532#discussion_r496105447



##########
File path: src/runtime/contrib/arm_compute_lib/acl_runtime.cc
##########
@@ -416,15 +422,54 @@ class ACLRuntime : public JSONRuntimeBase {
     auto function = std::make_shared<arm_compute::NEElementwiseMax>();
     function->configure(&layer->inputs[0], &layer->inputs[1], &layer->outputs[0]);
     layer->function = function;
-  }
 
-  /*! \brief Allow ACL functions to request auxiliary memory from TVM. */
-  ACLAllocator allocator_;
-  /*!
-   * \brief The network layers represented by acl functions.
-   * \note Currently only supports a single layer.
-   */
-  CachedLayer layer_;
+    /*!
+     * \brief Creates an add/qnn.add layer
+     *
+     * \param layer The ACL layer to build. Containing inputs, outputs and the ACL function.
+     * \param node  The JSON representation of the operator.
+     */
+    void CreateAddLayer(CachedLayer * layer, const JSONGraphNode& node) {
+      auto op_name = node.GetOpName();
+      if ("add" == op_name) {
+        layer->inputs.push_back(MakeACLTensorFromJSONEntry(node.GetInputs()[0]));
+        layer->inputs.push_back(MakeACLTensorFromJSONEntry(node.GetInputs()[1]));
+        layer->outputs.push_back(MakeACLTensorFromJSONNode(node));
+      } else if ("qnn.add" == op_name) {
+        layer->inputs.push_back(MakeACLTensorFromJSONEntry(
+            node.GetInputs()[0], &node.GetInputs()[2], &node.GetInputs()[3]));
+        layer->inputs.push_back(MakeACLTensorFromJSONEntry(
+            node.GetInputs()[1], &node.GetInputs()[4], &node.GetInputs()[5]));
+        layer->outputs.push_back(
+            MakeACLTensorFromJSONNode(node, &node.GetInputs()[6], &node.GetInputs()[7]));
+      } else {
+        LOG(FATAL) << "Unsupported op: " << op_name;

Review comment:
       replaced with throw 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-tvm] d-smirnov commented on a change in pull request #6532: [BYOC][ACL] Support add operation

Posted by GitBox <gi...@apache.org>.
d-smirnov commented on a change in pull request #6532:
URL: https://github.com/apache/incubator-tvm/pull/6532#discussion_r494840783



##########
File path: python/tvm/relay/op/contrib/arm_compute_lib.py
##########
@@ -337,3 +354,17 @@ def global_avg_pool2d(attrs, args):
     if attrs.layout != "NHWC":
         return False
     return True
+
+
+@tvm.ir.register_op_attr("add", "target.arm_compute_lib")
+@tvm.ir.register_op_attr("qnn.add", "target.arm_compute_lib")
+def add(attrs, args):
+    """Check if the external ACL codegen for add should be used."""
+    for typ in [args[0].checked_type, args[1].checked_type]:
+        if typ.dtype not in ["float32", "uint8"]:

Review comment:
       1. Yep. The pattern is not needed. Removed. 
   2. Yep. Has been split




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-tvm] lhutton1 commented on a change in pull request #6532: [BYOC][ACL] Support add operation

Posted by GitBox <gi...@apache.org>.
lhutton1 commented on a change in pull request #6532:
URL: https://github.com/apache/incubator-tvm/pull/6532#discussion_r494901379



##########
File path: tests/python/contrib/test_arm_compute_lib/test_add.py
##########
@@ -0,0 +1,142 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Arm Compute Library integration reshape tests."""
+
+import numpy as np
+
+import tvm
+import tvm.testing
+from tvm import relay
+
+from test_arm_compute_lib.infrastructure import (
+    skip_runtime_test,
+    skip_codegen_test,
+    build_and_run,
+    verify,
+    verify_codegen,
+)
+from test_arm_compute_lib.infrastructure import Device
+
+
+def _get_model(shape, dtype, var_names, op, op_params):
+    a = relay.var(next(var_names), shape=shape, dtype=dtype)
+    b = relay.var(next(var_names), shape=shape, dtype=dtype)
+    return op(a, b, **op_params)
+
+
+def _get_expected_codegen(shape, dtype):
+    node = {
+        "op": "kernel",
+        "name": "add",
+        "inputs": [[0, 0, 0], [1, 0, 0]],
+        "attrs": {
+            "num_inputs": "2",
+            "num_outputs": "1",
+            "shape": [[list(shape)]],
+            "dtype": [[dtype]],
+        },
+    }
+
+    input_a = {"op": "input", "name": "", "attrs": {"shape": [[list(shape)]], "dtype": [[dtype]]}}
+    input_b = {"op": "input", "name": "", "attrs": {"shape": [[list(shape)]], "dtype": [[dtype]]}}
+    return [input_a, input_b, node]
+
+
+def test_add():
+    Device.load("test_config.json")
+
+    if skip_runtime_test():
+        return
+
+    device = Device()
+    np.random.seed(0)
+
+    for dtype, low, high, atol, rtol, op, op_params in [
+        ("float32", -127, 128, 1e-7, 1e-7, relay.add, {}),
+        # different qnn params
+        (
+            "uint8",
+            0,
+            255,
+            0.0,
+            1.0,
+            relay.qnn.op.add,
+            {
+                "lhs_scale": relay.const(0.0156863, "float32"),
+                "lhs_zero_point": relay.const(127, "int32"),
+                "rhs_scale": relay.const(0.0117647, "float32"),
+                "rhs_zero_point": relay.const(85, "int32"),
+                "output_scale": relay.const(0.0235294, "float32"),
+                "output_zero_point": relay.const(128, "int32"),
+            },
+        ),
+        # same qnn params
+        (
+            "uint8",
+            0,
+            255,
+            0.0,
+            1.0,
+            relay.qnn.op.add,
+            {
+                "lhs_scale": relay.const(0.0126863, "float32"),
+                "lhs_zero_point": relay.const(127, "int32"),
+                "rhs_scale": relay.const(0.0126863, "float32"),
+                "rhs_zero_point": relay.const(127, "int32"),
+                "output_scale": relay.const(0.0126863, "float32"),
+                "output_zero_point": relay.const(127, "int32"),
+            },
+        ),
+    ]:
+        shape = (2, 2)
+        for inputs in [
+            {
+                "a": tvm.nd.array(np.random.uniform(low, high, shape).astype(dtype)),
+                "b": tvm.nd.array(np.random.uniform(low, high, shape).astype(dtype)),
+            }
+        ]:
+            outputs = []
+            func = _get_model(shape, dtype, iter(inputs), op, op_params)
+            for acl in [True, False]:
+                outputs.append(build_and_run(func, inputs, 1, None, device, enable_acl=acl)[0])
+
+            config = {
+                "shape": shape,
+                "dtype": dtype,
+                "inputs": inputs,
+                "operation": op,
+                "op_params": op_params,
+            }
+            verify(outputs, atol=atol, rtol=rtol, config=config)

Review comment:
       The intention was to ensure that most of the values output are not saturated i.e. most values are not 255 or 0. I can see why this would cause an issue with test cases of 2x2 as a single value that is 255 or 0 would trigger the asserts. I think the best approach here would be to increase the size of the test case?

##########
File path: tests/python/contrib/test_arm_compute_lib/test_add.py
##########
@@ -0,0 +1,142 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Arm Compute Library integration reshape tests."""
+
+import numpy as np
+
+import tvm
+import tvm.testing
+from tvm import relay
+
+from test_arm_compute_lib.infrastructure import (
+    skip_runtime_test,
+    skip_codegen_test,
+    build_and_run,
+    verify,
+    verify_codegen,
+)
+from test_arm_compute_lib.infrastructure import Device
+
+
+def _get_model(shape, dtype, var_names, op, op_params):
+    a = relay.var(next(var_names), shape=shape, dtype=dtype)
+    b = relay.var(next(var_names), shape=shape, dtype=dtype)
+    return op(a, b, **op_params)
+
+
+def _get_expected_codegen(shape, dtype):
+    node = {
+        "op": "kernel",
+        "name": "add",
+        "inputs": [[0, 0, 0], [1, 0, 0]],
+        "attrs": {
+            "num_inputs": "2",
+            "num_outputs": "1",
+            "shape": [[list(shape)]],
+            "dtype": [[dtype]],
+        },
+    }
+
+    input_a = {"op": "input", "name": "", "attrs": {"shape": [[list(shape)]], "dtype": [[dtype]]}}
+    input_b = {"op": "input", "name": "", "attrs": {"shape": [[list(shape)]], "dtype": [[dtype]]}}
+    return [input_a, input_b, node]
+
+
+def test_add():
+    Device.load("test_config.json")
+
+    if skip_runtime_test():
+        return
+
+    device = Device()
+    np.random.seed(0)
+
+    for dtype, low, high, atol, rtol, op, op_params in [
+        ("float32", -127, 128, 1e-7, 1e-7, relay.add, {}),
+        # different qnn params
+        (
+            "uint8",
+            0,
+            255,
+            0.0,
+            1.0,
+            relay.qnn.op.add,
+            {
+                "lhs_scale": relay.const(0.0156863, "float32"),
+                "lhs_zero_point": relay.const(127, "int32"),
+                "rhs_scale": relay.const(0.0117647, "float32"),
+                "rhs_zero_point": relay.const(85, "int32"),
+                "output_scale": relay.const(0.0235294, "float32"),
+                "output_zero_point": relay.const(128, "int32"),
+            },
+        ),
+        # same qnn params
+        (
+            "uint8",
+            0,
+            255,
+            0.0,
+            1.0,
+            relay.qnn.op.add,
+            {
+                "lhs_scale": relay.const(0.0126863, "float32"),
+                "lhs_zero_point": relay.const(127, "int32"),
+                "rhs_scale": relay.const(0.0126863, "float32"),
+                "rhs_zero_point": relay.const(127, "int32"),
+                "output_scale": relay.const(0.0126863, "float32"),
+                "output_zero_point": relay.const(127, "int32"),
+            },
+        ),
+    ]:
+        shape = (2, 2)
+        for inputs in [
+            {
+                "a": tvm.nd.array(np.random.uniform(low, high, shape).astype(dtype)),
+                "b": tvm.nd.array(np.random.uniform(low, high, shape).astype(dtype)),
+            }
+        ]:
+            outputs = []
+            func = _get_model(shape, dtype, iter(inputs), op, op_params)
+            for acl in [True, False]:
+                outputs.append(build_and_run(func, inputs, 1, None, device, enable_acl=acl)[0])
+
+            config = {
+                "shape": shape,
+                "dtype": dtype,
+                "inputs": inputs,
+                "operation": op,
+                "op_params": op_params,
+            }
+            verify(outputs, atol=atol, rtol=rtol, config=config)

Review comment:
       The intention was to ensure that most of the values output are not saturated i.e. most values are not 255 or 0. I can see why this would cause an issue with test cases of 2x2 as a single value that is 255 or 0 would trigger the asserts. I think the best approach here would be to increase the size of the test case, or to reduce 0.25 down to a value less than 25%?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org