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/19 03:11:42 UTC

[GitHub] [incubator-tvm] Beya2019 opened a new pull request #6516: [RELAY][OP] roi_pool operator alter layout

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


   According to the discussion of roi_align(https://github.com/apache/incubator-tvm/pull/6443)
   
   add aoi_pool operator(in fasterrcnn) convert_op_layout and related test case in test_pass_convert_op_layout.py.
   
   Meantime, I also test the case tests/python/frontend/pytorch/test_object_detection.py(changed the generate_jit_model to fasterrcnn) to verify this roi pool operator.
   
   Would you please have a look at this @kevinthesun @anijain2305. Thanks very much.


----------------------------------------------------------------
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] Beya2019 commented on pull request #6516: [RELAY][OP] roi_pool operator alter layout

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


   Hi @yzhliu, can you help to have a look at this submit? Thanks very much.


----------------------------------------------------------------
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] tqchen merged pull request #6516: [RELAY][OP] roi_pool operator alter layout

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


   


----------------------------------------------------------------
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] Beya2019 commented on pull request #6516: [RELAY][OP] roi_pool operator alter layout

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


   > my bad, was thinking of other stuffs.
   
   Can you help merge the pull request directly? Thanks very much


----------------------------------------------------------------
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] tkonolige commented on a change in pull request #6516: [RELAY][OP] roi_pool operator alter layout

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



##########
File path: src/relay/op/vision/rcnn_op.cc
##########
@@ -119,14 +119,34 @@ bool ROIPoolRel(const Array<Type>& types, int num_inputs, const Attrs& attrs,
   CHECK(roi_pool_attrs);
   CHECK_EQ(dshape.size(), 4) << "Input data should be 4-D.";
   CHECK_EQ(rshape.size(), 2) << "Input rois should be 2-D.";
-  CHECK_EQ(roi_pool_attrs->layout, "NCHW") << "ROI Pool only supports NCHW layout";
   // assign output type
-  std::vector<IndexExpr> oshape(
-      {rshape[0], dshape[1], roi_pool_attrs->pooled_size[0], roi_pool_attrs->pooled_size[1]});
+  std::vector<IndexExpr> oshape;
+  if (roi_pool_attrs->layout == "NCHW") {
+    oshape = {rshape[0], dshape[1], roi_pool_attrs->pooled_size[0], roi_pool_attrs->pooled_size[1]};
+  } else {
+    CHECK_EQ(roi_pool_attrs->layout, "NHWC") << "Unexpected ROI Pool layout";

Review comment:
       Maybe you could add what the expected layout should be?




----------------------------------------------------------------
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] Beya2019 commented on a change in pull request #6516: [RELAY][OP] roi_pool operator alter layout

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



##########
File path: src/relay/op/vision/rcnn_op.cc
##########
@@ -119,14 +119,34 @@ bool ROIPoolRel(const Array<Type>& types, int num_inputs, const Attrs& attrs,
   CHECK(roi_pool_attrs);
   CHECK_EQ(dshape.size(), 4) << "Input data should be 4-D.";
   CHECK_EQ(rshape.size(), 2) << "Input rois should be 2-D.";
-  CHECK_EQ(roi_pool_attrs->layout, "NCHW") << "ROI Pool only supports NCHW layout";
   // assign output type
-  std::vector<IndexExpr> oshape(
-      {rshape[0], dshape[1], roi_pool_attrs->pooled_size[0], roi_pool_attrs->pooled_size[1]});
+  std::vector<IndexExpr> oshape;
+  if (roi_pool_attrs->layout == "NCHW") {
+    oshape = {rshape[0], dshape[1], roi_pool_attrs->pooled_size[0], roi_pool_attrs->pooled_size[1]};
+  } else {
+    CHECK_EQ(roi_pool_attrs->layout, "NHWC") << "Unexpected ROI Pool layout";

Review comment:
       Hi @tkonolige :
     Do you mean this?
   ```
     if (roi_pool_attrs->layout == "NCHW") {
       oshape = {rshape[0], dshape[1], roi_pool_attrs->pooled_size[0], roi_pool_attrs->pooled_size[1]};
     } else if (roi_pool_attrs->layout == "NHWC") {
       oshape = {rshape[0], roi_pool_attrs->pooled_size[0], roi_pool_attrs->pooled_size[1], dshape[3]};
     } else {
       LOG(FATAL) << "vision.roi_pool does not support " << roi_pool_attrs->layout << " layout";
     }
   ```
   




----------------------------------------------------------------
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] Beya2019 commented on pull request #6516: [RELAY][OP] roi_pool operator alter layout

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


   Hi @kevinthesun:
   
     can you help to have a look at this submit?  Thanks very much.
   


----------------------------------------------------------------
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] yzhliu commented on a change in pull request #6516: [RELAY][OP] roi_pool operator alter layout

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



##########
File path: python/tvm/relay/op/vision/_rcnn.py
##########
@@ -69,11 +69,53 @@ def convert_roi_align(attrs, inputs, tinfos, desired_layouts):
     raise ValueError("Layout %s is not yet supported." % desired_data_layout)
 
 
+@reg.register_convert_op_layout("vision.roi_pool")
+def convert_roi_pool(attrs, inputs, tinfos, desired_layouts):
+    """Convert Layout pass registration for roi_pool op.
+
+    Parameters
+    ----------
+    attrs : tvm.ir.Attrs
+        Attributes of current roi_pool
+    inputs : list of tvm.relay.Expr
+        The args of the Relay expr to be legalized
+    tinfos : list of types
+        List of input and output types
+    desired_layouts : list of layout strings
+        List of layouts defining our desired
+        layout for the data and rois inputs respectively.
+
+    Returns
+    -------
+    result : tvm.relay.Expr
+        The transformed expr
+    """
+    # pylint: disable=import-outside-toplevel
+    from tvm import relay
+
+    data, rois = inputs
+    new_attrs = dict(attrs)
+    assert (
+        len(desired_layouts) == 2
+    ), "A desired layout is expected for both of vision.roi_pool's inputs"
+
+    desired_data_layout, desired_rois_layout = map(str, desired_layouts)
+    assert desired_data_layout != "default", "Data layout cannot be default"
+    assert desired_rois_layout == "default", "Rois layout must be default"
+
+    new_attrs["layout"] = desired_data_layout
+    # rois layout not change
+    if desired_data_layout in ["NCHW", "NHWC"]:
+        return relay.vision.roi_pool(data, rois, **new_attrs)
+
+    raise ValueError("Layout %s is not yet supported." % desired_data_layout)

Review comment:
       return None to keep original layout instead of raising exception?




----------------------------------------------------------------
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