follow coments

paddle/operators/CMakeLists.txt

@@ -75,10 +75,11 @@ 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(maxPool2dWithIndex);\n")
+        file(APPEND ${pybind_file} "USE_OP(max_pool2d_with_index);\n")
     endif()
 
     # pybind USE_NO_KERNEL_OP

paddle/operators/math/pooling.h

@@ -21,15 +21,26 @@ limitations under the License. */
 namespace paddle {
 namespace operators {
 namespace math {
-//////////////////////
-#define FLT_MAX __FLT_MAX__  //
 
+#define FLT_MAX \
+  __FLT_MAX__  // It might need to be placed in another file, but I'm still
+               // wondering where to put it
+
+/*
+ * \brief Extracting simple operations from pooling.
+ *        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.
+ *        MaxPoolGrad and AvgPoolGrad are gradient operations respectively.
+ */
 template <class T>
 class MaxPool {
  public:
   DEVICE inline T initial() { return static_cast<T>(-FLT_MAX); }
   DEVICE inline void compute(T& y, const T& x) { y = y > x ? y : x; }
-  DEVICE inline void finalize(T& y, const T& poo_size) {}
+  DEVICE inline void finalize(T& y, const T& pool_field) {}
 };
 
 template <class T>
@@ -37,8 +48,9 @@ class AvgPool {
  public:
   DEVICE inline T initial() { return static_cast<T>(0); }
   DEVICE inline void compute(T& y, const T& x) { y += x; }
-  DEVICE inline void finalize(T& y, const T& poo_size) { y /= poo_size; }
+  DEVICE inline void finalize(T& y, const T& pool_field) { y /= pool_field; }
 };
+
 template <class T>
 class MaxPoolGrad {
  public:
@@ -57,6 +69,20 @@ 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 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.
+ * This is different from average pooling. So we rewrite the max_pool_grad:
+ * MaxPool2dGradFunctor, MaxPool3dGradFunctor.
+ *
+ */
+
 template <typename Place, typename PoolProcess, typename T>
 class Pool2dFunctor {
  public:
@@ -117,6 +143,14 @@ class MaxPool3dGradFunctor {
                   std::vector<int>& strides, std::vector<int>& paddings);
 };
 
+/*
+ * \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 pool2d, all tensors are in NCHW format. In pool3d, all tensors are in
+ * NCDHW format.
+ *
+ */
 template <typename Place, typename T>
 class MaxPool2dWithIndexFunctor {
  public:

paddle/operators/pool_with_index_op.cc

@@ -17,7 +17,7 @@ limitations under the License. */
 namespace paddle {
 namespace operators {
 
-int OutputSizeMaxPool(int input_size, int filter_size, int padding,
+inline int OutputSizeMaxPool(int input_size, int filter_size, int padding,
                              int stride) {
   int output_size = (input_size - filter_size + 2 * padding) / stride + 1;
   return output_size;
@@ -194,24 +194,24 @@ the input and ksize, strides, paddings parameters.
 
 namespace ops = paddle::operators;
 
-REGISTER_OP(maxPool2dWithIndex, ops::MaxPoolWithIndexOp,
-            ops::MaxPool2dWithIndexOpMaker, maxPool2dWithIndex_grad,
+REGISTER_OP(max_pool2d_with_index, ops::MaxPoolWithIndexOp,
+            ops::MaxPool2dWithIndexOpMaker, max_pool2d_with_index_grad,
             ops::MaxPoolWithIndexOpGrad);
 
 REGISTER_OP_CPU_KERNEL(
-    maxPool2dWithIndex,
+    max_pool2d_with_index,
     ops::MaxPoolWithIndexKernel<paddle::platform::CPUPlace, float>);
 REGISTER_OP_CPU_KERNEL(
-    maxPool2dWithIndex_grad,
+    max_pool2d_with_index_grad,
     ops::MaxPoolWithIndexGradKernel<paddle::platform::CPUPlace, float>)
 
-REGISTER_OP(maxPool3dWithIndex, ops::MaxPoolWithIndexOp,
-            ops::MaxPool3dWithIndexOpMaker, maxPool3dWithIndex_grad,
+REGISTER_OP(max_pool3d_with_index, ops::MaxPoolWithIndexOp,
+            ops::MaxPool3dWithIndexOpMaker, max_pool3d_with_index_grad,
             ops::MaxPoolWithIndexOpGrad);
 
 REGISTER_OP_CPU_KERNEL(
-    maxPool3dWithIndex,
+    max_pool3d_with_index,
     ops::MaxPoolWithIndexKernel<paddle::platform::CPUPlace, float>);
 REGISTER_OP_CPU_KERNEL(
-    maxPool3dWithIndex_grad,
+    max_pool3d_with_index_grad,
     ops::MaxPoolWithIndexGradKernel<paddle::platform::CPUPlace, float>)

paddle/operators/pool_with_index_op.cu

@@ -17,15 +17,15 @@ limitations under the License. */
 namespace ops = paddle::operators;
 
 REGISTER_OP_GPU_KERNEL(
-    maxPool2dWithIndex,
+    max_pool2d_with_index,
     ops::MaxPoolWithIndexKernel<paddle::platform::GPUPlace, float>);
 REGISTER_OP_GPU_KERNEL(
-    maxPool2dWithIndex_grad,
+    max_pool2d_with_index_grad,
     ops::MaxPoolWithIndexGradKernel<paddle::platform::GPUPlace, float>)
 
 REGISTER_OP_GPU_KERNEL(
-    maxPool3dWithIndex,
+    max_pool3d_with_index,
     ops::MaxPoolWithIndexKernel<paddle::platform::GPUPlace, float>);
 REGISTER_OP_GPU_KERNEL(
-    maxPool3dWithIndex_grad,
+    max_pool3d_with_index_grad,
     ops::MaxPoolWithIndexGradKernel<paddle::platform::GPUPlace, float>)

python/paddle/v2/framework/tests/test_pool_max_op.py

@@ -100,7 +100,8 @@ class TestMaxPoolWithIndex_Op(OpTest):
 
     def initTestCase(self):
         self.global_pool = True
-        self.op_type = "maxPool3dWithIndex"
+        self.index = "max_pool3d_with_index"
+        self.op_type = "%s" % self.index
         self.pool_forward_naive = max_pool3D_forward_naive
         self.shape = [2, 3, 5, 5, 5]
         self.ksize = [3, 3, 3]
@@ -111,7 +112,7 @@ class TestMaxPoolWithIndex_Op(OpTest):
 class TestCase1(TestMaxPoolWithIndex_Op):
     def initTestCase(self):
         self.global_pool = True
-        self.op_type = "maxPool3dWithIndex"
+        self.op_type = "max_pool3d_with_index"
         self.pool_forward_naive = max_pool3D_forward_naive
         self.shape = [2, 3, 5, 5, 5]
         self.ksize = [3, 3, 3]
@@ -122,7 +123,7 @@ class TestCase1(TestMaxPoolWithIndex_Op):
 class TestCase2(TestMaxPoolWithIndex_Op):
     def initTestCase(self):
         self.global_pool = False
-        self.op_type = "maxPool3dWithIndex"
+        self.op_type = "max_pool3d_with_index"
         self.pool_forward_naive = max_pool3D_forward_naive
         self.shape = [2, 3, 7, 7, 7]
         self.ksize = [3, 3, 3]
@@ -133,7 +134,7 @@ class TestCase2(TestMaxPoolWithIndex_Op):
 class TestCase3(TestMaxPoolWithIndex_Op):
     def initTestCase(self):
         self.global_pool = False
-        self.op_type = "maxPool3dWithIndex"
+        self.op_type = "max_pool3d_with_index"
         self.pool_forward_naive = max_pool3D_forward_naive
         self.shape = [2, 3, 7, 7, 7]
         self.ksize = [3, 3, 3]
@@ -144,7 +145,7 @@ class TestCase3(TestMaxPoolWithIndex_Op):
 class TestCase4(TestMaxPoolWithIndex_Op):
     def initTestCase(self):
         self.global_pool = True
-        self.op_type = "maxPool3dWithIndex"
+        self.op_type = "max_pool3d_with_index"
         self.pool_forward_naive = max_pool3D_forward_naive
         self.shape = [2, 3, 5, 5, 5]
         self.ksize = [3, 3, 3]
@@ -155,7 +156,7 @@ class TestCase4(TestMaxPoolWithIndex_Op):
 class TestCase5(TestMaxPoolWithIndex_Op):
     def initTestCase(self):
         self.global_pool = True
-        self.op_type = "maxPool3dWithIndex"
+        self.op_type = "max_pool3d_with_index"
         self.pool_forward_naive = max_pool3D_forward_naive
         self.shape = [2, 3, 5, 5, 5]
         self.ksize = [3, 3, 3]
@@ -166,7 +167,7 @@ class TestCase5(TestMaxPoolWithIndex_Op):
 class TestCase6(TestMaxPoolWithIndex_Op):
     def initTestCase(self):
         self.global_pool = False
-        self.op_type = "maxPool2dWithIndex"
+        self.op_type = "max_pool2d_with_index"
         self.pool_forward_naive = max_pool2D_forward_naive
         self.shape = [2, 3, 7, 7]
         self.ksize = [3, 3]
@@ -177,7 +178,7 @@ class TestCase6(TestMaxPoolWithIndex_Op):
 class TestCase7(TestMaxPoolWithIndex_Op):
     def initTestCase(self):
         self.global_pool = False
-        self.op_type = "maxPool2dWithIndex"
+        self.op_type = "max_pool2d_with_index"
         self.pool_forward_naive = max_pool2D_forward_naive
         self.shape = [2, 3, 7, 7]
         self.ksize = [3, 3]
@@ -188,7 +189,7 @@ class TestCase7(TestMaxPoolWithIndex_Op):
 class TestCase8(TestMaxPoolWithIndex_Op):
     def initTestCase(self):
         self.global_pool = True
-        self.op_type = "maxPool2dWithIndex"
+        self.op_type = "max_pool2d_with_index"
         self.pool_forward_naive = max_pool2D_forward_naive
         self.shape = [2, 3, 5, 5]
         self.ksize = [3, 3]
@@ -199,7 +200,7 @@ class TestCase8(TestMaxPoolWithIndex_Op):
 class TestCase9(TestMaxPoolWithIndex_Op):
     def initTestCase(self):
         self.global_pool = True
-        self.op_type = "maxPool2dWithIndex"
+        self.op_type = "max_pool2d_with_index"
         self.pool_forward_naive = max_pool2D_forward_naive
         self.shape = [2, 3, 5, 5]
         self.ksize = [3, 3]