Adapt the convertor to tensorrt backend #47

kuke · 2018-05-09T09:21:53Z

Resolve #46

The TensorRT backend for onnx only supports onnx==1.0.1 right now, while we were carrying out experiments on master branch of ONNX and caffe2 backend. Hence, we need to make some adaptions to enable the converted models running with TensorRT backend.

Now the conversion's correctness of all the current supported models has been validated on TensorRT, including:

fit_a_line
recognize_digits
VGG16 & ResNet50
MobileNet
SE_ResNeXt

kuke · 2018-05-09T10:19:41Z

fluid_to_onnx.py

+        onnx_graph = helper.make_graph(
+            nodes=onnx_nodes,
+            name=model_name,
+            initializer=weights,


TensorRT backend distinguishes weights and tensors, so we have to use initializer to initialize parameters.

Great - so this is consistent with our original plan to add the initializer in at some point! Very happy to see this

This also solves some problems we are having when trying to use other ONNX convertors

Do we have to initialize the tmp inputs and outputs to the ops too? @nickyfantasy is having issues converting the model to the qualcomm DLC spec

Yeah. initializer is the officially recommended to initialize parameters, and the issues under onnx/models show that they may deprecate the Constant op initialization someday.

It should not be the proper way to initialize the tmp inputs and outputs to the ops. Let's figure out what happened here after Qualcomm prepare related operators.

@kuke I think @nickyfantasy found out why that issue was happening. you are right, it isn't the right way to initialize tmp inputs/outputs, so we are good.

kuke · 2018-05-09T10:20:48Z

fluid_onnx/ops.py

-        outputs=outputs['Out'],
-        scale=1.0 - attrs['dropout_prob'])
-    return (dropout_op, scale_op)
+    if __onnx_ver__ == '1.0.1':


TensorRT doesn't support Scale op

Is there a way we could use a version range or max / min comparison instead of hardcoding a specific version? And in the last possible resort where we were going to, perhaps make it a constant at the top?

I wish I understood what this means fully: "note that our implementation of Dropout does scaling in the training phase", but do we still need to scale given this context

How about we temporarily fix the version number to 1.0.1 right now? because there are several releases, and I have not checked the changes in the definition of operators.

You got the point. Yes, we need to scale the output of Dropout op, otherwise inference wouldn't get right result.

@kuke Okay, no worries, but we would probably need to come back to this, as it will bite as we try to make our ONNX models compatible for various backends.

kuke · 2018-05-09T10:22:06Z

fluid_onnx/ops.py

-        inputs=inputs['Y'],
-        outputs=y_flat_out,
-        axis=attrs['y_num_col_dims'])
+    if __onnx_ver__ == '1.0.1':


The input dim of Flatten op is limited to equal to 3.

This would be awesome as a comment in the code :)

kuke · 2018-05-09T10:23:16Z

fluid_onnx/ops.py

+        output_node = make_node(
+            'Reshape',
+            inputs=matmul_out,
+            shape=out_shape,


shape is an attribute in onnx v1.0.1

Same as above, code comment

varunarora

I think its fair to update the pip requirements file also to 1.0.1.

varunarora · 2018-05-09T23:00:34Z

fluid_to_onnx.py

+        onnx_graph = helper.make_graph(
+            nodes=onnx_nodes,
+            name=model_name,
+            initializer=weights,


Great - so this is consistent with our original plan to add the initializer in at some point! Very happy to see this

varunarora · 2018-05-09T23:56:26Z

fluid_to_onnx.py

+        onnx_graph = helper.make_graph(
+            nodes=onnx_nodes,
+            name=model_name,
+            initializer=weights,


This also solves some problems we are having when trying to use other ONNX convertors

varunarora · 2018-05-10T00:01:13Z

fluid_onnx/ops.py

@@ -68,13 +72,6 @@ def activation_ops(act_type, operator, block):
        act_type, inputs=inputs.values()[0], outputs=outputs.values()[0])


-def and_op():


Is there a reason for removing this placeholder function?

Oh nvm I realize this is merged into the elementwise stuff

Yes. It is no longer needed.

varunarora · 2018-05-10T00:10:21Z

fluid_onnx/ops.py

@@ -87,36 +84,52 @@ def batch_norm_op(operator, block):
    inputs, attrs, outputs = op_io_info(operator)

    x_shape = block.vars[get_old_name(inputs['X'][0])].shape
-    reshape_node = None
+    nodes = ()
    if len(x_shape) == 2:


A couple of us are stuck on why we do it this way. Then using a constant-less version for v1.0.1. Would love if you could add in some comments here - mostly for educating us.

Added the comment about why we need reshape here. And why didn't use Constant op in branch 1.0.1? the answer is that in onnx1.0.1 shape is used as an attribute and doesn't need a constant op to feed value.

varunarora · 2018-05-10T00:15:05Z

fluid_onnx/ops.py

+        'momentum': attrs['momentum']
+    }
+    if __onnx_ver__ == u'1.0.1':
+        kwargs['consumed_inputs'] = [0, 0, 0, 1, 1]


How did you figure the value of this out? Any reference to papers / code?

No docs found. If don't set the consumed_inputs, ONNX will give an error

onnx.onnx_cpp2py_export.checker.ValidationError: Input index 3 must be set to consumed for operator BatchNormalization

So I looked into the code in ONNX 1.0.1, finally find this attribute and its proper usage.

varunarora · 2018-05-10T00:41:36Z

fluid_to_onnx.py

-                onnx_nodes.append(param_node)
+                weight, val_info = paddle_onnx_weight(
+                    var=var, scope=inference_scope)
+                weights.append(weight), weights_value_info.append(val_info)


Oh just do the two appends over two lines, so this doesn't confuse anyone

varunarora · 2018-05-10T00:46:13Z

fluid_to_onnx.py

+        onnx_graph = helper.make_graph(
+            nodes=onnx_nodes,
+            name=model_name,
+            initializer=weights,


Do we have to initialize the tmp inputs and outputs to the ops too? @nickyfantasy is having issues converting the model to the qualcomm DLC spec

varunarora · 2018-05-10T00:57:19Z

fluid_onnx/ops.py

-        inputs=inputs['Y'],
-        outputs=y_flat_out,
-        axis=attrs['y_num_col_dims'])
+    if __onnx_ver__ == '1.0.1':


This would be awesome as a comment in the code :)

varunarora · 2018-05-10T00:57:57Z

fluid_onnx/ops.py

+        output_node = make_node(
+            'Reshape',
+            inputs=matmul_out,
+            shape=out_shape,


Same as above, code comment

varunarora · 2018-05-10T01:06:23Z

fluid_onnx/ops.py

+            'Flatten',
+            inputs=inputs['Y'],
+            outputs=y_flat_out,
+            axis=attrs['y_num_col_dims'])

    # Mat mul 
    matmul_out = [outputs['Out'][0] + '@matmul_0']
    matmul_node = make_node(
        'MatMul', inputs=x_flat_out + y_flat_out, outputs=matmul_out)


@nickyfantasy is asking if we can use the Mul op here

Seems cannot, Mul op is for element-wise production

kuke

@varunarora Thanks for the review!

I find that the ONNX release 1.1.2 is far behind the master branch, not the same as I thought. Maybe we'd better only consider v1.0.1 and the latest master branch of ONNX temporarily, and go back to the
compatibility some day in the future.

kuke · 2018-05-15T02:47:21Z

fluid_onnx/ops.py

@@ -68,13 +72,6 @@ def activation_ops(act_type, operator, block):
        act_type, inputs=inputs.values()[0], outputs=outputs.values()[0])


-def and_op():


Yes. It is no longer needed.

kuke · 2018-05-15T02:58:57Z

validate.py

+        from caffe2.python.onnx.backend import Caffe2Backend
+        rep = Caffe2Backend.prepare(onnx_model, device='CPU')
+    else:
+        import onnx_tensorrt.backend as backend


Great! Appreciate that.

kuke · 2018-05-15T03:17:28Z

fluid_to_onnx.py

+        onnx_graph = helper.make_graph(
+            nodes=onnx_nodes,
+            name=model_name,
+            initializer=weights,


Yeah. initializer is the officially recommended to initialize parameters, and the issues under onnx/models show that they may deprecate the Constant op initialization someday.

It should not be the proper way to initialize the tmp inputs and outputs to the ops. Let's figure out what happened here after Qualcomm prepare related operators.

kuke · 2018-05-15T03:19:02Z

fluid_to_onnx.py

-                onnx_nodes.append(param_node)
+                weight, val_info = paddle_onnx_weight(
+                    var=var, scope=inference_scope)
+                weights.append(weight), weights_value_info.append(val_info)


kuke · 2018-05-15T03:19:17Z

fluid_onnx/ops.py

+        output_node = make_node(
+            'Reshape',
+            inputs=matmul_out,
+            shape=out_shape,


kuke · 2018-05-15T03:20:37Z

fluid_onnx/ops.py

+            'Flatten',
+            inputs=inputs['Y'],
+            outputs=y_flat_out,
+            axis=attrs['y_num_col_dims'])

    # Mat mul 
    matmul_out = [outputs['Out'][0] + '@matmul_0']
    matmul_node = make_node(
        'MatMul', inputs=x_flat_out + y_flat_out, outputs=matmul_out)


Seems cannot, Mul op is for element-wise production

kuke · 2018-05-15T03:34:52Z

fluid_onnx/ops.py

+        'momentum': attrs['momentum']
+    }
+    if __onnx_ver__ == u'1.0.1':
+        kwargs['consumed_inputs'] = [0, 0, 0, 1, 1]


No docs found. If don't set the consumed_inputs, ONNX will give an error

onnx.onnx_cpp2py_export.checker.ValidationError: Input index 3 must be set to consumed for operator BatchNormalization

So I looked into the code in ONNX 1.0.1, finally find this attribute and its proper usage.

kuke · 2018-05-15T03:53:08Z

fluid_onnx/ops.py

@@ -87,36 +84,52 @@ def batch_norm_op(operator, block):
    inputs, attrs, outputs = op_io_info(operator)

    x_shape = block.vars[get_old_name(inputs['X'][0])].shape
-    reshape_node = None
+    nodes = ()
    if len(x_shape) == 2:


Added the comment about why we need reshape here. And why didn't use Constant op in branch 1.0.1? the answer is that in onnx1.0.1 shape is used as an attribute and doesn't need a constant op to feed value.

kuke · 2018-05-15T08:28:43Z

fluid_onnx/ops.py

-        outputs=outputs['Out'],
-        scale=1.0 - attrs['dropout_prob'])
-    return (dropout_op, scale_op)
+    if __onnx_ver__ == '1.0.1':


How about we temporarily fix the version number to 1.0.1 right now? because there are several releases, and I have not checked the changes in the definition of operators.

You got the point. Yes, we need to scale the output of Dropout op, otherwise inference wouldn't get right result.

kuke · 2018-05-15T08:30:18Z

fluid_onnx/ops.py

-        inputs=inputs['Y'],
-        outputs=y_flat_out,
-        axis=attrs['y_num_col_dims'])
+    if __onnx_ver__ == '1.0.1':


varunarora · 2018-05-15T21:06:17Z

Got it. Instead, why don't we just make 1.0.1 as our main version supported. And then do a if version > 1.0.1: use original op written for a higher version.

We can't guarantee support of master, but we can expect 1.0.0 or 1.0.1 as a stable point for all backends.

kuke · 2018-05-16T01:28:39Z

@varunarora Thanks! It is good to use 1.0.1 as the stable point. I'll try to test changes and make them work on both 1.0.1 and latest master. We can go back to make the compatibility more elegant in future.

Yibing Liu added 4 commits May 7, 2018 06:04

Avoid shape op to be compatible with lower version onnx

9d0027d

Run fit_a_line model on tensorrt

d4e571d

Make changes in batch_norm_op & dropout_op

de18330

Tiny fixes in ops

d306c7f

kuke commented May 9, 2018

View reviewed changes

kuke requested review from Superjomn, varunarora, pkuyym and sidgoyal78 May 9, 2018 10:25

varunarora reviewed May 10, 2018

View reviewed changes

Yibing Liu added 2 commits May 10, 2018 20:37

Add the conversion of lrn_op

dea22ab

Add some necessary comments

b7337f0

kuke force-pushed the remove_shape branch from 746e142 to b7337f0 Compare May 15, 2018 07:08

kuke commented May 15, 2018

View reviewed changes

varunarora approved these changes May 15, 2018

View reviewed changes

kuke merged commit f90177e into PaddlePaddle:develop May 16, 2018

varunarora mentioned this pull request May 16, 2018

We need distinguish persistable vars and const op #27

Closed

		@@ -68,13 +72,6 @@ def activation_ops(act_type, operator, block):
		act_type, inputs=inputs.values()[0], outputs=outputs.values()[0])


		def and_op():

Adapt the convertor to tensorrt backend #47

Adapt the convertor to tensorrt backend #47

Conversation

kuke commented May 9, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

varunarora left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kuke left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

varunarora commented May 15, 2018

kuke commented May 16, 2018

kuke commented May 9, 2018 •

edited

Loading

kuke left a comment •

edited

Loading