You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mxnet.apache.org by GitBox <gi...@apache.org> on 2020/03/22 06:08:44 UTC
[GitHub] [incubator-mxnet] wuxun-zhang opened a new pull request #17884:
[MKL-DNN] Integrate Conv3d and Pool3d/1d
wuxun-zhang opened a new pull request #17884: [MKL-DNN] Integrate Conv3d and Pool3d/1d
URL: https://github.com/apache/incubator-mxnet/pull/17884
## Description ##
This PR aims to integrate mkl-dnn 3d conv/pool primitive, including fp32 and quantization pass.
@pengzhao-intel @TaoLv @ciyongch @rongzha1 @zixuanweeei
## Checklist ##
### Essentials ###
- [x] Changes are complete (i.e. I finished coding on this PR)
- [x] All changes have test coverage:
- Unit tests are added for small changes to verify correctness (e.g. adding a new operator)
### Changes ###
- [x] support 5d data layout for MKLDNN
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] wuxun-zhang commented on a change in pull request
#17884: [MKL-DNN] Integrate Conv3d and Pool3d/1d
Posted by GitBox <gi...@apache.org>.
wuxun-zhang commented on a change in pull request #17884: [MKL-DNN] Integrate Conv3d and Pool3d/1d
URL: https://github.com/apache/incubator-mxnet/pull/17884#discussion_r397620390
##########
File path: src/operator/nn/mkldnn/mkldnn_base-inl.h
##########
@@ -153,9 +153,8 @@ static inline bool SupportMKLDNN(int dtype, const mxnet::TShape &shape) {
// MKLDNN currently does not support 0-dim Tensor and 0-size Tensor
return false;
}
-
return (dtype == mshadow::kFloat32 || dtype == mshadow::kBfloat16) &&
- (ndim == 1 || ndim == 2 || ndim == 4);
+ (ndim >= 1 && ndim <= 5);
Review comment:
Maybe it's better not to touch this function. I will enable 5d layout in `SupportPooling` instead.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] TaoLv commented on issue #17884: [MKL-DNN]
Integrate Conv3d and Pool3d/1d
Posted by GitBox <gi...@apache.org>.
TaoLv commented on issue #17884: [MKL-DNN] Integrate Conv3d and Pool3d/1d
URL: https://github.com/apache/incubator-mxnet/pull/17884#issuecomment-609039683
The CI issue should be already addressed. Please rebase your PR and resolve the comments. Thanks.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] pengzhao-intel commented on issue #17884:
[MKL-DNN] Integrate Conv3d and Pool3d/1d
Posted by GitBox <gi...@apache.org>.
pengzhao-intel commented on issue #17884: [MKL-DNN] Integrate Conv3d and Pool3d/1d
URL: https://github.com/apache/incubator-mxnet/pull/17884#issuecomment-611282749
Thanks for the improvement. Merging now.
We will share the performance data soon.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] TaoLv commented on a change in pull request
#17884: [MKL-DNN] Integrate Conv3d and Pool3d/1d
Posted by GitBox <gi...@apache.org>.
TaoLv commented on a change in pull request #17884: [MKL-DNN] Integrate Conv3d and Pool3d/1d
URL: https://github.com/apache/incubator-mxnet/pull/17884#discussion_r396517882
##########
File path: src/operator/nn/mkldnn/mkldnn_base-inl.h
##########
@@ -324,20 +323,27 @@ inline static mkldnn::memory::desc GetWeightDesc(const NDArray &arr,
if (num_groups == 1) {
return GetMemDesc(arr, dtype);
} else {
- auto ndim = arr.shape().ndim();
- CHECK((ndim == 3) || (ndim == 4))
- << "MKL-DNN weight currectly supports 3d and 4d layout";
+ const auto ndim = arr.shape().ndim();
+ CHECK((ndim == 3) || (ndim == 4) || (ndim == 5))
+ << "MKL-DNN weight currently supports 3d or 4d or 5d layout";
auto tz = mkldnn::memory::dims{0};
- const int N = 0, H = 2, W = 3, C = 1;
- if (ndim == 3) {
- tz = mkldnn::memory::dims{
- num_groups, static_cast<int>(arr.shape()[N] / num_groups),
- static_cast<int>(arr.shape()[C]), static_cast<int>(arr.shape()[H])};
- } else {
- tz = mkldnn::memory::dims{
- num_groups, static_cast<int>(arr.shape()[N] / num_groups),
- static_cast<int>(arr.shape()[C]), static_cast<int>(arr.shape()[H]),
- static_cast<int>(arr.shape()[W])};
+ const int D = (ndim == 5) ? 2 : 1;
+ const int N = 0, C = 1, H = D + 1, W = D + 2;
Review comment:
Let's be more descriptive here:
```suggestion
int N = 0, C = 1, H = 2, W = 3;
int D = -1;
if (ndim == 5) {
D = 2;
H = 3;
W = 4;
}
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] wuxun-zhang commented on a change in pull request
#17884: [MKL-DNN] Integrate Conv3d and Pool3d/1d
Posted by GitBox <gi...@apache.org>.
wuxun-zhang commented on a change in pull request #17884: [MKL-DNN] Integrate Conv3d and Pool3d/1d
URL: https://github.com/apache/incubator-mxnet/pull/17884#discussion_r397620361
##########
File path: src/operator/nn/mkldnn/mkldnn_pooling-inl.h
##########
@@ -114,15 +115,21 @@ inline bool SupportMKLDNNPooling(const PoolingParam ¶m,
return true;
} else {
if (param.pool_type == pool_enum::kAvgPooling) {
- CHECK_EQ(dshape.ndim(), 4);
+ CHECK(dshape.ndim() == 3 || dshape.ndim() == 4 || dshape.ndim() == 5);
// mkldnn works differently when padding is asymmetric, so let's skip this case.
- if (param.pad[0] == GetPaddingSizeFull(dshape[2], param.pad[0], param.pad[0], param.kernel[0],
- param.stride[0]) &&
- param.pad[1] == GetPaddingSizeFull(dshape[3], param.pad[1], param.pad[1], param.kernel[1],
- param.stride[1])) {
- return true;
+ bool is_symmetric = true;
+ switch (dshape.ndim()) {
+ case 5:
+ is_symmetric = is_symmetric && (param.pad[2] == GetPaddingSizeFull(dshape[4],
+ param.pad[2], param.pad[2], param.kernel[2], param.stride[2]));
+ case 4:
+ is_symmetric = is_symmetric && (param.pad[1] == GetPaddingSizeFull(dshape[3],
+ param.pad[1], param.pad[1], param.kernel[1], param.stride[1]));
Review comment:
Could you please show me where you saw these checks? Thanks
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] TaoLv commented on a change in pull request
#17884: [MKL-DNN] Integrate Conv3d and Pool3d/1d
Posted by GitBox <gi...@apache.org>.
TaoLv commented on a change in pull request #17884: [MKL-DNN] Integrate Conv3d and Pool3d/1d
URL: https://github.com/apache/incubator-mxnet/pull/17884#discussion_r397869347
##########
File path: src/operator/nn/pooling.cc
##########
@@ -274,12 +274,12 @@ void PoolingComputeExCPU(const nnvm::NodeAttrs &attrs, const OpContext &ctx,
// Pooling does not currently support working with views
if (inputs[0].IsView() || outputs[0].IsView()) {
+ std::cout << "Fall back to Pooling backward pass..." << std::endl;
Review comment:
Is this intended? We didn't have any log message for fallback execution.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] mxnet-bot commented on issue #17884: [MKL-DNN]
Integrate Conv3d and Pool3d/1d
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on issue #17884: [MKL-DNN] Integrate Conv3d and Pool3d/1d
URL: https://github.com/apache/incubator-mxnet/pull/17884#issuecomment-604172759
Jenkins CI successfully triggered : [windows-gpu, unix-gpu, centos-cpu]
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] TaoLv commented on a change in pull request
#17884: [MKL-DNN] Integrate Conv3d and Pool3d/1d
Posted by GitBox <gi...@apache.org>.
TaoLv commented on a change in pull request #17884: [MKL-DNN] Integrate Conv3d and Pool3d/1d
URL: https://github.com/apache/incubator-mxnet/pull/17884#discussion_r396524720
##########
File path: src/operator/nn/mkldnn/mkldnn_pooling.cc
##########
@@ -127,61 +116,139 @@ mkldnn::algorithm GetMKLDNNPoolAlgo(const PoolingParam ¶m) {
}
}
+void InitPoolingPrimitiveParams(const PoolingParam ¶m,
+ const mkldnn::memory::desc &data_md,
+ mkldnn::memory::dims *new_kernel,
Review comment:
How about pass-by-reference?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] ChaiBapchya commented on a change in pull request
#17884: [MKL-DNN] Integrate Conv3d and Pool3d/1d
Posted by GitBox <gi...@apache.org>.
ChaiBapchya commented on a change in pull request #17884: [MKL-DNN] Integrate Conv3d and Pool3d/1d
URL: https://github.com/apache/incubator-mxnet/pull/17884#discussion_r405664630
##########
File path: src/operator/nn/mkldnn/mkldnn_base-inl.h
##########
@@ -153,9 +153,8 @@ static inline bool SupportMKLDNN(int dtype, const mxnet::TShape &shape) {
// MKLDNN currently does not support 0-dim Tensor and 0-size Tensor
return false;
}
-
return (dtype == mshadow::kFloat32 || dtype == mshadow::kBfloat16) &&
- (ndim == 1 || ndim == 2 || ndim == 4);
Review comment:
What's the issue with 3D tensor?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] TaoLv commented on a change in pull request
#17884: [MKL-DNN] Integrate Conv3d and Pool3d/1d
Posted by GitBox <gi...@apache.org>.
TaoLv commented on a change in pull request #17884: [MKL-DNN] Integrate Conv3d and Pool3d/1d
URL: https://github.com/apache/incubator-mxnet/pull/17884#discussion_r397866258
##########
File path: src/operator/nn/mkldnn/mkldnn_pooling-inl.h
##########
@@ -114,15 +115,21 @@ inline bool SupportMKLDNNPooling(const PoolingParam ¶m,
return true;
} else {
if (param.pool_type == pool_enum::kAvgPooling) {
- CHECK_EQ(dshape.ndim(), 4);
+ CHECK(dshape.ndim() == 3 || dshape.ndim() == 4 || dshape.ndim() == 5);
// mkldnn works differently when padding is asymmetric, so let's skip this case.
- if (param.pad[0] == GetPaddingSizeFull(dshape[2], param.pad[0], param.pad[0], param.kernel[0],
- param.stride[0]) &&
- param.pad[1] == GetPaddingSizeFull(dshape[3], param.pad[1], param.pad[1], param.kernel[1],
- param.stride[1])) {
- return true;
+ bool is_symmetric = true;
+ switch (dshape.ndim()) {
+ case 5:
+ is_symmetric = is_symmetric && (param.pad[2] == GetPaddingSizeFull(dshape[4],
+ param.pad[2], param.pad[2], param.kernel[2], param.stride[2]));
+ case 4:
+ is_symmetric = is_symmetric && (param.pad[1] == GetPaddingSizeFull(dshape[3],
+ param.pad[1], param.pad[1], param.kernel[1], param.stride[1]));
Review comment:
okay, i didn't realize that you don't have `break` for each case. So if `ndim == 4`, both pad[1] and pad[0] are checked.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] wuxun-zhang commented on issue #17884: [MKL-DNN]
Integrate Conv3d and Pool3d/1d
Posted by GitBox <gi...@apache.org>.
wuxun-zhang commented on issue #17884: [MKL-DNN] Integrate Conv3d and Pool3d/1d
URL: https://github.com/apache/incubator-mxnet/pull/17884#issuecomment-609364511
Now DNNL version in master branch has been wrongly changed (from v1.2.2 to v1.1.2). This PR is now waiting for DNNL v1.2 or higher version coming back.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] ChaiBapchya commented on issue #17884: [MKL-DNN]
Integrate Conv3d and Pool3d/1d
Posted by GitBox <gi...@apache.org>.
ChaiBapchya commented on issue #17884: [MKL-DNN] Integrate Conv3d and Pool3d/1d
URL: https://github.com/apache/incubator-mxnet/pull/17884#issuecomment-611056744
@wuxun-zhang Thanks a lot for the work.
Can you share perf numbers for this patch?
What's the time taken 3d conv now w/MKLDNN vs then w/o MKLDNN?
Can we profile it?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] wuxun-zhang commented on issue #17884: [MKL-DNN]
Integrate Conv3d and Pool3d/1d
Posted by GitBox <gi...@apache.org>.
wuxun-zhang commented on issue #17884: [MKL-DNN] Integrate Conv3d and Pool3d/1d
URL: https://github.com/apache/incubator-mxnet/pull/17884#issuecomment-604172727
@mxnet-bot run ci [centos-cpu, unix-gpu, windows-gpu]
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] wuxun-zhang commented on issue #17884: [MKL-DNN]
Integrate Conv3d and Pool3d/1d
Posted by GitBox <gi...@apache.org>.
wuxun-zhang commented on issue #17884: [MKL-DNN] Integrate Conv3d and Pool3d/1d
URL: https://github.com/apache/incubator-mxnet/pull/17884#issuecomment-610747504
Now CI has finally passed. @pengzhao-intel @TaoLv please help review again.
@ChaiBapchya please also double check if this PR fixes https://github.com/apache/incubator-mxnet/issues/17915.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] wuxun-zhang commented on issue #17884: [MKL-DNN]
Integrate Conv3d and Pool3d/1d
Posted by GitBox <gi...@apache.org>.
wuxun-zhang commented on issue #17884: [MKL-DNN] Integrate Conv3d and Pool3d/1d
URL: https://github.com/apache/incubator-mxnet/pull/17884#issuecomment-610103164
Thank you @ChaiBapchya for investigating this. 8e96ef should be OneDNN v1.2.2. Actually we are working on confirming with the PR author of #17084 if such downgrade is intended or a mistake.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] wuxun-zhang commented on a change in pull request
#17884: [MKL-DNN] Integrate Conv3d and Pool3d/1d
Posted by GitBox <gi...@apache.org>.
wuxun-zhang commented on a change in pull request #17884: [MKL-DNN] Integrate Conv3d and Pool3d/1d
URL: https://github.com/apache/incubator-mxnet/pull/17884#discussion_r405850533
##########
File path: src/operator/nn/mkldnn/mkldnn_base-inl.h
##########
@@ -153,9 +153,8 @@ static inline bool SupportMKLDNN(int dtype, const mxnet::TShape &shape) {
// MKLDNN currently does not support 0-dim Tensor and 0-size Tensor
return false;
}
-
return (dtype == mshadow::kFloat32 || dtype == mshadow::kBfloat16) &&
- (ndim == 1 || ndim == 2 || ndim == 4);
Review comment:
No. This is not needed for this PR. We have already enabled mkldnn conv with 3D tensor previously.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] TaoLv commented on a change in pull request
#17884: [MKL-DNN] Integrate Conv3d and Pool3d/1d
Posted by GitBox <gi...@apache.org>.
TaoLv commented on a change in pull request #17884: [MKL-DNN] Integrate Conv3d and Pool3d/1d
URL: https://github.com/apache/incubator-mxnet/pull/17884#discussion_r396512804
##########
File path: src/operator/nn/mkldnn/mkldnn_base-inl.h
##########
@@ -153,9 +153,8 @@ static inline bool SupportMKLDNN(int dtype, const mxnet::TShape &shape) {
// MKLDNN currently does not support 0-dim Tensor and 0-size Tensor
return false;
}
-
return (dtype == mshadow::kFloat32 || dtype == mshadow::kBfloat16) &&
- (ndim == 1 || ndim == 2 || ndim == 4);
+ (ndim >= 1 && ndim <= 5);
Review comment:
Please fix the indent and make sure you also want to enable ndim=3 here.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] pengzhao-intel commented on issue #17884:
[MKL-DNN] Integrate Conv3d and Pool3d/1d
Posted by GitBox <gi...@apache.org>.
pengzhao-intel commented on issue #17884: [MKL-DNN] Integrate Conv3d and Pool3d/1d
URL: https://github.com/apache/incubator-mxnet/pull/17884#issuecomment-605735996
@wuxun-zhang please take a look for CI
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] pengzhao-intel merged pull request #17884:
[MKL-DNN] Integrate Conv3d and Pool3d/1d
Posted by GitBox <gi...@apache.org>.
pengzhao-intel merged pull request #17884: [MKL-DNN] Integrate Conv3d and Pool3d/1d
URL: https://github.com/apache/incubator-mxnet/pull/17884
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] wuxun-zhang commented on a change in pull request
#17884: [MKL-DNN] Integrate Conv3d and Pool3d/1d
Posted by GitBox <gi...@apache.org>.
wuxun-zhang commented on a change in pull request #17884: [MKL-DNN] Integrate Conv3d and Pool3d/1d
URL: https://github.com/apache/incubator-mxnet/pull/17884#discussion_r397892283
##########
File path: src/operator/nn/pooling.cc
##########
@@ -274,12 +274,12 @@ void PoolingComputeExCPU(const nnvm::NodeAttrs &attrs, const OpContext &ctx,
// Pooling does not currently support working with views
if (inputs[0].IsView() || outputs[0].IsView()) {
+ std::cout << "Fall back to Pooling backward pass..." << std::endl;
Review comment:
Sorry. Forgot to remove this line. Will fix.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] TaoLv commented on a change in pull request
#17884: [MKL-DNN] Integrate Conv3d and Pool3d/1d
Posted by GitBox <gi...@apache.org>.
TaoLv commented on a change in pull request #17884: [MKL-DNN] Integrate Conv3d and Pool3d/1d
URL: https://github.com/apache/incubator-mxnet/pull/17884#discussion_r396523088
##########
File path: src/operator/nn/mkldnn/mkldnn_pooling-inl.h
##########
@@ -114,15 +115,21 @@ inline bool SupportMKLDNNPooling(const PoolingParam ¶m,
return true;
} else {
if (param.pool_type == pool_enum::kAvgPooling) {
- CHECK_EQ(dshape.ndim(), 4);
+ CHECK(dshape.ndim() == 3 || dshape.ndim() == 4 || dshape.ndim() == 5);
// mkldnn works differently when padding is asymmetric, so let's skip this case.
- if (param.pad[0] == GetPaddingSizeFull(dshape[2], param.pad[0], param.pad[0], param.kernel[0],
- param.stride[0]) &&
- param.pad[1] == GetPaddingSizeFull(dshape[3], param.pad[1], param.pad[1], param.kernel[1],
- param.stride[1])) {
- return true;
+ bool is_symmetric = true;
+ switch (dshape.ndim()) {
+ case 5:
+ is_symmetric = is_symmetric && (param.pad[2] == GetPaddingSizeFull(dshape[4],
+ param.pad[2], param.pad[2], param.kernel[2], param.stride[2]));
+ case 4:
+ is_symmetric = is_symmetric && (param.pad[1] == GetPaddingSizeFull(dshape[3],
+ param.pad[1], param.pad[1], param.kernel[1], param.stride[1]));
Review comment:
I see both pad[0] and pad[1] are checked in previous code.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] wuxun-zhang commented on a change in pull request
#17884: [MKL-DNN] Integrate Conv3d and Pool3d/1d
Posted by GitBox <gi...@apache.org>.
wuxun-zhang commented on a change in pull request #17884: [MKL-DNN] Integrate Conv3d and Pool3d/1d
URL: https://github.com/apache/incubator-mxnet/pull/17884#discussion_r397620385
##########
File path: src/operator/nn/mkldnn/mkldnn_base-inl.h
##########
@@ -324,20 +323,27 @@ inline static mkldnn::memory::desc GetWeightDesc(const NDArray &arr,
if (num_groups == 1) {
return GetMemDesc(arr, dtype);
} else {
- auto ndim = arr.shape().ndim();
- CHECK((ndim == 3) || (ndim == 4))
- << "MKL-DNN weight currectly supports 3d and 4d layout";
+ const auto ndim = arr.shape().ndim();
+ CHECK((ndim == 3) || (ndim == 4) || (ndim == 5))
+ << "MKL-DNN weight currently supports 3d or 4d or 5d layout";
auto tz = mkldnn::memory::dims{0};
- const int N = 0, H = 2, W = 3, C = 1;
- if (ndim == 3) {
- tz = mkldnn::memory::dims{
- num_groups, static_cast<int>(arr.shape()[N] / num_groups),
- static_cast<int>(arr.shape()[C]), static_cast<int>(arr.shape()[H])};
- } else {
- tz = mkldnn::memory::dims{
- num_groups, static_cast<int>(arr.shape()[N] / num_groups),
- static_cast<int>(arr.shape()[C]), static_cast<int>(arr.shape()[H]),
- static_cast<int>(arr.shape()[W])};
+ const int D = (ndim == 5) ? 2 : 1;
+ const int N = 0, C = 1, H = D + 1, W = D + 2;
Review comment:
Thanks. Done.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] ChaiBapchya commented on a change in pull request
#17884: [MKL-DNN] Integrate Conv3d and Pool3d/1d
Posted by GitBox <gi...@apache.org>.
ChaiBapchya commented on a change in pull request #17884: [MKL-DNN] Integrate Conv3d and Pool3d/1d
URL: https://github.com/apache/incubator-mxnet/pull/17884#discussion_r405865801
##########
File path: src/operator/nn/mkldnn/mkldnn_base-inl.h
##########
@@ -153,9 +153,8 @@ static inline bool SupportMKLDNN(int dtype, const mxnet::TShape &shape) {
// MKLDNN currently does not support 0-dim Tensor and 0-size Tensor
return false;
}
-
return (dtype == mshadow::kFloat32 || dtype == mshadow::kBfloat16) &&
- (ndim == 1 || ndim == 2 || ndim == 4);
Review comment:
can you point me to where it's handled? I didn't understand the separate treatment of 3D
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] ChaiBapchya commented on issue #17884: [MKL-DNN]
Integrate Conv3d and Pool3d/1d
Posted by GitBox <gi...@apache.org>.
ChaiBapchya commented on issue #17884: [MKL-DNN] Integrate Conv3d and Pool3d/1d
URL: https://github.com/apache/incubator-mxnet/pull/17884#issuecomment-609983862
PR https://github.com/apache/incubator-mxnet/pull/17084 updated 3rdparty/mkldnn from 8e96ef to cb2cc7 [1.1.0 to 1.1.2]
@wuxun-zhang Can you confirm when was it v1.2.2? and what's the path forward?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] wuxun-zhang commented on issue #17884: [MKL-DNN]
Integrate Conv3d and Pool3d/1d
Posted by GitBox <gi...@apache.org>.
wuxun-zhang commented on issue #17884: [MKL-DNN] Integrate Conv3d and Pool3d/1d
URL: https://github.com/apache/incubator-mxnet/pull/17884#issuecomment-605753245
Seems CI system is now experiencing failures. The errors are unrelated to this PR.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] ChaiBapchya commented on issue #17884: [MKL-DNN] Integrate Conv3d and Pool3d/1d
Posted by GitBox <gi...@apache.org>.
ChaiBapchya commented on issue #17884:
URL: https://github.com/apache/incubator-mxnet/pull/17884#issuecomment-616771837
@pengzhao-intel @wuxun-zhang Gentle ping. Can we get the perf numbers after this PR changes?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] bartekkuncer commented on issue #17884: [MKL-DNN] Integrate Conv3d and Pool3d/1d
Posted by GitBox <gi...@apache.org>.
bartekkuncer commented on issue #17884:
URL: https://github.com/apache/incubator-mxnet/pull/17884#issuecomment-617739799
**Some more performance numbers for Conv3d:**
| Shape | Kernel shape | w/o mkldnn | w/mkldnn |
| ------------- | ------------- | ------- | --- |
| (3, 3, 12, 128, 64) | (2, 2, 2) | 0.56526 sec | 0.00332 sec |
| (3, 3, 12, 128, 128) | (2, 2, 2) | 1.06378 sec | 0.00453 sec |
| (3, 3, 128, 128, 128) | (2, 2, 2) | 10.53919 sec | 0.04117 sec |
| (5, 5, 12, 128, 128) | (2, 2, 2) | 2.76165 sec | 0.00688 sec |
| (5, 5, 128, 128, 128) | (2, 2, 2) | 29.06880 sec | 0.07062 sec |
| (3, 3, 12, 128, 64) | (3, 3, 3) | 1.48476 sec | 0.00172 sec |
| (3, 3, 12, 128, 128) | (3, 3, 3) | 2.87235 sec | 0.00424 sec |
| (3, 3, 128, 128, 128) | (3, 3, 3) | 34.26913 sec | 0.04124 sec |
| (5, 5, 12, 128, 128) | (3, 3, 3) | 7.77486 sec | 0.00697 sec |
| (5, 5, 128, 128, 128) | (3, 3, 3) | 141.04758 sec | 0.07259 sec |
**Test I used:**
```
import mxnet as mx
import time
def perf_conv3d():
shapes = [(3, 3, 12, 128, 64), (3, 3, 12, 128, 128), (3, 3, 128, 128, 128), (5, 5, 16, 128, 128), (5, 5, 128, 128, 128)]
kernel_shapes = [(2, 2, 2), (3, 3, 3)]
num_filter = 32
warmup = 10
runs = 40
for shape in shapes:
for kernel_shape in kernel_shapes:
a = mx.random.uniform(shape=shape)
w = mx.random.uniform(shape=(num_filter, shape[1], kernel_shape[0], kernel_shape[1], kernel_shape[2]))
tic = 0
for i in range(runs + warmup):
if i == warmup:
tic = time.time()
_ = mx.nd.Convolution(data=a, weight=w, kernel=kernel_shape, num_filter=num_filter, stride=(1,1,1), pad=(0,0,0), no_bias=True, cudnn_off=True)
mx.nd.waitall()
toc = time.time()
print('conv3d benchmark, shape={}, kernel_shape={} time={} ms/iter'.format(shape, kernel_shape, (toc-tic)*1000/(runs-warmup)))
if __name__ == '__main__':
perf_conv3d()
```
Sorry for the delay.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] wuxun-zhang commented on issue #17884: [MKL-DNN] Integrate Conv3d and Pool3d/1d
Posted by GitBox <gi...@apache.org>.
wuxun-zhang commented on issue #17884:
URL: https://github.com/apache/incubator-mxnet/pull/17884#issuecomment-616912500
**Performance numbers for Conv3d op:**
shape | w/o mkldnn | w/mkldnn |
-- | -- | -- |
(3, 3, 16, 224, 224) | 3.696679 sec | 0.046571 sec|
(3, 3, 128, 128, 128) | 11.716535 sec | 0.165749 sec|
**Test script:**
```
import mxnet as mx
from mxnet import nd, gluon
import time
data_shape = [(3, 3, 16, 224, 224), (3, 3, 128, 128, 128)]
for shape in data_shape:
data = mx.random.uniform(shape=shape)
weight_shape = (32, shape[1], 3, 3, 3)
weight = mx.nd.ones(shape=weight_shape)
num_iter = 10
dry_run = 5
for i in range(num_iter):
if i == dry_run:
tic = time.time()
out = mx.nd.Convolution(data, weight, kernel=(3,3,3), stride=(1,1,1), num_filter=weight_shape[0], pad=(0,0,0), dilate=(2,2,2), no_bias=True, cudnn_off=True, name='conv3d')
out.asnumpy()
print("For shape : {}".format(shape))
print("Average time cost is %f sec" % ((time.time() - tic)/(num_iter-dry_run))
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org