Merge pull request #3 from reyoung/feature/refactorize_framework_proto

Step 1: Make code compile well.
qingqing01 · Aug 8, 2017 · d97a2b4 · d97a2b4
2 parents 72e3ba5 + dba618c
commit d97a2b4
Show file tree

Hide file tree

Showing 35 changed files with 927 additions and 960 deletions.
diff --git a/.gitignore b/.gitignore
@@ -24,4 +24,5 @@ cmake-build-*
 python/paddle/v2/framework/core.so
 CMakeFiles
 cmake_install.cmake
-
+paddle/.timestamp
+python/paddlepaddle.egg-info/
diff --git a/paddle/framework/attribute.cc b/paddle/framework/attribute.cc
@@ -44,7 +44,7 @@ AttrType AttrTypeID<std::vector<std::string>>() {
   return STRINGS;
 }
 
-Attribute GetAttrValue(const AttrDesc& attr_desc) {
+Attribute GetAttrValue(const OpDesc::Attr& attr_desc) {
   switch (attr_desc.type()) {
     case paddle::framework::AttrType::INT: {
       return attr_desc.i();

diff --git a/paddle/framework/attribute.h b/paddle/framework/attribute.h
@@ -21,8 +21,7 @@ limitations under the License. */
 #include <unordered_set>
 #include <vector>
 
-#include "paddle/framework/attribute.pb.h"
-#include "paddle/framework/op_desc.pb.h"
+#include "paddle/framework/framework.pb.h"
 #include "paddle/platform/enforce.h"
 
 namespace paddle {
@@ -37,7 +36,7 @@ typedef std::unordered_map<std::string, Attribute> AttributeMap;
 template <typename T>
 AttrType AttrTypeID();
 
-Attribute GetAttrValue(const AttrDesc& attr_desc);
+Attribute GetAttrValue(const OpDesc::Attr& attr_desc);
 
 // check whether a value(attribute) fit a certain limit
 template <typename T>

diff --git a/paddle/framework/backward.cc b/paddle/framework/backward.cc
@@ -20,15 +20,24 @@
 namespace paddle {
 namespace framework {
 
-static bool AllInSet(const std::vector<std::string>& names,
-                     const std::string& suffix,
-                     const std::unordered_set<std::string>& set) {
+template <typename Map, typename T>
+static void ForEachVarName(Map& names, T callback) {
   for (auto& name : names) {
-    if (set.find(name + suffix) == set.end()) {
-      return false;
+    for (auto& n : name.second) {
+      if (callback(n)) break;
     }
   }
-  return true;
+}
+
+static bool AllInSet(
+    const std::unordered_map<std::string, std::vector<std::string>>& names,
+    const std::string& suffix, const std::unordered_set<std::string>& set) {
+  bool ret_val = true;
+  ForEachVarName(names, [&ret_val, &set, &suffix](const std::string& n) {
+    ret_val = set.find(n + suffix) == set.end();
+    return !ret_val;
+  });
+  return ret_val;
 }
 
 static std::shared_ptr<OperatorBase> NOP() {
@@ -67,10 +76,11 @@ std::shared_ptr<OperatorBase> BackwardRecursive(
   //  Then all input gradients cannot be computed at all, and we put them into
   //  `no_grad_names` set. Return an NOP.
   if (AllInSet(forwardOp.outputs_, kGradVarSuffix, no_grad_names)) {
-    for (auto& name : forwardOp.inputs_) {
-      // Mark all input is not need
-      no_grad_names.insert(name + kGradVarSuffix);
-    }
+    ForEachVarName(forwardOp.inputs_,
+                   [&no_grad_names](const std::string& name) -> bool {
+                     no_grad_names.insert(GradVarName(name));
+                     return false;
+                   });
     return NOP();
   }
 
@@ -92,9 +102,11 @@ std::shared_ptr<OperatorBase> BackwardRecursive(
       auto fwd = *it;
       auto bwd = BackwardRecursive(*fwd, no_grad_names, uniq_id);
       net->AddOp(bwd);
-      for (auto& out : bwd->outputs_) {
-        dup_output_ops[out].emplace_back(local_op_id);
-      }
+      ForEachVarName(bwd->outputs_,
+                     [&dup_output_ops, local_op_id](const std::string& out) {
+                       dup_output_ops[out].emplace_back(local_op_id);
+                       return false;
+                     });
     }
     // Get unique ID for this method.
     auto uid = uniq_id++;
@@ -116,7 +128,7 @@ std::shared_ptr<OperatorBase> BackwardRecursive(
       insert_position.push_back(
           {dup_op.back(),
            OpRegistry::CreateOp(
-               "add", {dup_outputs}, {name},
+               "add", {{"X", {dup_outputs}}}, {{"Out", {name}}},
                {{"input_format",
                  std::vector<int>{0, static_cast<int>(dup_outputs.size())}}})});
     }
@@ -130,24 +142,29 @@ std::shared_ptr<OperatorBase> BackwardRecursive(
 
   } else {
     std::shared_ptr<OperatorBase> grad_op = OpRegistry::CreateGradOp(forwardOp);
-    for (std::string& grad_input : grad_op->inputs_) {
+
+    ForEachVarName(grad_op->inputs_, [&no_grad_names,
+                                      &net](std::string& grad_input) {
       if (no_grad_names.count(grad_input)) {
         std::string prefix =
             grad_input.substr(0, grad_input.size() - kGradVarSuffix.size());
         grad_input = prefix + kZeroVarSuffix;
 
         // If part of input gradient of that operator is not calculated, fill
         // zero variables to that input gradient.
-        net->AddOp(OpRegistry::CreateOp("fill_zeros_like", {prefix},
-                                        {grad_input}, {}));
+        net->AddOp(OpRegistry::CreateOp("fill_zeros_like", {{"Src", {prefix}}},
+                                        {{"Dst", {grad_input}}}, {}));
       }
-    }
-
-    for (std::string& grad_output : grad_op->outputs_) {
-      if (no_grad_names.count(grad_output)) {
-        grad_output = kEmptyVarName;
-      }
-    }
+      return false;
+    });
+
+    ForEachVarName(grad_op->outputs_,
+                   [&no_grad_names](std::string& grad_output) {
+                     if (no_grad_names.count(grad_output)) {
+                       grad_output = kEmptyVarName;
+                     }
+                     return false;
+                   });
 
     if (net->ops_.empty()) {  // Current no aux op is added to network
       return grad_op;