Skip to content

[MKL-DNN]Bump up mkl-dnn to 0.20#18370

Merged
luotao1 merged 1 commit intoPaddlePaddle:developfrom
jczaja:develop-0.20
Jul 19, 2019
Merged

[MKL-DNN]Bump up mkl-dnn to 0.20#18370
luotao1 merged 1 commit intoPaddlePaddle:developfrom
jczaja:develop-0.20

Conversation

@jczaja
Copy link
Contributor

@jczaja jczaja commented Jun 27, 2019

Those changes are updating MKL-DNN from 0.19 to 0.20.

Justification:

  • Some new post operations (needed for some int8 workloads)
  • better performance of sofmax & transpose ops (AVX512 platforms)
  • inner product does work on 3D,4D,5D.. dimensional data so we can enable some workloads like BERT to use mkl-dnn FC more (enabling to be done after this is merged)

@luotao1
Copy link
Contributor

luotao1 commented Jun 28, 2019

This commit will fail on one of the internal models, which runs successfully on 0.19.

@jczaja
Copy link
Contributor Author

jczaja commented Jun 28, 2019

@luotao1 could you please share some logs/description of failure?

@jczaja
Copy link
Contributor Author

jczaja commented Jun 28, 2019

@luotao1 In particular I'm interested in MKLDNN_VERBOSE log

@luotao1
Copy link
Contributor

luotao1 commented Jun 28, 2019

The model was sent to you Dec 2018. If you don't find the model, I will ask @bingyanghuang to sync with you.

@jczaja
Copy link
Contributor Author

jczaja commented Jul 15, 2019

@luotao1 I have updated changes with needed fix to have crash in internal workload you reported, goes away. We are testing currently those changes.

@luotao1
Copy link
Contributor

luotao1 commented Jul 16, 2019

@jczaja Do you mean current PR is OK for review?

@jczaja
Copy link
Contributor Author

jczaja commented Jul 16, 2019

@luotao1 I'm sorry for being not very clear. We haven't finished testing it yet (Should be done today/tomorrow) and then those changes will be ready for review. But what I would like You to do is to test your internal workloads if possible on this branch and tell me if you see any problems . If you also let us know if there is some performance change that would be good as well.

@jczaja jczaja requested review from Sand3r- and kbinias July 17, 2019 13:55
@jczaja
Copy link
Contributor Author

jczaja commented Jul 17, 2019

@baojun-nervana Just to let You know that we are bumping up mkl-dnn to 0.20 in this PR.

@jczaja
Copy link
Contributor Author

jczaja commented Jul 17, 2019

@luotao1 We finished internal testing. From highlights, there is slight performance improvement on BERT inference on AVX512 platforms eg. Skylake and Cascadelake. So PR is now ready for review, if you have any data on how this PR working with your internal workloads then please share

@baojun-nervana
Copy link
Contributor

@baojun-nervana Just to let You know that we are bumping up mkl-dnn to 0.20 in this PR.

Thanks, will update ngraph.

Copy link
Contributor

@Sand3r- Sand3r- left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jczaja
Copy link
Contributor Author

jczaja commented Jul 19, 2019

@luotao1 Have you had a chance to test this PR on your internal workloads? If yes and positive then please consider merging

@luotao1
Copy link
Contributor

luotao1 commented Jul 19, 2019

It works OK on internal workloads, thanks very much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants