[MKLDNN]Add quantized relu by huangzhiyuan · Pull Request #14604 · apache/mxnet

huangzhiyuan · 2019-04-03T05:00:53Z

Description

This PR is to add quantized relu op and its MKLDNN implementation.
@pengzhao-intel @ZhennanQin @TaoLv

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

The PR title starts with [MXNET-$JIRA_ID], where $JIRA_ID refers to the relevant JIRA issue created (except PRs with tiny changes)
Changes are complete (i.e. I finished coding on this PR)
All changes have test coverage:
Unit tests are added for small changes to verify correctness (e.g. adding a new operator)
Nightly tests are added for complicated/long-running ones (e.g. changing distributed kvstore)
Build tests will be added for build configuration changes (e.g. adding a new build option with NCCL)
Code is well-documented:
For user-facing API changes, API doc string has been updated.
For new C++ functions in header files, their functionalities and arguments are documented.
For new examples, README.md is added to explain the what the example does, the source of the dataset, expected performance on test set and reference to the original paper if applicable
Check the API doc at http://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-$PR_ID/$BUILD_ID/index.html
To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Changes

Feature1, tests, (and when applicable, API doc)
Feature2, tests, (and when applicable, API doc)

Comments

If this change is a backward incompatible change, why must this change be made.
Interesting edge cases to note here

piyushghai · 2019-04-03T17:17:42Z

Thanks for your contributions @huangzhiyuan.
Can you please look into the CI failures ?

piyushghai · 2019-04-03T17:17:57Z

@mxnet-label-bot Add [MKLDNN, pr-awaiting-review]

src/operator/nn/mkldnn/mkldnn_act-inl.h

pengzhao-intel · 2019-04-04T01:55:33Z

src/operator/nn/mkldnn/mkldnn_act-inl.h

+
+mkldnn::algorithm GetMKLDNNActAlgo(const ActivationParam& param);
+mkldnn::eltwise_forward::primitive_desc GetActFwdDescImpl(
+    const ActivationParam& param, bool is_train,


unify the format of "&"

Not necessary there for bool or int type, please refer to https://github.com/apache/incubator-mxnet/blob/master/src/operator/nn/mkldnn/mkldnn_pooling-inl.h#L139

src/operator/quantization/mkldnn/mkldnn_quantized_act.cc

pengzhao-intel · 2019-04-04T02:19:33Z

cc @anirudh2290

src/operator/nn/mkldnn/mkldnn_act.cc

TaoLv · 2019-04-08T12:45:46Z

src/operator/quantization/mkldnn/mkldnn_quantized_act.cc

+ */
+/*!
+ * Copyright (c) 2019 by Contributors
+ * \file mkldnn_act-inl.h


Fix file name.

src/operator/quantization/mkldnn/mkldnn_quantized_act.cc

TaoLv · 2019-04-08T12:47:26Z

src/operator/quantization/mkldnn/mkldnn_quantized_act.cc

+                                      const std::vector<NDArray>& out_data) {
+  CHECK(in_data[0].dtype() == mshadow::kUint8 ||
+        in_data[0].dtype() == mshadow::kInt8)
+      << "mkldnn_quantized_activation op only supports uint8 and int8 as input "


mkldnn_quantized_activation is not a valid operator name.

TaoLv · 2019-04-08T12:50:42Z

src/operator/quantization/quantized_activation.cc

+  if (param.act_type == activation::kReLU) {
+    TYPE_ASSIGN_CHECK(*out_type, 0, mshadow::kInt8);
+  } else {
+    LOG(FATAL) << "QuantizedActivationOp only supports act_type=relu for now";


QuantizedActivationOp is not a valid operator name.

TaoLv · 2019-04-08T12:55:18Z

tests/python/quantization/test_quantization.py

        check_quantized_flatten((3, 4, 23, 23), qdtype)

+@with_seed()
+def test_quantized_act():


Do we have cases for non-relu activation type?

Negative test cases are not necessary there, it well prompt that _contrib_quantized_act only supports act_type=relu for now.

Fine. I'm personally okay about there is no non-relu cases in UT. But do you mind running one on your machine and paste the error log here? I just want to make sure that the error happens at a appropriate place and the message is accurate.

Sure, the error log just like below when change activation type from relu to no-relu type (e.g. sigmoid):
[16:03:13] src/operator/quantization/quantized_activation.cc:54: _contrib_quantized_act only supports act_type=relu for now
Aborted

pengzhao-intel · 2019-04-11T03:41:37Z

Please retrigger the CI

pengzhao-intel · 2019-04-12T07:44:24Z

@TaoLv @ZhennanQin @ciyongch please help review the PR again.

src/operator/nn/mkldnn/mkldnn_act.cc

src/operator/quantization/quantize_graph_pass.cc

src/operator/quantization/quantized_activation.cc

TaoLv

Thank you for addressing my comments. Approved now.

pengzhao-intel

Thanks for your contribution. LGTM

pengzhao-intel · 2019-04-18T01:21:26Z

@huangzhiyuan please rebase the code since I have merged the quantize v2 changes.
Let's see if there are any impact on your change.

I will merge the PR if everything is fine.

pengzhao-intel · 2019-04-18T12:18:08Z

Merging now, thanks for your contribution :)

* add quantized relu * fix testcase * add author and skip quantized-relu for gpu * fix comments * retrigger ci * retrigger ci * comment fix * retrigger ci * retrigger ci

huangzhiyuan added 2 commits April 3, 2019 12:57

add quantized relu

3dcf0ae

fix testcase

e218338

marcoabreu added MKLDNN pr-awaiting-review PR is waiting for code review labels Apr 3, 2019

pengzhao-intel reviewed Apr 4, 2019

View reviewed changes

pengzhao-intel mentioned this pull request Apr 5, 2019

[Discussion] 1.5.0 Roadmap #14619

Closed

azai91 reviewed Apr 6, 2019

View reviewed changes

src/operator/nn/mkldnn/mkldnn_act.cc Outdated Show resolved Hide resolved

add author and skip quantized-relu for gpu

d75d30f

TaoLv reviewed Apr 8, 2019

View reviewed changes

huangzhiyuan added 2 commits April 9, 2019 09:56

Merge remote-tracking branch 'upstream/master' into quantized-relu

6a8f206

fix comments

7781d4d

pengzhao-intel added the Quantization Issues/Feature Requests related to Quantization label Apr 11, 2019

huangzhiyuan added 4 commits April 11, 2019 12:03

Merge remote-tracking branch 'upstream/master' into quantized-relu

19a2df8

Merge remote-tracking branch 'upstream/master' into quantized-relu

c8f7b6c

retrigger ci

75088f3

retrigger ci

a954d5e

TaoLv reviewed Apr 12, 2019

View reviewed changes

comment fix

66d6fc0

TaoLv approved these changes Apr 16, 2019

View reviewed changes

pengzhao-intel approved these changes Apr 16, 2019

View reviewed changes

huangzhiyuan added 3 commits April 18, 2019 09:32

Merge remote-tracking branch 'upstream/master' into quantized-relu

4995cef

retrigger ci

c4d8b94

retrigger ci

7fa9373

pengzhao-intel merged commit 0da4b67 into apache:master Apr 18, 2019

huangzhiyuan deleted the quantized-relu branch April 19, 2019 02:51

Conversation

huangzhiyuan commented Apr 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Essentials

Changes

Comments

Uh oh!

piyushghai commented Apr 3, 2019

Uh oh!

piyushghai commented Apr 3, 2019

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pengzhao-intel commented Apr 4, 2019

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pengzhao-intel commented Apr 11, 2019

Uh oh!

pengzhao-intel commented Apr 12, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

TaoLv left a comment

Choose a reason for hiding this comment

Uh oh!

pengzhao-intel left a comment

Choose a reason for hiding this comment

Uh oh!

pengzhao-intel commented Apr 18, 2019

Uh oh!

pengzhao-intel commented Apr 18, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

huangzhiyuan commented Apr 3, 2019 •

edited

Loading