Update activations for MKL-DNN #10597

kbinias · 2018-05-11T10:22:58Z

Updated activations for MKL-DNN after changes in PaddlePaddle activations.

luotao1 · 2018-05-11T13:55:33Z

after changes in PaddlePaddle activations.

What's the changes do you mean?

kbinias · 2018-05-11T15:51:28Z

Changes for register activation operators (look at changes in activation_op.cc file, e.g. macros to create REGISTER_ACTIVATION_OP_MAKER class XXXOpMaker, new macros for register operators e.g. INPLACE, etc.)

luotao1 · 2018-05-14T06:44:12Z

paddle/fluid/operators/activation_op.cc

  __macro(SoftRelu, soft_relu);              \
  __macro(Relu6, relu6);                     \
  __macro(Reciprocal, reciprocal);           \
  __macro(HardSigmoid, hard_sigmoid);

+#define FOR_EACH_MKLDNN_INPLACE_OP_FUNCTOR(__macro) \


Why changes of activation_op.cc?

Are the previous registration not worked? But I see that the unit-test test_activation_mkldnn_op.py works well.

Current changes are not suitable for other devices. For example, if there is amd_relu_op, how should it be defined?

tensor-tang · 2018-05-14T06:06:27Z

paddle/fluid/operators/activation_mkldnn_op.cc

-                     static_cast<void *>(const_cast<float *>(src_data)));
+                     static_cast<void *>(const_cast<float *>(src_data))));
+  // save source memory to device context to be referred in backward path
+  dev_ctx.SetBlob("InputX@eltwise_pd", src_memory);
  auto dst_memory =


Why only save src_memory, how about dst_memory?

eltwise_grad needs input data (e.g. look at activation_op.cc ReluAddInput("X", ...)). I have no access to this data directly in eltwise_grad function. Only eltwise_forward has access to this input data.
dst_memory is not used in eltwise_grad.

For example, in MKL-DNN sqrt_bwd has different implementation than PaddlePaddle:

// MKL-DNN
template
T sqrt_bwd(T dd, T s) {
return s > 0 ? dd / (2 * ::sqrtf(s)) : 0;
}

// PP
void operator()(Device d, X x, Out out, dOut dout, dX dx) const {
const Out out_conj = Eigen::numext::conj(out);
dx.device(d) = static_cast(0.5) * dout / out_conj;
}

this the reason I need input data in eltwise_grad

I know why src_memory is needed, but my point is why not also save dst_memory to context to save time since you have already saved src.

tensor-tang · 2018-05-14T06:11:34Z

paddle/fluid/operators/activation_mkldnn_op.cc

@@ -69,7 +71,7 @@ void eltwise_forward(const ExecContext &ctx, mkldnn::algorithm algorithm,
      forward_desc, mkldnn_engine);
  dev_ctx.SetBlob(key_eltwise_pd, forward_pd);


I can only see setting forward_pd every forward iteration but not retrieving the existed one?

forward_pd is retrieved in eltwise_grad() function, line with dev_ctx.GetBlob(key_eltwise_pd)

Actually, my point is I can only see you always create and set this pd to context every forward iteration, but no reuse it in next iteration. Could we avoid this recreation among every iterations to enhance performance?

Yes, of course. I will improve it in next commit.

I added suport for reusing memory buffers to MKL-DNN activations.

kbinias · 2018-05-18T13:30:26Z

Could you possibly continue your review ? I added suport for reusing memory buffers.

tensor-tang

Sorry for the late reply, I have one question.

tensor-tang · 2018-05-21T02:37:26Z

paddle/fluid/operators/activation_mkldnn_op.cc

@@ -23,6 +24,18 @@ using paddle::framework::Tensor;
 using paddle::platform::MKLDNNDeviceContext;

 namespace {
+std::string gethash(const mkldnn::memory::dims &operand_dims,
+                    const mkldnn::algorithm algorithm) {
+  auto dim2str = [](const mkldnn::memory::dims &operand_dims) {


Is that possible two fc ops with same src dims, then would this be right? They would share same hash code?

They will be reusable except input data. I improved hash code for input data (key_src_data)

tensor-tang

LGTM

kbinias added the Intel label May 11, 2018

kbinias force-pushed the mkldnn-activations-improvments branch from 771ac6f to 598ae8e Compare May 11, 2018 11:39

kbinias requested a review from luotao1 May 11, 2018 13:20

luotao1 requested a review from tensor-tang May 14, 2018 01:56

luotao1 reviewed May 14, 2018

View reviewed changes

kbinias force-pushed the mkldnn-activations-improvments branch from 598ae8e to bf447fc Compare May 14, 2018 10:24

tensor-tang reviewed May 14, 2018

View reviewed changes

kbinias force-pushed the mkldnn-activations-improvments branch from bf447fc to f6404f0 Compare May 17, 2018 15:57

tensor-tang reviewed May 21, 2018

View reviewed changes

kbinias added 6 commits May 21, 2018 12:20

Update activations for MKL-DNN

1c81301

MKL-DNN activations improvements

a76d0dd

Realloc for forward

0cc25a4

Add backward

0aa0192

Cache input data

32929cd

Unique key for input data

24904b9

kbinias force-pushed the mkldnn-activations-improvments branch from f6404f0 to 24904b9 Compare May 21, 2018 10:23

tensor-tang approved these changes May 22, 2018

View reviewed changes

tensor-tang merged commit 7205d33 into PaddlePaddle:develop May 22, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update activations for MKL-DNN #10597

Update activations for MKL-DNN #10597

kbinias commented May 11, 2018

luotao1 commented May 11, 2018

kbinias commented May 11, 2018

luotao1 May 14, 2018

tensor-tang May 14, 2018

kbinias May 14, 2018

kbinias May 14, 2018

tensor-tang May 15, 2018 •

edited

Loading

tensor-tang May 14, 2018 •

edited

Loading

kbinias May 14, 2018

tensor-tang May 15, 2018

kbinias May 15, 2018

kbinias May 17, 2018

kbinias commented May 18, 2018

tensor-tang left a comment

tensor-tang May 21, 2018 •

edited

Loading

kbinias May 21, 2018 •

edited

Loading

tensor-tang May 22, 2018

tensor-tang left a comment

		@@ -69,7 +71,7 @@ void eltwise_forward(const ExecContext &ctx, mkldnn::algorithm algorithm,
		forward_desc, mkldnn_engine);
		dev_ctx.SetBlob(key_eltwise_pd, forward_pd);

Update activations for MKL-DNN #10597

Update activations for MKL-DNN #10597

Conversation

kbinias commented May 11, 2018

luotao1 commented May 11, 2018

kbinias commented May 11, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tensor-tang May 15, 2018 • edited Loading

Choose a reason for hiding this comment

tensor-tang May 14, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kbinias commented May 18, 2018

tensor-tang left a comment

Choose a reason for hiding this comment

tensor-tang May 21, 2018 • edited Loading

Choose a reason for hiding this comment

kbinias May 21, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tensor-tang left a comment

Choose a reason for hiding this comment

tensor-tang May 15, 2018 •

edited

Loading

tensor-tang May 14, 2018 •

edited

Loading

tensor-tang May 21, 2018 •

edited

Loading

kbinias May 21, 2018 •

edited

Loading