Mxnet parser for Qnn dialect #4714

shoubhik · 2020-01-15T18:25:23Z

In this parser, I am including the changes needed for QNN dialect for Mxnet. It has been tested with most of the open-source Mxnet models.

…to qnn-mxnet-parser # Conflicts: # python/tvm/relay/frontend/mxnet_qnn_op_utils.py - Working code.

- Small refactoring - Remove "with_sum" from conv - Simplified code

shoubhik · 2020-01-15T18:27:20Z

@anijain2305 , @jackwish can you take a look at this.

liangfu

Thanks for bringing the favorable change.

Please add test scripts following test_qnn_ops_utils.py.

liangfu · 2020-01-16T01:35:07Z

python/tvm/relay/frontend/mxnet.py

@@ -14,12 +14,14 @@
 # KIND, either express or implied.  See the License for the
 # specific language governing permissions and limitations
 # under the License.
-# pylint: disable=invalid-name, import-self, len-as-condition, no-else-return
+# pylint: disable=invalid-name, import-self, len-as-condition, no-else-return, too-many-lines


Avoid disabling linter in head of a file. Instead, you can disable them in specific functions or classes.

This particular one is about the file, where pylint complains about too many lines in the file.

liangfu · 2020-01-16T01:45:33Z

python/tvm/relay/frontend/mxnet_qnn_op_utils.py


-zero_centered_uint8_quantized_range = np.float32(255)
-zero_centered_int8_quantized_range = np.float32(127)
+zero_centered_uint8_quantized_range = np.float32(255.5)


Can you explain why are we adding an extra 0.5 here?

Added reference.

Can you explain why are we adding an extra 0.5 here?

I guess this is related to the rounding policy and the quantization arithmetic?

src/relay/qnn/op/convolution.cc

liangfu · 2020-01-16T01:55:01Z

python/tvm/relay/frontend/mxnet.py

+    data_layout = attrs.get_str("layout", "NCHW")
+    if len(kernel_size) != 2:
+        raise tvm.error.OpAttributeInvalid(
+            'Non 1D or 2D kernels are not supported for operator Convolution')


Only 2D kernels are supported.

Fixed the message

actually we do have support for 1d and 3d conv in topi now.

This condition seems to be from an older version of mxnet. It makes more sense to put this change in a separate PR after more extensive testing.
Latest mxnet https://github.com/apache/incubator-mxnet/blob/master/src/operator/nn/convolution.cc#L364-L368 does support 1,2,3 dim kernels

liangfu · 2020-01-16T02:00:03Z

python/tvm/relay/frontend/mxnet.py

@@ -1075,6 +1098,377 @@ def _mx_cond(inputs, attrs, subgraphs):
    return ret


+def _qnn_mx_contrib_quantize(inputs, attrs):


Can we put functions with _qnn_mx prefix into mxnet_qnn_op_utils.py file as well?

Renamed the ops so that they no longer have mx prefix in them.

- Removing redundant commented code.

- Removing unused methods.

masahi · 2020-01-17T02:59:28Z

it would be great if you could add an test case on end to end network translation like in our tflite test case.

# Conflicts: # src/relay/qnn/op/convolution.cc

zhenhuaw-me

I looked into some code, however, as I am not familiar with these MxNet code, and not very understand of the naming (codebase issue maybe), have only these comments so far... Hoping some other can further check :)

zhenhuaw-me · 2020-01-19T12:59:38Z

python/tvm/relay/frontend/mxnet.py

 def _mx_fully_connected(inputs, attrs):
-    import mxnet as mx
+    import mxnet as mx #pylint: disable=import-outside-toplevel


Why we need this change?

running the pylint locally i was getting this warning.

zhenhuaw-me · 2020-01-19T13:00:45Z

python/tvm/relay/frontend/mxnet.py

@@ -1033,6 +1054,7 @@ def _mx_contrib_fifo_buffer(inputs, attrs):
    new_attrs['axis'] = attrs.get_int('axis')
    return _op.nn.fifo_buffer(*inputs, **new_attrs)

+


not needed?

zhenhuaw-me · 2020-01-19T13:03:37Z

python/tvm/relay/frontend/mxnet.py

+                out_dtype = 'int8'
+    else:
+        out_dtype = out_type
+    if out_dtype not in {'int8', 'uint8'}:


It seems that this assert checks out_type actually? If that is the case, can we move it to where we obtained the out_type value?

out_dtype can be auto too. in which case we infer it based on the min and max values.

zhenhuaw-me · 2020-01-19T13:05:10Z

python/tvm/relay/frontend/mxnet.py

+    min_calib_range = attrs.get_float('min_calib_range', 0.0)
+    max_calib_range = attrs.get_float('max_calib_range', 0.0)


That's interesting, if there is not min/max_calib_range provided, we are expecting the range to be [0.0, 0.0)? I am not very familiar with MxNet calibration method, but it appears to me that the detault min/max can derive from out_dtype?

You are correct. The zeros should not be there. In fact there min and max operators. i'll fix this.

zhenhuaw-me · 2020-01-19T13:11:01Z

python/tvm/relay/frontend/mxnet_qnn_op_utils.py


-zero_centered_uint8_quantized_range = np.float32(255)
-zero_centered_int8_quantized_range = np.float32(127)
+zero_centered_uint8_quantized_range = np.float32(255.5)


Can you explain why are we adding an extra 0.5 here?

I guess this is related to the rounding policy and the quantization arithmetic?

shoubhik · 2020-01-20T23:22:08Z

it would be great if you could add an test case on end to end network translation like in our tflite test case.

I have created a post on discuss on how we can do this - https://discuss.tvm.ai/t/use-mxnet-mkldnn-distribution-in-ci-instead-of-stock-mxnet-distribution/5474

anijain2305 · 2020-01-21T22:03:47Z

With the latest commit, we have good accuracy for all MxNet-MKLDNN quantized models

@tmoreau89 @yzhliu @tqchen @jackwish @FrozenGene @liangfu @vinx13

--	Mxnet-Top1	Mxnet-Top5	TVM-Top1	TVM-Top5	Degradation-Top1	Degradation-Top5
Resnet18_v1	69.76	89.02	69.85	89.09	-0.09	-0.07
Resnet50_v1	76.13	92.6	75.9	92.66	0.23	-0.06
Resnet50_v1b	76.66	92.6	76.45	92.57	0.21	0.03
Resnet101_v1	77.13	93.06	77	93.06	0.13	0
Resnet152_v2	75.99	92.52	75.32	92.26	0.67	0.26
Inception-V3	77.84	93.52	77.28	93.32	0.56	0.2
Inception-BN	71.96	90.38	71.79	90.25	0.17	0.13
MobileNetV1	71.27	90.09	71.13	90.16	0.14	-0.07
MobileNetV2	70.35	89.45	70.19	89.5	0.16	-0.05

Quantized models are generated using this - https://github.com/apache/incubator-mxnet/tree/master/example/quantization

Accuracy is collected over 10,000 inputs with target = 'llvm'

shoubhik · 2020-02-03T23:41:02Z

@tmoreau89 @yzhliu @tqchen @jackwish @FrozenGene @liangfu @vinx13 @anijain2305 I want to discuss this PR from testability POV. Mxnet suggests to use MKLDNN as backend for quantization. The operators for MKLDNN quantization in Mxnet is different from stock Mxnet quantization operators. Also, for some ops the implementation of these ops are also different.
Due to this PR #4753 and #4764, it is not possible to merge the Mxnet-MKLDNN installation in TVM docker. One potential solution is to test stock quantization operators but that would be extra work that may not be too useful in future. All the testing we have done and the edge cases we have covered are for the MKLDNN operators.
My suggestion is at this point we check in the parser code along with the the test scripts we have for benchmarking the QNN networks. When MKLDNN issues are fixed and we can safely upgrade we can add test cases at that point.

anijain2305 · 2020-02-04T06:58:07Z

Given the constraints, I am ok with MxNet-parser code w/o any MxNet-MKLDNN CI testing, as long as we have a test script linked in the PR, that somebody can locally use to test the changes.

Once Mxnet-MKLDNN CI/accuracy issues are resolved, we can re-write that script as a test.

Is that ok - @tqchen @icemelon9

FrozenGene · 2020-02-04T08:42:01Z

@tmoreau89 @yzhliu @tqchen @jackwish @FrozenGene @liangfu @vinx13 @anijain2305 I want to discuss this PR from testability POV. Mxnet suggests to use MKLDNN as backend for quantization. The operators for MKLDNN quantization in Mxnet is different from stock Mxnet quantization operators. Also, for some ops the implementation of these ops are also different.
Due to this PR #4753 and #4764, it is not possible to merge the Mxnet-MKLDNN installation in TVM docker. One potential solution is to test stock quantization operators but that would be extra work that may not be too useful in future. All the testing we have done and the edge cases we have covered are for the MKLDNN operators.
My suggestion is at this point we check in the parser code along with the the test scripts we have for benchmarking the QNN networks. When MKLDNN issues are fixed and we can safely upgrade we can add test cases at that point.

I am OK with this.

shoubhik · 2020-02-04T22:04:17Z

@tqchen @yzhliu I think the code can be merged now.

masahi · 2020-02-04T22:10:15Z

tests/python/frontend/mxnet/test_forward.py

+    _, arg_params, aux_params = get_mxnet_output(mx_symbol, x)
+    quantize_api = quantize_with_old_api if use_old_mxnet_quantization_api() else quantized_with_new_api
+
+


remove this white spaces

masahi · 2020-02-04T22:11:17Z

@shoubhik I'll have a look and merge. I'm interested in this PR.

…ache#4753, mxnet could not be updated to mxnet-mkldnn.

masahi · 2020-02-05T13:28:26Z

thanks @shoubhik @anijain2305 @jackwish @FrozenGene @liangfu this is merged.

masahi · 2020-02-09T00:43:06Z

My suggestion is at this point we check in the parser code along with the the test scripts we have for benchmarking the QNN networks

Where are that test scripts? I want to run them @shoubhik @anijain2305

* - Additional util methods needed for mxnet frontend for qnn dialect. * - Fixing call to quantize. * [QNN] MxNet-MKLDNN parser support for QNN * [QNN] Relax conv check. * - Merge from origin * [QNN] Channel wise changes * [QNN] Dense changes * Dense fix for QNN ops. * - Removed non-mkl code from utils. - Small refactoring - Remove "with_sum" from conv - Simplified code * - Fixing ring buffer name. * - Fixing pylint issues. * - Fixing lint - Removing redundant commented code. * - Adding test cases - Removing unused methods. * [WIP] end to end test case for mxnet qnn parser * Changes to parse large CV models. * Pylint issues. * Fix Conv2D with sum and quantized pooling. * Reverting the changes made for mxnet-mkldnn test cases. Because of apache#4753, mxnet could not be updated to mxnet-mkldnn. Co-authored-by: Animesh Jain <anijain@umich.edu>

apache#4714

shoubhik and others added 14 commits January 7, 2020 14:52

- Additional util methods needed for mxnet frontend for qnn dialect.

fdb033f

- Fixing call to quantize.

9f16c63

[QNN] MxNet-MKLDNN parser support for QNN

24ca89a

[QNN] Relax conv check.

92095fd

- Merge from origin

387fea3

wqMerge branch 'master' into qnn-mxnet-parser

7057307

[QNN] Channel wise changes

e23576f

[QNN] Dense changes

555862b

Dense fix for QNN ops.

f3c0f53

Merge branch 'mx_qnn_parser' of https://github.com/anijain2305/tvm in…

8227c95

…to qnn-mxnet-parser # Conflicts: # python/tvm/relay/frontend/mxnet_qnn_op_utils.py - Working code.

- Removed non-mkl code from utils.

e75ba69

- Small refactoring - Remove "with_sum" from conv - Simplified code

- Fixing ring buffer name.

63d6fc0

- Fixing pylint issues.

e484ef8

Merge branch 'master' into qnn-mxnet-parser

1c55919

tqchen added the status: need review label Jan 15, 2020

liangfu requested changes Jan 16, 2020

View reviewed changes

shoubhik added 2 commits January 15, 2020 18:13

- Fixing lint

20f0d70

- Removing redundant commented code.

- Adding test cases

4db6843

- Removing unused methods.

Merge branch 'master' into qnn-mxnet-parser

8191ffe

# Conflicts: # src/relay/qnn/op/convolution.cc

zhenhuaw-me reviewed Jan 19, 2020

View reviewed changes

shoubhik and others added 4 commits January 20, 2020 16:35

[WIP] end to end test case for mxnet qnn parser

db4381e

Merge branch 'master' into qnn-mxnet-parser

02e568d

Changes to parse large CV models.

f1c0827

Pylint issues.

27701ba

masahi self-assigned this Jan 26, 2020

Fix Conv2D with sum and quantized pooling.

86c9e84

masahi reviewed Feb 4, 2020

View reviewed changes

Reverting the changes made for mxnet-mkldnn test cases. Because of ap…

6fb6a9d

…ache#4753, mxnet could not be updated to mxnet-mkldnn.

masahi approved these changes Feb 5, 2020

View reviewed changes

masahi merged commit 7d263c3 into apache:master Feb 5, 2020

shoubhik added a commit to shoubhik/incubator-tvm that referenced this pull request May 12, 2020

[QNN]Mxnet parser for Qnn dialect apache#4714

c5a818e

apache#4714

ZihengJiang mentioned this pull request Sep 17, 2020

TVM v0.7 Release Note Candidate #6486

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Mxnet parser for Qnn dialect #4714

Mxnet parser for Qnn dialect #4714

shoubhik commented Jan 15, 2020

shoubhik commented Jan 15, 2020

liangfu left a comment

liangfu Jan 16, 2020

shoubhik Jan 16, 2020

liangfu Jan 16, 2020

shoubhik Jan 16, 2020

zhenhuaw-me Jan 19, 2020

liangfu Jan 16, 2020

shoubhik Jan 16, 2020

masahi Jan 16, 2020

shoubhik Jan 17, 2020 •

edited

Loading

liangfu Jan 16, 2020

shoubhik Jan 16, 2020

masahi commented Jan 17, 2020 •

edited

Loading

zhenhuaw-me left a comment

zhenhuaw-me Jan 19, 2020

shoubhik Jan 20, 2020

zhenhuaw-me Jan 19, 2020

zhenhuaw-me Jan 19, 2020

shoubhik Jan 20, 2020

zhenhuaw-me Jan 19, 2020

shoubhik Jan 20, 2020

zhenhuaw-me Jan 19, 2020

shoubhik commented Jan 20, 2020

anijain2305 commented Jan 21, 2020 •

edited

Loading

shoubhik commented Feb 3, 2020

anijain2305 commented Feb 4, 2020

FrozenGene commented Feb 4, 2020

shoubhik commented Feb 4, 2020

masahi Feb 4, 2020

masahi commented Feb 4, 2020

masahi commented Feb 5, 2020

masahi commented Feb 9, 2020

		@@ -1075,6 +1098,377 @@ def _mx_cond(inputs, attrs, subgraphs):
		return ret


		def _qnn_mx_contrib_quantize(inputs, attrs):

		@@ -1033,6 +1054,7 @@ def _mx_contrib_fifo_buffer(inputs, attrs):
		new_attrs['axis'] = attrs.get_int('axis')
		return _op.nn.fifo_buffer(inputs, *new_attrs)

		min_calib_range = attrs.get_float('min_calib_range', 0.0)
		max_calib_range = attrs.get_float('max_calib_range', 0.0)

		_, arg_params, aux_params = get_mxnet_output(mx_symbol, x)
		quantize_api = quantize_with_old_api if use_old_mxnet_quantization_api() else quantized_with_new_api

Mxnet parser for Qnn dialect #4714

Mxnet parser for Qnn dialect #4714

Conversation

shoubhik commented Jan 15, 2020

shoubhik commented Jan 15, 2020

liangfu 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

shoubhik Jan 17, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

masahi commented Jan 17, 2020 • edited Loading

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

shoubhik commented Jan 20, 2020

anijain2305 commented Jan 21, 2020 • edited Loading

shoubhik commented Feb 3, 2020

anijain2305 commented Feb 4, 2020

FrozenGene commented Feb 4, 2020

shoubhik commented Feb 4, 2020

Choose a reason for hiding this comment

masahi commented Feb 4, 2020

masahi commented Feb 5, 2020

masahi commented Feb 9, 2020

shoubhik Jan 17, 2020 •

edited

Loading

masahi commented Jan 17, 2020 •

edited

Loading

anijain2305 commented Jan 21, 2020 •

edited

Loading