Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fix][Frontend][TOPI] minor bugs #8622

Merged
merged 3 commits into from
Aug 5, 2021
Merged

[Fix][Frontend][TOPI] minor bugs #8622

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ecdd872
fix
hzfan Aug 2, 2021
15e2d13
fix
hzfan Aug 3, 2021
d3a3ae1
lint
hzfan Aug 3, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion include/tvm/topi/detail/ravel_unravel.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,9 @@ using namespace tvm::te;
*/
inline PrimExpr RavelIndex(Array<PrimExpr> indices, Array<PrimExpr> shape) {
ICHECK_EQ(indices.size(), shape.size()) << "indices and shape must have equal size";
ICHECK_GT(indices.size(), 0) << "indices must not be empty";
if (indices.size() == 0U) {
return 0;
}
PrimExpr idx;
for (size_t i = 0; i < indices.size(); ++i) {
if (i == 0) {
Expand Down
2 changes: 1 addition & 1 deletion python/tvm/relay/frontend/pytorch.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1445,7 +1445,7 @@ def linear(self, inputs, input_types):
# 0 - input
# 1 - weight
bias = inputs[2]
mm_out = self.matmul(inputs[:2], input_types[:2])
mm_out = self.matmul([inputs[0], _op.transpose(inputs[1], axes=(1, 0))], input_types[:2])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can use _op.nn.dense without transposing its second input. Also see https://discuss.tvm.apache.org/t/pytorch-unable-to-import-a-simple-torch-nn-linear-to-relay/10383/4

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I missed something, does that mean the current linear layers converted from PyTorch are wrong?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, see my comment in the discuss thread above. Until PT 1.8, nn.Linear converts to aten::addmm, so this code path never hit unless users explicitly call something that converts to aten::linear. Since PT 1.9, nn.Linear converts to aten::linear, so this becomes a major issue.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I see, so this bug doesn't explode until PT 1.9. Thanks for the explanation!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@masahi My concern is that linear is supposed to support inputs with arbitrary ranks like 1d, 2d, 3d, 4d.... The different input dimensions are normalized in self.matmul. If _op.nn.dense is directly used, the usability of linear would be limited to inputs whose ranks are 2.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about we just make an exception: when the input rank is 2 then use dense; otherwise use matmul?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't op.nn.dense also support ND input?

tvm/python/tvm/relay/op/nn/nn.py

Lines 1528 to 1530 in 850abb0

data : tvm.relay.Expr
The input data to the operator,
of shape `(d_1, d_2, ..., d_n, units_in)`.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also if we want to support ND input, please add more tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@masahi Tests with 3d input are added. For now I use dense when both inputs are 2d and fallback to matmul otherwise.

I tried op.nn.dense in ND scenario, but it fails at

tvm/python/tvm/topi/x86/dense_alter_op.py

Lines 29 to 37 in 4b9d43e

@dense_alter_layout.register(["cpu", "arm_cpu"])
def _alter_dense_layout(attrs, inputs, tinfos, out_type):
target = tvm.target.Target.current(allow_none=False)
dispatch_ctx = autotvm.task.DispatchContext.current
data_tensor, weight_tensor = tinfos
out_dtype = out_type.dtype
M, K = get_const_tuple(data_tensor.shape)
N, _ = get_const_tuple(weight_tensor.shape)

where data_tensor.shape is unpacked into a tuple of size 2. I also check that in tests/python/relay/test_op_level1.py dense is tested numerically for 2d inputs only. Not sure if I miss anything.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it looks like relay doc is wrong and topi dense indeed only supports 2D #8412

if isinstance(bias, _expr.Expr):
bias_ndims = len(self.infer_shape_with_prelude(bias))
if bias_ndims == 1:
Expand Down
3 changes: 3 additions & 0 deletions tests/python/frontend/pytorch/test_forward.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1571,6 +1571,7 @@ def forward(self, input, weight):
input2d = torch.rand([2, 2]).float()
weight1d = torch.rand([2]).float()
weight2d = torch.rand([2, 2]).float()
weight3x2 = torch.rand([3, 2]).float()
bias1d = torch.rand([2]).float()
bias2d = torch.rand([2, 2]).float()
# 2D input, 2D weight, 1D bias
Expand All @@ -1579,6 +1580,7 @@ def forward(self, input, weight):
verify_model(Linear(), input_data=[input2d, weight2d, bias2d])
# 2D input, 2D weight, no bias
verify_model(LinearNoBias(), input_data=[input2d, weight2d])
verify_model(LinearNoBias(), input_data=[input2d, weight3x2])
# 2D input, 1D weight, 1D bias is not supported by torch.linear()
# 2D input, 1D weight, no bias
verify_model(LinearNoBias(), input_data=[input2d, weight1d])
Expand Down Expand Up @@ -3981,6 +3983,7 @@ def forward(self, x):
test_forward_logsoftmax()
test_forward_sigmoid()
test_forward_dense()
test_forward_linear()
test_forward_avgpool1d()
test_forward_avgpool2d()
test_forward_avgpool3d()
Expand Down
1 change: 1 addition & 0 deletions tests/python/relay/test_op_level3.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,7 @@ def verify_reshape(shape, newshape, oshape):
verify_reshape((2, 3, 4), (-3, -2), (6, 4))
verify_reshape((2, 3, 4), (-4, 1, 2, -2), (1, 2, 3, 4))
verify_reshape((2, 3, 4), (2, -4, -1, 3, -2), (2, 1, 3, 4))
verify_reshape((1,), (), ())


def test_reshape_fail():
Expand Down