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/23 09:49:51 UTC

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

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