anijain2305 commented on a change in pull request #6443:
URL: https://github.com/apache/incubator-tvm/pull/6443#discussion_r487234456



##########
File path: python/tvm/relay/op/vision/rcnn.py
##########
@@ -24,7 +24,7 @@ def roi_align(data, rois, pooled_size, spatial_scale, 
sample_ratio=-1, layout='N
     Parameters
     ----------
     data : relay.Expr
-        4-D tensor with shape [batch, channel, height, width]
+        4-D tensor with shape [batch, channel, height, width] or [batch, 
height, width, channel]

Review comment:
       Hi @Beya2019 ,
   We understand your usecase with the special target. But, when we put add 
changes in TVM, we want to ensure that we don't break things for x86, arm and 
gpus. Now, what @kevinthesun and I are saying is that if you try compiling a 
graph for x86(or gpu) with this PR, you should see compilation failure. Our 
reasoning is that the ConvertLayout will introduce roi_align with NHWC layout, 
and then InferType will be called, this will cause failure. Can you please try 
this and confirm?

##########
File path: python/tvm/relay/op/vision/rcnn.py
##########
@@ -24,7 +24,7 @@ def roi_align(data, rois, pooled_size, spatial_scale, 
sample_ratio=-1, layout='N
     Parameters
     ----------
     data : relay.Expr
-        4-D tensor with shape [batch, channel, height, width]
+        4-D tensor with shape [batch, channel, height, width] or [batch, 
height, width, channel]

Review comment:
       Hi @Beya2019 ,
   We understand your usecase with the special target. But, when we put add 
changes in TVM, we want to ensure that we don't break things for x86, arm and 
gpus. Now, what @kevinthesun and I are saying is that if you try compiling a 
graph for x86(or gpu) with this PR, you should see compilation failure. Our 
reasoning is that the ConvertLayout will introduce roi_align with NHWC layout, 
and then InferType will be called, this will cause failure. Can you please try 
this and confirm?

##########
File path: python/tvm/relay/op/vision/rcnn.py
##########
@@ -24,7 +24,7 @@ def roi_align(data, rois, pooled_size, spatial_scale, 
sample_ratio=-1, layout='N
     Parameters
     ----------
     data : relay.Expr
-        4-D tensor with shape [batch, channel, height, width]
+        4-D tensor with shape [batch, channel, height, width] or [batch, 
height, width, channel]

Review comment:
       Hi @Beya2019 ,
   We understand your usecase with the special target. But, when we put add 
changes in TVM, we want to ensure that we don't break things for x86, arm and 
gpus. Now, what @kevinthesun and I are saying is that if you try compiling a 
graph for x86(or gpu) with this PR, you should see compilation failure. Our 
reasoning is that the ConvertLayout will introduce roi_align with NHWC layout, 
and then InferType will be called, this will cause failure. Can you please try 
this and confirm?




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


Reply via email to