Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shyrma concat #24

Merged
merged 16 commits into from Nov 3, 2019
Merged

Shyrma concat #24

merged 16 commits into from Nov 3, 2019

Conversation

@shyrma
Copy link

shyrma commented Nov 2, 2019

  • provide deconv2d/3d (ff and bp) based on corresponding mkl dnn api
  • add possibility to pass axis as last input array in concat op
  • corrcect sumation in bias_add_bp op for NHWC case
shyrma and others added 15 commits Oct 28, 2019
- corrcect sumation in bias_add_bp op for NHWC case

Signed-off-by: Yurii <iuriish@yahoo.com>
Signed-off-by: raver119 <raver119@gmail.com>
Signed-off-by: raver119 <raver119@gmail.com>
Signed-off-by: Yurii <iuriish@yahoo.com>
Signed-off-by: Yurii <iuriish@yahoo.com>
Signed-off-by: Yurii <iuriish@yahoo.com>
Signed-off-by: Yurii <iuriish@yahoo.com>
Signed-off-by: Yurii <iuriish@yahoo.com>
Signed-off-by: Yurii <iuriish@yahoo.com>
Signed-off-by: Yurii <iuriish@yahoo.com>
Signed-off-by: Yurii <iuriish@yahoo.com>
Signed-off-by: Yurii <iuriish@yahoo.com>
@shyrma shyrma requested a review from raver119 Nov 2, 2019
Copy link

raver119 left a comment

Few minor changes required. Looks great otherwise.

std::string expectedWeightsShape = ShapeUtils::shapeAsString({kH, kW, oC, iC});
REQUIRE_TRUE(expectedWeightsShape == ShapeUtils::shapeAsString(weights), 0, "CUSTOM DECONV2D OP: wrong shape of weights array, expected is %s, but got %s instead !", expectedWeightsShape.c_str(), ShapeUtils::shapeAsString(weights).c_str());
std::vector<Nd4jLong> expectedWeightsShape = {kH, kW, oC, iC};
REQUIRE_TRUE(expectedWeightsShape == weights->getShapeAsVector(), 0, "CUSTOM DECONV2D OP: wrong shape of weights array, expected is %s, but got %s instead !", ShapeUtils::shapeAsString(expectedWeightsShape).c_str(), ShapeUtils::shapeAsString(weights).c_str());

This comment has been minimized.

Copy link
@raver119

raver119 Nov 2, 2019

Why are we doing creating 2 vectors here? Can't we make a method, that will compare EXISTING shapeInfo to expectedWeightsShape?

std::string expectedWeightsShape = ShapeUtils::shapeAsString({kH, kW, oC, iC});
REQUIRE_TRUE(expectedWeightsShape == ShapeUtils::shapeAsString(weightsShapeInfo), 0, "CUSTOM DECONV2D OP: wrong shape of weights array, expected is %s, but got %s instead !", expectedWeightsShape.c_str(), ShapeUtils::shapeAsString(weightsShapeInfo).c_str());
std::vector<Nd4jLong> expectedWeightsShape = {kH, kW, oC, iC};
REQUIRE_TRUE(expectedWeightsShape == ShapeUtils::shapeAsVector(weightsShapeInfo), 0, "CUSTOM DECONV2D OP: wrong shape of weights array, expected is %s, but got %s instead !", ShapeUtils::shapeAsString(expectedWeightsShape).c_str(), ShapeUtils::shapeAsString(weightsShapeInfo).c_str());

This comment has been minimized.

Copy link
@raver119

raver119 Nov 2, 2019

Same as above

if(isSameMode) // SAME
ConvolutionUtils::calcPadding2D(pH, pW, oH, oW, iH, iW, kH, kW, sH, sW, dH, dW);
if(isSameMode){ // SAME
//Note: we're intentionally swapping iH and oH, to calculated the padding for a"normal" conv (not deconv) forward pass

This comment has been minimized.

Copy link
@raver119
REQUIRE_TRUE(expectedWeightsShape == ShapeUtils::shapeAsString(weights), 0, "CUSTOM DECONV2D_TF OP: wrong shape of weights array, expected is %s, but got %s instead !", expectedWeightsShape.c_str(), ShapeUtils::shapeAsString(weights).c_str());
std::vector<Nd4jLong> expectedGradOShape = ShapeUtils::composeShapeUsingDimsAndIdx({bS,oC,trueoH,trueoW, 0,indIOioC,indOoH,indOoH+1});
std::vector<Nd4jLong> expectedWeightsShape = {kH, kW, iC, oC};
REQUIRE_TRUE(expectedGradOShape == gradO->getShapeAsVector(), 0, "CUSTOM DECONV2D_TF OP: wrong shape of input array, basing on array with output shape expected is %s, but got %s instead !", ShapeUtils::shapeAsString(expectedGradOShape).c_str(), ShapeUtils::shapeAsString(gradO).c_str());

This comment has been minimized.

Copy link
@raver119

raver119 Nov 2, 2019

Same as above

REQUIRE_TRUE(weightsShapeInfo[0] == rank, 0, "CUSTOM DECONV2D_TF OP: rank of weights array must be equal to %i, but got %i instead !", rank, weightsShapeInfo[0]);
REQUIRE_TRUE(gradOShapeInfo[0] == rank, 0, "CUSTOM DECONV2D_TF OP: rank of input array must be equal to %i, but got %i instead !", rank, gradOShapeInfo[0]);
REQUIRE_TRUE(gradIShapeShapeInfo[0] == 1, 0, "CUSTOM DECONV2D_TF OP: rank of array with output shape must be equal to %i, but got %i instead !", 1, gradIShapeShapeInfo[0]);

REQUIRE_TRUE(gradIShapeShapeInfo[0] == 1, 0, "CUSTOM DECONV2D_TF OP: rank of array with output shape must be equal to %i, but got %i instead !", 1, gradIShapeShapeInfo[0]);

This comment has been minimized.

Copy link
@raver119

raver119 Nov 2, 2019

Please use shape::sizeAt() and shape::rank() instead. Don't use direct buffer comparison.

Signed-off-by: Yurii <iuriish@yahoo.com>
@shyrma

This comment has been minimized.

Copy link
Author

shyrma commented Nov 3, 2019

thank for reasonable comments,
corrrected !

@shyrma shyrma merged commit 0cdb575 into master Nov 3, 2019
@shyrma shyrma deleted the shyrma_concat branch Nov 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.