cleanup Hexagon conv2d tests #9473
Conversation
adstraw
requested review from
areusch,
comaniac,
icemelon,
jroesch,
junrushao,
merrymercy,
tqchen and
yzhliu
as code owners
|
@csullivan and @cconvey please review |
| import tvm | ||
| from .infrastructure import get_packed_filter_layout | ||
|
|
||
|
|
||
| @tvm.testing.fixture | ||
| def shape_nhwc(batch, in_channel, in_size): | ||
| return (batch, in_size, in_size, in_channel) | ||
|
|
||
|
|
||
| @tvm.testing.fixture | ||
| def shape_oihw(out_channel, in_channel, kernel): | ||
| return (out_channel, in_channel, kernel, kernel) | ||
|
|
||
|
|
||
| @tvm.testing.fixture | ||
| def shape_oihw8i32o4i(out_channel, in_channel, kernel): | ||
| return get_packed_filter_layout(out_channel, in_channel, kernel, kernel) |
There was a problem hiding this comment.
The goal being to check to see if the testing fixtures are the source of the i386 flakiness?
There was a problem hiding this comment.
I removed the fixtures because of their effect in terms of the ordering of parameters on the pytest command lines which I personally find confusing. I just realized, though, that the maxpool test in this same directory relies on these fixtures so I either need to remove that dependence or add back these fixtures.
| shape.extend(block_shape) | ||
| else: | ||
| shape.extend([off_h, off_w, shape_nhwc[3]]) | ||
| shape.append(ceildiv(shape_nhwc[3], off_c)) |
There was a problem hiding this comment.
In this code we handle ceildiv as-is, but in the get_packed_filter_shape function (below) we cast its result to a Python int type.
Is there a good reason for doing it two different ways?
Specifically: casting to int works when the runtime return type of ceildiv is tvm.tir.expr.IntImm. But I'm wondering if we're okay with this test making that assumption. I.e., are we testing everything we intend to test when we assume that the exact shapes are known at this point in time?
There was a problem hiding this comment.
Indeed, casting to int is a means of converting from tvm.tir.expr.IntImm to int. I am not sure why it works differently for get_packed_shape (no cast) vs. get_packed_filter_shape (requires cast). Also not sure about your question RE knowing exact shapes. More investigation required.
NOTE: I did change get_packed_filter_shape to take a shape instead of a list of dimensions to match the API of get_packed_shape.
adstraw
force-pushed
the
hexagon-conv2d-cleanup
branch
from
9a4c73f to
3be018b
Compare
|
Thank you @adstraw @cconvey @csullivan ; the PR has been merged! |
* cleanup Hexagon conv2d tests * add back fixtures; skip tests only on i386 * fix pylint error * fix maxpool failures * fix `skipif` statement and verify error + code review feedback * fix typo "physical_physical" * retrigger ci * determining i386 processor string from CI * hopefully the correct test disable
* cleanup Hexagon conv2d tests * add back fixtures; skip tests only on i386 * fix pylint error * fix maxpool failures * fix `skipif` statement and verify error + code review feedback * fix typo "physical_physical" * retrigger ci * determining i386 processor string from CI * hopefully the correct test disable
* cleanup Hexagon conv2d tests * add back fixtures; skip tests only on i386 * fix pylint error * fix maxpool failures * fix `skipif` statement and verify error + code review feedback * fix typo "physical_physical" * retrigger ci * determining i386 processor string from CI * hopefully the correct test disable
* cleanup Hexagon conv2d tests * add back fixtures; skip tests only on i386 * fix pylint error * fix maxpool failures * fix `skipif` statement and verify error + code review feedback * fix typo "physical_physical" * retrigger ci * determining i386 processor string from CI * hopefully the correct test disable
* cleanup Hexagon conv2d tests * add back fixtures; skip tests only on i386 * fix pylint error * fix maxpool failures * fix `skipif` statement and verify error + code review feedback * fix typo "physical_physical" * retrigger ci * determining i386 processor string from CI * hopefully the correct test disable
Deletes unnecessary cases from the test_conv2d_blocked.py including cases covering logical
oihwfilter and packednhw8h8wcpacked input layouts. Moved quite a lot of shared code to the infrastructure file reducing maintenance costs. And, some other general cleanup items --- comments, variable names, etc.