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