tvm-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [incubator-tvm] anijain2305 commented on a change in pull request #5422: [RELAY][Convert Layout] Specify additional layouts in convert layout pass
Date Tue, 05 May 2020 21:55:25 GMT

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



##########
File path: docs/dev/convert_layout.rst
##########
@@ -116,16 +117,21 @@ These steps happen for each operator in sequence, where ConvertLayout
pass keeps
         data_layout = attrs['data_layout']
         kernel_layout = attrs['kernel_layout']
         data, weight = inputs
-        assert desired_layout == 'NCHW', \
-                "Currently only transformation to NCHW layout is supported."
+
+        # Use the first entry in desired layouts which specifies the data layout.
+        # The expected ordering of layouts for this operator is defined by this function.
+        desired_data_layout = str(desired_layouts[0])
+
         if desired_layout == 'NCHW':
             new_attrs = dict(attrs)
-            new_attrs['data_layout'] = desired_layout
+            new_attrs['data_layout'] = desired_data_layout
             new_attrs['kernel_layout'] = 'OIHW'
             # Actual insertion of layout transforms is taken care internally
             # by ConvertLayout pass.
             return relay.nn.conv2d(data, weight, **new_attrs)
-        return None
+        else:
+            assert "Layout %s is not yet supported" % desired_data_layout
+            return None

Review comment:
       One way to do this is to move the assert to L124.
   
   ~~~
   assert desired_layout == ...
   if desired_layout == 'NCHW':
       .....
   return None
   ~~~

##########
File path: python/tvm/relay/op/nn/_nn.py
##########
@@ -141,20 +142,29 @@ def convert_conv2d(attrs, inputs, tinfos, desired_layout):
     from tvm import relay
     data, weight = inputs
     new_attrs = dict(attrs)
-    new_attrs['data_layout'] = desired_layout
-    if desired_layout == 'NCHW':
+    desired_data_layout = str(desired_layouts[0])
+    assert desired_data_layout != "default", "Data layout cannot be default"
+    new_attrs['data_layout'] = desired_data_layout
+
+    if len(desired_layouts) > 1:
+        desired_kernel_layout = str(desired_layouts[1])
+        if desired_kernel_layout != "default":
+            new_attrs['kernel_layout'] = desired_kernel_layout
+            return relay.nn.conv2d(data, weight, **new_attrs)
+
+    # Handle default kernel layouts
+    if desired_data_layout == 'NCHW':
         new_attrs['kernel_layout'] = 'OIHW'
         return relay.nn.conv2d(data, weight, **new_attrs)
-    elif desired_layout == 'NHWC':
+    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']):
             new_attrs['kernel_layout'] = 'HWOI'
         else:
             new_attrs['kernel_layout'] = 'HWIO'
         return relay.nn.conv2d(data, weight, **new_attrs)
-    else:
-        assert "Layout %s is not yet supported." % (desired_layout)
+    assert "Layout %s is not yet supported." % (desired_data_layout)
     return None

Review comment:
       Ditto

##########
File path: docs/dev/convert_layout.rst
##########
@@ -218,24 +224,49 @@ Second example is for a lightly-layout sensitive operator - batch normalization.
 
 ConvertLayout pass is extremely easy to use. The pass is not a part of default relay.build
pipeline. The intended usage is to call it between the framework-to-relay parser and relay.build
module call.
 
+In order to specify the layouts to convert to, we create a mapping of heavily-layout sensitive
operators to a list of the desired layouts for that operator.
+
 .. code-block:: python
 
     # TFlite framework to Relay parser - Default layout is NHWC
     mod, params = relay.frontend.from_tflite(tflite_model,
                                              shape_dict=shape_dict,
                                              dtype_dict=dtype_dict)
 
+    # We assume our model's heavily-layout sensitive operators only consist of nn.conv2d
+    desired_layouts = {'nn.conv2d': ['NCHW']}

Review comment:
       How about having stricter requirement here?
   
   User must specify default
   ~~~
   desired_layouts = {'nn.conv2d': ['NCHW', 'default']}
   ~~~
   
   @comaniac What do you think? We can add a comment in the Conv2d convert layout function
to specify what default means.

##########
File path: tests/python/relay/test_pass_convert_op_layout.py
##########
@@ -625,3 +685,5 @@ def expected():
     test_qnn_conv_requantize_convert_layout()
     test_qnn_conv_concat_convert_layout()
     test_qnn_conv_add_convert_layout()
+    test_conv_convert_kernel_layout()
+    test_default_keyword()

Review comment:
       A soft suggestion - Does it make sense to add a test that tests conversion for 2 operators
simultaneously - Like conv3d and conv2d in the same network? Or qnn.conv2d and nn.conv2d in
the same network.




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



Mime
View raw message