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/04/06 08:11:01 UTC
[GitHub] [incubator-mxnet] PawelGlomski-Intel opened a new pull request #20129: [BUGFIX][BACKPORT] Impose a plain format on padded concat output
PawelGlomski-Intel opened a new pull request #20129:
URL: https://github.com/apache/incubator-mxnet/pull/20129
## Description ##
Backport of #19735
> Right now, oneDNN may choose a blocked format for the output of concat, in cases that may require additional padding (thus more memory). In (de)convolution we make sure to only select a primitive_desc which has the expected (from tensor size) memory size requirements, so the same has to be done in this case.
>
> Unlike convolution's, concat's primitive_desc does not support iteration over multiple implementations, so here I manually impose a plain format when oneDNN would choose a blocked one with padding.
>
> Resolves #19586
--
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 #20129: [BUGFIX][BACKPORT] Impose a plain format on padded concat output
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20129:
URL: https://github.com/apache/incubator-mxnet/pull/20129#issuecomment-813922463
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**: [unix-cpu, windows-gpu, windows-cpu, website, sanity, centos-cpu, miscellaneous, centos-gpu, clang, unix-gpu, edge]
***
_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 #20129: [BUGFIX][BACKPORT] Impose a plain format on padded concat output
Posted by GitBox <gi...@apache.org>.
PawelGlomski-Intel commented on pull request #20129:
URL: https://github.com/apache/incubator-mxnet/pull/20129#issuecomment-821006825
@mxnet-bot run ci [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 #20129: [BUGFIX][BACKPORT] Impose a plain format on padded concat output
Posted by GitBox <gi...@apache.org>.
PawelGlomski-Intel commented on pull request #20129:
URL: https://github.com/apache/incubator-mxnet/pull/20129#issuecomment-816490630
@mxnet-bot run ci [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 a change in pull request #20129: [BUGFIX][BACKPORT] Impose a plain format on padded concat output
Posted by GitBox <gi...@apache.org>.
PawelGlomski-Intel commented on a change in pull request #20129:
URL: https://github.com/apache/incubator-mxnet/pull/20129#discussion_r614140568
##########
File path: tests/python/mkl/test_mkldnn.py
##########
@@ -618,6 +618,35 @@ def check_concat_training(stype):
for stype in stypes:
check_concat_training(stype)
+def test_concat_blocked():
+ ctx = mx.cpu()
+ axis = 1
+ filters = 32 # must be a multiple of 16
+ kernel = (3, 3)
+ for in_dim_size in range(1, 17): # check cases with and without padding
+ in_shape = (1, in_dim_size, 64, 64)
+ in_data = mx.nd.random.uniform(-1, 1, in_shape, ctx=ctx)
+ conv_weights = mx.nd.random.uniform(-1, 1, (filters, in_shape[1], kernel[0], kernel[1]), ctx=ctx)
+
+ def calc_output_of_layer(layer):
+ ex = layer._simple_bind(ctx, x=in_shape)
+ in_data.copyto(ex.arg_arrays[0])
+ conv_weights.copyto(ex.arg_arrays[1])
+ return ex.forward()[0].asnumpy()
+
+ x = mx.sym.Variable('x')
+ w = mx.sym.Variable('w')
+ # convolution, so a blocked format is selected
+ conv = mx.sym.Convolution(data=x, weight=w, num_filter=filters, kernel=kernel, pad=(1, 1), no_bias=True)
Review comment:
AFAIK numpy concatenate does not use oneDNN. I believe this implementation is used: https://github.com/apache/incubator-mxnet/blob/master/src/operator/numpy/np_matrix_op.cc#L681.
Should we add oneDNN support?
--
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 a change in pull request #20129: [BUGFIX][BACKPORT] Impose a plain format on padded concat output
Posted by GitBox <gi...@apache.org>.
szha commented on a change in pull request #20129:
URL: https://github.com/apache/incubator-mxnet/pull/20129#discussion_r611247865
##########
File path: tests/python/mkl/test_mkldnn.py
##########
@@ -618,6 +618,35 @@ def check_concat_training(stype):
for stype in stypes:
check_concat_training(stype)
+def test_concat_blocked():
+ ctx = mx.cpu()
+ axis = 1
+ filters = 32 # must be a multiple of 16
+ kernel = (3, 3)
+ for in_dim_size in range(1, 17): # check cases with and without padding
+ in_shape = (1, in_dim_size, 64, 64)
+ in_data = mx.nd.random.uniform(-1, 1, in_shape, ctx=ctx)
+ conv_weights = mx.nd.random.uniform(-1, 1, (filters, in_shape[1], kernel[0], kernel[1]), ctx=ctx)
+
+ def calc_output_of_layer(layer):
+ ex = layer._simple_bind(ctx, x=in_shape)
+ in_data.copyto(ex.arg_arrays[0])
+ conv_weights.copyto(ex.arg_arrays[1])
+ return ex.forward()[0].asnumpy()
+
+ x = mx.sym.Variable('x')
+ w = mx.sym.Variable('w')
+ # convolution, so a blocked format is selected
+ conv = mx.sym.Convolution(data=x, weight=w, num_filter=filters, kernel=kernel, pad=(1, 1), no_bias=True)
Review comment:
could you add a separate test for npx interface?
--
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 a change in pull request #20129: [BUGFIX][BACKPORT] Impose a plain format on padded concat output
Posted by GitBox <gi...@apache.org>.
szha commented on a change in pull request #20129:
URL: https://github.com/apache/incubator-mxnet/pull/20129#discussion_r614210811
##########
File path: tests/python/mkl/test_mkldnn.py
##########
@@ -618,6 +618,35 @@ def check_concat_training(stype):
for stype in stypes:
check_concat_training(stype)
+def test_concat_blocked():
+ ctx = mx.cpu()
+ axis = 1
+ filters = 32 # must be a multiple of 16
+ kernel = (3, 3)
+ for in_dim_size in range(1, 17): # check cases with and without padding
+ in_shape = (1, in_dim_size, 64, 64)
+ in_data = mx.nd.random.uniform(-1, 1, in_shape, ctx=ctx)
+ conv_weights = mx.nd.random.uniform(-1, 1, (filters, in_shape[1], kernel[0], kernel[1]), ctx=ctx)
+
+ def calc_output_of_layer(layer):
+ ex = layer._simple_bind(ctx, x=in_shape)
+ in_data.copyto(ex.arg_arrays[0])
+ conv_weights.copyto(ex.arg_arrays[1])
+ return ex.forward()[0].asnumpy()
+
+ x = mx.sym.Variable('x')
+ w = mx.sym.Variable('w')
+ # convolution, so a blocked format is selected
+ conv = mx.sym.Convolution(data=x, weight=w, num_filter=filters, kernel=kernel, pad=(1, 1), no_bias=True)
Review comment:
Since there shouldn't be functionality difference, I'd recommend making that decision based on expected performance.
--
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 #20129: [BUGFIX][BACKPORT] Impose a plain format on padded concat output
Posted by GitBox <gi...@apache.org>.
PawelGlomski-Intel commented on pull request #20129:
URL: https://github.com/apache/incubator-mxnet/pull/20129#issuecomment-829944145
@szha I believe this can be merged (oneDNN support for npx operators won't be included in 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
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #20129: [BUGFIX][BACKPORT] Impose a plain format on padded concat output
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20129:
URL: https://github.com/apache/incubator-mxnet/pull/20129#issuecomment-816490693
Jenkins CI successfully triggered : [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 #20129: [BUGFIX][BACKPORT] Impose a plain format on padded concat output
Posted by GitBox <gi...@apache.org>.
PawelGlomski-Intel commented on pull request #20129:
URL: https://github.com/apache/incubator-mxnet/pull/20129#issuecomment-829118950
@mxnet-bot run ci [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
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #20129: [BUGFIX][BACKPORT] Impose a plain format on padded concat output
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20129:
URL: https://github.com/apache/incubator-mxnet/pull/20129#issuecomment-829118997
Jenkins CI successfully triggered : [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
[GitHub] [incubator-mxnet] szha merged pull request #20129: [BUGFIX][BACKPORT] Impose a plain format on padded concat output
Posted by GitBox <gi...@apache.org>.
szha merged pull request #20129:
URL: https://github.com/apache/incubator-mxnet/pull/20129
--
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 #20129: [BUGFIX][BACKPORT] Impose a plain format on padded concat output
Posted by GitBox <gi...@apache.org>.
PawelGlomski-Intel edited a comment on pull request #20129:
URL: https://github.com/apache/incubator-mxnet/pull/20129#issuecomment-829944145
@szha I believe this can be merged (adding oneDNN support for npx operators won't be included in 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
[GitHub] [incubator-mxnet] szha commented on pull request #20129: [BUGFIX][BACKPORT] Impose a plain format on padded concat output
Posted by GitBox <gi...@apache.org>.
szha commented on pull request #20129:
URL: https://github.com/apache/incubator-mxnet/pull/20129#issuecomment-830133798
@PawelGlomski-Intel 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
[GitHub] [incubator-mxnet] PawelGlomski-Intel commented on pull request #20129: [BUGFIX][BACKPORT] Impose a plain format on padded concat output
Posted by GitBox <gi...@apache.org>.
PawelGlomski-Intel commented on pull request #20129:
URL: https://github.com/apache/incubator-mxnet/pull/20129#issuecomment-814991505
@mxnet-bot run ci [all]
--
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 #20129: [BUGFIX][BACKPORT] Impose a plain format on padded concat output
Posted by GitBox <gi...@apache.org>.
PawelGlomski-Intel commented on pull request #20129:
URL: https://github.com/apache/incubator-mxnet/pull/20129#issuecomment-822386446
@mxnet-bot run ci [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