add the LinearChainCrf operator. #5084

lcy-seso · 2017-10-25T15:13:36Z

Currently, this operator only runs on CPU.

This operator has been manually tested with the old CRFLayer. Their computation results have no differences.
~~2. Currently, the linear chain CRF operator does not backpropagate the gradients it receives, so it can only be used at the end of the network. This can be easily fixed.~~
The following things still need to be optimized:
~~- It is better to use double in unitest for this operator. But, currently, I come into a coredump and I haven't checked it yet. I will fix this later.~~
- It is better if some local intermediate variable is optimized to avoid frequently allocating/freeing memory.

pengli09 · 2017-10-26T03:05:28Z

paddle/framework/tensor_impl.h

  PADDLE_ENFORCE_LT(begin_idx, end_idx,
-                    "Begin index must be less than end index.");
+                    "The start row index must be less than the end row index.");


s/less/smaller/g

pengli09 · 2017-10-26T03:12:16Z

paddle/operators/linear_chain_crf_op.cc

+  // Right now, we just bet that sum won't be zero. If this really happens, we
+  // will figure out what should be done then.
+  PADDLE_ENFORCE(sum,
+                 "The unnormalized probabilites of all possible unfinished "


These are not probabilities. Normalization is performed for numerical stability.

The sum is used to normalize one row of the alpha to avoid overflow or underflow, but one row of Alpha(in forward computation)/Beta(in backward computation) is meaningful (alpha dotmul beta is also meanningful).

Beta is calculated in "reverse order" with compared to Alpha (not mathmatically accurate). They all caputres unnormalzied probabilities for unfinished seqences. The accumulation of this sum come to the normalizion factor.

I try to make the warning information meaningful.

pengli09 · 2017-10-26T03:17:53Z

paddle/operators/linear_chain_crf_op.cc

+  PADDLE_ENFORCE(sum,
+                 "The unnormalized probabilites of all possible unfinished "
+                 "sequences must be greater than 0.");
+  for (size_t i = 0; i < len; ++i) x[i] /= sum;


I think we should retain Wei Xu's code here. Because multiplication needs less CPU cycles than division.

pengli09 · 2017-10-26T03:20:27Z

paddle/operators/linear_chain_crf_op.cc

+        "batch. A two dimensional tensor with shape [N x D], "
+        "denoted as \f$\alpha\f$. \f$\alpha$\f is a memo table used to "
+        "calculate the normalization factor in CRF. \f$\alpha[k, v]$\f stores "
+        "the unnormalized probabilites of all possible unfinished sequences of "


I think ``unnormalized probabilites'' is not a proper interpretation.

The "unnormalized probabilities" is just from the paper.

I am guessing you are referring to this paper: http://cseweb.ucsd.edu/~elkan/250Bwinter2012/loglinearCRFs.pdf

It would be valuable to cite this in the comments for reference.

You are quite right. It is this paper. I will add it. Thank you very much for all your comments.

Done. I add this into the operator's comments.

pengli09 · 2017-10-26T03:21:43Z

paddle/operators/linear_chain_crf_op.cc

+        "tensor with shape [S x 1], where S is the sequence number in a "
+        "mini-batch. "
+        "Note: S is equal to the sequence number in a mini-batch. The "
+        "output "


Merge line 91 and 92 to a single line.

pengli09 · 2017-10-26T04:59:53Z

paddle/operators/linear_chain_crf_op.cc

+        }
+        beta_value[k * tag_num + i] = sum;
+      }
+      NormalizeL1<T>(beta_value + k * tag_num, tag_num);


Restore comment "// normalizeL1 is to avoid underflow or overflow at (**)"

pengli09 · 2017-10-26T05:02:59Z

paddle/operators/linear_chain_crf_op.cc

+    auto alpha_mat = EigenMatrix<T>::From(*alpha);
+    auto beta_mat = EigenMatrix<T>::From(*beta);
+    auto x_grad_mat = EigenMatrix<T>::From(*emission_grad);
+    auto* place = ctx.GetEigenDevice<platform::CPUPlace>();


change auto* to auto

using auto* is suggested by our style.

pengli09 · 2017-10-26T05:03:15Z

paddle/operators/linear_chain_crf_op.cc

+    auto row_sum = prob.sum(Eigen::DSizes<int, 1>(1))
+                       .reshape(Eigen::DSizes<int, 2>(seq_length, 1))
+                       .broadcast(Eigen::DSizes<int, 2>(1, tag_num));
+    x_grad_mat.device(*place) = prob / row_sum;


change *place to place

auto* is suggested by our style, so I do not change this.

pengli09 · 2017-10-26T05:33:53Z

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

+        # the linear_chain_crf operator only supports sequence (LoD level = 1)
+        lod = [[0]]
+        for i in range(SEQ_NUM):
+            lod[-1].append(lod[-1][-1] + random.randint(1, MAX_SEQ_LEN))


Sequence of length 1, 2 and 3 must be included in the test. These are high-risk boundary conditions.

I have tested all these cases offline. Actually, I agree the unit test needs some fixed boundary cases. I will fix this.

Thank you. I will fix the unit test in the next PR, for the unit test in the latest develop branch framework seems to have a certain problem.

pengli09 · 2017-10-26T05:37:18Z

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

+from op_test import OpTest
+
+
+class LinearChainCrfForward(object):


A test case of LinearChainCrfForward() should also be included to make sure the implementation is correct. Furthermore, it is also useful for detecting unexpected errors introduced by future modifications. Reference: https://github.com/PaddlePaddle/Paddle/blob/develop/paddle/gserver/tests/test_CRFLayerGrad.cpp#L56

The forward computation has already been added in the unit test. I can how the original codes do the check later.

lcy-seso

follow comments.

lcy-seso · 2017-10-26T07:53:15Z

paddle/framework/tensor_impl.h

  PADDLE_ENFORCE_LT(begin_idx, end_idx,
-                    "Begin index must be less than end index.");
+                    "The start row index must be less than the end row index.");


lcy-seso · 2017-10-26T08:01:06Z

paddle/operators/linear_chain_crf_op.cc

+  // Right now, we just bet that sum won't be zero. If this really happens, we
+  // will figure out what should be done then.
+  PADDLE_ENFORCE(sum,
+                 "The unnormalized probabilites of all possible unfinished "


The sum is used to normalize one row of the alpha to avoid overflow or underflow, but one row of Alpha(in forward computation)/Beta(in backward computation) is meaningful (alpha dotmul beta is also meanningful).

Beta is calculated in "reverse order" with compared to Alpha (not mathmatically accurate). They all caputres unnormalzied probabilities for unfinished seqences. The accumulation of this sum come to the normalizion factor.

I try to make the warning information meaningful.

lcy-seso · 2017-10-26T08:05:14Z

paddle/operators/linear_chain_crf_op.cc

+        "tensor with shape [S x 1], where S is the sequence number in a "
+        "mini-batch. "
+        "Note: S is equal to the sequence number in a mini-batch. The "
+        "output "


lcy-seso · 2017-10-26T08:06:18Z

paddle/operators/linear_chain_crf_op.cc

+Linear chain CRF is a special case of CRF that is useful for sequence labeling
+task. Sequence labeling tasks do not assume a lot of conditional
+independences among inputs. They only concern about the input and the output
+being linear sequences. Thus, the graph model of CRF is a simple chain or


lcy-seso · 2017-10-26T08:06:37Z

paddle/operators/linear_chain_crf_op.cc

+    : public framework::OpKernel<T> {
+ public:
+  void Compute(const framework::ExecutionContext& ctx) const override {
+    PADDLE_ENFORCE(platform::is_cpu_place(ctx.GetPlace()),


Yes. I am doing this. I will make this operator running on GPU by simply copy the inputs to GPU.

lcy-seso · 2017-10-26T08:42:32Z

paddle/operators/linear_chain_crf_op.cc

+
+    // Beta is the memo table used in dynamic programming to calculate the
+    // backwark vectors. For a backward vector i (the i-th row of beta), it
+    // captures the unnormalized probabilities of partial sequences starting at


As explained above, each element in alpha and beta, and also alpha dotmul beta are meaningful. I try to make the comments meaningful.

lcy-seso · 2017-10-26T08:43:51Z

paddle/operators/linear_chain_crf_op.cc

+      for (size_t i = 0; i < tag_num; ++i) {
+        T sum = 0.;
+        for (size_t j = 0; j < tag_num; ++j) {
+          sum += w_exps[(i + state_trans_base_idx) * tag_num + j] *


lcy-seso · 2017-10-26T08:44:00Z

paddle/operators/linear_chain_crf_op.cc

+        }
+        beta_value[k * tag_num + i] = sum;
+      }
+      NormalizeL1<T>(beta_value + k * tag_num, tag_num);


lcy-seso · 2017-10-26T08:45:46Z

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

+from op_test import OpTest
+
+
+class LinearChainCrfForward(object):


The forward computation has already been added in the unit test. I can how the original codes do the check later.

lcy-seso · 2017-10-26T08:49:48Z

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

+        # the linear_chain_crf operator only supports sequence (LoD level = 1)
+        lod = [[0]]
+        for i in range(SEQ_NUM):
+            lod[-1].append(lod[-1][-1] + random.randint(1, MAX_SEQ_LEN))


I have tested all these cases offline. Actually, I agree the unit test needs some fixed boundary cases. I will fix this.

qingqing01 · 2017-10-26T11:21:54Z

paddle/operators/linear_chain_crf_op.cc

+    // resized to its correct size in the function Compute.
+    ctx->SetOutputDim("LogLikelihood", {emission_dims[0], 1});
+
+    ctx->ShareLoD("Emission", /*->*/ "EmissionExps");


If the output is not a LoDTensor, this line can be removed.

qingqing01 · 2017-10-26T11:32:53Z

paddle/operators/linear_chain_crf_op.cc

+    PADDLE_ENFORCE(ctx->HasOutput(framework::GradVarName("Emission")),
+                   "Output(Emission@GRAD) should be not null.");
+    PADDLE_ENFORCE(ctx->HasOutput(framework::GradVarName("Transition")),
+                   "Output(Transition@GRAD) should be not null.");


Remove these two checks. The input gradients is not to be calculated at any time. Just like the old framework, some times, one branch of the network does not need to do backward.

qingqing01 · 2017-10-26T11:33:17Z

paddle/operators/linear_chain_crf_op.cc

+
+    ctx->SetOutputDim(framework::GradVarName("Emission"), emission_exps_dims);
+    ctx->SetOutputDim(framework::GradVarName("Transition"),
+                      transition_exps_dims);


if (ctx->HasOutput(framework::GradVarName("Emission")) { ... } if (ctx->HasOutput(framework::GradVarName("Transition")) { ... }

see mul_op.cc

qingqing01 · 2017-10-26T11:35:05Z

paddle/operators/linear_chain_crf_op.cc

+                           const Tensor* transition_exps, const Tensor* alpha,
+                           const Tensor* label, Tensor* beta,
+                           Tensor* transition_grad,
+                           Tensor* emission_grad) const {


For the const input arguments: const Tensor&,
For the output arguments: Tensor*

Done. Thank you for pointing this out.

qingqing01 · 2017-10-26T11:41:29Z

paddle/operators/linear_chain_crf_op.h

@@ -0,0 +1,54 @@
+/* Copyright (c) 2016 PaddlePaddle Authors. All Rights Reserve.


Will this header file also be used for the CRF decoding code? If not, it seems this header file can be removed in this PR.

After our discussion. I will keep this header and move the function Compute into the header file and add GPU kernel function.

pengli09 · 2017-10-27T05:40:35Z

paddle/operators/linear_chain_crf_op.cc

+      for (size_t i = 0; i < tag_num; ++i) {
+        T sum = 0.;
+        for (size_t j = 0; j < tag_num; ++j) {
+          sum += alpha_value[(k - 1) * tag_num + j] *


Wei's add a comment "// (*)" at the end of this line. Line 321 refers to this line by this mark.

pengli09 · 2017-10-27T05:43:12Z

paddle/operators/linear_chain_crf_op.cc

+      for (size_t i = 0; i < tag_num; ++i) {
+        T sum = 0.;
+        for (size_t j = 0; j < tag_num; ++j) {
+          sum += w_exps[(i + state_trans_base_idx) * tag_num + j] *


Wei added a mark "// (**)" at the end of this line, which is referred by line 495.

pengli09 · 2017-10-27T05:44:41Z

paddle/operators/linear_chain_crf_op.cc

+        framework::make_ddim({static_cast<int>(batch_size), 1}),
+        platform::CPUPlace());
+
+    auto place = ctx.GetEigenDevice<platform::CPUPlace>();


Change auto to auto*, and change place to *place accordingly in the following lines.

abhinavarora

In addition to @pengli09 comments, there are some style issues. Apart from that the PR looks very solid. Great Job!

abhinavarora · 2017-10-26T23:42:37Z

paddle/framework/tensor_impl.h

+  PADDLE_ENFORCE_LE(end_idx, dims_[0], "The end row index is out of bound.");
+  PADDLE_ENFORCE_LT(
+      begin_idx, end_idx,
+      "The start row index must be smaller than the end row index.");


smaller -> lesser

abhinavarora · 2017-10-27T18:47:40Z

paddle/operators/linear_chain_crf_op.cc

+        "See more details in the operator's comments.");
+    AddInput(
+        "Label",
+        "(LoDTensor, default: LoDTensor<int>). The groundtruth which is a 2-D "


There should be a space in groundtruth. It should be ground truth

Done. Thank you.

abhinavarora · 2017-10-27T18:48:25Z

paddle/operators/linear_chain_crf_op.cc

+        "Emission",
+        "(LoDTensor, default: LoDTensor<float>). "
+        "The unscaled emission weight matrix for the linear chain CRF. "
+        "This input is a LoDTensor with shape [N x D] where N is the total "


Can we simply write that N is the size of the minibatch?

Yes, I think this is no problem. Thank you. Done.

abhinavarora · 2017-10-27T18:55:37Z

paddle/operators/linear_chain_crf_op.cc

+        "batch. A two dimensional tensor with shape [N x D], "
+        "denoted as \f$\alpha\f$. \f$\alpha$\f is a memo table used to "
+        "calculate the normalization factor in CRF. \f$\alpha[k, v]$\f stores "
+        "the unnormalized probabilites of all possible unfinished sequences of "


I am guessing you are referring to this paper: http://cseweb.ucsd.edu/~elkan/250Bwinter2012/loglinearCRFs.pdf

It would be valuable to cite this in the comments for reference.

abhinavarora · 2017-10-27T19:01:12Z

paddle/operators/linear_chain_crf_op.cc

+
+Linear chain CRF is a special case of CRF that is useful for sequence labeling
+task. Sequence labeling tasks do not assume a lot of conditional
+independences among inputs. They only concern about the input and the output


We can write this sentence as: The only constraint they impose is that the input and output must be linear sequences

Thank you. Done.

abhinavarora · 2017-10-27T22:17:53Z

paddle/operators/linear_chain_crf_op.cc

+    ctx->SetOutputDim("Alpha", emission_dims);
+    ctx->SetOutputDim("EmissionExps", emission_dims);
+    ctx->SetOutputDim("TransitionExps", transition_dims);
+    // (TODO caoying) This is tricky. The 1st dimension of Output(LogLikelihood)


You can follow the Google C++ stye guide for TODO comments. http://google.github.io/styleguide/cppguide.html#TODO_Comments

Done, thank you.

abhinavarora · 2017-10-27T22:20:00Z

paddle/operators/linear_chain_crf_op.cc

+
+ protected:
+  // Explicitly set that the data type of output of the linear_chain_crf_grad
+  // operator is determined by its input: graidents of LogLikelihood.


Typo: graidents -> gradients

Done. Thank you.

abhinavarora · 2017-10-27T22:26:39Z

paddle/operators/linear_chain_crf_op.h

+}
+}  // namespace
+
+using framework::LoDTensor;


This violates the Google Style Guide. For more details check out https://google.github.io/styleguide/cppguide.html#Namespaces

abhinavarora · 2017-10-27T22:28:35Z

paddle/operators/linear_chain_crf_op.h

+    auto* label = ctx.Input<LoDTensor>("Label");
+
+    auto in_lod = emission_weights->lod();
+    PADDLE_ENFORCE(in_lod.size(), "Input(Emission) is not a sequence.");


Error message language could be Input(Emission) must be a sequence

abhinavarora · 2017-10-27T22:29:01Z

paddle/operators/linear_chain_crf_op.h

+    auto in_lod = emission_weights->lod();
+    PADDLE_ENFORCE(in_lod.size(), "Input(Emission) is not a sequence.");
+
+    // TODO(caoying) The checks related to LoD information should be


You can follow the Google C++ stye guide for TODO comments. http://google.github.io/styleguide/cppguide.html#TODO_Comments

lcy-seso · 2017-10-30T02:06:22Z

I have not finished yet, so, please do not begin the review. Thank you all.

lcy-seso · 2017-10-31T11:13:35Z

After discussion with @qingqing01, to make this operator can also run on GPU device, in this PR, I register the GPU kernel which simply copies the inputs and outputs between the CPU/GPU devices. This copy introduces many local variables and makes the codes looks not very clean. I will keep on optimizing in the following PR.

2. Unfortunately, after I merged the lasted develop branch, the previously succeeded gradient check fails. Many related issues have been created and some the operators have been removed from the unittest (for example, the gru_unit_op, recurrent_op, lstm_unit_op, softmax_with_cross_entropy ). It seems that some changes of the gradient check framework make the unit test of this operator fail. I create this issue #5250.

I will enhance the unit test by adding more fixed boundary cases in the next PR.

Thank you all for all of the comments ~

qingqing01

I approve this PR. The enhancements if have can be done in next PR.

qingqing01 · 2017-11-01T02:19:48Z

paddle/operators/linear_chain_crf_op.h

+    if (platform::is_gpu_place(ctx.GetPlace())) {
+      emission_weights = &emission_weight_tensor;
+      transition_weights = &transition_weight_tensor;
+      label = &label_tensor;


For short lines code, the LoDTensor xx and Tensor xx from line 72 to line 86 can be removed, then use new LoDTensor() and new Tensor() here for the pointers from line 72 to line 86. And delete these pointers at last.

qingqing01 · 2017-11-01T02:21:43Z

paddle/operators/linear_chain_crf_op.h

+
+    Tensor emission_row_max;
+    emission_row_max.mutable_data<T>(
+        framework::make_ddim({static_cast<int>(batch_size), 1}),


static_cast<int64_t>

The number type in DDim is int64_t.

qingqing01 · 2017-11-01T02:24:52Z

paddle/operators/cross_entropy_op.cc

@@ -38,8 +39,8 @@ class CrossEntropyOp : public framework::OperatorWithKernel {
                        "If Attr(soft_label) == true, the 2nd dimension of "
                        "Input(X) and Input(Label) should be equal.");
    } else {
-      PADDLE_ENFORCE_EQ(label_dims[1], 1,
-                        "If Attr(soft_label) == false, the 2nd dimension of "
+      PADDLE_ENFORCE_EQ(label_dims[1], 1UL,


The dimension type is int64_t.

pengli09

Approve as requested by @lcy-seso . However, there are still a few places need to be updated/fixed. Please revise the code as soon as possible once you have time.

lcy-seso · 2017-11-01T03:15:04Z

I will enhance the unittest, and refine the codes in the following PR. Thanks for all the comments.

lcy-seso added 13 commits September 29, 2017 21:38

initialize crf opreator.

735737d

Merge branch 'develop' into crf

dcfbbd3

add python forward unittest.

d92c671

Merge branch 'develop' into crf

515981d

add the forward operator.

91cc5d6

Merge branch 'develop' into crf

cbcf11d

add forward computation of crf operator.

cc220ee

Merge branch 'develop' into crf

3123e3c

fix forward and add backward.

80a5ee0

Merge branch 'develop' into crf

c8d0d37

fix the computation kernels.

427644b

Merge branch 'develop' into crf

6a630f2

fix backward computation.

c74107b

lcy-seso force-pushed the crf branch from 3fc5dd0 to c74107b Compare October 25, 2017 15:37

Merge branch 'develop' into crf

4c63086

lcy-seso requested review from abhinavarora and qingqing01 October 26, 2017 02:51

qingqing01 added the OpPorting label Oct 26, 2017

qingqing01 mentioned this pull request Oct 26, 2017

Review operators required by books. #4786

Closed

36 tasks

pengli09 requested changes Oct 26, 2017

View reviewed changes

lcy-seso commented Oct 26, 2017

View reviewed changes

qingqing01 reviewed Oct 26, 2017

View reviewed changes

follow comments.

bce4f7d

lcy-seso force-pushed the crf branch from f873138 to bce4f7d Compare October 27, 2017 01:55

use double in unittest.

3afb9dc

lcy-seso force-pushed the crf branch 3 times, most recently from a8dea8f to 0bcfc00 Compare October 27, 2017 05:52

follow comments.

cca383c

lcy-seso force-pushed the crf branch from 0bcfc00 to cca383c Compare October 27, 2017 06:58

pengli09 requested changes Oct 27, 2017

View reviewed changes

abhinavarora reviewed Oct 27, 2017

View reviewed changes

lcy-seso force-pushed the crf branch from 6796d94 to 76de60b Compare October 30, 2017 23:10

add gpu kernel by copying inputs/outputs between cpu and gpu.

86fd6b6

lcy-seso force-pushed the crf branch 4 times, most recently from cdc0db5 to e95a4c7 Compare October 31, 2017 09:23

NHZlX mentioned this pull request Oct 31, 2017

consider dilation in cnn_output_size Calculation of python api #5245

Closed

lcy-seso force-pushed the crf branch from e95a4c7 to aa7686e Compare October 31, 2017 09:53

Merge branch 'develop' into crf

dd2be3d

lcy-seso force-pushed the crf branch 2 times, most recently from a1a21bb to b990c32 Compare October 31, 2017 10:53

follow comments.

2ac9a3d

lcy-seso force-pushed the crf branch from b990c32 to 2ac9a3d Compare October 31, 2017 12:05

backpropagate gradients the CRF operator receives.

ebd992e

lcy-seso force-pushed the crf branch from c68c9d9 to ebd992e Compare October 31, 2017 15:18

qingqing01 approved these changes Nov 1, 2017

View reviewed changes

pengli09 approved these changes Nov 1, 2017

View reviewed changes

lcy-seso merged commit 8401039 into PaddlePaddle:develop Nov 1, 2017

lcy-seso deleted the crf branch November 1, 2017 03:35

		from op_test import OpTest


		class LinearChainCrfForward(object):

		@@ -0,0 +1,54 @@
		/* Copyright (c) 2016 PaddlePaddle Authors. All Rights Reserve.

add the LinearChainCrf operator. #5084

add the LinearChainCrf operator. #5084

Conversation

lcy-seso commented Oct 25, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lcy-seso Oct 26, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lcy-seso Oct 26, 2017 • edited

Choose a reason for hiding this comment

lcy-seso left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lcy-seso Oct 26, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qingqing01 Oct 26, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pengli09 Oct 27, 2017 • edited by lcy-seso

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abhinavarora left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lcy-seso commented Oct 30, 2017

lcy-seso commented Oct 31, 2017 • edited

qingqing01 left a comment

lcy-seso commented Oct 25, 2017 •

edited

lcy-seso Oct 26, 2017 •

edited

lcy-seso Oct 26, 2017 •

edited

lcy-seso Oct 26, 2017 •

edited

qingqing01 Oct 26, 2017 •

edited

pengli09 Oct 27, 2017 •

edited by lcy-seso

lcy-seso commented Oct 31, 2017 •

edited