Add LeakyRelu MKLDNN support by grygielski · Pull Request #18656 · PaddlePaddle/Paddle

grygielski · 2019-07-16T12:47:02Z

LeakyRelu MKLDNN support along with unit tests have been added.

CLAassistant · 2019-07-16T12:47:05Z

All committers have signed the CLA.

bingyanghuang · 2019-07-17T03:02:46Z

This PR is to fix #17654

test=develop

jczaja

LGTM

luotao1 · 2019-07-18T11:35:53Z

paddle/fluid/operators/activation_op.cc

    // input1: X
    op->SetInput("X", Input("X"));
+    // input2: Out
+    op->SetInput("Out", Input("Out"));


Why add an Input here?

It's because MKLDNN activation function grad functor expects to have input called "Out" to create hash key for caching. Without this change activation grad test crashed.

create hash key for caching

Why should we add this input only for creating hash key, since this op doesn't need this input indeed?

Well, I did it as it felt like the fastest way to get around this error but you are right, this Input unneccesary so I will change the way MKLDNN key is generated.

Thanks very much! If we change the input, then many trained models before will fail after update the develop branch.

@luotao1 Could you please explain why trained model will fail when we add additional input to grad op? This is just leaky relu grad op , so this grad op is not holding any params, so why extending inputs for grad op would cause failure? I'm asking because as described in #17960 I would like to extend all mkl-dnn grad ops to have X as input of grad ops as mkl-dnn backward implementations are requiring X to compute backward/grad.

@phlrain Could you help see this problem?

@jczaja @grygielski
Discussed with @phlrain, it's OK to add additional input to grad op as this PR does. But we could not addinput() in OpMaker().

Ok, so my sollution is acceptable? Because during key generation we would have to add another cases to check if specific operator grad has only X input what causes additional problems when adding MKLDNN support for further activations.

Yes, it is.

luotao1 · 2019-07-18T15:11:05Z

Do you have any benchmark result for this mkldnn op on face model in #17654

jczaja · 2019-07-18T15:15:17Z

@luotao1 When we wanted to test performance we noticed that face model no longer works on develop for us #18658 . So until we have demark model fixed we cannot test performance of leaky rely of mkl-dnn

luotao1 · 2019-07-18T15:27:30Z

Got it. I will check it tomorrow

luotao1 · 2019-07-19T05:04:25Z

Thanks very much for your contribution.
I test it on demark model. leaky_relu speedup from 132ms->38ms on 4 threads, 3.47x speedup!

before:

Event                        Calls       Total       Min.        Max.        Ave.        Ratio.
thread0::conv2d_transpose    400         3794.74     2.86285     17.8366     9.48685     0.768815
thread0::conv2d              400         899.1       0.729642    3.6068      2.24775     0.182158
thread0::leaky_relu          400         132.44      0.085567    1.10562     0.3311      0.0268324
thread0::batch_norm          300         50.5053     0.1174      0.256459    0.168351    0.0102324
thread0::relu                300         39.7742     0.067462    0.242046    0.132581    0.00805825
thread0::fetch               100         11.2578     0.106524    0.123902    0.112578    0.00228083
thread0::tanh                100         7.4732      0.069471    0.090934    0.074732    0.00151407
thread0::feed                100         0.541868    0.004703    0.006246    0.00541868  0.000109783

after:

Event                        Calls       Total       Min.        Max.        Ave.        Ratio.
thread0::conv2d_transpose    400         3809.5      2.86721     17.8962     9.52376     0.786597
thread0::conv2d              400         882.729     0.732503    3.49819     2.20682     0.182269
thread0::batch_norm          300         50.5289     0.119364    0.270264    0.16843     0.0104334
thread0::relu                300         42.0752     0.074177    0.234958    0.140251    0.00868782
thread0::leaky_relu          400         38.5553     0.047176    0.194089    0.0963881   0.007961
thread0::fetch               100         10.8161     0.099839    0.117321    0.108161    0.00223335
thread0::tanh                100         8.28066     0.078006    0.108433    0.0828066   0.00170981
thread0::feed                100         0.526761    0.004705    0.006533    0.00526761  0.000108767

luotao1 · 2019-07-19T05:11:30Z

Paddle/paddle/fluid/framework/ir/is_test_pass.cc

Lines 27 to 35 in 067ed70

    
           auto op_list = {"pool2d",      "sigmoid",      "logsigmoid", 
        
                           "softshrink",  "exp",          "brelu", 
        
                           "pow",         "leaky_relu",   "stanh", 
        
                           "relu",        "tanh",         "tanh_shrink", 
        
                           "sqrt",        "abs",          "ceil", 
        
                           "elu",         "floor",        "cos", 
        
                           "sin",         "round",        "reciprocal", 
        
                           "hard_shrink", "hard_sigmoid", "relu6", 
        
                           "soft_relu",   "swish",        "thresholded_relu",

Do you need to update the op_list in is_test_pass?

grygielski · 2019-07-19T06:51:36Z

I guess leaky_relu is already there

luotao1 · 2019-07-19T08:04:27Z

I guess leaky_relu is already there

But I don't see it. @grygielski @jczaja

grygielski · 2019-07-19T08:24:06Z

What about 3rd row, 2nd column in op_list you quoted in your last comment?

luotao1 · 2019-07-19T08:52:34Z

Sorry, I miss finding it.

test=develop

jczaja requested review from Sand3r- and jczaja July 16, 2019 12:49

luotao1 added the Intel label Jul 16, 2019

grygielski force-pushed the grygielski/leaky_relu_mkldnn branch 2 times, most recently from 123e958 to d5aad2a Compare July 16, 2019 13:34

Add LeakyRelu MKLDNN support

2d47bd7

test=develop

grygielski force-pushed the grygielski/leaky_relu_mkldnn branch from d5aad2a to 2d47bd7 Compare July 17, 2019 08:02

jczaja approved these changes Jul 17, 2019

View reviewed changes

jczaja assigned luotao1 Jul 17, 2019

Sand3r- approved these changes Jul 18, 2019

View reviewed changes

luotao1 reviewed Jul 18, 2019

View reviewed changes

luotao1 requested a review from phlrain July 18, 2019 14:50

luotao1 merged commit d6b6a33 into PaddlePaddle:develop Jul 19, 2019

grygielski deleted the grygielski/leaky_relu_mkldnn branch July 19, 2019 07:55

jczaja mentioned this pull request Jul 19, 2019

[MKL-DNN] Add Leaky-relu support #17654

Closed

This was referenced Jul 19, 2019

PR #18656 add unnecessary input in leaky_relu_grad op #18707

Closed

PR #18656 add unnecessary input in leaky_relu_grad op #18708

Closed

luotao1 mentioned this pull request Jul 22, 2019

Revert "Add LeakyRelu MKLDNN support (#18656)" #18723

Merged

luotao1 added a commit that referenced this pull request Jul 22, 2019

Revert "Add LeakyRelu MKLDNN support (#18656)" (#18723)

bd22453

test=develop

luotao1 mentioned this pull request Jul 26, 2019

[MKL-DNN] Add Leaky-relu support and conv+leaky_relu fuse pass #18788

Closed

Conversation

grygielski commented Jul 16, 2019

Uh oh!

CLAassistant commented Jul 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bingyanghuang commented Jul 17, 2019

Uh oh!

jczaja left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

grygielski Jul 18, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

grygielski Jul 18, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

luotao1 commented Jul 18, 2019

Uh oh!

jczaja commented Jul 18, 2019

Uh oh!

luotao1 commented Jul 18, 2019

Uh oh!

luotao1 commented Jul 19, 2019

Uh oh!

luotao1 commented Jul 19, 2019

Uh oh!

grygielski commented Jul 19, 2019

Uh oh!

luotao1 commented Jul 19, 2019

Uh oh!

grygielski commented Jul 19, 2019

Uh oh!

luotao1 commented Jul 19, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

CLAassistant commented Jul 16, 2019 •

edited

Loading

grygielski Jul 18, 2019 •

edited

Loading

grygielski Jul 18, 2019 •

edited

Loading