This repository has been archived by the owner on Nov 17, 2023. It is now read-only.
[MXNET-486] Create CPP test for concat MKLDNN operator #11371
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
LGTM :) Thanks for the test and fix. |
zheng-da
reviewed
Jun 28, 2018
tests/cpp/operator/mkldnn.cc
Outdated
| @@ -804,6 +1014,24 @@ TEST(IMPERATIVE, SumBackwardsOp) { | |||
| TestOp(attrs, VerifySumBackwardsResult); | |||
| } | |||
|
|
|||
| TEST(IMPERATIVE, ConcatOp) { | |||
| for (int num_inputs = 2; num_inputs < 3; num_inputs++) { | |||
There was a problem hiding this comment.
why do you only test num_inputs=2 and write the code like this?
| for (auto &dispatch : dispatches) { | ||
| std::vector<std::vector<NDArrayAttrs>> out_arrs(attrs.num_outputs); | ||
| for (int i = 0; i < attrs.num_outputs; i++) | ||
| out_arrs[i] = GetTestOutputArrays(in_arr.arr.shape(), pds); |
There was a problem hiding this comment.
it doesn't seem out_arrs generated here is used.
zheng-da
reviewed
Jun 28, 2018
tests/cpp/operator/mkldnn.cc
Outdated
| } | ||
|
|
||
| TEST(IMPERATIVE, ConcatBackwardsOp) { | ||
| for (int num_inputs = 2; num_inputs < 3; num_inputs++) { |
There was a problem hiding this comment.
the same comment for here.
zheng-da
approved these changes
Jun 29, 2018
XinYao1994
pushed a commit
to XinYao1994/incubator-mxnet
that referenced
this pull request
Aug 29, 2018
* create func to generate new outputs for concat output * provide type for vectors * fix concat dict * change value to string * use different datatype * create GetExpandedMemPD helper * update input num * fix dim range * use only new shape * add comments * replace all with new_shape * consolidate testop * remove init interface * use GetTestOutputArraysConcat for concat op * out arrays in correct scope * add VerifyConcatResult * noop for kWriteInPlace for concat * refactor GetTestOutputArrays and GetTestOutputArraysConcat into one method * create temp ndarrays in same scope as assert * add message for GetTestOutputArraysConcat * filter when dim too large * fix print message * reshape concat output so it can be read * check concat on diff dim * add VerifyConcatBackwardsResult bp * reshape if view and mkldnn * finish VerifyConcatBackwardsResult * reverse input output for concat backwards * do not rand output for concat backwards * make mulitple copies of inputs for ops that need mult unique inputs * swap input/output msg * create test inputs can create expanded inputs * add verify msg to test * fix slice of input * remove unused test * missing assignment * fix slice amount for diff dim concat * shrink outputs for concat backwards * revert switching input/output for concat/backwards * create multiple copies of output * reorder concat input grad * increase num of input for concat backwards * concat dst is smaller array * use output vs input mem to determine shape and as tmp storage * do not support mkldnn concat if mkl layout diff from nd layout * reorder if view /mkldnn * exclude views from concat * remove unused header * remove check for view in mkldnn_concat * remove unused heaeder * skip test * rename target_shape to shape * remove rand var and default outputs to rand * rename target_pd to pd * fix lint issues * add space to error msg * do not use mkldnn for forward concat if layout mismatch * create temp shape var * do not check if view in concat * convert dim to unsigned int * fix lint * check view first * check type before creating mem * check all inputs for concat mkldnn * remove getshapestring * add comments for verify concat helpres * revert adding USE_MKLDNN flag * use reference for arrays in concat mkldnn check * fix indent * set default num_inputs to 1 * revert change to test_ctc_loss_train * add error message to check * use reference of arr in loops * remove extra space * use default num_inputs * use reference for all loops * fix lint * use separate concat test * remove reference from pd * do not use reference for shape * change conditional in gettestinputarray * remove reference * fix lint * increase num_inputs to 3 * remove extra out_arr var * retrigger * increase num_inputs
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Add test for MKLDNN concat forward / backward operator
Checklist
Essentials
Please feel free to remove inapplicable items for your PR.
Changes
num_inputs/dimso that the inputs / outputs can be scaled (used to create larger ndarrays scaled bynum_inputsondim)Comments