Improve log_softmax op performance by using DNNL support #18320

bgawrych · 2020-05-14T11:06:50Z

Description

Improve log_softmax op performance by using DNNL support

Native implementation:

shape	time (ms/iter)
(5, 512, 512)	5.3858582973480225
(5, 512, 1536)	18.289981842041016
(5, 512, 2048)	23.175915956497192
(5, 2048, 512)	24.09080457687378
(4, 512, 512)	4.271230220794678

MKLDNN implementation:

shape	time (ms/iter)
(5, 512, 512)	1.0982356071472168
(5, 512, 1536)	3.9154343605041504
(5, 512, 2048)	5.493232250213623
(5, 2048, 512)	6.6350884437561035
(4, 512, 512)	0.8532240390777588

Checklist

Essentials

Changes are complete (i.e. I finished coding on this PR)
To the best of my knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Comments

Tests have increased tolerance - the reason of this is that DNNL sometimes have significantly different value of single element in array

mxnet-bot · 2020-05-14T11:06:54Z

Hey @bgawrych , Thanks for submitting the PR
All tests are already queued to run once. If tests fail, you can trigger one or more tests again with the following commands:

To trigger all jobs: @mxnet-bot run ci [all]
To trigger specific jobs: @mxnet-bot run ci [job1, job2]

CI supported jobs: [clang, windows-gpu, miscellaneous, unix-gpu, website, unix-cpu, centos-gpu, edge, windows-cpu, sanity, centos-cpu]

Note:
Only following 3 categories can trigger CI :PR Author, MXNet Committer, Jenkins Admin.
All CI tests must pass before the PR can be merged.

bgawrych · 2020-05-15T09:25:38Z

@mxnet-bot run ci [centos-cpu, unix-cpu]

mxnet-bot · 2020-05-15T09:25:43Z

Jenkins CI successfully triggered : [unix-cpu, centos-cpu]

bgawrych · 2020-05-18T06:28:06Z

@TaoLv @pengzhao-intel Can you look at this and ping interested people also?

TaoLv · 2020-05-18T06:49:01Z

src/operator/nn/log_softmax.cc

+}
+
+inline static bool LogSoftmaxStorageType(const nnvm::NodeAttrs& attrs,
+                                      const int dev_mask,


Please fix indent.

TaoLv · 2020-05-18T06:49:34Z

src/operator/nn/log_softmax.cc

+}
+
+inline static bool LogSoftmaxGradStorageType(const nnvm::NodeAttrs& attrs,
+                                          const int dev_mask,


TaoLv · 2020-05-18T06:50:18Z

src/operator/nn/log_softmax.cc

 .set_attr<nnvm::FGradient>("FGradient", SoftmaxFGradient{"_backward_log_softmax"})
 .set_attr<nnvm::FInferType>("FInferType", SoftmaxOpType)
 .set_num_inputs(1)
 .set_num_outputs(1)
-.set_attr<mxnet::FInferShape>("FInferShape", ElemwiseShape<1, 1>)
+.set_attr<mxnet::FInferShape>("FInferShape", SoftmaxOpShape)


Could you please elaborate?

Residue after use_length support

TaoLv · 2020-05-18T06:54:47Z

src/operator/nn/mkldnn/mkldnn_log_softmax.cc

+                             const OpReqType &req,
+                             const NDArray &out_data) {
+  if (req == kNullOp) return;
+  // same as the FCompute path, softmax only supports kWriteTo and kWriteInplace for now.


Fix the comment - should it be log_softmax?

TaoLv · 2020-05-18T06:55:31Z

src/operator/nn/mkldnn/mkldnn_log_softmax.cc

+                             const NDArray &data,
+                             const NDArray &output) {
+  // MKLDNN does not support temperature argument in their softmax function
+  // now. Need update this once they start to support it.


Duplicated comments. Also change softmax to log_softmax.

bgawrych · 2020-05-20T08:38:44Z

@mxnet-bot run ci [miscellaneous]

mxnet-bot · 2020-05-20T08:38:49Z

Jenkins CI successfully triggered : [miscellaneous]

pengzhao-intel · 2020-05-21T05:32:14Z

Thanks, @bgawrych thanks for the contribution :)

You can cc Tao and me when you filed a PR so that we can notice reviewing your code in time and please cherry-pick the change to 1.x after the code merge.

pengzhao-intel · 2020-05-21T05:35:07Z

Please add the new op into our list
https://mxnet.apache.org/versions/1.5.0/tutorials/mkldnn/operator_list.html

bgawrych · 2020-05-25T10:53:11Z

I have tested accuracy of Tree LSTM model to make sure MKLDNN log_softmax
doesn't cause accuracy drop.
Trainings was done on the same dataset and the same seed.

Tree LSTM model accuracy with MKLDNN log_softmax

epoch	pearsonr	mse
0	0,097245	1,096977
1	0,100926	1,051807
2	0,124511	1,039598
3	0,104311	1,040804
4	0,095321	1,04387
5	0,126679	1,040978
6	0,108294	1,043001
7	0,068355	1,048689
8	0,11179	1,04342
9	0,089656	1,044691

Tree LSTM model accuracy with native log_softmax

epoch	pearsonr	mse
0	0,097245	1,096978
1	0,100925	1,051807
2	0,124513	1,039598
3	0,104312	1,040804
4	0,095321	1,04387
5	0,126685	1,040978
6	0,108294	1,043001
7	0,068355	1,048689
8	0,111792	1,04342
9	0,089656	1,044691

There are some slightly differences (bolded), but overall it looks like floating point precision issue.
CC: @TaoLv
@pengzhao-intel

Please add the new op into our list
https://mxnet.apache.org/versions/1.5.0/tutorials/mkldnn/operator_list.html

Is this message to me or someone from amazon? Because I can't find any file responsible for this page

pengzhao-intel · 2020-05-27T02:57:37Z

@pengzhao-intel

Please add the new op into our list
https://mxnet.apache.org/versions/1.5.0/tutorials/mkldnn/operator_list.html

Is this message to me or someone from amazon? Because I can't find any file responsible for this page

@xinyu-intel could you help point out where to change the doc?

pengzhao-intel

LGTM, thanks for your contribution.

bgawrych · 2020-06-01T11:32:35Z

@pengzhao-intel @TaoLv
Is this change gonna be merged or need more approves?

pengzhao-intel · 2020-06-01T11:34:08Z

Thanks for the contribution. Merging now.

…upport (apache#18320) * Improve log_softmax performance by OneDNN library * Adapt tests for MKLDNN log_softmax * Fix lint errors * Fix indent and comments

* Improve log_softmax performance by OneDNN library * Adapt tests for MKLDNN log_softmax * Fix lint errors * Fix indent and comments

…upport (#18320) (#18469) * Improve log_softmax performance by OneDNN library * Adapt tests for MKLDNN log_softmax * Fix lint errors * Fix indent and comments

* Improve log_softmax performance by OneDNN library * Adapt tests for MKLDNN log_softmax * Fix lint errors * Fix indent and comments

…upport (apache#18320) (apache#18469) * Improve log_softmax performance by OneDNN library * Adapt tests for MKLDNN log_softmax * Fix lint errors * Fix indent and comments

TaoLv reviewed May 18, 2020

View reviewed changes

TaoLv added this to In progress in CPU Performance and Quantization via automation May 18, 2020

bgawrych added 4 commits May 19, 2020 11:31

Improve log_softmax performance by OneDNN library

f5a706c

Adapt tests for MKLDNN log_softmax

deca041

Fix lint errors

b393cc1

Fix indent and comments

957eaf0

bgawrych force-pushed the log_softmax branch from 7aba63a to 957eaf0 Compare May 19, 2020 09:34

pengzhao-intel added the MKLDNN label May 21, 2020

CPU Performance and Quantization automation moved this from In progress to Reviewer approved May 25, 2020

TaoLv approved these changes May 25, 2020

View reviewed changes

pengzhao-intel approved these changes May 28, 2020

View reviewed changes

pengzhao-intel merged commit cbbb864 into apache:master Jun 1, 2020

CPU Performance and Quantization automation moved this from Reviewer approved to Done Jun 1, 2020

bgawrych mentioned this pull request Jun 2, 2020

[v1.x] Backport of improve log_softmax op performance by using DNNL #18469

Merged

bgawrych mentioned this pull request Jun 3, 2020

Improve log_softmax performance by OneDNN library #18065

Closed

pengzhao-intel added this to In progress in MKLDNN improvements via automation Aug 24, 2020

pengzhao-intel moved this from In progress to Reviewer approved in MKLDNN improvements Aug 24, 2020

pengzhao-intel moved this from Reviewer approved to Done in MKLDNN improvements Aug 24, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve log_softmax op performance by using DNNL support #18320

Improve log_softmax op performance by using DNNL support #18320

bgawrych commented May 14, 2020

mxnet-bot commented May 14, 2020

bgawrych commented May 15, 2020

mxnet-bot commented May 15, 2020

bgawrych commented May 18, 2020

TaoLv May 18, 2020

TaoLv May 18, 2020

TaoLv May 18, 2020

bgawrych May 19, 2020

TaoLv May 18, 2020

TaoLv May 18, 2020

bgawrych commented May 20, 2020

mxnet-bot commented May 20, 2020

pengzhao-intel commented May 21, 2020

pengzhao-intel commented May 21, 2020

bgawrych commented May 25, 2020

pengzhao-intel commented May 27, 2020 •

edited

pengzhao-intel left a comment

bgawrych commented Jun 1, 2020

pengzhao-intel commented Jun 1, 2020

Improve log_softmax op performance by using DNNL support #18320

Improve log_softmax op performance by using DNNL support #18320

Conversation

bgawrych commented May 14, 2020

Description

Checklist

Essentials

Comments

mxnet-bot commented May 14, 2020

bgawrych commented May 15, 2020

mxnet-bot commented May 15, 2020

bgawrych commented May 18, 2020

TaoLv May 18, 2020

Choose a reason for hiding this comment

TaoLv May 18, 2020

Choose a reason for hiding this comment

TaoLv May 18, 2020

Choose a reason for hiding this comment

bgawrych May 19, 2020

Choose a reason for hiding this comment

TaoLv May 18, 2020

Choose a reason for hiding this comment

TaoLv May 18, 2020

Choose a reason for hiding this comment

bgawrych commented May 20, 2020

mxnet-bot commented May 20, 2020

pengzhao-intel commented May 21, 2020

pengzhao-intel commented May 21, 2020

bgawrych commented May 25, 2020

pengzhao-intel commented May 27, 2020 • edited

pengzhao-intel left a comment

Choose a reason for hiding this comment

bgawrych commented Jun 1, 2020

pengzhao-intel commented Jun 1, 2020

pengzhao-intel commented May 27, 2020 •

edited