From 6c6474cbd8514011b1c63d3439d49bd4700e46c8 Mon Sep 17 00:00:00 2001 From: chengduoZH Date: Tue, 10 Oct 2017 10:32:19 +0800 Subject: [PATCH] follow coments --- paddle/operators/CMakeLists.txt | 15 +++---- paddle/operators/math/pooling.h | 23 ++++++----- paddle/operators/pool_with_index_op.cc | 57 +++++++++++++++----------- 3 files changed, 54 insertions(+), 41 deletions(-) diff --git a/paddle/operators/CMakeLists.txt b/paddle/operators/CMakeLists.txt index 39af318ca..31ae4b2cc 100644 --- a/paddle/operators/CMakeLists.txt +++ b/paddle/operators/CMakeLists.txt @@ -55,12 +55,20 @@ function(op_library TARGET) set(pybind_flag 1) endif() + # pool_op contains several operators if ("${TARGET}" STREQUAL "pool_op") set(pybind_flag 1) # It's enough to just adding one operator to pybind file(APPEND ${pybind_file} "USE_OP(pool2d);\n") endif() + # pool_with_index_op contains several operators + if ("${TARGET}" STREQUAL "pool_with_index_op") + set(pybind_flag 1) + # It's enough to just adding one operator to pybind + file(APPEND ${pybind_file} "USE_OP(max_pool2d_with_index);\n") + endif() + # activation_op contains several operators if ("${TARGET}" STREQUAL "activation_op") set(pybind_flag 1) @@ -75,13 +83,6 @@ function(op_library TARGET) file(APPEND ${pybind_file} "USE_OP(reduce_sum);\n") endif() - # pool_with_index_op contains several operators - if ("${TARGET}" STREQUAL "pool_with_index_op") - set(pybind_flag 1) - # It's enough to just adding one operator to pybind - file(APPEND ${pybind_file} "USE_OP(max_pool2d_with_index);\n") - endif() - # pybind USE_NO_KERNEL_OP file(READ ${TARGET}.cc TARGET_CONTENT) string(REGEX MATCH "OperatorWithKernel" regex_result "${TARGET_CONTENT}") diff --git a/paddle/operators/math/pooling.h b/paddle/operators/math/pooling.h index f15ddca69..c50c57b5c 100644 --- a/paddle/operators/math/pooling.h +++ b/paddle/operators/math/pooling.h @@ -24,15 +24,16 @@ namespace math { #define FLT_MAX \ __FLT_MAX__ // It might need to be placed in another file, but I'm still - // wondering where to put it + // wondering where to put it. /* * \brief Extracting simple operations from pooling. - * Both MaxPool and AvgPool need initial, compute and finalize operation. + * Both MaxPool and AvgPool need "initial", "compute" and "finalize" + * operation. * MaxPool initializes temp variable to the negative maximum to find the * maximum value in the pooling field. * AvgPool initializes temp variable to the zero to accumulate all values - * in pool pooling, and takes the average. + * in pool pooling, and finally takes the average. * MaxPoolGrad and AvgPoolGrad are gradient operations respectively. */ template @@ -72,17 +73,17 @@ class AvgPoolGrad { /* * \brief Getting pooling results, and calculating gradient. * - * In pool2d, all tensors are in NCHW format. In pool3d, all tensors are in - * NCDHW format. + * In pool2d, all tensors are in NCHW format. Where N is batch size, C is the + * number of channels, H and W is the height and width of feature. + * In pool3d, all tensors are in NCDHW format. Where N is batch size, C is the + * number of channels, D, H and W is the depth, height and width of feature. * * In max pooling, it is possible that the pooling region has multiple maximum - * elements. - * In this case, we should compute the gradient of the first maximum element. + * elements. In this case, we should compute the gradient of the first maximum + * element. * This is different from average pooling. So we rewrite the max_pool_grad: * MaxPool2dGradFunctor, MaxPool3dGradFunctor. - * */ - template class Pool2dFunctor { public: @@ -146,10 +147,9 @@ class MaxPool3dGradFunctor { /* * \brief Getting max pooling results and corresponding max index, and * calculating gradient. - * In sub-sampling-pooling, it is necessary to know max element index. + * In up-sampling-pooling, it is necessary to know max element index. * In pool2d, all tensors are in NCHW format. In pool3d, all tensors are in * NCDHW format. - * */ template class MaxPool2dWithIndexFunctor { @@ -188,6 +188,7 @@ class MaxPool3dWithIndexGradFunctor { const framework::Tensor& mask, std::vector& ksize, std::vector& strides, std::vector& paddings); }; + } // namespace math } // namespace operators } // namespace paddle diff --git a/paddle/operators/pool_with_index_op.cc b/paddle/operators/pool_with_index_op.cc index 2e6a5f255..ab933a340 100644 --- a/paddle/operators/pool_with_index_op.cc +++ b/paddle/operators/pool_with_index_op.cc @@ -34,7 +34,7 @@ class MaxPoolWithIndexOp : public framework::OperatorWithKernel { PADDLE_ENFORCE(ctx->HasOutput("Out"), "Out(Output) of Pooling should not be null."); PADDLE_ENFORCE(ctx->HasOutput("Mask"), - "Out(Output) of Pooling should not be null."); + "Mask(Output) of Pooling should not be null."); auto in_x_dims = ctx->GetInputDim("X"); @@ -52,13 +52,11 @@ class MaxPoolWithIndexOp : public framework::OperatorWithKernel { } PADDLE_ENFORCE(in_x_dims.size() - ksize.size() == 2U, - "Pooling intput size and pooling size should be consistent"); - PADDLE_ENFORCE(ksize.size() == 2 || ksize.size() == 3, - "Pooling size size should be 2 elements. or 3 elements."); + "Intput size and pooling size should be consistent."); PADDLE_ENFORCE_EQ(ksize.size(), strides.size(), - "strides size and pooling size should be the same."); + "Strides size and pooling size should be the same."); PADDLE_ENFORCE_EQ(ksize.size(), paddings.size(), - "paddings size and pooling size should be the same."); + "Paddings size and pooling size should be the same."); std::vector output_shape({in_x_dims[0], in_x_dims[1]}); for (size_t i = 0; i < ksize.size(); ++i) { @@ -76,11 +74,9 @@ class MaxPoolWithIndexOpGrad : public framework::OperatorWithKernel { protected: void InferShape(framework::InferShapeContextBase *ctx) const override { - PADDLE_ENFORCE(ctx->HasInput("X"), - "X(Input) of Pooling should not be null."); - PADDLE_ENFORCE( - ctx->HasOutput(framework::GradVarName("X")), - "X@GRAD(Input@GRAD) of MaxPoolWithIndexOpGrad should not be null."); + PADDLE_ENFORCE(ctx->HasInput("X"), "Input(X) must not be null."); + PADDLE_ENFORCE(ctx->HasOutput(framework::GradVarName("X")), + "Input(X@GRAD) should not be null."); ctx->SetOutputDim(framework::GradVarName("X"), ctx->GetInputDim("X")); } }; @@ -110,9 +106,10 @@ class MaxPool2dWithIndexOpMaker : public framework::OpProtoAndCheckerMaker { AddAttr>( "ksize", - "Pooling size(height, width) of pooling operator." + "The pooling size(height, width) of pooling operator." "If globalPooling = true, ksize is ignored and need not be " - "specified."); // TODO(Add checker) + "specified."); // TODO(Chengduo): Add checker. (Currently, + // TypedAttrChecker don't support vector type.) AddAttr( "globalPooling", "Whether to use the globalPooling." @@ -123,15 +120,21 @@ class MaxPool2dWithIndexOpMaker : public framework::OpProtoAndCheckerMaker { AddAttr>("strides", "Strides(height, width) of pooling operator." "Default {1,1}.") - .SetDefault({1, 1}); // TODO(Add checker) + .SetDefault({1, 1}); // TODO(Chengduo): Add checker. (Currently, + // TypedAttrChecker don't support vector type.) AddAttr>("paddings", "Paddings(height, width) of pooling operator." "Default {0,0}.") - .SetDefault({0, 0}); // TODO(Add checker) + .SetDefault({0, 0}); // TODO(Chengduo): Add checker. (Currently, + // TypedAttrChecker don't support vector type.) AddComment(R"DOC( -The maxPooling2d with index operation calculates the output and the mask based on -the input and ksize, strides, paddings parameters. +The maxPooling2d with index operation calculates the output and the mask +based on the input and ksize, strides, paddings parameters. Input(X) and +output(Out, Mask) are in NCHW format. Where N is batch size, C is the +number of channels, H and W is the height and width of feature. +Parameters(ksize, strides, paddings) are two elements. +These two elements represent height and width, respectively. )DOC"); } }; @@ -162,9 +165,10 @@ class MaxPool3dWithIndexOpMaker : public framework::OpProtoAndCheckerMaker { AddAttr>( "ksize", - "Pooling size(depth, height, width) of pooling operator." + "The pooling size(depth, height, width) of pooling operator." "If globalPooling = true, ksize is ignored and need not be " - "specified."); // TODO(Add checker) + "specified."); // TODO(Chengduo): Add checker. (Currently, + // TypedAttrChecker don't support vector type.) AddAttr( "globalPooling", "Whether to use the globalPooling." @@ -176,19 +180,26 @@ class MaxPool3dWithIndexOpMaker : public framework::OpProtoAndCheckerMaker { "strides", "Strides(depth, height, width) of pooling operator." "Default {1,1,1}.") - .SetDefault({1, 1, 1}); // TODO(Add checker) + .SetDefault({1, 1, 1}); // TODO(Chengduo): Add checker. (Currently, + // TypedAttrChecker don't support vector type.) AddAttr>( "paddings", "Paddings(depth, height, width) of pooling operator." "Default {0,0,0}.") - .SetDefault({0, 0, 0}); // TODO(Add checker) + .SetDefault({0, 0, 0}); // TODO(Chengduo): Add checker. (Currently, + // TypedAttrChecker don't support vector type.) AddComment(R"DOC( -The maxpooling3d with index operation calculates the output and the mask based on -the input and ksize, strides, paddings parameters. +The maxpooling3d with index operation calculates the output and the mask +based on the input and ksize, strides, paddings parameters. +Input(X) and output(Out, Mask) are in NCDHW format. Where N is batch +size, C is the number of channels, D, H and W is the depth, height and +width of feature. Parameters(ksize, strides, paddings) are three elements. +These three elements represent depth, height and width, respectively. )DOC"); } }; + } // namespace operators } // namespace paddle -- GitLab