Fix several bugs in compile time backward and Protobuf desc (#4894)

* Implement FC layer with helper

* Update LayerHelper

* Add debug string for Python ProtoBuf

and Rename `Sync` to `Flush`

* Add check of ProtoBuf initialization

* Layer wrapper for FC

* Fix unittest

* Fix CI

* Add code generator

* AttributeChecker Better error log and speicalize bool

Since lots of types can be cast to bool

* Complete mlp, fit_a_line

* Implementation of simple conv_2d layer

* Fix bugs

* Correct implement BlockDesc destructor

* Fix bugs

* Fix unit test error

* Follow comments

paddle/framework/backward.cc

@@ -309,8 +309,7 @@ static void CreateGradVarInBlock(
 }
 
 std::vector<std::unique_ptr<OpDescBind>> MakeOpGrad(
-    const std::unique_ptr<OpDescBind>& op_desc,
-    std::unordered_set<std::string>* no_grad_vars,
+    const OpDescBind* op_desc, std::unordered_set<std::string>* no_grad_vars,
     std::unordered_map<std::string, std::string>* grad_to_var) {
   std::vector<std::unique_ptr<OpDescBind>> grad_op_descs;
   // All input gradients of forwarding operator do not need to calculate.
@@ -357,7 +356,7 @@ std::vector<std::unique_ptr<OpDescBind>> MakeBlockBackward(
     std::unordered_set<std::string>* no_grad_vars,
     std::unordered_map<std::string, std::string>* grad_to_var) {
   BlockDescBind* cur_block = program_desc.Block(block_idx);
-  std::deque<std::unique_ptr<OpDescBind>>& op_descs = cur_block->ops_;
+  std::vector<OpDescBind*> op_descs = cur_block->AllOps();
   std::unordered_map<std::string, std::vector<size_t>> dup_out_ops;
   size_t grad_desc_idx = 0;
   std::vector<std::unique_ptr<OpDescBind>> backward_descs;
@@ -375,7 +374,7 @@ std::vector<std::unique_ptr<OpDescBind>> MakeBlockBackward(
           program_desc, step_block_idx, no_grad_vars, grad_to_var);
       BlockDescBind* backward_block = program_desc.AppendBlock(*cur_block);
       for (auto& ptr : backward_block_op_descs) {
-        backward_block->ops_.push_back(std::move(ptr));
+        backward_block->AppendAllocatedOp(std::move(ptr));
       }
       op_grads[0]->SetBlockAttr("step_block", *backward_block);
     }
@@ -432,7 +431,6 @@ ParamGradInfoMap AppendBackward(
 
   const int root_block_idx = 0;
   auto root_block = program_desc.Block(root_block_idx);
-  auto& all_ops = root_block->ops_;
 
   // insert fill one op for target
   // TODO(qiao) add some check to the target.
@@ -447,8 +445,8 @@ ParamGradInfoMap AppendBackward(
                      {{"shape", target_shape},
                       {"value", static_cast<float>(1.0)},
                       {"data_type", framework::DataType::FP32}}));
-  all_ops.push_back(std::move(fill_one_op));
-  size_t forward_op_num = all_ops.size();
+  root_block->AppendAllocatedOp(std::move(fill_one_op));
+  size_t forward_op_num = root_block->OpSize();
   size_t forward_block_num = program_desc.Size();
 
   // Insert backward operators
@@ -457,7 +455,7 @@ ParamGradInfoMap AppendBackward(
                                              &no_grad_var_names, &grad_to_var);
 
   for (auto& ptr : backward_op_descs) {
-    all_ops.push_back(std::move(ptr));
+    root_block->AppendAllocatedOp(std::move(ptr));
   }
   // Create Variable
 

paddle/framework/block_desc.cc

@@ -19,11 +19,11 @@ namespace paddle {
 namespace framework {
 
 VarDescBind *BlockDescBind::Var(const std::string &name) {
-  need_update_ = true;
   auto it = vars_.find(name);
   if (it != vars_.end()) {
     return it->second.get();
   }
+  need_update_ = true;
   auto *var = new VarDescBind(name);
   vars_[name].reset(var);
   return var;
@@ -55,6 +55,11 @@ OpDescBind *BlockDescBind::AppendOp() {
   return ops_.back().get();
 }
 
+void BlockDescBind::AppendAllocatedOp(std::unique_ptr<OpDescBind> &&op_desc) {
+  need_update_ = true;
+  ops_.emplace_back(std::move(op_desc));
+}
+
 OpDescBind *BlockDescBind::PrependOp() {
   need_update_ = true;
   ops_.emplace_front(new OpDescBind());
@@ -70,6 +75,10 @@ std::vector<OpDescBind *> BlockDescBind::AllOps() const {
 }
 
 void BlockDescBind::Flush() {
+  for (auto &op_desc : ops_) {
+    op_desc->Flush();
+  }
+
   if (need_update_) {
     auto &op_field = *this->desc_->mutable_ops();
     this->ClearPBOps();

paddle/framework/block_desc.h

@@ -57,10 +57,16 @@ class BlockDescBind {
 
   OpDescBind *AppendOp();
 
+  void AppendAllocatedOp(std::unique_ptr<OpDescBind> &&op_desc);
+
   OpDescBind *PrependOp();
 
   std::vector<OpDescBind *> AllOps() const;
 
+  size_t OpSize() const { return ops_.size(); }
+
+  OpDescBind *Op(int idx) { return ops_.at(idx).get(); }
+
   void Flush();
 
   BlockDesc *Proto();
@@ -69,9 +75,7 @@ class BlockDescBind {
   void ClearPBOps();
   void ClearPBVars();
 
-  // FIXME(yuyang18): backward will access private data of BlockDesc.
-  // Mark it public temporary. We can fix it later.
- public:
+ private:
   ProgramDescBind *prog_;  // not_own
   BlockDesc *desc_;        // not_own
   bool need_update_;

paddle/pybind/protobuf.cc

@@ -162,8 +162,8 @@ void BindBlockDesc(py::module &m) {
            py::return_value_policy::reference)
       .def("all_vars", &BlockDescBind::AllVars,
            py::return_value_policy::reference)
-      .def("all_ops", &BlockDescBind::AllOps,
-           py::return_value_policy::reference)
+      .def("op_size", &BlockDescBind::OpSize)
+      .def("op", &BlockDescBind::Op, py::return_value_policy::reference)
       .def("serialize_to_string", [](BlockDescBind &block_desc) -> py::bytes {
         const BlockDesc *desc = block_desc.Proto();
         PADDLE_ENFORCE(desc->IsInitialized(),

python/paddle/v2/framework/framework.py

@@ -344,7 +344,10 @@ class Block(object):
                 self.create_var(name=var.name(), desc=var, type=var.type())
 
         # sync operators from cpp
-        ops_in_cpp = self.desc.all_ops()
+        ops_in_cpp = []
+        for op_idx in range(0, self.desc.op_size()):
+            ops_in_cpp.append(self.desc.op(op_idx))
+
         first_op_in_python = self.ops[0].desc
         last_op_in_python = self.ops[len(self.ops) - 1].desc
         start_index = None

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

@@ -17,6 +17,7 @@ class TestBook(unittest.TestCase):
 
         avg_cost = mean(x=cost, program=program)
         self.assertIsNotNone(avg_cost)
+        program.append_backward(avg_cost, set())
         print str(program)
 
     def test_recognize_digits_mlp(self):
@@ -34,7 +35,17 @@ class TestBook(unittest.TestCase):
         cost = cross_entropy(input=predict, label=label, program=program)
         avg_cost = mean(x=cost, program=program)
         self.assertIsNotNone(avg_cost)
-        print str(program)
+        # print str(program)
+
+    def test_simple_conv2d(self):
+        pd = core.ProgramDesc.__create_program_desc__()
+        program = Program(desc=pd)
+        images = data_layer(
+            name='pixel', shape=[3, 48, 48], data_type='int32', program=program)
+        conv2d_layer(
+            input=images, num_filters=3, filter_size=[4, 4], program=program)
+
+        # print str(program)
 
     def test_simple_conv2d(self):
         pd = core.ProgramDesc.__create_program_desc__()

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

@@ -133,7 +133,9 @@ class TestBlockDesc(unittest.TestCase):
         op1 = block.append_op()
         op2 = block.append_op()
         op0 = block.prepend_op()
-        all_ops = block.all_ops()
+        all_ops = []
+        for idx in xrange(0, block.op_size()):
+            all_ops.append(block.op(idx))
         self.assertEqual(all_ops, [op0, op1, op2])