areusch commented on code in PR #13051:
URL: https://github.com/apache/tvm/pull/13051#discussion_r998244813


##########
python/tvm/topi/utils.py:
##########
@@ -431,13 +431,13 @@ def get_shape(src_shape, src_layout, dst_layout):
     return get_const_tuple(tuple([src_shape[i.value] for i in dst_indices]))
 
 
-def change_constant_shape(src, src_layout, dst_layout):
-    """Makes a copy of a Relay constant, reshaping it to a new data layout.
+def change_ndarray_layout(arr, src_layout, dst_layout):

Review Comment:
   should this still live in prod if it's just a test util?



##########
tests/python/relay/strategy/arm_cpu/test_group_conv2d.py:
##########
@@ -14,121 +14,44 @@
 # KIND, either express or implied.  See the License for the
 # specific language governing permissions and limitations
 # under the License.
-import numpy as np
-import tvm
-import tvm.testing
-from tvm import relay
-from tvm.testing.aot import AOTTestModel, compile_and_run, generate_ref_data
-from tvm.micro.testing.aot_test_utils import AOT_CORSTONE300_RUNNER
+"""Tests for arm_cpu schedules for grouped conv2d."""
 
+from test_generalized_conv2d import GeneralizedConv2dTests
+from tvm.testing import main, parameter, parameters
 
-class BasicGroupConv2dTests:
-    @tvm.testing.requires_corstone300
-    def test_conv2d(
-        self,
-        data_shape,
-        data_layout,
-        kernel_size,
-        kernel_layout,
-        num_filter,
-        strides,
-        padding,
-        dilation,
-        groups,
-        dtype,
-        schedule_name,
-    ):
-        """Test a subgraph with a single conv2d operator."""
-        ishape = data_shape
 
-        assert groups > 1, f"groups should be more than 1 to create a group 
conv2d."
+class GroupConv2dTests(GeneralizedConv2dTests):
+    """Helper for constructing group Conv2ds. Sets the kernel layout to what 
x86 code supports."""

Review Comment:
   nit: sets the reference kernel layout...



##########
tests/python/relay/strategy/arm_cpu/test_generalized_conv2d.py:
##########
@@ -0,0 +1,122 @@
+# 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.
+"""Helper class for testing variations of 2D convolution. Should be used by 
subclassing
+`GeneralizedConv2dTests`, and then setting the arguments using 
tvm.testing.parameter(s)."""
+
+from numpy.random import randint
+import tvm
+import tvm.testing
+from tvm import relay
+from tvm.testing.aot import AOTTestModel, compile_and_run, generate_ref_data
+from tvm.micro.testing.aot_test_utils import AOT_CORSTONE300_RUNNER
+from tvm.topi.utils import change_ndarray_layout
+
+
+class GeneralizedConv2dTests:
+    """Superclass which can be used to test regular, depthwise, or grouped 
conv2D. Cannot be used
+    for 5D data formats (NCHWc and such) as written, but could be extended. 
Might also be worth
+    abstracting some of this logic into an even more general class that could 
be used for other
+    operators.
+
+    Note that data_shape should always be a tuple of length four indicating 
the data shape in NHWC
+    format (it will later be reshaped according to the given data_layout), and 
kernel_size should be
+    a length two tuple giving the height and width of the kernel."""
+
+    @tvm.testing.requires_corstone300
+    def test_conv2d(
+        self,
+        data_shape,
+        kernel_size,
+        num_filter,
+        in_dtype,
+        strides,
+        padding,
+        groups,
+        dilation,
+        data_layout,
+        kernel_layout,
+        out_layout,
+        schedule_name,
+    ):
+        """Test a subgraph with a single conv2d operator."""
+
+        ref_input_data = randint(low=-128, high=127, size=data_shape, 
dtype=in_dtype)
+        ref_input_var = relay.var("input", relay.TensorType(data_shape, 
in_dtype))  # NHWC layout
+        kernel_shape = (*kernel_size, data_shape[-1] // groups, num_filter)  # 
HWIO layout
+        ref_kernel_data = randint(low=-10, high=10, size=kernel_shape, 
dtype=in_dtype)
+
+        # Our x86 depthwise implementation only supports HWOI with NHWC, so we 
need to change our
+        # kernel layout to work around this. We can't just change the whole 
thing to HWIO or
+        # something else, as then group conv2d would not work. Eventually, we 
should switch to using
+        # TensorFlow to create the reference output so we can ensure our 
implementation is right.

Review Comment:
   can you file a bug for this and link to it from this comment?



##########
tests/python/relay/strategy/arm_cpu/test_generalized_conv2d.py:
##########
@@ -0,0 +1,122 @@
+# 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.
+"""Helper class for testing variations of 2D convolution. Should be used by 
subclassing
+`GeneralizedConv2dTests`, and then setting the arguments using 
tvm.testing.parameter(s)."""
+
+from numpy.random import randint
+import tvm
+import tvm.testing
+from tvm import relay
+from tvm.testing.aot import AOTTestModel, compile_and_run, generate_ref_data
+from tvm.micro.testing.aot_test_utils import AOT_CORSTONE300_RUNNER
+from tvm.topi.utils import change_ndarray_layout
+
+
+class GeneralizedConv2dTests:
+    """Superclass which can be used to test regular, depthwise, or grouped 
conv2D. Cannot be used
+    for 5D data formats (NCHWc and such) as written, but could be extended. 
Might also be worth
+    abstracting some of this logic into an even more general class that could 
be used for other
+    operators.
+
+    Note that data_shape should always be a tuple of length four indicating 
the data shape in NHWC
+    format (it will later be reshaped according to the given data_layout), and 
kernel_size should be
+    a length two tuple giving the height and width of the kernel."""
+
+    @tvm.testing.requires_corstone300
+    def test_conv2d(

Review Comment:
   is this case prevented from being run because it takes undefined parameters?



-- 
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: commits-unsubscr...@tvm.apache.org

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

Reply via email to