Enable hidden/cell state initialization and enhance unit testing in LSTM operator. #5429

qingqing01 · 2017-11-07T07:56:23Z

Enable hidden/cell state initialization in LSTM operator.
Users can disable peephole connections.
Enhance unit testing.
Also, fix the attribute names

1. user can disable peephole connections. 2. not calculate some gradients.

… lstm_fix

QiJune · 2017-11-07T21:04:43Z

paddle/operators/lstm_op.h

        int pre_h_start = static_cast<int>(batch_starts[n - 1]);
        int pre_h_end = pre_h_start + cur_batch_size;
        auto pre_hidden_t = batch_hidden.Slice(pre_h_start, pre_h_end);
        math::matmul<Place, T>(device_ctx, pre_hidden_t, false, *weight, false,
                               static_cast<T>(1.0), &gate_t,
                               static_cast<T>(1.0));
+      } else if (hidden_t0) {


Since H0 is optional, what will happen if hidden_t0 is nullptr?

hidden_t0 is nullptr, means there is no hidden initialization, that is to say the hidden_t0 is zero. And there is no need to compute matmul in line 122.

QiJune · 2017-11-07T23:47:56Z

paddle/operators/lstm_op.cc

                  "whether to enable diagonal/peephole connections.")
        .SetDefault(true);
-    AddAttr<bool>("isReverse",
-                  "(bool, default False) "
+    AddAttr<bool>("is_reverse",


Does LSTM operator only support one level RNN? or can have Nesting LSTM?

It only supports one level.

luotao1

#5420 和 #5428 两个issue内容能补充一下么

luotao1 · 2017-11-08T09:23:15Z

paddle/operators/lstm_op.cc

+
+    auto c0_g_name = framework::GradVarName("C0");
+    if (ctx->HasOutput(c0_g_name))
+      ctx->SetOutputDim(c0_g_name, ctx->GetInputDim("C0"));
  }


248-267能写成（函数名称可再修改下）：

auto setOutputDim = [](string name) { auto g_name = framework::GradVarName(name); if (ctx->HasOutput(g_name)) ctx->SetOutputDim(g_name, ctx->GetInputDim(name)); }; setOutputDim("Input"); setOutputDim("Weight"); ...

luotao1 · 2017-11-08T09:24:21Z

paddle/operators/lstm_op.h

@@ -109,15 +115,23 @@ class LSTMKernel : public framework::OpKernel<T> {

      int cur_batch_size = bend - bstart;

-      if (n != 0) {
+      if (n > 0) {


if (n)就行了

统一改成 n > 0，表明第2时刻开始，意义明确。

luotao1 · 2017-11-08T09:29:35Z

paddle/operators/lstm_op.h

+        row_shuffle(device_ctx, *hidden_t0, order, ordered_h0, true);
+        math::matmul<Place, T>(device_ctx, ordered_h0, false, *weight, false,
+                               static_cast<T>(1.0), &gate_t,
+                               static_cast<T>(1.0));


126-133行的代码，和86-92行的代码，是实现什么功能呢？

需要在这里简单加一下公式或注释么？

这两段代码，除了最后一行不一样，其他实现的功能是一样的（后面190-196行也是）。需要封装一个函数么？

126行和127行，需要在每个for循环里面都定义一遍么？

需要在这里简单加一下公式或注释么？

在代码中加了注释。

这两段代码，除了最后一行不一样，其他实现的功能是一样的（后面190-196行也是）。需要封装一个函数么？

Done.

126行和127行，需要在每个for循环里面都定义一遍么？

不会的，只有n == 0的时候才走这个分支，只执行一边。

luotao1 · 2017-11-08T09:32:38Z

paddle/operators/lstm_op.h

    auto& device_ctx = ctx.device_context();
    math::SetConstant<Place, T> zero;
    if (weight_g) {
      weight_g->mutable_data<T>(ctx.GetPlace());
      zero(device_ctx, weight_g, static_cast<T>(0.0));
    }

+    Tensor ordered_h0, ordered_c0, ordered_h0_g, ordered_c0_g;


这些变量需要加注释说明一下各自代表什么吗？

luotao1 · 2017-11-08T09:34:50Z

paddle/operators/lstm_op.h

      zero(device_ctx, bias_g, static_cast<T>(0.0));
+    }
+    if (bias && bias_g && ctx.Attr<bool>("use_peepholes")) {
+      T* bias_g_data = bias_g->data<T>();


这里的逻辑是，只要221行的三个条件中有一个是false，lstm_grad.checkXXX就为空么

是这样的。没有bias，backward就不会bias_grad；lstm_grad.checkXXX 表示的peephole connection的weight，是个向量，存储在bias里，如果use_peepholes = False, lstm_grad.checkXXX 就是null。

luotao1 · 2017-11-08T09:38:11Z

paddle/operators/lstm_op.h

@@ -226,9 +258,9 @@ class LSTMGradKernel : public framework::OpKernel<T> {
    batch_gate_g.mutable_data<T>(batch_gate->dims(), ctx.GetPlace());
    batch_gate_g.set_lod(batch_gate->lod());


235-259行也可以使用一个函数来缩短代码。同上。

luotao1 · 2017-11-08T09:39:13Z

paddle/operators/lstm_op.h

@@ -250,23 +282,32 @@ class LSTMGradKernel : public framework::OpKernel<T> {
      lstm_grad.gateGrad = gate_g.data<T>();
      lstm_grad.outputGrad = out_g.data<T>();

-      if (n) {
+      if (n > 0) {


n就行了，为什么要改成n>0呢

luotao1 · 2017-11-08T09:39:47Z

paddle/operators/lstm_op.h

+        if (c0) {
+          lstm_value.prevStateValue = ordered_c0.data<T>();
+        } else {
+          lstm_value.prevStateValue = nullptr;


lstm_value.prevStateValue = c0 ? ordered_c0.data(): nullptr;

luotao1 · 2017-11-08T09:42:18Z

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

+            ['Input', 'Weight', 'Bias', 'H0', 'C0'], ['Hidden'],
+            max_relative_error=5e-4)
+
+    # In order to speed up, skip following testing


请问这几个单测很耗时么，大概需要多少时间呢
如果不测的话，这几个单测可以都删掉，下同。

这几个是梯度检测，时间相对较长，具体时间没统计。
改了下单测方式，把这些从基类移到一个子类里测了。

… lstm_fix

qingqing01

@luotao1 Thanks very much.

qingqing01 · 2017-11-09T05:51:08Z

paddle/operators/lstm_op.cc

+
+    auto c0_g_name = framework::GradVarName("C0");
+    if (ctx->HasOutput(c0_g_name))
+      ctx->SetOutputDim(c0_g_name, ctx->GetInputDim("C0"));
  }


qingqing01 · 2017-11-09T05:52:33Z

paddle/operators/lstm_op.h

@@ -109,15 +115,23 @@ class LSTMKernel : public framework::OpKernel<T> {

      int cur_batch_size = bend - bstart;

-      if (n != 0) {
+      if (n > 0) {


统一改成 n > 0，表明第2时刻开始，意义明确。

qingqing01 · 2017-11-09T05:54:04Z

paddle/operators/lstm_op.h

+        row_shuffle(device_ctx, *hidden_t0, order, ordered_h0, true);
+        math::matmul<Place, T>(device_ctx, ordered_h0, false, *weight, false,
+                               static_cast<T>(1.0), &gate_t,
+                               static_cast<T>(1.0));


需要在这里简单加一下公式或注释么？

在代码中加了注释。

这两段代码，除了最后一行不一样，其他实现的功能是一样的（后面190-196行也是）。需要封装一个函数么？

Done.

126行和127行，需要在每个for循环里面都定义一遍么？

不会的，只有n == 0的时候才走这个分支，只执行一边。

qingqing01 · 2017-11-09T05:54:37Z

paddle/operators/lstm_op.h

    auto& device_ctx = ctx.device_context();
    math::SetConstant<Place, T> zero;
    if (weight_g) {
      weight_g->mutable_data<T>(ctx.GetPlace());
      zero(device_ctx, weight_g, static_cast<T>(0.0));
    }

+    Tensor ordered_h0, ordered_c0, ordered_h0_g, ordered_c0_g;


qingqing01 · 2017-11-09T05:56:19Z

paddle/operators/lstm_op.h

      zero(device_ctx, bias_g, static_cast<T>(0.0));
+    }
+    if (bias && bias_g && ctx.Attr<bool>("use_peepholes")) {
+      T* bias_g_data = bias_g->data<T>();


是这样的。没有bias，backward就不会bias_grad；lstm_grad.checkXXX 表示的peephole connection的weight，是个向量，存储在bias里，如果use_peepholes = False, lstm_grad.checkXXX 就是null。

qingqing01 · 2017-11-09T06:11:31Z

paddle/operators/lstm_op.h

@@ -226,9 +258,9 @@ class LSTMGradKernel : public framework::OpKernel<T> {
    batch_gate_g.mutable_data<T>(batch_gate->dims(), ctx.GetPlace());
    batch_gate_g.set_lod(batch_gate->lod());


qingqing01 · 2017-11-09T06:17:03Z

paddle/operators/lstm_op.h

+        if (c0) {
+          lstm_value.prevStateValue = ordered_c0.data<T>();
+        } else {
+          lstm_value.prevStateValue = nullptr;


qingqing01 · 2017-11-09T06:22:49Z

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

+            ['Input', 'Weight', 'Bias', 'H0', 'C0'], ['Hidden'],
+            max_relative_error=5e-4)
+
+    # In order to speed up, skip following testing


这几个是梯度检测，时间相对较长，具体时间没统计。
改了下单测方式，把这些从基类移到一个子类里测了。

luotao1

LGTM

qingqing01 added 4 commits November 6, 2017 22:39

Enable initial hidden state and cell state in LSTM Operator.

0c70bd2

Update and resolve conflicts.

d851daf

Enhance unit testing.

d94c936

1. user can disable peephole connections. 2. not calculate some gradients.

Merge branch 'develop' of https://github.com/PaddlePaddle/Paddle into…

ed2d30b

… lstm_fix

qingqing01 requested review from luotao1 and guoshengCS November 7, 2017 07:56

qingqing01 changed the title ~~Enable initial hidden/cell state and enhance unit testing in LSTM Operator.~~ Enable hidden/cell state initialization and enhance unit testing in LSTM Operator. Nov 7, 2017

qingqing01 changed the title ~~Enable hidden/cell state initialization and enhance unit testing in LSTM Operator.~~ Enable hidden/cell state initialization and enhance unit testing in LSTM operator. Nov 7, 2017

QiJune reviewed Nov 7, 2017

View reviewed changes

qingqing01 mentioned this pull request Nov 8, 2017

Some operators do not follow naming convention #5366

Closed

6 tasks

luotao1 reviewed Nov 8, 2017

View reviewed changes

qingqing01 added 3 commits November 9, 2017 12:55

Merge branch 'develop' of https://github.com/PaddlePaddle/Paddle into…

4098ce7

… lstm_fix

follow comments.

d60fe75

Merge branch 'develop' of https://github.com/PaddlePaddle/Paddle into…

8bf3799

… lstm_fix

qingqing01 commented Nov 9, 2017

View reviewed changes

luotao1 approved these changes Nov 9, 2017

View reviewed changes

qingqing01 merged commit 41d0533 into PaddlePaddle:develop Nov 9, 2017

qingqing01 deleted the lstm_fix branch March 7, 2018 12:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable hidden/cell state initialization and enhance unit testing in LSTM operator. #5429

Enable hidden/cell state initialization and enhance unit testing in LSTM operator. #5429

qingqing01 commented Nov 7, 2017 •

edited

Loading

QiJune Nov 7, 2017

qingqing01 Nov 8, 2017

QiJune Nov 7, 2017 •

edited

Loading

qingqing01 Nov 8, 2017

luotao1 left a comment

luotao1 Nov 8, 2017

qingqing01 Nov 9, 2017

luotao1 Nov 8, 2017

qingqing01 Nov 9, 2017

luotao1 Nov 8, 2017

qingqing01 Nov 9, 2017

luotao1 Nov 8, 2017

qingqing01 Nov 9, 2017

luotao1 Nov 8, 2017

qingqing01 Nov 9, 2017

luotao1 Nov 8, 2017

qingqing01 Nov 9, 2017

luotao1 Nov 8, 2017

luotao1 Nov 8, 2017

qingqing01 Nov 9, 2017

luotao1 Nov 8, 2017

qingqing01 Nov 9, 2017

qingqing01 left a comment

qingqing01 Nov 9, 2017

qingqing01 Nov 9, 2017

qingqing01 Nov 9, 2017

qingqing01 Nov 9, 2017

qingqing01 Nov 9, 2017

qingqing01 Nov 9, 2017

qingqing01 Nov 9, 2017

qingqing01 Nov 9, 2017

luotao1 left a comment

		@@ -226,9 +258,9 @@ class LSTMGradKernel : public framework::OpKernel<T> {
		batch_gate_g.mutable_data<T>(batch_gate->dims(), ctx.GetPlace());
		batch_gate_g.set_lod(batch_gate->lod());

Enable hidden/cell state initialization and enhance unit testing in LSTM operator. #5429

Enable hidden/cell state initialization and enhance unit testing in LSTM operator. #5429

Conversation

qingqing01 commented Nov 7, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

QiJune Nov 7, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

luotao1 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

qingqing01 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

luotao1 left a comment

Choose a reason for hiding this comment

qingqing01 commented Nov 7, 2017 •

edited

Loading

QiJune Nov 7, 2017 •

edited

Loading