Enable slice embedding concat split fuse #14491
Conversation
|
Thank you for your contribution @JustForFun099 ! |
|
@apeforest, testcase are added now, please help review again, ths. |
|
@apeforest Ping for review... |
tests/python/mkl/test_subgraph.py
Outdated
| aux_array = [mx.nd.random.uniform(shape=shape) for shape in aux_shapes] | ||
| exe = sym.bind(ctx=mx.current_context(), args=arg_array, aux_states=aux_array, grad_req='null') | ||
| exe.forward() | ||
| os.environ['MXNET_SUBGRAPH_BACKEND'] = 'MKLDNN_WIDE_AND_DEEP_INPUT_FUSE' |
There was a problem hiding this comment.
is this environment variable newly introduced? where is the documentation?
There was a problem hiding this comment.
I didn' see anything about MKLDNN primitive and API in the code.
I suggest removing the keyword of MKLDNN to make the thing clear.
There was a problem hiding this comment.
@pengzhao-intel This PR is related to MKLDNN. Please help review. Thanks!
|
@JustForFun099 Can you provide more context in the PR description: (1) why do we need this? (2) who is the customer? (3) any workaround today without this (4) who will maintain this. Thanks! |
The PR belongs to operator improvement rather than MKLDNN implementation. |
|
Thanks for @apeforest 's review. below are answers to your questions:
|
|
Please retrigger the CI |
|
@apeforest can you review this PR again. Thanks |
| using namespace mshadow; | ||
| CHECK_EQ(in_shape->size(), 1U); | ||
| mxnet::TShape dshape = in_shape->at(slice_enum::kData); | ||
| mxnet::TShape ishape = in_shape->at(slice_enum::kData); |
There was a problem hiding this comment.
does in_shape->at have to accessed twice? can we do it once and reuse?
| // The Assumption is only base on W&D which all | ||
| // embedding occur at the beginning and output to 1 concat node | ||
| if (op_name == EMBEDDING_NODE_NAME) { | ||
| emb_nodes.push_back(node); |
There was a problem hiding this comment.
can push_back be replaced by emplace_back wherever applicable?
|
@JustForFun099 Could you address the review comments? Thanks! |
|
@JustForFun099 Ping for an update! Thanks. |
|
@JustForFun099 Gentle ping... |
|
@mxnet-label-bot update [pr-awaiting-response] |
marcoabreu
added
pr-awaiting-response
|
@JustForFun099 Thank you for you contribution. Could you please provide an update on the PR? There is no activity since 90+ days and we can close it if you don't have a bandwidth to work right now. |
|
@JustForFun099 Can you give an update on this PR ? |
|
@JustForFun099 gentle ping to rebase and update, thanks! |
Description
As title, fuse Slice Split Embedding Concat for Wide & Deep Model
Checklist
Essentials
Please feel free to remove inapplicable items for your PR.
Changes
Comments