feature/dynamic recurrent op forward and backward #4799

Superjomn · 2017-10-13T21:34:25Z

commit cc50ca9 Author: superjom <superjom@gmail.com> Date: Wed Oct 11 16:21:36 2017 -0400 clean code commit 3850325 Merge: 15d54bb b504a23 Author: superjom <superjom@gmail.com> Date: Wed Oct 11 16:16:30 2017 -0400 Merge branch 'develop' of github.com:PaddlePaddle/Paddle into feature/dynamic-recurrent-op-forward-test commit 15d54bb Author: superjom <superjom@gmail.com> Date: Wed Oct 11 16:13:48 2017 -0400 forward run smoothly commit d8b0f14 Author: superjom <superjom@gmail.com> Date: Tue Oct 10 20:39:26 2017 -0400 using namespace related commit a8ad874 Merge: 150acac c193f82 Author: superjom <superjom@gmail.com> Date: Tue Oct 10 20:38:45 2017 -0400 Merge branch 'develop' of github.com:PaddlePaddle/Paddle into feature/dynamic-recurrent-op-forward-test commit 150acac Author: superjom <superjom@gmail.com> Date: Tue Oct 10 19:40:57 2017 -0400 add DySeqBatch commit f282903 Author: superjom <superjom@gmail.com> Date: Tue Oct 10 18:23:25 2017 -0400 create outputs in step scopes commit 066cbb6 Author: superjom <superjom@gmail.com> Date: Tue Oct 10 18:21:57 2017 -0400 format commit 50c033b Author: superjom <superjom@gmail.com> Date: Tue Oct 10 18:19:23 2017 -0400 init reorder boot commit b5e8a8a Author: superjom <superjom@gmail.com> Date: Tue Oct 10 13:56:31 2017 -0400 merge dynamic op changes commit dccf0cc Author: superjom <superjom@gmail.com> Date: Mon Oct 9 19:45:24 2017 -0400 small fix commit 4a0cc85 Merge: 50c364e d30ada2 Author: superjom <superjom@gmail.com> Date: Mon Oct 9 19:44:40 2017 -0400 Merge branch 'feature/dynamic-recurrent-op' into feature/dynamic-recurrent-op-forward-test commit d30ada2 Author: superjom <superjom@gmail.com> Date: Mon Oct 9 19:01:20 2017 -0400 fix missing lib commit 50c364e Merge: e99a4ce 383faaf Author: superjom <superjom@gmail.com> Date: Mon Oct 9 17:44:16 2017 -0400 Merge branch 'develop' of github.com:PaddlePaddle/Paddle into feature/dynamic-recurrent-op-forward-test commit 3cfaa8b Merge: 58d53f1 d23cd51 Author: superjom <superjom@gmail.com> Date: Mon Oct 9 16:59:02 2017 -0400 Merge branch 'develop' of github.com:PaddlePaddle/Paddle into feature/dynamic-recurrent-op commit 58d53f1 Merge: e578fc3 e12ec95 Author: superjom <superjom@gmail.com> Date: Mon Oct 9 15:57:32 2017 -0400 Merge branch 'develop' of github.com:PaddlePaddle/Paddle into feature/dynamic-recurrent-op commit e578fc3 Author: superjom <superjom@gmail.com> Date: Mon Oct 9 15:56:59 2017 -0400 add definition for WITH_TESTING commit e99a4ce Author: superjom <superjom@gmail.com> Date: Mon Oct 9 15:56:18 2017 -0400 add pybind commit df46223 Author: superjom <superjom@gmail.com> Date: Mon Oct 9 14:19:05 2017 -0400 fix maker name commit 0e40af5 Merge: 0c97e0c 0ff540c Author: superjom <superjom@gmail.com> Date: Mon Oct 9 14:14:36 2017 -0400 Merge branch 'develop' of github.com:PaddlePaddle/Paddle into feature/dynamic-recurrent-op commit 0c97e0c Author: superjom <superjom@gmail.com> Date: Mon Oct 9 14:14:16 2017 -0400 fix gtest_prod.h loss error commit 7245bac Author: superjom <superjom@gmail.com> Date: Mon Oct 9 14:13:37 2017 -0400 make states a map commit 72706ae Merge: 9b1bfc5 d9585f9 Author: superjom <superjom@gmail.com> Date: Mon Oct 9 13:38:33 2017 -0400 Merge branch 'develop' of github.com:PaddlePaddle/Paddle into feature/dynamic-recurrent-op-forward-test commit 21b84bd Author: superjom <superjom@gmail.com> Date: Mon Oct 9 13:35:24 2017 -0400 fix cuda error commit b83ffae Merge: cdc6e5d 4c96008 Author: superjom <superjom@gmail.com> Date: Fri Oct 6 20:06:06 2017 -0400 Merge branch 'develop' of github.com:PaddlePaddle/Paddle into feature/dynamic-recurrent-op commit 9b1bfc5 Merge: 9b4991c 3e82922 Author: superjom <superjom@gmail.com> Date: Fri Oct 6 18:11:55 2017 -0400 Merge branch 'develop' of github.com:PaddlePaddle/Paddle into feature/dynamic-recurrent-op-forward-test commit 9b4991c Author: superjom <superjom@gmail.com> Date: Thu Oct 5 13:59:47 2017 -0400 fix gtest commit cdc6e5d Author: superjom <superjom@gmail.com> Date: Thu Oct 5 13:55:05 2017 -0400 fix gtest commit d300ce6 Author: superjom <superjom@gmail.com> Date: Thu Oct 5 13:54:27 2017 -0400 add pybind commit d5349e1 Author: superjom <superjom@gmail.com> Date: Wed Oct 4 18:41:56 2017 -0400 clean code commit 7d59aee Author: superjom <superjom@gmail.com> Date: Wed Oct 4 18:40:11 2017 -0400 recover op_registry commit 13c1d6d Author: superjom <superjom@gmail.com> Date: Wed Oct 4 18:38:58 2017 -0400 recover op_registry commit 728b4de Merge: f94f0a8 76169e0 Author: superjom <superjom@gmail.com> Date: Wed Oct 4 18:06:43 2017 -0400 Merge branch 'feature/dynamic-recurrent-op' of github.com:Superjom/Paddle into feature/dynamic-recurrent-op commit f94f0a8 Merge: cf87cd6 15b35f9 Author: superjom <superjom@gmail.com> Date: Wed Oct 4 18:06:36 2017 -0400 Merge branch 'develop' of github.com:PaddlePaddle/Paddle into feature/dynamic-recurrent-op commit cf87cd6 Author: superjom <superjom@gmail.com> Date: Wed Oct 4 18:06:08 2017 -0400 finish commit 76169e0 Author: superjom <superjom@gmail.com> Date: Wed Oct 4 18:04:54 2017 -0400 finish debug commit d432f17 Author: superjom <superjom@gmail.com> Date: Wed Oct 4 13:16:15 2017 -0400 finish forward implementation commit 85dac0f Author: superjom <superjom@gmail.com> Date: Mon Oct 2 22:52:59 2017 -0400 add test for dynamic recurrent op commit fceaad8 Merge: aaaed8b c705f06 Author: superjom <superjom@gmail.com> Date: Mon Oct 2 16:34:46 2017 -0400 Merge branch 'develop' of github.com:PaddlePaddle/Paddle into feature/dynamic-recurrent-op commit aaaed8b Merge: cf228a7 a80e010 Author: superjom <superjom@gmail.com> Date: Mon Oct 2 13:44:41 2017 -0400 Merge branch 'develop' of github.com:PaddlePaddle/Paddle into feature/dynamic-recurrent-op commit cf228a7 Author: superjom <superjom@gmail.com> Date: Mon Oct 2 13:40:51 2017 -0400 add registry commit c138e6b Author: superjom <superjom@gmail.com> Date: Sat Sep 30 19:53:59 2017 -0400 add Run commit b3d28ae Merge: d77fb89 2719ff6 Author: superjom <superjom@gmail.com> Date: Sat Sep 30 18:44:39 2017 -0400 Merge branch 'feature/tensor_array' into feature/dynamic-recurrent-op commit 2719ff6 Merge: 956f949 0900aed Author: superjom <superjom@gmail.com> Date: Sat Sep 30 18:17:15 2017 -0400 Merge branch 'develop' of github.com:PaddlePaddle/Paddle into feature/tensor_array commit 956f949 Author: superjom <superjom@gmail.com> Date: Sat Sep 30 15:21:22 2017 -0400 set type commit 870be95 Author: superjom <superjom@gmail.com> Date: Sat Sep 30 15:17:10 2017 -0400 update commit d77fb89 Author: superjom <superjom@gmail.com> Date: Sat Sep 30 15:03:27 2017 -0400 add implementation commit 661f30b Author: superjom <superjom@gmail.com> Date: Wed Sep 27 22:09:41 2017 -0400 add tensor array

…/dynamic_recurrent_op_forward_and_backward

wangkuiyi · 2017-10-15T14:29:34Z

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

+    def create_forward_op(self):
+        # create RNNOp
+        self.forward_op = DynamicRecurrentOp(
+            # inputs


inlinks => inputs
boot_memories => initial_memory_value
step_net => step
outlinks => outputs
pre_memories => ex_memory

As you are making something innovative, I guess you don't want to copy †he terminology simply from Caffe2.

wangkuiyi · 2017-10-15T14:31:08Z

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

+            boot_memories=["h_boot"],
+            step_net="stepnet",
+            # outputs
+            outlinks=["h@mem"],


What is the convention to of the name h@mem and h@pre? What does the @ here mean?

Currently, the python API is not completed yet, so the high-level syntax as we described before cannot be used in this unit test.
Both h@mem and h@pre are some underlying names of memory and ex-memory, the work to define these two names should be done by the python API in the future.

wangkuiyi · 2017-10-15T14:32:44Z

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

+        a = set()
+        backward_op = core.DynamicRecurrentOp.backward(self.forward_op, a)
+
+    def create_step_net(self):


In the design, we use Python's with to construct the step block. It seems that this Python test should also do that.

This is an underlying API, the with-statement syntax is not supported yet because the python API is still under developing.

wangkuiyi · 2017-10-15T14:33:33Z

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

+        a = set()
+        backward_op = core.DynamicRecurrentOp.backward(self.forward_op, a)
+
+    def create_step_net(self):


We are not going to do NetOp, but block, here the step is not represented by a NetOp instance, but a block. We mustn't go on using the name "step_net".

Currently, these codes still focus on the primary logic of RNN its own. This implementation is based on NetOp because that is stable and simpler.

I will start the migration to block and executor on next Monday.

wangkuiyi · 2017-10-15T14:34:45Z

paddle/operators/dynamic_recurrent_op.h

+   * Link the pre-state of the first time step to the `boot-state` in parent's
+   * scope.
+   */
+  void LinkBootState(const rnn::MemoryAttr& memory);


As we talked before, it is not boot, but bootstrap. A more plain name is initial.

wangkuiyi · 2017-10-15T14:35:19Z

paddle/operators/dynamic_recurrent_op.h

+  /*
+   * different `Run` method for forward and backward.
+   */
+  template <ComputeMode _>


What are the possible values of ComputeMode? Why do we need the _ here? We need at least a class comment here to explain such stuff.

wangkuiyi · 2017-10-15T14:36:05Z

paddle/operators/dynamic_recurrent_op.cc

+
+    // Attributes stored in AttributeMap
+    AddAttr<std::vector<std::string>>(name.pre_memories,
+                                      "names of pre-memories");


pre-memory => ex-memory

wangkuiyi · 2017-10-15T14:37:33Z

paddle/operators/dynamic_recurrent_op.h

+  std::map<std::string, framework::TensorArray> step_outputs_;
+  std::map<std::string, std::vector<framework::DySeqMeta>> dy_seq_metas_;
+  rnn::Argument arg_;
+  ArgCache cache_;


What is the purpose of ArgCache? I saw that the terminology in your code includes "inlinks", "outlink", and "memory". Are they in "ArgCache", or what is the relationship between them and "argument"?

This is a cache of all the arguments that located in the parent scope. RNN's computation is split into a bunch of protected methods, it is painful and ugly to pass in the arguments throughout all the member functions, so a cache called ArgCache is added as a member.

There is only one public member function of RNNAlgorthm, which is Run, refresh the ArgCache when Run is invoked.

including inlinks, outlinks, memories and so on.

…/dynamic_recurrent_op_forward_and_backward

jacquesqiao

LGTM!

Superjomn added 5 commits October 12, 2017 19:01

finish factor for forward and backward

c358062

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

94b484e

…/dynamic_recurrent_op_forward_and_backward

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

7e859d7

…/dynamic_recurrent_op_forward_and_backward

add gradient test

35aa445

Superjomn requested review from jacquesqiao, qingqing01 and luotao1 October 13, 2017 23:44

Superjomn added 2 commits October 13, 2017 19:58

clean mutable and const

63fd336

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

530630a

…/dynamic_recurrent_op_forward_and_backward

wangkuiyi requested changes Oct 15, 2017

View reviewed changes

Superjomn added 11 commits October 15, 2017 14:58

refactor the names

8bcb965

including inlinks, outlinks, memories and so on.

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

85f72a4

…/dynamic_recurrent_op_forward_and_backward

fix py test

a3f3ed6

fix recurrent_op test

621f599

init

cdde5cf

restore

3b403ef

restore rnn.md

f26a627

format

74a6d02

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

04dbb3a

…/dynamic_recurrent_op_forward_and_backward

update

68cac3c

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

1d3cd8b

…/dynamic_recurrent_op_forward_and_backward

wangkuiyi previously approved these changes Oct 20, 2017

View reviewed changes

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

603ce33

…/dynamic_recurrent_op_forward_and_backward

Superjomn dismissed wangkuiyi’s stale review via 603ce33 October 20, 2017 03:44

wangkuiyi previously approved these changes Oct 20, 2017

View reviewed changes

fix conflicts

97b5df6

Superjomn dismissed wangkuiyi’s stale review via 97b5df6 October 20, 2017 04:17

jacquesqiao approved these changes Oct 20, 2017

View reviewed changes

Superjomn merged commit 07ea9ad into PaddlePaddle:develop Oct 20, 2017

Superjomn deleted the feature/dynamic_recurrent_op_forward_and_backward branch October 20, 2017 17:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature/dynamic recurrent op forward and backward #4799

feature/dynamic recurrent op forward and backward #4799

Superjomn commented Oct 13, 2017 •

edited

wangkuiyi Oct 15, 2017

Superjomn Oct 20, 2017

wangkuiyi Oct 15, 2017

Superjomn Oct 15, 2017

wangkuiyi Oct 15, 2017

Superjomn Oct 15, 2017

wangkuiyi Oct 15, 2017

Superjomn Oct 15, 2017

wangkuiyi Oct 15, 2017

wangkuiyi Oct 15, 2017

wangkuiyi Oct 15, 2017

wangkuiyi Oct 15, 2017

Superjomn Oct 15, 2017

jacquesqiao left a comment

feature/dynamic recurrent op forward and backward #4799

feature/dynamic recurrent op forward and backward #4799

Conversation

Superjomn commented Oct 13, 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

jacquesqiao left a comment

Choose a reason for hiding this comment

Superjomn commented Oct 13, 2017 •

edited