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.

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

AttributeError during ConvertLayout to NHWC #6410

Closed
leandron opened this issue Sep 7, 2020 · 6 comments
Closed

AttributeError during ConvertLayout to NHWC #6410

leandron opened this issue Sep 7, 2020 · 6 comments

Comments

@leandron
Copy link
Contributor

leandron commented Sep 7, 2020

When using tvm.relay.transform.ConvertLayout (in the context of #6302), I'm getting some errors that are not really clear on how they are related to the convert process.

It seems ConvertLayout it is looking for shape (this check was introduced in #5284), when trying to find depthwise convolutions, but it doesn't seem to be the case that shape is there for some imported models, and might need some fixing.
https://github.com/apache/incubator-tvm/blob/4b48d89c79a72f7799606e845bdb1ed938baa115/python/tvm/relay/op/nn/_nn.py#L158-L164

This is the error I see, when trying to run ConvertLayout on an ONNX model:

AttributeError: <class 'tvm.relay.expr.TupleGetItem'> has no attribute shape

To reproduce the issue, you can run the script below:

  1. Download resnet50:
wget https://github.com/onnx/models/raw/master/vision/classification/resnet/model/resnet50-v2-7.onnx
  1. Run the ConvertLayout following the script below:
import onnx
import tvm
from tvm import relay

# boilerplate to load the ONNX model
model = onnx.load("resnet50-v2-7.onnx")
name = model.graph.input[0].name
proto_shape = model.graph.input[0].type.tensor_type.shape.dim
shape = [d.dim_value for d in proto_shape]
shape_dict = {name: shape}
mod, params = relay.frontend.from_onnx(model, shape_dict)

# converting layout
desired_layouts = {'nn.conv2d': ['NHWC', 'default']}
seq = tvm.transform.Sequential([relay.transform.RemoveUnusedFunctions(),
                                relay.transform.ConvertLayout(desired_layouts)])

with tvm.transform.PassContext(opt_level=3):
    mod = seq(mod)

with relay.build_config(opt_level=3):
     graph, lib, params = relay.build(mod, "llvm", params=params)

PS: It also happens if I get a NHWC TFlite model and try to force it into an NHWC convertion, but that is a negative test, that is why I didn't add it here.

cc @comaniac @jwfromm

@lhutton1
Copy link
Contributor

lhutton1 commented Sep 7, 2020

I think this is happening because the shape of data is accessed by data.shape, at some point something in TVM changed meaning shape needs to be accessed by data.checked_type.shape. It looks like the convert layout tests didn't pick this up.

lhutton1 added a commit to lhutton1/tvm that referenced this issue Sep 8, 2020
Fixes an issue described in apache#6410. In order to retrieve the shape a tensor `checked_type` should be used.

Change-Id: I991d194d9cc15ee20464ff2e239fd05c035000c8
lhutton1 added a commit to lhutton1/tvm that referenced this issue Sep 8, 2020
Fixes an issue described in apache#6410. In order to retrieve the shape a tensor `checked_type` should be used.

Change-Id: I991d194d9cc15ee20464ff2e239fd05c035000c8
lhutton1 added a commit to lhutton1/tvm that referenced this issue Sep 8, 2020
Fixes an issue described in apache#6410. In order to retrieve the shape a tensor `checked_type` should be used.

Change-Id: I991d194d9cc15ee20464ff2e239fd05c035000c8
lhutton1 added a commit to lhutton1/tvm that referenced this issue Sep 8, 2020
Fixes an issue described in apache#6410. In order to retrieve the shape a tensor `checked_type` should be used.

Change-Id: I991d194d9cc15ee20464ff2e239fd05c035000c8
@zhiics
Copy link
Member

zhiics commented Sep 8, 2020

cc @anijain2305

@anijain2305
Copy link
Contributor

Thanks @leandron for raising the issue. Your fixes look good to me.

@leandron
Copy link
Contributor Author

leandron commented Sep 9, 2020

Just to mention, I checked PR #6419 and it completely fixes this issue here, so we can close it once that PR is merged.

zhiics pushed a commit that referenced this issue Sep 9, 2020
…WC (#6419)

Fixes an issue described in #6410. In order to retrieve the shape a tensor `checked_type` should be used.

Change-Id: I991d194d9cc15ee20464ff2e239fd05c035000c8
@zhiics
Copy link
Member

zhiics commented Sep 9, 2020

Thanks @leandron. This can be closed now.

@zhiics zhiics closed this as completed Sep 9, 2020
kevinthesun pushed a commit to kevinthesun/tvm that referenced this issue Sep 17, 2020
…WC (apache#6419)

Fixes an issue described in apache#6410. In order to retrieve the shape a tensor `checked_type` should be used.

Change-Id: I991d194d9cc15ee20464ff2e239fd05c035000c8
kevinthesun pushed a commit to kevinthesun/tvm that referenced this issue Sep 18, 2020
…WC (apache#6419)

Fixes an issue described in apache#6410. In order to retrieve the shape a tensor `checked_type` should be used.

Change-Id: I991d194d9cc15ee20464ff2e239fd05c035000c8
trevor-m pushed a commit to neo-ai/tvm that referenced this issue Sep 18, 2020
…WC (apache#6419)

Fixes an issue described in apache#6410. In order to retrieve the shape a tensor `checked_type` should be used.

Change-Id: I991d194d9cc15ee20464ff2e239fd05c035000c8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants