Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

how to add MKL Packed interface #6612

Closed
tensor-tang opened this issue Dec 14, 2017 · 6 comments · Fixed by #6680
Closed

how to add MKL Packed interface #6612

tensor-tang opened this issue Dec 14, 2017 · 6 comments · Fixed by #6680

Comments

@tensor-tang
Copy link
Contributor

tensor-tang commented Dec 14, 2017

Need discuss about the python interface how to use mkl packed implements.

As this issue is long, I summarize the key points in Chinese as first:

intel对GRU等layer优化(还是使用MKL库,非MKLDNN库)完并通过测试后,python api封装时,cpu模式(同时开启with_mkl=on),python端是否可以默认调用优化过后的c++ layer?

Background

We plan to add MKL Packed implements for RNN layers,(Recurrent Layer, LSTM, and GRU). related #6512 and #6506 .

They are total different with MKLDNN, and they will be compiled only if WITH_MKL=ON,

Then we got a problem about how to use it from python side?

Solutions

There are two options:

  • Add one more flag: use_mkl_packed just like use_mkldnn. This would make things flexible but complex. If user want to use both. They should add both use_mkldnn=True and use_mkl_packed=True.

  • Change use_mkldnn ==> use_mkl, and make use_mkl cover both. This can make users happy, Just use_mkl=True is enough, do not need care which one to choose. But it's not flexible enough.

Which one we should choose? Any suggestion is appreciated.

@wangkuiyi @luotao1

@luotao1
Copy link
Contributor

luotao1 commented Dec 15, 2017

As MKL Packed optimization on RNN are totally different with MKLDNN, and is compiled only when WITH_MKL=ON, thus, above two options are not good enough:

  • The first one adds use_mkl_packed, which will make users more confused. Users cannot distinguish the difference between use_mkldnn and use_mkl_packed. If we tell them, when configure RNN, they can use use_mkl_packed, when configure CNN, they can use use_mkldnn, they will be crazy.
  • The second one changes use_mkldnn ==> use_mkl. However, in cpu mode, the default library is mklml now, thus, the default value of use_mkl in ON? and default library is both mklml and mkldnn?

I have another option: when MKL Packed optimization on RNN, in cpu mode and with_MKL=ON, can we default use these optimized C++ code to replace the original recurrent layer/LSTMLayer/GRULayer?

@hedaoyuan How do you think about it ? Any suggestion is appreciated.

@tensor-tang
Copy link
Contributor Author

The second one changes use_mkldnn ==> use_mkl. However, in cpu mode, the default library is mklml now, thus, the default value of use_mkl in ON? and default library is both mklml and mkldnn?

About this suggestion. In CPU mode, default compiling is WITH_MKL=ON, so will build both mklml
and mkldnn, But the default value use_mkl=False. Only if user set use_mkl=True, both MKL pakced function and MKLDNN functions will turn on.

I have another option: when MKL Packed optimization on RNN, in cpu mode and with_MKL=ON, can we default use these optimized C++ code to replace the original recurrent layer/LSTMLayer/GRULayer?

I think this is my option one, just default set use_mkl_packed=true in C++ code. Then users do not need to handle too much options.

@luotao1
Copy link
Contributor

luotao1 commented Dec 15, 2017

default compiling is WITH_MKL=ON, but default value use_mkl=False

This will make both develops and users confused. As original Paddle c++ code uses MKL library as well, use_mkl=False will make user think that Paddle use openblas library in this case.

@tensor-tang
Copy link
Contributor Author

tensor-tang commented Dec 15, 2017

OK, I get your point.

It seems option one is better, at least we can separate them.

@luotao1
Copy link
Contributor

luotao1 commented Dec 15, 2017

Discussed with @hedaoyuan , python api (both v2 and v1) can default use MKL packed C++ codes after testing.

@tensor-tang
Copy link
Contributor Author

OK, Thx. I see.

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

Successfully merging a pull request may close this issue.

2 participants