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 2021/05/21 07:40:12 UTC
[GitHub] [incubator-mxnet] PawelGlomski-Intel opened a new pull request #20292: [BACKPORT][BUGFIX][FEATURE] Add oneDNN 1D and 3D deconvolution support and fix bias
PawelGlomski-Intel opened a new pull request #20292:
URL: https://github.com/apache/incubator-mxnet/pull/20292
## Description ##
Backport of #20107 and #20137
> Currently, deconvolution 2D is implemented with oneDNN convolution primitives. Deconvolution forward pass is implemented with convolution backward primitives that do not include bias. This requires you to manually add the bias, which is currently broken (#19768).
>
> This change implements oneDNN deconvolution primitives to deconvolution, fixing (#19768).
> As I couldn't reproduce (#12579) for the 2D case, I enabled the 2D part of this test.
> Adds Deconvolution 1D and 3D support with MKLDNN #19904
## Checklist ##
### Essentials ###
- [ ] PR's title starts with a category (e.g. [BUGFIX], [MODEL], [TUTORIAL], [FEATURE], [DOC], etc)
- [ ] Changes are complete (i.e. I finished coding on this PR)
- [ ] All changes have test coverage
- [ ] Code is well-documented
### Changes ###
- [ ] Implemented oneDNN deconvolution primitives to deconvolution (resolves #19768)
- [ ] Added Deconvolution 3D and 1D support with oneDNN (resolves #19904)
- [ ] Fixed flaky deconvolution test (resolves #12579)
--
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] mxnet-bot commented on pull request #20292: [BACKPORT][BUGFIX][FEATURE] Add oneDNN 1D and 3D deconvolution support and fix bias
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20292:
URL: https://github.com/apache/incubator-mxnet/pull/20292#issuecomment-914195758
Jenkins CI successfully triggered : [windows-gpu, unix-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.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] szha commented on pull request #20292: [BACKPORT][BUGFIX][FEATURE] Add oneDNN 1D and 3D deconvolution support and fix bias
Posted by GitBox <gi...@apache.org>.
szha commented on pull request #20292:
URL: https://github.com/apache/incubator-mxnet/pull/20292#issuecomment-930297689
@PawelGlomski-Intel thank you!
--
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.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #20292: [BACKPORT][BUGFIX][FEATURE] Add oneDNN 1D and 3D deconvolution support and fix bias
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20292:
URL: https://github.com/apache/incubator-mxnet/pull/20292#issuecomment-847650939
Jenkins CI successfully triggered : [unix-cpu, centos-gpu, unix-gpu, windows-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
[GitHub] [incubator-mxnet] szha merged pull request #20292: [BACKPORT][BUGFIX][FEATURE] Add oneDNN 1D and 3D deconvolution support and fix bias
Posted by GitBox <gi...@apache.org>.
szha merged pull request #20292:
URL: https://github.com/apache/incubator-mxnet/pull/20292
--
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.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] PawelGlomski-Intel commented on pull request #20292: [BACKPORT][BUGFIX][FEATURE] Add oneDNN 1D and 3D deconvolution support and fix bias
Posted by GitBox <gi...@apache.org>.
PawelGlomski-Intel commented on pull request #20292:
URL: https://github.com/apache/incubator-mxnet/pull/20292#issuecomment-923009647
@szha The changes I made to the native convolution introduce a slight (~8% on my machine) drop in performance in its backward pass. Is the performance of native implementation crucial (backward pass in particular)?
Native 2D deconvolution seems to be slower after my changes too, but as it wasn't working properly (#11203), I am not sure if I can compare them 1:1.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] PawelGlomski-Intel commented on pull request #20292: [BACKPORT][BUGFIX][FEATURE] Add oneDNN 1D and 3D deconvolution support and fix bias
Posted by GitBox <gi...@apache.org>.
PawelGlomski-Intel commented on pull request #20292:
URL: https://github.com/apache/incubator-mxnet/pull/20292#issuecomment-925699789
@szha Can you please review (excluding mkldnn files)?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] PawelGlomski-Intel commented on pull request #20292: [BACKPORT][BUGFIX][FEATURE] Add oneDNN 1D and 3D deconvolution support and fix bias
Posted by GitBox <gi...@apache.org>.
PawelGlomski-Intel commented on pull request #20292:
URL: https://github.com/apache/incubator-mxnet/pull/20292#issuecomment-852979638
> thanks for the forward-port. Could you help make sure that the functionality is tested from the mx.npx module instead of from the legacy Symbol API?
Since 3D deconvolution is not natively supported, such tests should be only added to test_mkldnn.py? Is there a designated file for npx interface tests?
--
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] szha commented on pull request #20292: [BACKPORT][BUGFIX][FEATURE] Add oneDNN 1D and 3D deconvolution support and fix bias
Posted by GitBox <gi...@apache.org>.
szha commented on pull request #20292:
URL: https://github.com/apache/incubator-mxnet/pull/20292#issuecomment-930297689
@PawelGlomski-Intel thank you!
--
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.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #20292: [BACKPORT][BUGFIX][FEATURE] Add oneDNN 1D and 3D deconvolution support and fix bias
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20292:
URL: https://github.com/apache/incubator-mxnet/pull/20292#issuecomment-847808339
Jenkins CI successfully triggered : [unix-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
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #20292: [BACKPORT][BUGFIX][FEATURE] Add oneDNN 1D and 3D deconvolution support and fix bias
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20292:
URL: https://github.com/apache/incubator-mxnet/pull/20292#issuecomment-925582093
Jenkins CI successfully triggered : [website, miscellaneous]
--
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.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] PawelGlomski-Intel edited a comment on pull request #20292: [BACKPORT][BUGFIX][FEATURE] Add oneDNN 1D and 3D deconvolution support and fix bias
Posted by GitBox <gi...@apache.org>.
PawelGlomski-Intel edited a comment on pull request #20292:
URL: https://github.com/apache/incubator-mxnet/pull/20292#issuecomment-923009647
@szha The changes I made to the native convolution introduce a slight (~8% on my machine) drop in performance in its backward pass. Is the performance of native implementation crucial (backward pass in particular)?
Native 2D deconvolution seems to be slower after my changes too, but as it wasn't working properly (#11203), I am not sure if I can compare them 1:1.
Also, are you aware why #20101 introduced [this](https://github.com/barry-jin/incubator-mxnet/blob/4006219d83d151a48707b2ad292d3579e52c1561/python/mxnet/ndarray/numpy_extension/_op.py#L616) assert?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] szha commented on pull request #20292: [BACKPORT][BUGFIX][FEATURE] Add oneDNN 1D and 3D deconvolution support and fix bias
Posted by GitBox <gi...@apache.org>.
szha commented on pull request #20292:
URL: https://github.com/apache/incubator-mxnet/pull/20292#issuecomment-853327229
`tests/python/unittest/test_numpy_op.py` would be the appropriate file
--
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] PawelGlomski-Intel commented on pull request #20292: [BACKPORT][BUGFIX][FEATURE] Add oneDNN 1D and 3D deconvolution support and fix bias
Posted by GitBox <gi...@apache.org>.
PawelGlomski-Intel commented on pull request #20292:
URL: https://github.com/apache/incubator-mxnet/pull/20292#issuecomment-851308642
@szha can you preview review?
--
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] PawelGlomski-Intel commented on pull request #20292: [BACKPORT][BUGFIX][FEATURE] Add oneDNN 1D and 3D deconvolution support and fix bias
Posted by GitBox <gi...@apache.org>.
PawelGlomski-Intel commented on pull request #20292:
URL: https://github.com/apache/incubator-mxnet/pull/20292#issuecomment-847808272
@mxnet-bot run ci [unix-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
[GitHub] [incubator-mxnet] PawelGlomski-Intel edited a comment on pull request #20292: [BACKPORT][BUGFIX][FEATURE] Add oneDNN 1D and 3D deconvolution support and fix bias
Posted by GitBox <gi...@apache.org>.
PawelGlomski-Intel edited a comment on pull request #20292:
URL: https://github.com/apache/incubator-mxnet/pull/20292#issuecomment-852979638
> thanks for the forward-port. Could you help make sure that the functionality is tested from the mx.npx module instead of from the legacy Symbol API?
Since 3D deconvolution is not natively supported, such tests should be only added to test_mkldnn.py? Is there a designated file for npx interface tests?
@szha
--
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] szha commented on pull request #20292: [BACKPORT][BUGFIX][FEATURE] Add oneDNN 1D and 3D deconvolution support and fix bias
Posted by GitBox <gi...@apache.org>.
szha commented on pull request #20292:
URL: https://github.com/apache/incubator-mxnet/pull/20292#issuecomment-923481901
@PawelGlomski-Intel it's ok if the mkldnn version has good performance since we turn on mkldnn by default.
@barry-jin can comment on the assert
--
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.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] PawelGlomski-Intel commented on pull request #20292: [BACKPORT][BUGFIX][FEATURE] Add oneDNN 1D and 3D deconvolution support and fix bias
Posted by GitBox <gi...@apache.org>.
PawelGlomski-Intel commented on pull request #20292:
URL: https://github.com/apache/incubator-mxnet/pull/20292#issuecomment-847650847
@mxnet-bot run ci [centos-gpu, unix-cpu, unix-gpu, windows-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
[GitHub] [incubator-mxnet] PawelGlomski-Intel commented on pull request #20292: [BACKPORT][BUGFIX][FEATURE] Add oneDNN 1D and 3D deconvolution support and fix bias
Posted by GitBox <gi...@apache.org>.
PawelGlomski-Intel commented on pull request #20292:
URL: https://github.com/apache/incubator-mxnet/pull/20292#issuecomment-913424642
@szha is 3D deconvolution enabled for the GPU? I thought it was, but as I see now, there are no tests for it ([here](https://github.com/apache/incubator-mxnet/blob/4b384027de0f324dd0cb2ce2d64e85fcdfa84777/tests/python/gpu/test_operator_gpu.py#L865-L877) it is even stated that it's not enabled). If my changes are not the cause of these errors, should I move these tests (their 3D part) to some other file or do I somehow check at runtime if GPU is enabled and skip them if it is?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] szha merged pull request #20292: [BACKPORT][BUGFIX][FEATURE] Add oneDNN 1D and 3D deconvolution support and fix bias
Posted by GitBox <gi...@apache.org>.
szha merged pull request #20292:
URL: https://github.com/apache/incubator-mxnet/pull/20292
--
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.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] PawelGlomski-Intel commented on pull request #20292: [BACKPORT][BUGFIX][FEATURE] Add oneDNN 1D and 3D deconvolution support and fix bias
Posted by GitBox <gi...@apache.org>.
PawelGlomski-Intel commented on pull request #20292:
URL: https://github.com/apache/incubator-mxnet/pull/20292#issuecomment-916039806
@szha Can you check if tests are ok?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] PawelGlomski-Intel commented on pull request #20292: [BACKPORT][BUGFIX][FEATURE] Add oneDNN 1D and 3D deconvolution support and fix bias
Posted by GitBox <gi...@apache.org>.
PawelGlomski-Intel commented on pull request #20292:
URL: https://github.com/apache/incubator-mxnet/pull/20292#issuecomment-914195641
@mxnet-bot run ci [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.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] barry-jin commented on pull request #20292: [BACKPORT][BUGFIX][FEATURE] Add oneDNN 1D and 3D deconvolution support and fix bias
Posted by GitBox <gi...@apache.org>.
barry-jin commented on pull request #20292:
URL: https://github.com/apache/incubator-mxnet/pull/20292#issuecomment-923485061
@PawelGlomski-Intel Thanks for pointing it out. I will provide a quick fix on the assert. The intention of introducing assert here is that move the [lower bound check](https://github.com/apache/incubator-mxnet/blob/179e7db21693162eda5b254ef4ae2c36e577b97e/src/operator/nn/convolution-inl.h#L83) to frontend.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] PawelGlomski-Intel commented on pull request #20292: [BACKPORT][BUGFIX][FEATURE] Add oneDNN 1D and 3D deconvolution support and fix bias
Posted by GitBox <gi...@apache.org>.
PawelGlomski-Intel commented on pull request #20292:
URL: https://github.com/apache/incubator-mxnet/pull/20292#issuecomment-919749310
With the native implementation of deconvolution changed, I believe this PR will also fix #11203 (I will add a test for it) and replace #14031.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] szha commented on pull request #20292: [BACKPORT][BUGFIX][FEATURE] Add oneDNN 1D and 3D deconvolution support and fix bias
Posted by GitBox <gi...@apache.org>.
szha commented on pull request #20292:
URL: https://github.com/apache/incubator-mxnet/pull/20292#issuecomment-913848618
@PawelGlomski-Intel let's skip it for now. You can use `pytest.skip` inside the test to do it. In GPU tests, `mx.context.current_context()` will be set to gpu so that you can differentiate between cpu and gpu test runs.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #20292: [BACKPORT][BUGFIX][FEATURE] Add oneDNN 1D and 3D deconvolution support and fix bias
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20292:
URL: https://github.com/apache/incubator-mxnet/pull/20292#issuecomment-845733084
Hey @PawelGlomski-Intel , Thanks for submitting the PR
All tests are already queued to run once. If tests fail, you can trigger one or more tests again with the following commands:
- To trigger all jobs: @mxnet-bot run ci [all]
- To trigger specific jobs: @mxnet-bot run ci [job1, job2]
***
**CI supported jobs**: [miscellaneous, edge, windows-cpu, unix-cpu, unix-gpu, website, windows-gpu, centos-cpu, clang, sanity, centos-gpu]
***
_Note_:
Only following 3 categories can trigger CI :PR Author, MXNet Committer, Jenkins Admin.
All CI tests must pass before the PR can be merged.
--
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] PawelGlomski-Intel commented on pull request #20292: [BACKPORT][BUGFIX][FEATURE] Add oneDNN 1D and 3D deconvolution support and fix bias
Posted by GitBox <gi...@apache.org>.
PawelGlomski-Intel commented on pull request #20292:
URL: https://github.com/apache/incubator-mxnet/pull/20292#issuecomment-849634353
@mxnet-bot run ci [unix-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
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #20292: [BACKPORT][BUGFIX][FEATURE] Add oneDNN 1D and 3D deconvolution support and fix bias
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20292:
URL: https://github.com/apache/incubator-mxnet/pull/20292#issuecomment-849634411
Jenkins CI successfully triggered : [unix-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
[GitHub] [incubator-mxnet] PawelGlomski-Intel commented on pull request #20292: [BACKPORT][BUGFIX][FEATURE] Add oneDNN 1D and 3D deconvolution support and fix bias
Posted by GitBox <gi...@apache.org>.
PawelGlomski-Intel commented on pull request #20292:
URL: https://github.com/apache/incubator-mxnet/pull/20292#issuecomment-925581930
@mxnet-bot run ci [miscellaneous, website]
--
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.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org