Softmax MKLDNN FLUID operator #9214
Conversation
jczaja
force-pushed
the
prv-softmax-mkldnn-operator-PR
branch
from
083b8bd
to
29b68ef
Compare
|
LGTM. @tensor-tang Could you help review the |
| using mkldnn::stream; | ||
|
|
||
| template <typename T> | ||
| class SoftmaxMkldnnKernel : public paddle::framework::OpKernel<T> { |
There was a problem hiding this comment.
SoftmaxMkldnnKernel ==> SoftmaxMKLDNNKernel
| @@ -81,6 +81,7 @@ def fc(input, | |||
| num_flatten_dims=1, | |||
| param_attr=None, | |||
| bias_attr=None, | |||
| use_mkldnn=False, | |||
There was a problem hiding this comment.
Why softmax PR would change fc?
| attrs={ | ||
| "x_num_col_dims": num_flatten_dims, | ||
| "y_num_col_dims": 1, | ||
| 'use_mkldnn': use_mkldnn |
There was a problem hiding this comment.
Save as above, this should be added in FC PR, right?
There was a problem hiding this comment.
@tensor-tang You are correct, but without those changes to FC Softmax MKLDNN would remain unused code. Till FC of MKLDNN is not ready (it is under development) then there is no other option for users to take advantage of softmax mkldnn operator as it is usually used as activation of FC layer. Do you suggest to remove use_mkldnn attribute/param from FC ?
There was a problem hiding this comment.
@tensor-tang, @luotao1 Should I remove use_mkldnn attrbute from FC , in this PR?
There was a problem hiding this comment.
I think there should be some other ways to use mkldnn softmax without change fc. But since we will implement MKLDNN FC soon, I think it's fine, you do not need revert back now.
jczaja
force-pushed
the
prv-softmax-mkldnn-operator-PR
branch
2 times, most recently
from
9df2a09
to
228e0dc
Compare
removed diagnostic - Added Unit tests for Softmax MKLDNN Forward Added fix for div by 0 to happen in cross_entropy backward Conflicts: paddle/fluid/operators/CMakeLists.txt - Cosmetic fixes to SoftMax MKLDNN fluid operator Added misssing softmax fluid operator file Disabled MKLDNN softmax operator by default Fix to softmax op unittest merge clang_formater fixes clang_formatter fixes - Name changing of softmax mkldnn operator to maintin consistency across codebase - updated comment fix to comment
jczaja
force-pushed
the
prv-softmax-mkldnn-operator-PR
branch
from
228e0dc
to
3b95b55
Compare
| attrs={ | ||
| "x_num_col_dims": num_flatten_dims, | ||
| "y_num_col_dims": 1, | ||
| 'use_mkldnn': use_mkldnn |
There was a problem hiding this comment.
I think there should be some other ways to use mkldnn softmax without change fc. But since we will implement MKLDNN FC soon, I think it's fine, you do not need revert back now.
This PR provides MKLDNN based Softmax op implementation.
Performance and testing:
On tested models softmax MKLDNN op is roughly ~10x faster than Plain CPU version.
RNN Search (https://github.com/dzhwinter/benchmark/blob/master/fluid/machine_translation.py)
executes training in ~90% of Plain CPU time. RNN search does converge with Softmax MKLDNN op used.
Notes