tkonolige commented on code in PR #11341:
URL: https://github.com/apache/tvm/pull/11341#discussion_r876347585


##########
src/relay/op/tensor/transform.cc:
##########
@@ -346,7 +346,7 @@ RELAY_REGISTER_OP("concatenate")
     .set_support_level(1)
     .add_type_rel("Concatenate", ConcatenateRel<ConcatenateAttrs>)
     .set_attr<FInferCorrectLayout>("FInferCorrectLayout", ConcatenateLayout)
-    .set_attr<FTVMCompute>("FTVMCompute", ConcatenateCompute)
+    // .set_attr<FTVMCompute>("FTVMCompute", ConcatenateCompute)

Review Comment:
   nit: uncomment or delete



##########
src/te/schedule/schedule_dataflow_rewrite.cc:
##########
@@ -511,6 +511,23 @@ void InjectInline(ScheduleNode* sch, bool 
feature_extraction_mode) {
   std::vector<bool> changed(sch->stages.size(), false);
   std::vector<Stmt> new_hybrid_body(sch->stages.size());
   std::vector<bool> hybrid_changed(sch->stages.size(), false);
+  // (sshtin): this workaround allows to inline extern ops.
+  // All inputs for extern op should not be inlined because inlining happens
+  // before generation of TE script for particular extern op. That may lead to
+  // crash during lowering or building stages.
+  std::unordered_map<Operation, Operation> ext_ops;

Review Comment:
   Can you document how this workaround works and in what cases it might fail?



##########
tests/python/relay/test_op_level1.py:
##########
@@ -430,6 +430,112 @@ def test_batch_norm():
         )
 
 
+def do_concat_test(shapes, t_shape, dtype, axis, dev, target):
+    varsToConcat = []
+    inputData = []
+    pos = 0
+    for s in shapes:
+        varsToConcat.append(relay.var("x{}".format(pos), shape=s))
+        inputData.append(np.random.rand(*s).astype(dtype))
+        pos += 1
+    t = relay.var("z", shape=t_shape, dtype=dtype)
+    z = relay.concatenate(varsToConcat, axis=axis)
+    z = relay.add(z, t)
+    params = varsToConcat
+    params.append(t)
+    func = relay.Function(params, z)
+    t_data = np.random.uniform(low=-10, high=10, size=t_shape).astype(dtype)
+    ref_res = np.concatenate((tuple(inputData)), axis=axis) + t_data
+    mod = tvm.IRModule.from_expr(func)
+
+    executor = relay.create_executor("graph", mod=mod, device=dev, 
target=target)
+    op_res1 = executor.evaluate()(*inputData, t_data)
+
+    tvm.testing.assert_allclose(op_res1.numpy(), ref_res, rtol=0.000001)
+    op_res2 = relay.create_executor("debug", device=dev, 
target=target).evaluate(func)(
+        *inputData, t_data
+    )
+    tvm.testing.assert_allclose(op_res2.numpy(), ref_res, rtol=0.000001)
+
+
[email protected]_gpu
+def test_concatenate1():
+    for target, dev in tvm.testing.enabled_targets():
+        if target != "llvm":
+            continue

Review Comment:
   ```suggestion
   @tvm.testing.parameterize_targets("llvm")
   def test_concatenate1(target, dev):
   ```



##########
tests/python/relay/test_op_level1.py:
##########
@@ -430,6 +430,112 @@ def test_batch_norm():
         )
 
 
+def do_concat_test(shapes, t_shape, dtype, axis, dev, target):
+    varsToConcat = []
+    inputData = []
+    pos = 0
+    for s in shapes:
+        varsToConcat.append(relay.var("x{}".format(pos), shape=s))
+        inputData.append(np.random.rand(*s).astype(dtype))
+        pos += 1
+    t = relay.var("z", shape=t_shape, dtype=dtype)
+    z = relay.concatenate(varsToConcat, axis=axis)
+    z = relay.add(z, t)
+    params = varsToConcat
+    params.append(t)
+    func = relay.Function(params, z)
+    t_data = np.random.uniform(low=-10, high=10, size=t_shape).astype(dtype)
+    ref_res = np.concatenate((tuple(inputData)), axis=axis) + t_data
+    mod = tvm.IRModule.from_expr(func)
+
+    executor = relay.create_executor("graph", mod=mod, device=dev, 
target=target)
+    op_res1 = executor.evaluate()(*inputData, t_data)
+
+    tvm.testing.assert_allclose(op_res1.numpy(), ref_res, rtol=0.000001)
+    op_res2 = relay.create_executor("debug", device=dev, 
target=target).evaluate(func)(
+        *inputData, t_data
+    )
+    tvm.testing.assert_allclose(op_res2.numpy(), ref_res, rtol=0.000001)
+
+
[email protected]_gpu
+def test_concatenate1():
+    for target, dev in tvm.testing.enabled_targets():
+        if target != "llvm":
+            continue
+        np.random.seed(471)
+        maxNumDimensions = 6
+        shape = [4, 32, 16, 1, 31, 20, 21, 8, 28, 7]  # just randomly selected 
10 numbers
+        for dtype in ["float32"]:
+            for dimsNum in range(1, maxNumDimensions):
+                np.random.shuffle(shape)
+                for axis in range(0, dimsNum):  # range should be (-dimsNum + 
1, dimsNum)
+                    numToConcat = np.random.uniform(low=2, high=10, 
size=(1)).astype("int64")[0]
+                    shapes = []
+                    # the code below to normalize axes index. For some reasons 
tvm notifies about error if the axis is negative
+                    normalizedAxis = axis
+                    if axis < 0:
+                        normalizedAxis += dimsNum
+                    finalSize = 0
+                    for i in range(0, numToConcat):
+                        shp = tuple(shape[:dimsNum])
+                        finalSize += shape[(i % len(shape))]
+                        shapes.append(
+                            shp[:normalizedAxis]
+                            + tuple([shape[(i % len(shape))]])
+                            + shp[normalizedAxis + 1 :]
+                        )
+                    t_shape = shp[:normalizedAxis] + tuple([finalSize]) + 
shp[normalizedAxis + 1 :]
+                    do_concat_test(shapes, t_shape, dtype, axis, dev, target)
+
+
[email protected]_gpu
+def test_concatenate2():
+    # test to cover cases (1, .. , x, 1, .. , 1)
+    for target, dev in tvm.testing.enabled_targets():
+        if target != "llvm":
+            continue

Review Comment:
   ```suggestion
   @tvm.testing.parametrize_targets("llvm")
   def test_concatenate2(target, dev):
       # test to cover cases (1, .. , x, 1, .. , 1)
   ```



##########
python/tvm/topi/x86/concat.py:
##########
@@ -0,0 +1,134 @@
+# 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.
+# pylint: disable=invalid-name,unused-variable,unused-argument,invalid-name

Review Comment:
   It would be better to not disable all these lints.



##########
tests/python/relay/test_op_level1.py:
##########
@@ -430,6 +430,112 @@ def test_batch_norm():
         )
 
 
+def do_concat_test(shapes, t_shape, dtype, axis, dev, target):
+    varsToConcat = []
+    inputData = []
+    pos = 0
+    for s in shapes:
+        varsToConcat.append(relay.var("x{}".format(pos), shape=s))
+        inputData.append(np.random.rand(*s).astype(dtype))
+        pos += 1
+    t = relay.var("z", shape=t_shape, dtype=dtype)
+    z = relay.concatenate(varsToConcat, axis=axis)
+    z = relay.add(z, t)
+    params = varsToConcat
+    params.append(t)
+    func = relay.Function(params, z)
+    t_data = np.random.uniform(low=-10, high=10, size=t_shape).astype(dtype)
+    ref_res = np.concatenate((tuple(inputData)), axis=axis) + t_data
+    mod = tvm.IRModule.from_expr(func)
+
+    executor = relay.create_executor("graph", mod=mod, device=dev, 
target=target)
+    op_res1 = executor.evaluate()(*inputData, t_data)
+
+    tvm.testing.assert_allclose(op_res1.numpy(), ref_res, rtol=0.000001)
+    op_res2 = relay.create_executor("debug", device=dev, 
target=target).evaluate(func)(
+        *inputData, t_data
+    )
+    tvm.testing.assert_allclose(op_res2.numpy(), ref_res, rtol=0.000001)
+
+
[email protected]_gpu
+def test_concatenate1():
+    for target, dev in tvm.testing.enabled_targets():
+        if target != "llvm":
+            continue
+        np.random.seed(471)
+        maxNumDimensions = 6
+        shape = [4, 32, 16, 1, 31, 20, 21, 8, 28, 7]  # just randomly selected 
10 numbers
+        for dtype in ["float32"]:
+            for dimsNum in range(1, maxNumDimensions):
+                np.random.shuffle(shape)
+                for axis in range(0, dimsNum):  # range should be (-dimsNum + 
1, dimsNum)
+                    numToConcat = np.random.uniform(low=2, high=10, 
size=(1)).astype("int64")[0]
+                    shapes = []
+                    # the code below to normalize axes index. For some reasons 
tvm notifies about error if the axis is negative
+                    normalizedAxis = axis
+                    if axis < 0:
+                        normalizedAxis += dimsNum
+                    finalSize = 0
+                    for i in range(0, numToConcat):
+                        shp = tuple(shape[:dimsNum])
+                        finalSize += shape[(i % len(shape))]
+                        shapes.append(
+                            shp[:normalizedAxis]
+                            + tuple([shape[(i % len(shape))]])
+                            + shp[normalizedAxis + 1 :]
+                        )
+                    t_shape = shp[:normalizedAxis] + tuple([finalSize]) + 
shp[normalizedAxis + 1 :]
+                    do_concat_test(shapes, t_shape, dtype, axis, dev, target)
+
+
[email protected]_gpu
+def test_concatenate2():
+    # test to cover cases (1, .. , x, 1, .. , 1)
+    for target, dev in tvm.testing.enabled_targets():
+        if target != "llvm":
+            continue
+        np.random.seed(13)
+        maxNumDimensions = 6
+        shape = [8, 3, 25, 33, 12, 29, 5, 11, 29, 11]  # just randomly 
selected 10 numbers
+        ind = 0
+        for dtype in ["float32"]:
+            for dimsNum in range(2, maxNumDimensions):
+                np.random.shuffle(shape)
+                for axis in range(-dimsNum + 1, dimsNum):  # range should be 
(-dimsNum + 1, dimsNum)
+                    numToConcat = np.random.uniform(low=2, high=10, 
size=(1)).astype("int64")[0]
+                    shapes = []
+                    # the code below to normalize axes index. For some reasons 
tvm notifies about error if the axis is negative
+                    normalizedAxis = axis
+                    if axis < 0:
+                        normalizedAxis += dimsNum
+                    finalSize = 0
+                    for i in range(0, numToConcat):
+                        axisVal = [1] * dimsNum
+                        axisVal[axis] = shape[(ind % len(shape))]
+                        ind += 1
+                        finalSize += axisVal[axis]
+                        shapes.append(tuple(axisVal))
+                    temp = [1] * dimsNum
+                    temp[axis] = finalSize
+                    t_shape = tuple(temp)
+                    do_concat_test(shapes, t_shape, dtype, axis, dev, target)
+
+
[email protected]_gpu
+def test_concatenate3():
+    for target, dev in tvm.testing.enabled_targets():
+        if target != "llvm":
+            continue

Review Comment:
   ```suggestion
   @tvm.testing.parametrize_targets("llvm")
   def test_concatenate3(target, dev):
   ```



-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to