Added Relu6 mkldnn op #25713

arlesniak · 2020-07-24T10:45:27Z

PR types

New features

PR changes

OPs

Describe

Dygraph: Added Relu6 mkldnn op used in mobilenetV2 from models repo

paddle-bot-old · 2020-07-24T10:45:33Z

Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

paddle-bot-old · 2020-07-24T10:45:35Z

✅ This PR's description meets the template requirements!
Please wait for other CI results.

sfraczek

LGTM

sfraczek

LGTM!

grygielski

LGTM

luotao1

LGTM

luotao1 · 2020-07-28T02:40:33Z

2020-07-25 05:40:02 0. You must have one RD (XiaoguangHu01 or lanxianghit) and one TPM (saxon-zh or jzhang533 or swtkiwi or Heeenrrry or TCChenlong) approval for the api change for the management reason of API interface.
2020-07-25 05:40:02 1. You must have one TPM (saxon-zh or jzhang533 or swtkiwi or Heeenrrry or TCChenlong) approval for the api change for the management reason of API document.
2020-07-25 05:40:02 2. You must have one RD (zhiqiu (Recommend) or phlrain) approval for the api change for the opreator-related api without 'core.ops'.
2020-07-25 05:40:02 For more details, please click [https://github.com/PaddlePaddle/Paddle/wiki/paddle_api_development_manual.md]
2020-07-25 05:40:02 Related APIs: paddle.fluid.layers.relu6
2020-07-25 05:40:02 There are 3 approved errors.
2020-07-25 05:40:02 ****************
2020-07-25 05:40:02 API Difference is: 
2020-07-25 05:40:02 - paddle.fluid.layers.relu6 (ArgSpec(args=['x', 'threshold', 'name'], varargs=None, keywords=None, defaults=(6.0, None)), ('document', '09b5e5decc444b742f737ece2f5b9f7e'))
2020-07-25 05:40:02 ?                                                                                                                                       ^^ ^ ^^^^^^^^^^^^^    ^^^^^  ^^^
2020-07-25 05:40:02 
2020-07-25 05:40:02 + paddle.fluid.layers.relu6 (ArgSpec(args=['x', 'threshold', 'name', 'use_mkldnn'], varargs=None, keywords=None, defaults=(6.0, None, False)), ('document', '114eba28e873971013636dbeabb9051c'))
2020-07-25 05:40:02 ?                                                                  ++++++++++++++                                                   +++++++                  ^^^^ ^^^ ^  + +++++++++ ^^  ^^^^
2020-07-25 05:40:02 
2020-07-25 05:40:04 + approval_error=1

zhiqiu

LGTM

luotao1 · 2020-07-28T04:42:40Z

python/paddle/fluid/layers/nn.py

@@ -9313,7 +9313,7 @@ def elu(x, alpha=1.0, name=None):


 @templatedoc()
-def relu6(x, threshold=6.0, name=None):
+def relu6(x, threshold=6.0, name=None, use_mkldnn=False):


Why do you add the attribute use_mkldnn here?

If we use FLAGS_use_mkldnn=true to global control the model, we don't need add use_mkldnn attribute in each API.

add dnnl sigmoid (logistic) activation #25745 sigmoid mkldnn op does not add the attribute use_mkldnn

Why we add use_mkldnn here

This Relu6 is used in models repo. It is needed for mobilenetv2 here https://github.com/PaddlePaddle/models/blob/develop/dygraph/mobilenet/mobilenet_v2.py#L69.

We wouldn't have to do this if models repo would use the approach as here:
https://github.com/PaddlePaddle/models/blob/develop/dygraph/resnet/train.py#L207
which calls:
https://github.com/PaddlePaddle/Paddle/blob/develop/python/paddle/fluid/dygraph/nn.py#L233.
which is passed to convolution here: https://github.com/PaddlePaddle/Paddle/blob/develop/python/paddle/fluid/dygraph/nn.py#L170

There are few ways how ops are added in models repo, for example this: https://github.com/PaddlePaddle/models/blob/develop/dygraph/resnet/train.py#L243
I am guessing that this is inconsistent because it isn't fully migrated yet but I am not sure.

Value of use_mkldnn has to be evaluated before this line for dygraph to use mkldnn kernel of relu6: https://github.com/PaddlePaddle/Paddle/pull/25713/files/3edaaad6bc84ed19715aaca04f9ed6bdbf06e25e#diff-f8fc1afdc1f06d4c47bc0933d53bf3c6R9359.

The case of sigmoid

For sigmoid I haven't added any use_mkldnn yet because we haven't enabled it yet. However, It will have to be handled with in the same way as we will eventually decide to handle relu6.

Old vs New approach

This use_mkldnn=False is from previous approach before we decided ot add FLAGS_use_mkldnn.
The new approach we have is to pass use_mkldnn=True to ops and check it with FLAGS_use_mkldnn. For details please refer to this PR
https://github.com/PaddlePaddle/Paddle/pull/25773/files#diff-aeec709d2310c344dd0f235478865b7cR182. We pass use_mkldnn=True and we call CheckUseMkldnnWithFlag() to compare against FLAGS_use_mkldnn.

How we can proceed

We could apply the same approach to this PR with checking against FLAGS_use_mkldnn. If you think this is correct, I can do that here.
Alternatively, maybe models repo could call relu6 in different way? Or maybe you have other ideas?

I found the same situation here: #25773 (comment)
We should definitely decide how to deal with that so I can change both places.

We could apply the same approach to this PR with checking against FLAGS_use_mkldnn. If you think this is correct, I can do that here.
Alternatively, maybe models repo could call relu6 in different way? Or maybe you have other ideas?

@phlrain Please have a look, and which way do you prefer?

I prefer to use FLAGS_use_mkldnn. It's seems an easier way

luotao1 · 2020-07-28T04:44:08Z

paddle/fluid/operators/activation_op.cc

@@ -504,6 +504,9 @@ class Relu6OpMaker : public framework::OpProtoAndCheckerMaker {
    AddAttr<float>("threshold",
                   "The threshold value of Relu6. Default is 6.0. ")
        .SetDefault(6.0f);
+    AddAttr<bool>("use_mkldnn",
+                  "(bool, default false) Only used in mkldnn kernel")
+        .SetDefault(false);


Do we need to add an attribute here? A lot of activation ops already support the use_mkldnn attribute.

Without it it crashes. We run modified mobilenet script:
https://github.com/PaddlePaddle/models/blob/a7fb45f650e0d32ba697b5818aa4799c2ccc9b05/dygraph/mobilenet/mobilenet_v2.py#L69
passing use_mkldnn=True in that line.
The error is:

Traceback (most recent call last): File "train.py", line 262, in <module> train_mobilenet() File "train.py", line 195, in train_mobilenet avg_loss.backward() File "</nfs/pdx/home/sfraczek/.local/lib/python2.7/site-packages/decorator.pyc:decorator-gen-97>", line 2, in backward File "/data/sfraczek/Paddle/build/python/paddle/fluid/wrapped_decorator.py", line 25, in __impl__ return wrapped_func(*args, **kwargs) File "/data/sfraczek/Paddle/build/python/paddle/fluid/framework.py", line 219, in __impl__ return func(*args, **kwargs) File "/data/sfraczek/Paddle/build/python/paddle/fluid/dygraph/varbase_patch_methods.py", line 172, in backward framework._dygraph_tracer(), retain_graph) KeyboardInterrupt Reservation status error sfraczek@aipg-ra-clx-86:/data/sfraczek/models/dygraph/mobilenet$ bash run_cpu_v2.sh | grep relu Traceback (most recent call last): File "train.py", line 262, in <module> train_mobilenet() File "train.py", line 180, in train_mobilenet out = net(img) File "/data/sfraczek/Paddle/build/python/paddle/fluid/dygraph/layers.py", line 600, in __call__ outputs = self.forward(*inputs, **kwargs) File "/data/sfraczek/models/dygraph/mobilenet/mobilenet_v2.py", line 212, in forward y = self._conv1(inputs, if_act=True) File "/data/sfraczek/Paddle/build/python/paddle/fluid/dygraph/layers.py", line 600, in __call__ outputs = self.forward(*inputs, **kwargs) File "/data/sfraczek/models/dygraph/mobilenet/mobilenet_v2.py", line 69, in forward y = fluid.layers.relu6(y, use_mkldnn=True) File "/data/sfraczek/Paddle/build/python/paddle/fluid/layers/nn.py", line 9359, in relu6 'use_mkldnn': use_mkldnn}) File "/data/sfraczek/Paddle/build/python/paddle/fluid/layer_helper.py", line 43, in append_op return self.main_program.current_block().append_op(*args, **kwargs) File "/data/sfraczek/Paddle/build/python/paddle/fluid/framework.py", line 2789, in append_op kwargs.get("stop_gradient", False)) File "/data/sfraczek/Paddle/build/python/paddle/fluid/dygraph/tracer.py", line 45, in trace_op not stop_gradient) paddle.fluid.core_avx.EnforceNotMet: -------------------------------------------- C++ Call Stacks (More useful to developers): -------------------------------------------- 0 std::string paddle::platform::GetTraceBackString<std::string >(std::string&&, char const*, int) 1 paddle::platform::EnforceNotMet::EnforceNotMet(paddle::platform::ErrorSummary const&, char const*, int) 2 std::conditional<std::is_pointer<paddle::framework::Attribute >::value, bool const*, bool const&>::type paddle::platform::details::SafeBoostGetConst<bool, paddle::framework::Attribute >(paddle::framework::Attribute const&, char const*, char const*, int) 3 paddle::operators::GetKernelType(paddle::framework::ExecutionContext const&, paddle::framework::OperatorWithKernel const&, std::string const&) 4 paddle::operators::ActivationOp::GetExpectedKernelType(paddle::framework::ExecutionContext const&) const 5 paddle::imperative::PreparedOp paddle::imperative::PrepareOpImpl<paddle::imperative::VarBase>(paddle::imperative::details::NameVarMapTrait<paddle::imperative::VarBase>::Type const&, paddle::imperative::details::NameVarMapTrait<paddle::imperative::VarBase>::Type const&, paddle::framework::OperatorWithKernel const&, paddle::platform::Place, paddle::framework::AttributeMap const&) 6 paddle::imperative::PreparedOp::Prepare(paddle::imperative::NameVarBaseMap const&, paddle::imperative::NameVarBaseMap const&, paddle::framework::OperatorWithKernel const&, paddle::platform::Place const&, paddle::framework::AttributeMap const&) 7 paddle::imperative::Tracer::TraceOp(std::string const&, paddle::imperative::NameVarBaseMap const&, paddle::imperative::NameVarBaseMap const&, paddle::framework::AttributeMap, paddle::platform::Place const&, bool) ---------------------- Error Message Summary: ---------------------- InvalidArgumentError: boost::get failed, cannot get value (GetAttr(name)) by type bool, its type is int. at (/data/sfraczek/Paddle/paddle/fluid/framework/operator.h:259) [operator < relu6 > error] Reservation status error sfraczek@aipg-ra-clx-86:/data/sfraczek/models/dygraph/mobilenet$ bash run_cpu_v2.sh | grep relu Traceback (most recent call last): File "train.py", line 262, in <module> train_mobilenet() File "train.py", line 180, in train_mobilenet out = net(img) File "/data/sfraczek/Paddle/build/python/paddle/fluid/dygraph/layers.py", line 600, in __call__ outputs = self.forward(*inputs, **kwargs) File "/data/sfraczek/models/dygraph/mobilenet/mobilenet_v2.py", line 212, in forward y = self._conv1(inputs, if_act=True) File "/data/sfraczek/Paddle/build/python/paddle/fluid/dygraph/layers.py", line 600, in __call__ outputs = self.forward(*inputs, **kwargs) File "/data/sfraczek/models/dygraph/mobilenet/mobilenet_v2.py", line 69, in forward y = fluid.layers.relu6(y, use_mkldnn=True) File "/data/sfraczek/Paddle/build/python/paddle/fluid/layers/nn.py", line 9359, in relu6 'use_mkldnn': use_mkldnn}) File "/data/sfraczek/Paddle/build/python/paddle/fluid/layer_helper.py", line 43, in append_op return self.main_program.current_block().append_op(*args, **kwargs) File "/data/sfraczek/Paddle/build/python/paddle/fluid/framework.py", line 2789, in append_op kwargs.get("stop_gradient", False)) File "/data/sfraczek/Paddle/build/python/paddle/fluid/dygraph/tracer.py", line 45, in trace_op not stop_gradient) paddle.fluid.core_avx.EnforceNotMet: -------------------------------------------- C++ Call Stacks (More useful to developers): -------------------------------------------- 0 std::string paddle::platform::GetTraceBackString<std::string >(std::string&&, char const*, int) 1 paddle::platform::EnforceNotMet::EnforceNotMet(paddle::platform::ErrorSummary const&, char const*, int) 2 std::conditional<std::is_pointer<paddle::framework::Attribute >::value, bool const*, bool const&>::type paddle::platform::details::SafeBoostGetConst<bool, paddle::framework::Attribute >(paddle::framework::Attribute const&, char const*, char const*, int) 3 paddle::operators::GetKernelType(paddle::framework::ExecutionContext const&, paddle::framework::OperatorWithKernel const&, std::string const&) 4 paddle::operators::ActivationOp::GetExpectedKernelType(paddle::framework::ExecutionContext const&) const 5 paddle::imperative::PreparedOp paddle::imperative::PrepareOpImpl<paddle::imperative::VarBase>(paddle::imperative::details::NameVarMapTrait<paddle::imperative::VarBase>::Type const&, paddle::imperative::details::NameVarMapTrait<paddle::imperative::VarBase>::Type const&, paddle::framework::OperatorWithKernel const&, paddle::platform::Place, paddle::framework::AttributeMap const&) 6 paddle::imperative::PreparedOp::Prepare(paddle::imperative::NameVarBaseMap const&, paddle::imperative::NameVarBaseMap const&, paddle::framework::OperatorWithKernel const&, paddle::platform::Place const&, paddle::framework::AttributeMap const&) 7 paddle::imperative::Tracer::TraceOp(std::string const&, paddle::imperative::NameVarBaseMap const&, paddle::imperative::NameVarBaseMap const&, paddle::framework::AttributeMap, paddle::platform::Place const&, bool) ---------------------- Error Message Summary: ---------------------- InvalidArgumentError: boost::get failed, cannot get value (GetAttr(name)) by type bool, its type is int. at (/data/sfraczek/Paddle/paddle/fluid/framework/operator.h:259) [operator < relu6 > error]

Got it. I think adding use_mkldnn attribute in activation_op.cc is OK.

luotao1 · 2020-08-05T04:18:36Z

Discussion Summary from @phlrain and I:

We prefer to use FLAGS_use_mkldnn.
You can add use_mkldnn attribute in C++ OpMaker, such as in activation_op.cc.
Please don't add use_mkldnn attribute in Python API, such as in def Relu6(xxx).

luotao1 · 2020-08-07T10:13:37Z

Is this PR closed due to duplicated #26037? @grygielski

grygielski · 2020-08-07T10:21:34Z

@luotao1 Sure, closing due to duplicate.

Dygraph: Added Relu6 mkldnn op used in mobilenetV2 from models repo

5fa470d

jczaja added Intel dygraph issues related to dygraph mode labels Jul 24, 2020

$sfraczek$

sfraczek previously approved these changes Jul 24, 2020

View reviewed changes

Fixes

3edaaad

arlesniak dismissed sfraczek’s stale review via 3edaaad July 24, 2020 21:07

$@sfraczek$ sfraczek requested a review from grygielski July 27, 2020 07:58

$sfraczek$

sfraczek previously approved these changes Jul 27, 2020

View reviewed changes

grygielski previously approved these changes Jul 27, 2020

View reviewed changes

luotao1 previously approved these changes Jul 28, 2020

View reviewed changes

zhiqiu self-requested a review July 28, 2020 02:42

zhiqiu previously approved these changes Jul 28, 2020

View reviewed changes

luotao1 reviewed Jul 28, 2020

View reviewed changes

$@sfraczek$ sfraczek mentioned this pull request Jul 28, 2020

add use_mkldnn attribute to ops in dygraph #25773

Merged

$@sfraczek$

Merge branch 'develop' into arlesniak/mkldnn_relu6_for_dygraph

1bc2b3b

$@sfraczek$ sfraczek dismissed stale reviews from zhiqiu, luotao1, grygielski, and themself via 1bc2b3b July 29, 2020 15:35

$@sfraczek$ sfraczek mentioned this pull request Jul 31, 2020

Verification of dygraph MKLDNN accuracy convergence #25872

Closed

grygielski mentioned this pull request Aug 7, 2020

Add oneDNN relu6 op #26037

Merged

grygielski closed this Aug 7, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added Relu6 mkldnn op #25713

Added Relu6 mkldnn op #25713

arlesniak commented Jul 24, 2020 •

edited

paddle-bot-old bot commented Jul 24, 2020

paddle-bot-old bot commented Jul 24, 2020 •

edited

$@sfraczek$ sfraczek left a comment

$@sfraczek$ sfraczek left a comment

grygielski left a comment

luotao1 left a comment

luotao1 commented Jul 28, 2020

zhiqiu left a comment

luotao1 Jul 28, 2020

$@sfraczek$ sfraczek Jul 28, 2020 •

edited

$@sfraczek$ sfraczek Jul 28, 2020

luotao1 Jul 30, 2020

phlrain Aug 4, 2020

luotao1 Jul 28, 2020 •

edited

$@sfraczek$ sfraczek Jul 29, 2020

luotao1 Aug 4, 2020

luotao1 commented Aug 5, 2020

luotao1 commented Aug 7, 2020

grygielski commented Aug 7, 2020

Added Relu6 mkldnn op #25713

Added Relu6 mkldnn op #25713

Conversation

arlesniak commented Jul 24, 2020 • edited

PR types

PR changes

Describe

paddle-bot-old bot commented Jul 24, 2020

paddle-bot-old bot commented Jul 24, 2020 • edited

sfraczek left a comment

Choose a reason for hiding this comment

sfraczek left a comment

Choose a reason for hiding this comment

grygielski left a comment

Choose a reason for hiding this comment

luotao1 left a comment

Choose a reason for hiding this comment

luotao1 commented Jul 28, 2020

zhiqiu left a comment

Choose a reason for hiding this comment

luotao1 Jul 28, 2020

Choose a reason for hiding this comment

sfraczek Jul 28, 2020 • edited

Choose a reason for hiding this comment

Why we add use_mkldnn here

The case of sigmoid

Old vs New approach

How we can proceed

sfraczek Jul 28, 2020

Choose a reason for hiding this comment

luotao1 Jul 30, 2020

Choose a reason for hiding this comment

phlrain Aug 4, 2020

Choose a reason for hiding this comment

luotao1 Jul 28, 2020 • edited

Choose a reason for hiding this comment

sfraczek Jul 29, 2020

Choose a reason for hiding this comment

luotao1 Aug 4, 2020

Choose a reason for hiding this comment

luotao1 commented Aug 5, 2020

luotao1 commented Aug 7, 2020

grygielski commented Aug 7, 2020

arlesniak commented Jul 24, 2020 •

edited

paddle-bot-old bot commented Jul 24, 2020 •

edited

$@sfraczek$ sfraczek left a comment

$@sfraczek$ sfraczek left a comment

$@sfraczek$ sfraczek Jul 28, 2020 •

edited

$@sfraczek$ sfraczek Jul 28, 2020

luotao1 Jul 28, 2020 •

edited

$@sfraczek$ sfraczek Jul 29, 2020