update comment

paddle/operators/sequence_concat_op.cc

@@ -48,11 +48,11 @@ class SequenceConcatOpMaker : public framework::OpProtoAndCheckerMaker {
                         framework::OpAttrChecker* op_checker)
       : OpProtoAndCheckerMaker(proto, op_checker) {
     AddInput("X",
-             "The input Multip LoDTensors, which are variable-length "
-             "sequence or nested sequence.")
+             "(A vector of LoDTensor), the input is a vector of LoDTensor, "
+             "each of which is a variable-length sequence or nested sequence.")
         .AsDuplicable();
     AddOutput("Out",
-              "A LoDTensor, the variable-length output of "
+              "(A LoDTensor), the variable-length output of "
               "sequence_concat Op.");
     AddAttr<int>("axis",
                  "(int, default 0)"
@@ -61,27 +61,36 @@ class SequenceConcatOpMaker : public framework::OpProtoAndCheckerMaker {
         .SetDefault(0);
     AddAttr<int>("level",
                  "(int, default 0)"
-                 "The level which the inputs will be joined with."
-                 "If level is 0, the inputs will be joined with "
-                 "nested sequences."
-                 "If level is 1, the inputs will be joined with sequences.")
+                 "The level at which the inputs will be joined."
+                 "If the level is 0, the inputs will be joined at the nested "
+                 "sequence level."
+                 "If the level is 1, the inputs will be joined at the "
+                 "sequence level.")
         .SetDefault(0);
     AddComment(R"DOC(
     The sequence_concat operator concatenates multiple LoDTensors. 
-    It only supports sequences ( LoD Tensor with level=1) 
-    or nested sequences (LoD tensor with level=0) as its inputs.
+    It only supports sequence (LoD Tensor with level number is 1) 
+    or a nested sequence (LoD tensor with level number is 2) as its input.
     - Case1:
-      If the axis is 1, level is 1, the LoD of Inputs are the same,
-      LoD(x0) = {{0,2,4},{0,1,2,3,4}}; Dims(x0) = (2,3,4)
-      LoD(x1) = {{0,2,4},{0,1,2,3,4}}; Dims(x1) = (2,4,4)
-      LoD(Out) = {{0,2,4},{0,1,2,3,4}}; Dims(Out) = (2,7,4)
+      If the axis is other than 0(here, axis is 1 and level is 1),
+      each input should have the same LoD information and the LoD 
+      information of the output keeps the same as the input.
+      LoD(x0) = {{0,2,4}, {0,1,2,3,4}}; Dims(x0) = (4,3,4)
+      LoD(x1) = {{0,2,4}, {0,1,2,3,4}}; Dims(x1) = (4,4,4)
+      LoD(Out) = {{0,2,4}, {0,1,2,3,4}}; Dims(Out) = (4,7,4)
     - Case2:
-      If the axis is 0, level is 1, the LoD of inputs are different,
-      LoD(x0) = {{0,2,4}, {0,1,2,3,4}}; Dims(x0) = (2,3,4)
-      LoD(x1) = {{0,3,5}, {0,1,3,4,5}}; Dims(x1) = (3,3,4)
-      LoD(Out) = {{0,5,9}, {0,1,2,4,5,6,7,8,9}}; Dims(Out) = (5,3,4)
+      If the axis is 0(here, leve is 0), the inputs are concatenated along 
+      time steps, the LoD information of the output need to re-compute.
+      LoD(x0) = {{0,2,4}, {0,1,2,3,4}}; Dims(x0) = (4,3,4)
+      LoD(x1) = {{0,3,5}, {0,1,2,3,5}}; Dims(x1) = (5,3,4)
+      LoD(Out) = {{0,5,9}, {0,1,2,3,4,5,6,7,9}}; Dims(Out) = (9,3,4)
+    - Case3:
+      If the axis is 0(here, level is 1).
+      LoD(x0) = {{0,2,4}, {0,1,2,3,4}}; Dims(x0) = (4,3,4)
+      LoD(x1) = {{0,3,5}, {0,1,3,4,5}}; Dims(x1) = (5,3,4)
+      LoD(Out) = {{0,5,9}, {0,2,5,7,9}}; Dims(Out) = (9,3,4)
       
-    NOTE: The level of all the inputs should be the same.
+    NOTE: The levels of all the inputs should be the same.
     )DOC");
   }
 };
@@ -95,7 +104,7 @@ class SequenceConcatGradOp : public framework::OperatorWithKernel {
     PADDLE_ENFORCE(ctx->HasInput(framework::GradVarName("Out")),
                    "The gradient of Out should not be null.");
     PADDLE_ENFORCE(ctx->HasOutputs(framework::GradVarName("X")),
-                   "The gradient of X should not be empty.");
+                   "The gradient of X should not be null.");
     ctx->SetOutputsDim(framework::GradVarName("X"), ctx->GetInputsDim("X"));
   }
 };

paddle/operators/sequence_concat_op.h

@@ -23,35 +23,22 @@ using Tensor = framework::Tensor;
 using LoDTensor = framework::LoDTensor;
 using LoD = framework::LoD;
 
-// Concat LoD, the initialized LoD of Output is lod(x0),
-// if axis is not 0, the LoD(Out) will be the same as Inputs, if axis is 0:
-// Case1:
-//  There is one level, the Output LoD will be modified:
-//  LoD(x0) = {{0,2,4}}
-//  LoD(x1) = {{0,1,5}}
-//  LoD(Out) = {{0,3,9}}
-// Case2:
-//  There is two level, and concat level is 1,
-//  the Output LoD will be modified as followed:
-//  LoD(x0) = {{0,2,4}, {0,1,2,3,4}}
-//  LoD(x1) = {{0,3,5}, {0,1,3,4,5}}
-//  LoD(Out) = {{0,5,9}, {0,1,2,4,5,6,7,8,9}}
 template <typename T>
 LoD concatLoD(const std::vector<const T*> ins, const size_t axis,
               const size_t level) {
   auto out_lod = ins[0]->lod();
   const size_t n = ins.size();
   if (axis == 0UL) {
-    if (level == 0) {
+    if (level == 0UL) {
       for (size_t i = 1; i < n; ++i) {
         for (size_t j = 0; j < ins[i]->lod()[0].size(); ++j) {
           out_lod[0][j] += ins[i]->lod()[0][j];
         }
       }
-    } else if (level == 1) {
+    } else if (level == 1UL) {
       PADDLE_ENFORCE_EQ(ins[0]->NumLevels(), 2UL,
                         "If the level is 1, all of the inputs "
-                        "should be the the nested sequence.");
+                        "should be the nested sequence.");
       for (size_t i = 1; i < n; ++i) {
         for (size_t j = 0; j < ins[i]->lod()[0].size(); ++j) {
           out_lod[0].push_back(ins[i]->lod()[0][j]);
@@ -80,16 +67,17 @@ class SequenceConcatOpKernel : public framework::OpKernel<T> {
                         "The level number of all the input LoDTensors "
                         "should be the same.");
       PADDLE_ENFORCE_EQ(ins[0]->dims().size(), ins[i]->dims().size(),
-                        "The dimensions size of all the input LoDTensors "
+                        "The dimension size of all the input LoDTensors "
                         "should be the same.");
 
       const size_t dims_size = ins[i]->dims().size();
       for (size_t j = 0; j < dims_size; ++j) {
         if (j == axis) continue;
         PADDLE_ENFORCE_EQ(ins[0]->dims()[j], ins[i]->dims()[j],
-                          "The dimensions of all the input LoDTensors "
-                          "except for the specify axis should be "
-                          "matched exactly.");
+                          "Except for the dimension of the specified "
+                          "axis along which all the inputs are concatenated, "
+                          "dimensions of all the other axises of the input "
+                          "LoDTensors should be the same.");
       }
     }
 

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

@@ -6,16 +6,16 @@ from op_test import OpTest
 class TestConcatOp(OpTest):
     def set_data(self):
         # two level, batch size is 3
-        x0 = np.random.random((11, 6, 3)).astype('float32')
-        lod0 = [[0, 2, 5, 11], [0, 1, 2, 5, 7, 11]]
-        x1 = np.random.random((11, 8, 3)).astype('float32')
-        lod1 = [[0, 2, 5, 11], [0, 1, 2, 5, 7, 11]]
+        x0 = np.random.random((4, 6, 3)).astype('float32')
+        lod0 = [[0, 2, 4], [0, 1, 2, 3, 4]]
+        x1 = np.random.random((4, 8, 3)).astype('float32')
+        lod1 = [[0, 2, 4], [0, 1, 2, 3, 4]]
         axis = 1
         level = 1
         self.inputs = {'X': [('x0', (x0, lod0)), ('x1', (x1, lod1))]}
         self.attrs = {'axis': axis, 'level': level}
         outs = []
-        for i in range(5):
+        for i in range(4):
             sub_x0 = x0[lod0[level][i]:lod0[level][i + 1], :]
             sub_x1 = x1[lod1[level][i]:lod1[level][i + 1], :]
             outs.append(np.concatenate((sub_x0, sub_x1), axis=axis))
@@ -36,16 +36,36 @@ class TestConcatOp(OpTest):
 class TestConcatOpDiffLod(TestConcatOp):
     def set_data(self):
         # two level, batch size is 3
-        x0 = np.random.random((12, 6, 3)).astype('float32')
-        lod0 = [[0, 3, 9, 12], [0, 2, 3, 5, 9, 12]]
-        x1 = np.random.random((11, 6, 3)).astype('float32')
-        lod1 = [[0, 2, 5, 11], [0, 1, 2, 5, 7, 11]]
+        x0 = np.random.random((4, 6, 3)).astype('float32')
+        lod0 = [[0, 2, 4], [0, 1, 2, 3, 4]]
+        x1 = np.random.random((5, 6, 3)).astype('float32')
+        lod1 = [[0, 3, 5], [0, 1, 2, 3, 5]]
         axis = 0
         level = 1
         self.inputs = {'X': [('x0', (x0, lod0)), ('x1', (x1, lod1))]}
         self.attrs = {'axis': axis, 'level': level}
         outs = []
-        for i in range(5):
+        for i in range(4):
+            sub_x0 = x0[lod0[level][i]:lod0[level][i + 1], :]
+            sub_x1 = x1[lod1[level][i]:lod1[level][i + 1], :]
+            outs.append(np.concatenate((sub_x0, sub_x1), axis=axis))
+
+        self.outputs = {'Out': np.concatenate(outs, axis=0)}
+
+
+class TestConcatOpLevelZero(TestConcatOp):
+    def set_data(self):
+        # two level, batch size is 3
+        x0 = np.random.random((4, 3, 4)).astype('float32')
+        lod0 = [[0, 2, 4], [0, 1, 2, 3, 4]]
+        x1 = np.random.random((5, 3, 4)).astype('float32')
+        lod1 = [[0, 3, 5], [0, 1, 3, 4, 5]]
+        axis = 0
+        level = 0
+        self.inputs = {'X': [('x0', (x0, lod0)), ('x1', (x1, lod1))]}
+        self.attrs = {'axis': axis, 'level': level}
+        outs = []
+        for i in range(2):
             sub_x0 = x0[lod0[level][i]:lod0[level][i + 1], :]
             sub_x1 = x1[lod1[level][i]:lod1[level][i + 1], :]
             outs.append(np.concatenate((sub_x0, sub_x1), axis=axis))