-
Notifications
You must be signed in to change notification settings - Fork 6.8k
Conversation
Thanks for your contribution. Could you add a test case to cover the specific size of 8? |
@@ -420,8 +419,7 @@ void BatchNormGradComputeExCPU(const nnvm::NodeAttrs &attrs, | |||
|
|||
mxnet::TShape shape = inputs[0].shape(); | |||
// MKLDNN batchnorm only works well on the special MKLDNN layout. | |||
if (SupportMKLDNNBN(inputs[0], param) | |||
&& (inputs[3].IsMKLDNNData() || inputs[0].IsMKLDNNData())) { | |||
if (SupportMKLDNNBN(inputs[0], param)) { |
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 we don't need to check these two conditions?
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.
It's the same reason as line 399, as described in Performance part, when a model has fp32 bn along with other int8 ops, it triggers the IsMKLDNNData() check and cannot run int MKLDNN, which cause a slower speed.
@@ -396,7 +395,7 @@ void BatchNormComputeExCPU(const nnvm::NodeAttrs &attrs, | |||
CHECK_EQ(inputs.size(), 5U); | |||
const BatchNormParam ¶m = nnvm::get<BatchNormParam>(attrs.parsed); | |||
// MKLDNN batchnorm only works well on the special MKLDNN layout. | |||
if (SupportMKLDNNBN(inputs[0], param) && inputs[0].IsMKLDNNData()) { | |||
if (SupportMKLDNNBN(inputs[0], param)) { | |||
std::vector<NDArray> in_data(inputs.begin(), inputs.begin() + batchnorm::kInMovingMean); |
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 remove the comments in L397
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.
LGTM
Please add the tests and make the CI pass.
Seems something wrong on CI or your code. Please rebase the code and try again. |
Seems something wrong on CI, retrigger it again. If it's still failed, will look into it. |
4b08a14
to
f0a5a7f
Compare
It's not easy to pass CI. Merging now and thanks for your contribution. |
Description
update BN conditions of when to run into MKLDNN.
Changes
Performance