comaniac commented on a change in pull request #7142:
URL: https://github.com/apache/tvm/pull/7142#discussion_r548090943
##########
File path: tests/python/topi/python/test_topi_conv2d_int8.py
##########
@@ -385,6 +387,18 @@ def get_ref_data():
a_np, w_np, b_np, c_np = get_ref_data()
+ def test_worload_padding(output_data, data, kernel, strides, padding,
out_dtype):
+ _, _, out_height, out_width = get_const_tuple(output_data.shape)
+ wkl = _get_workload(data, kernel, strides, padding, out_dtype)
+ int32_lanes, num_int8_elements = 16, 4
+
+ # check if tile_ow candidates are the factors of the right output
weight.
+ cfg = autotvm.get_config()
+ fallback_schedule_cpu_common_int8(cfg, wkl, int32_lanes,
num_int8_elements)
+ ow_tile = cfg["tile_ow"].size[-1] * cfg["tile_ow"].size[-2]
Review comment:
```suggestion
ow_tile_prod = np.prod(cfg["tile_ow"].size)
```
##########
File path: tests/python/topi/python/test_topi_conv2d_int8.py
##########
@@ -435,6 +449,7 @@ def check_device(device):
)
func(a, w, c)
tvm.testing.assert_allclose(c.asnumpy(), c_np, rtol=1e-5)
+ test_worload_padding(C, A, W, (stride, stride), padding, dtype)
Review comment:
It's ok to put this check in `verify_conv2d_nchw_int8` so that we can
cover all existing and future unit tests. On the other hand, it seems like you
don't need to test this function per device? For example,
`fallback_schedule_cpu_common_int8 ` is only for CPU, so it doesn't make sense
to test it within `check_device["cuda"]`. Maybe you can simply put this check
before `check_device`?
```
verify_workload_padding(...)
for device in ["cuda"]:
check_device(device)
```
##########
File path: tests/python/topi/python/test_topi_conv2d_int8.py
##########
@@ -385,6 +387,18 @@ def get_ref_data():
a_np, w_np, b_np, c_np = get_ref_data()
+ def test_worload_padding(output_data, data, kernel, strides, padding,
out_dtype):
Review comment:
1. Avoid using the prefix `test_`unless this is a standalone test
function.
2. This function name is too vague, as you are testing a very specific
schedule (i.e., `fallback_schedule_cpu_common_int8`). Something like
`verify_fallback_schedule_cpu_padding` might be better.
```suggestion
def verify_worload_padding(output_data, data, kernel, strides, padding,
out_dtype):
```
----------------------------------------------------------------
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:
[email protected]
