Add use of global flag 'use_mkldnn' to layer_helper #26497

arlesniak · 2020-08-20T10:54:34Z

PR types

New features

PR changes

Others

Describe

Get use of global FLAGS_use_mkldnn to append_activation in layer_helper

paddle-bot-old · 2020-08-20T10:54:57Z

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

arlesniak · 2020-08-27T04:34:51Z

PR-CI-Coverage does not update status but after clicking Details it's all green

python/paddle/fluid/layer_helper.py

luotao1 · 2020-08-27T07:49:23Z

PR-CI-Coverage does not update status but after clicking Details it's all green

Some network error. If you will fix #26497 (comment), you can push a new commit to trigger it.

grygielski

LGTM

luotao1 · 2020-08-28T07:39:55Z

2020-08-27 19:32:07 ****************
2020-08-27 19:32:07 0. You must have one RD (zhiqiu (Recommend) or phlrain) approval for the api change for the opreator-related api without 'core.ops'.
2020-08-27 19:32:07 For more details, please click [https://github.com/PaddlePaddle/Paddle/wiki/paddle_api_development_manual.md]
2020-08-27 19:32:07 Related APIs: paddle.common_ops_import.LayerHelper.append_activation
2020-08-27 19:32:07 paddle.common_ops_import.LayerHelper.append_op
2020-08-27 19:32:07 paddle.fluid.layers.utils.LayerHelper.append_activation
2020-08-27 19:32:07 paddle.fluid.layers.utils.LayerHelper.append_op
2020-08-27 19:32:07 
2020-08-27 19:32:07 There are 1 approved errors.
2020-08-27 19:32:07 ****************

python/paddle/fluid/tests/unittests/test_imperative_basic.py

zhiqiu · 2020-08-28T08:40:11Z

python/paddle/fluid/layer_helper.py

+        op = self.main_program.current_block().append_op(*args, **kwargs)
+        if self.debug_:
+            self.appended_op = op
+        return op


It seems appended_op is only used to get the operator for testing its attrs.
I think we don't need to add a member to LayerHelper, the first appended op (in your test case, only one op is appended) can be obtained by fluid.default_main_program().global_block().ops[0] .

Thank you for the comment

fluid.default_main_program().global_block().ops

is empty in dygraph mode, I'm working on different solution.

zhiqiu · 2020-08-28T08:49:15Z

python/paddle/fluid/layer_helper.py

+        use_mkldnn = self.kwargs.get(
+            'use_mkldnn', core.globals().get("FLAGS_use_mkldnn", False))
+        if use_mkldnn:
+            act['use_mkldnn'] = use_mkldnn
+


Maybe it is better to implement this functionality in C++ side. There are 2 reasons,

FLAGS_use_mkldnn is a GFLAG, which is declared and used in c++.

There may be other way to add an activation op, for example, _append_activation_in_dygraph.

I've changed tracer on the c++ side, so there is no need for the above changes anymore.

grygielski

LGTM

wojtuss

LGTM

luotao1 · 2020-08-31T08:10:14Z

@zhiqiu Please have a double review!

zhiqiu · 2020-08-31T08:01:44Z

paddle/fluid/imperative/tracer.cc

+  if (FLAGS_use_mkldnn) {
+    attrs["use_mkldnn"] = true;
+  }


LGTM for dygraph mode.
I don't see handling use_mkldnn in static mode, is it already done in other PR?

If you mean running through executor, it's already done in executor.cc:185

zhiqiu · 2020-08-31T08:10:54Z

python/paddle/fluid/tests/unittests/test_imperative_basic.py

+            res2 = fluid.layers.relu(a)
+            self.assertTrue(np.array_equal(res1.numpy(), res2.numpy()))
+
+    def test_append_activation_in_dygraph_global_use_mkldnn(self):


LGTM for the tests to check the accuracy of the result when use_mkldnn is True and False.

I wonder if there are another ways that we checked the mkldnn is really used. For example, we can add some VLOG in the mkldnn kernel, and see if the log is printed when fluid.set_flags({'FLAGS_use_mkldnn': True}) is called. And we can do it manually it is difficult for unittest.
Maybe you have already done that, I just wonder that.

The problem we had is that AFAIK there is no better way to check if mkldnn kernel has been run than to check the output of DNNL_VERBOSE for now. The other idea is to run kernel throwing Exception with extended information about type of kernel. The other problem is that AFAIK python unittest lib does not have parsing c++ stdout result out of the box. Do you have suggestion what is the best way to check what type of kernel has been run from the python level ?

Do you have suggestion what is the best way to check what type of kernel has been run from the python level

I think checking the output of DNNL_VERBOSE is Ok. You can do it in the next PR.

jczaja added Intel dygraph issues related to dygraph mode labels Aug 20, 2020

arlesniak added 4 commits August 26, 2020 15:41

get use of global 'use_mkldnn' in layer_helper

40c7792

update for CI

3709d53

update for CI, relu test

cbbe8ff

update for CI, relu test added, make FLAGS_use_mkldnn a public flag

700abe3

wojtuss reviewed Aug 27, 2020

View reviewed changes

python/paddle/fluid/layer_helper.py Outdated Show resolved Hide resolved

added more strict tests, fixes after review

2fbf8a9

arlesniak force-pushed the arlesniak/global_use_mkldnn_in_layer_helper branch from 3f7190d to 2fbf8a9 Compare August 27, 2020 10:10

grygielski previously approved these changes Aug 28, 2020

View reviewed changes

luotao1 requested a review from zhiqiu August 28, 2020 07:40

wojtuss reviewed Aug 28, 2020

View reviewed changes

python/paddle/fluid/tests/unittests/test_imperative_basic.py Outdated Show resolved Hide resolved

zhiqiu reviewed Aug 28, 2020

View reviewed changes

fixes after review

a9ca37d

arlesniak dismissed grygielski’s stale review via a9ca37d August 28, 2020 18:31

fixes after review, CI stuff

acd1ed1

grygielski approved these changes Aug 31, 2020

View reviewed changes

zhiqiu self-requested a review August 31, 2020 07:59

wojtuss approved these changes Aug 31, 2020

View reviewed changes

zhiqiu reviewed Aug 31, 2020

View reviewed changes

luotao1 merged commit 885c61f into PaddlePaddle:develop Aug 31, 2020

luotao1 mentioned this pull request Aug 31, 2020

support mnist and resnet dygraph_to_static test #25774

Merged

arlesniak mentioned this pull request Sep 2, 2020

Test for passing global env FLAGS_use_mkldnn #26938

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add use of global flag 'use_mkldnn' to layer_helper #26497

Add use of global flag 'use_mkldnn' to layer_helper #26497

arlesniak commented Aug 20, 2020 •

edited

paddle-bot-old bot commented Aug 20, 2020

arlesniak commented Aug 27, 2020

luotao1 commented Aug 27, 2020

grygielski left a comment

luotao1 commented Aug 28, 2020

zhiqiu Aug 28, 2020

arlesniak Aug 28, 2020 •

edited

zhiqiu Aug 28, 2020 •

edited

arlesniak Aug 31, 2020

grygielski left a comment

wojtuss left a comment

luotao1 commented Aug 31, 2020

zhiqiu Aug 31, 2020

arlesniak Aug 31, 2020

zhiqiu Aug 31, 2020 •

edited

arlesniak Aug 31, 2020

luotao1 Aug 31, 2020

Add use of global flag 'use_mkldnn' to layer_helper #26497

Add use of global flag 'use_mkldnn' to layer_helper #26497

Conversation

arlesniak commented Aug 20, 2020 • edited

PR types

PR changes

Describe

paddle-bot-old bot commented Aug 20, 2020

arlesniak commented Aug 27, 2020

luotao1 commented Aug 27, 2020

grygielski left a comment

Choose a reason for hiding this comment

luotao1 commented Aug 28, 2020

zhiqiu Aug 28, 2020

Choose a reason for hiding this comment

arlesniak Aug 28, 2020 • edited

Choose a reason for hiding this comment

zhiqiu Aug 28, 2020 • edited

Choose a reason for hiding this comment

arlesniak Aug 31, 2020

Choose a reason for hiding this comment

grygielski left a comment

Choose a reason for hiding this comment

wojtuss left a comment

Choose a reason for hiding this comment

luotao1 commented Aug 31, 2020

zhiqiu Aug 31, 2020

Choose a reason for hiding this comment

arlesniak Aug 31, 2020

Choose a reason for hiding this comment

zhiqiu Aug 31, 2020 • edited

Choose a reason for hiding this comment

arlesniak Aug 31, 2020

Choose a reason for hiding this comment

luotao1 Aug 31, 2020

Choose a reason for hiding this comment

arlesniak commented Aug 20, 2020 •

edited

arlesniak Aug 28, 2020 •

edited

zhiqiu Aug 28, 2020 •

edited

zhiqiu Aug 31, 2020 •

edited