-
Notifications
You must be signed in to change notification settings - Fork 6.8k
Conversation
src/operator/nn/softmax.cc
Outdated
@@ -38,10 +38,9 @@ static void SoftmaxComputeExCPU(const nnvm::NodeAttrs& attrs, | |||
const std::vector<NDArray>& inputs, | |||
const std::vector<OpReqType>& req, | |||
const std::vector<NDArray>& outputs) { | |||
const SoftmaxParam& param = nnvm::get<SoftmaxParam>(attrs.parsed); | |||
// It seems MKLDNN softmax doesn't support training. | |||
// and it only supports non-negative axis. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please also remove this comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why mkldnn softmax doesn't support training? https://github.com/intel/mkl-dnn/blob/master/include/mkldnn.hpp#L2680
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are not using the latest MKLDNN.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test?
@szha unit tests with negative axis have been added and passed. |
@szha what's the difference of softmax/log_softmax/SoftmaxActivation in mxnet? and which one is the most widely used by mxnet users? Seems only softmax is optimized here with mkldnn primitive. |
right, the SoftmaxActivation is an older operator and is not actively developed anymore. Going forward we are using softmax/log_softmax. |
@szha same question for SoftmaxOutput operator? https://github.com/apache/incubator-mxnet/blob/master/src/operator/softmax_output.cc#L47 |
* add softmax imporvement * reuse CheckAxis code * update comment * add tests with negative axis
* add softmax imporvement * reuse CheckAxis code * update comment * add tests with negative axis
Description
Apply CheckAxis to MKLDNN Softmax function in order to call MKLDNN correctly.
Checklist
Essentials
Please feel free to remove inapplicable items for your PR.
@pengzhao-intel @TaoLv