You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2020/09/08 10:16:27 UTC

[GitHub] [incubator-tvm] lhutton1 opened a new pull request #6419: [BUG][ConvertLayout] Fixes AttributeError during ConvertLayout to NHWC

lhutton1 opened a new pull request #6419:
URL: https://github.com/apache/incubator-tvm/pull/6419


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


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] lhutton1 commented on a change in pull request #6419: [BUG][ConvertLayout] Fixes AttributeError during ConvertLayout to NHWC

Posted by GitBox <gi...@apache.org>.
lhutton1 commented on a change in pull request #6419:
URL: https://github.com/apache/incubator-tvm/pull/6419#discussion_r484837153



##########
File path: python/tvm/relay/op/nn/_nn.py
##########
@@ -157,8 +157,9 @@ def convert_conv2d(attrs, inputs, tinfos, desired_layouts):
         return relay.nn.conv2d(data, weight, **new_attrs)
     elif desired_data_layout == 'NHWC':
         # Check for depthwise convolution.
-        if is_depthwise_conv2d(data.shape, attrs['data_layout'], weight.shape,
-                               attrs['kernel_layout'], attrs['groups']):
+        if is_depthwise_conv2d(data.checked_type.shape, attrs['data_layout'],

Review comment:
       I think it's ok to leave without an assert, getting `checked_type` will result in a check for its existence. See: https://github.com/apache/incubator-tvm/blob/0465ffda26a4e9d64944f01b33b566b6663d3afe/python/tvm/ir/expr.py#L36




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] lhutton1 edited a comment on pull request #6419: [BUG][ConvertLayout] Fixes AttributeError during ConvertLayout to NHWC

Posted by GitBox <gi...@apache.org>.
lhutton1 edited a comment on pull request #6419:
URL: https://github.com/apache/incubator-tvm/pull/6419#issuecomment-689104242


   Apologies, just realized I forgot to add a call from main in the tests.


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] lhutton1 commented on pull request #6419: [BUG][ConvertLayout] Fixes AttributeError during ConvertLayout to NHWC

Posted by GitBox <gi...@apache.org>.
lhutton1 commented on pull request #6419:
URL: https://github.com/apache/incubator-tvm/pull/6419#issuecomment-689104242


   Apologies, just realized I forgot to add a call to main in the tests.


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] comaniac commented on pull request #6419: [BUG][ConvertLayout] Fixes AttributeError during ConvertLayout to NHWC

Posted by GitBox <gi...@apache.org>.
comaniac commented on pull request #6419:
URL: https://github.com/apache/incubator-tvm/pull/6419#issuecomment-689072808


   cc @JoeyTPChou to take a look as well as he encountered the same issue weeks ago.


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] JoeyTPChou commented on pull request #6419: [BUG][ConvertLayout] Fixes AttributeError during ConvertLayout to NHWC

Posted by GitBox <gi...@apache.org>.
JoeyTPChou commented on pull request #6419:
URL: https://github.com/apache/incubator-tvm/pull/6419#issuecomment-689098892


   Thanks for the work!


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] leandron commented on a change in pull request #6419: [BUG][ConvertLayout] Fixes AttributeError during ConvertLayout to NHWC

Posted by GitBox <gi...@apache.org>.
leandron commented on a change in pull request #6419:
URL: https://github.com/apache/incubator-tvm/pull/6419#discussion_r484822607



##########
File path: python/tvm/relay/op/nn/_nn.py
##########
@@ -157,8 +157,9 @@ def convert_conv2d(attrs, inputs, tinfos, desired_layouts):
         return relay.nn.conv2d(data, weight, **new_attrs)
     elif desired_data_layout == 'NHWC':
         # Check for depthwise convolution.
-        if is_depthwise_conv2d(data.shape, attrs['data_layout'], weight.shape,
-                               attrs['kernel_layout'], attrs['groups']):
+        if is_depthwise_conv2d(data.checked_type.shape, attrs['data_layout'],

Review comment:
       It might worth putting an assert to make sure `*.checked_type.shape` exist at this point.




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] zhiics commented on pull request #6419: [BUG][ConvertLayout] Fixes AttributeError during ConvertLayout to NHWC

Posted by GitBox <gi...@apache.org>.
zhiics commented on pull request #6419:
URL: https://github.com/apache/incubator-tvm/pull/6419#issuecomment-689648231


   Thanks @lhutton1 @JoeyTPChou @leandron @anijain2305 


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] zhiics merged pull request #6419: [BUG][ConvertLayout] Fixes AttributeError during ConvertLayout to NHWC

Posted by GitBox <gi...@apache.org>.
zhiics merged pull request #6419:
URL: https://github.com/apache/incubator-tvm/pull/6419


   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] lhutton1 commented on a change in pull request #6419: [BUG][ConvertLayout] Fixes AttributeError during ConvertLayout to NHWC

Posted by GitBox <gi...@apache.org>.
lhutton1 commented on a change in pull request #6419:
URL: https://github.com/apache/incubator-tvm/pull/6419#discussion_r484837153



##########
File path: python/tvm/relay/op/nn/_nn.py
##########
@@ -157,8 +157,9 @@ def convert_conv2d(attrs, inputs, tinfos, desired_layouts):
         return relay.nn.conv2d(data, weight, **new_attrs)
     elif desired_data_layout == 'NHWC':
         # Check for depthwise convolution.
-        if is_depthwise_conv2d(data.shape, attrs['data_layout'], weight.shape,
-                               attrs['kernel_layout'], attrs['groups']):
+        if is_depthwise_conv2d(data.checked_type.shape, attrs['data_layout'],

Review comment:
       I think it's ok to leave without an assert, getting `checked_type` will result in a check for its existence. See: https://github.com/apache/incubator-tvm/blob/0465ffda26a4e9d64944f01b33b566b6663d3afe/python/tvm/ir/expr.py#L49




----------------------------------------------------------------
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:
users@infra.apache.org