Refine pad op

1. Rename variables by Google style.
2. Add more test cases.
3. Add more detail and meaningful comments.
4. Change type of "padding" to vector<int>

paddle/operators/pad_op.cc

@@ -26,13 +26,13 @@ class PadOp : public framework::OperatorWithKernel {
  protected:
   void InferShape(const framework::InferShapeContext &ctx) const override {
     auto dim0 = ctx.Input<Tensor>("X")->dims();
-    auto paddings = GetAttr<std::vector<std::pair<int, int>>>("paddings");
+    auto paddings = GetAttr<std::vector<int>>("paddings");
     PADDLE_ENFORCE_EQ(
-        dim0.size(), paddings.size(),
+        dim0.size(), (int)(paddings.size() / 2),
         "Paddings size should be equal to dimension size of input tensor.");
     std::vector<int> dim1(dim0.size());
     for (int i = 0; i < dim0.size(); ++i) {
-      dim1[i] = dim0[i] + paddings[i].first + paddings[i].second;
+      dim1[i] = dim0[i] + paddings[i * 2] + paddings[i * 2 + 1];
     }
     ctx.Output<Tensor>("Out")->Resize(paddle::framework::make_ddim(dim1));
   }
@@ -42,14 +42,40 @@ class PadOpMaker : public framework::OpProtoAndCheckerMaker {
  public:
   PadOpMaker(framework::OpProto *proto, framework::OpAttrChecker *op_checker)
       : OpProtoAndCheckerMaker(proto, op_checker) {
-    AddInput("X", "The input of pad op");
-    AddOutput("Out", "The output of pad op");
+    AddInput("X", "The input of pad op.");
+    AddOutput("Out", "The output of pad op.");
     AddComment(R"DOC(
-Pad Operator.
+Pad input into output, as specified by paddings and pad_value. The input should be a k-D tensor(k > 0 and k < 7). As an example:
+
+Given:
+
+X = [[1, 2],
+   [3, 4]]
+
+and 
+
+paddings = [(0,1),(1,2)]
+
+and
+ 
+pad_value = 0 
+
+then we get 
+
+Out = [[0, 1, 2, 0, 0]
+       [0, 3, 4, 0, 0]
+       [0, 0, 0, 0, 0]]
 )DOC");
-    AddAttr<std::vector<std::pair<int, int>>>(
-        "paddings", "The padding rules for each dimension");
-    AddAttr<float>("pad_value", "The value to be padded into tensor")
+    AddAttr<std::vector<int>>(
+        "paddings",
+        "A pair list to describes padding rules for each dimension."
+        " For 2-D image tensor, paddings=[(0, 1), (2, 3)] means"
+        " padding 0 row to top, 1 row to bottom, 2 columns to left"
+        " and 3 columns to right.Paddings size should be equal to"
+        " dimension size of input tensor.");
+    AddAttr<float>("pad_value",
+                   "(float) default to 0; "
+                   "The value to be padded into tensor. ")
         .SetDefault(0.0f);
   }
 };

paddle/operators/pad_op.h

@@ -28,23 +28,23 @@ using EigenTensor = framework::EigenTensor<T, D, MajorType, IndexType>;
 
 template <typename Place, typename T, size_t D>
 void PadFunction(const framework::ExecutionContext& context) {
-  auto pads =
-      context.op().GetAttr<std::vector<std::pair<int, int>>>("paddings");
+  auto pads = context.GetAttr<std::vector<int>>("paddings");
   Eigen::array<std::pair<int, int>, D> paddings;
-  for (int i = 0; i < pads.size(); ++i) {
-    paddings[i] = pads[i];
+  for (int i = 0; i < paddings.size(); ++i) {
+    paddings[i].first = pads[i * 2];
+    paddings[i].second = pads[i * 2 + 1];
   }
-  T pad_value = context.op().GetAttr<T>("pad_value");
+  T pad_value = context.GetAttr<T>("pad_value");
 
-  auto* X = context.Input<Tensor>("X");
-  auto* Out = context.Output<Tensor>("Out");
-  Out->mutable_data<T>(context.GetPlace());
-  auto dims = X->dims();
+  auto* x = context.Input<Tensor>("X");
+  auto* out = context.Output<Tensor>("Out");
+  out->mutable_data<T>(context.GetPlace());
+  auto dims = x->dims();
 
-  auto X_tensor = EigenTensor<T, D>::From(*X);
-  auto Out_tensor = EigenTensor<T, D>::From(*Out);
+  auto x_tensor = EigenTensor<T, D>::From(*x);
+  auto out_tensor = EigenTensor<T, D>::From(*out);
   auto place = context.GetEigenDevice<Place>();
-  Out_tensor.device(place) = X_tensor.pad(paddings, pad_value);
+  out_tensor.device(place) = x_tensor.pad(paddings, pad_value);
 }
 
 template <typename Place, typename T>
@@ -72,28 +72,27 @@ class PadKernel : public framework::OpKernel {
         PadFunction<Place, T, 6>(context);
         break;
       default:
-        LOG(ERROR) << "Only ranks up to 6 supported.";
+        PADDLE_THROW("Only ranks up to 6 supported.");
     }
   }
 };
 
 template <typename Place, typename T, size_t D>
 void PadGradFunction(const framework::ExecutionContext& context) {
-  auto pads =
-      context.op().GetAttr<std::vector<std::pair<int, int>>>("paddings");
+  auto pads = context.GetAttr<std::vector<int>>("paddings");
   Eigen::array<std::pair<int, int>, D> paddings;
-  for (int i = 0; i < pads.size(); ++i) {
-    paddings[i].first = -pads[i].first;
-    paddings[i].second = -pads[i].second;
+  for (int i = 0; i < paddings.size(); ++i) {
+    paddings[i].first = -pads[i * 2];
+    paddings[i].second = -pads[i * 2 + 1];
   }
-  auto* dOut = context.Input<Tensor>(framework::GradVarName("Out"));
-  auto* dX = context.Output<Tensor>(framework::GradVarName("X"));
-  dX->mutable_data<T>(context.GetPlace());
+  auto* d_out = context.Input<Tensor>(framework::GradVarName("Out"));
+  auto* d_x = context.Output<Tensor>(framework::GradVarName("X"));
+  d_x->mutable_data<T>(context.GetPlace());
 
-  auto dX_tensor = EigenTensor<T, D>::From(*dX);
-  auto dOut_tensor = EigenTensor<T, D>::From(*dOut);
+  auto d_x_tensor = EigenTensor<T, D>::From(*d_x);
+  auto d_out_tensor = EigenTensor<T, D>::From(*d_out);
   auto place = context.GetEigenDevice<Place>();
-  dX_tensor.device(place) = dOut_tensor.pad(paddings, 0);
+  d_x_tensor.device(place) = d_out_tensor.pad(paddings, 0);
 }
 
 template <typename Place, typename T>
@@ -122,7 +121,7 @@ class PadGradKernel : public framework::OpKernel {
         PadGradFunction<Place, T, 6>(context);
         break;
       default:
-        LOG(ERROR) << "Only ranks up to 6 supported.";
+        PADDLE_THROW("Only ranks up to 6 supported.");
     }
   }
 };

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

@@ -9,36 +9,89 @@ class TestPadOp(unittest.TestCase):
     __metaclass__ = OpTestMeta
 
     def setUp(self):
+        self.initTestCase()
         self.type = "pad"
-        self.inputs = {'X': np.random.random((16, 16)).astype("float32"), }
+        self.inputs = {'X': np.random.random(self.shape).astype("float32"), }
         self.attrs = {}
-        self.attrs['paddings'] = [(0, 1), (2, 3)]
-        self.attrs['pad_value'] = 0
+        self.attrs['paddings'] = np.array(self.paddings).flatten()
+        self.attrs['pad_value'] = self.pad_value
         self.outputs = {
             'Out': np.pad(self.inputs['X'],
-                          self.attrs['paddings'],
+                          self.paddings,
                           mode='constant',
-                          constant_values=0)
+                          constant_values=self.pad_value)
         }
 
+    def initTestCase(self):
+        self.shape = (16, 16)
+        self.paddings = [(0, 1), (2, 3)]
+        self.pad_value = 0
+
+
+class TestCase1(TestPadOp):
+    def initTestCase(self):
+        self.shape = (2, 3, 4, 4)
+        self.paddings = [(0, 1), (2, 3), (2, 1), (1, 1)]
+        self.pad_value = 0.5
+
+
+class TestCase2(TestPadOp):
+    def initTestCase(self):
+        self.shape = (2, 2, 2)
+        self.paddings = [(0, 0), (0, 0), (1, 2)]
+        self.pad_value = 1
+
+
+class TestCase3(TestPadOp):
+    def initTestCase(self):
+        self.shape = (8)
+        self.paddings = [(0, 1)]
+        self.pad_value = 0.9
+
 
 class TestPadGradOp(GradientChecker):
     def setUp(self):
+        self.initTestCase()
         self.op = Operator(
             type="pad",
             X="X",
             Out="Out",
-            paddings=[(0, 1), (2, 3)],
-            pad_value=0)
-        self.inputs = {'X': np.random.random((16, 16)).astype("float32"), }
+            paddings=np.array(self.paddings).flatten(),
+            pad_value=self.pad_value)
+        self.inputs = {'X': np.random.random(self.shape).astype("float32"), }
+
+    def initTestCase(self):
+        self.shape = (16, 16)
+        self.paddings = [(0, 1), (2, 3)]
+        self.pad_value = 0
 
     def test_normal(self):
-        self.check_grad(
-            self.op, self.inputs, set(["X"]), "Out", max_relative_error=0.5)
+        self.check_grad(self.op, self.inputs, set(["X"]), "Out")
 
     def test_cpu_gpu_compare(self):
         self.compare_grad(self.op, self.inputs)
 
 
+class TestiGradCase1(TestPadOp):
+    def initTestCase(self):
+        self.shape = (2, 3, 4, 4)
+        self.paddings = [(0, 1), (2, 3), (2, 1), (1, 1)]
+        self.pad_value = 0.5
+
+
+class TestGradCase2(TestPadOp):
+    def initTestCase(self):
+        self.shape = (2, 2, 2)
+        self.paddings = [(0, 0), (0, 0), (1, 2)]
+        self.pad_value = 1
+
+
+class TestGradCase3(TestPadOp):
+    def initTestCase(self):
+        self.shape = (8)
+        self.paddings = [(0, 1)]
+        self.pad_value = 0.9
+
+
 if __name__ == '__main__':
     unittest.main()