From a204fefe16814d9ef5fcad4071daf79207d5dc36 Mon Sep 17 00:00:00 2001 From: fengjiayi Date: Wed, 18 Oct 2017 14:45:32 -0700 Subject: [PATCH] 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 | 14 ++++++-------- paddle/framework/block_desc.cc | 11 ++++++++++- paddle/framework/block_desc.h | 10 +++++++--- paddle/pybind/protobuf.cc | 4 ++-- python/paddle/v2/framework/framework.py | 5 ++++- python/paddle/v2/framework/tests/test_layers.py | 13 ++++++++++++- .../v2/framework/tests/test_protobuf_descs.py | 4 +++- 7 files changed, 44 insertions(+), 17 deletions(-) diff --git a/paddle/framework/backward.cc b/paddle/framework/backward.cc index ac80879c541..fb552fe3448 100644 --- a/paddle/framework/backward.cc +++ b/paddle/framework/backward.cc @@ -309,8 +309,7 @@ static void CreateGradVarInBlock( } std::vector> MakeOpGrad( - const std::unique_ptr& op_desc, - std::unordered_set* no_grad_vars, + const OpDescBind* op_desc, std::unordered_set* no_grad_vars, std::unordered_map* grad_to_var) { std::vector> grad_op_descs; // All input gradients of forwarding operator do not need to calculate. @@ -357,7 +356,7 @@ std::vector> MakeBlockBackward( std::unordered_set* no_grad_vars, std::unordered_map* grad_to_var) { BlockDescBind* cur_block = program_desc.Block(block_idx); - std::deque>& op_descs = cur_block->ops_; + std::vector op_descs = cur_block->AllOps(); std::unordered_map> dup_out_ops; size_t grad_desc_idx = 0; std::vector> backward_descs; @@ -375,7 +374,7 @@ std::vector> 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(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 diff --git a/paddle/framework/block_desc.cc b/paddle/framework/block_desc.cc index ba970254e59..92ac302e46a 100644 --- a/paddle/framework/block_desc.cc +++ b/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 &&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 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(); diff --git a/paddle/framework/block_desc.h b/paddle/framework/block_desc.h index dd7b1228bef..5e1f10c1aef 100644 --- a/paddle/framework/block_desc.h +++ b/paddle/framework/block_desc.h @@ -57,10 +57,16 @@ class BlockDescBind { OpDescBind *AppendOp(); + void AppendAllocatedOp(std::unique_ptr &&op_desc); + OpDescBind *PrependOp(); std::vector 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_; diff --git a/paddle/pybind/protobuf.cc b/paddle/pybind/protobuf.cc index fbdd673295b..d9647717d2a 100644 --- a/paddle/pybind/protobuf.cc +++ b/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(), diff --git a/python/paddle/v2/framework/framework.py b/python/paddle/v2/framework/framework.py index 93e2218eab9..5a8ded46ea4 100644 --- a/python/paddle/v2/framework/framework.py +++ b/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 diff --git a/python/paddle/v2/framework/tests/test_layers.py b/python/paddle/v2/framework/tests/test_layers.py index 2ffadf7371f..2d8c2e55186 100644 --- a/python/paddle/v2/framework/tests/test_layers.py +++ b/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__() diff --git a/python/paddle/v2/framework/tests/test_protobuf_descs.py b/python/paddle/v2/framework/tests/test_protobuf_descs.py index 6ed8edf91c7..2fd3d5d165a 100644 --- a/python/paddle/v2/framework/tests/test_protobuf_descs.py +++ b/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]) -- GitLab