add piano graph executor

[thisjiang]

PR types

New features

PR changes

Others

Describe

An executor accept sub-graph which is generated by PianoCompilePass, run each op's PianoOpKernel, finally return the graph's ModuleProto.

Parameter:

1. graph_id: the unique graph id, used for generating unique notebuilder name
2. cluster: a vector which contains all graph op, non-topological-sorting.
3. cluster_inputs: a vector which contains all graph's input var, the var's input are outside op, the output are inside op
4. cluster_outputs: a vector which contains all graph's output var, the var's input are inside op, the output are outside op
5. pcluster_internals: a vector which contains all graph's internal var, the var's input and output are inside op

Describe:

The executor consisted by the following step:

1. create a NoteBuilder, it's name is unique for each graph
2. create PianoScope, initially, scope only consist graph's input var and its operand
3. topological sorting graph
4. create PianoOpKernelContext and run each op's PianoOpKernel
5. run NoteBuilder's Build function to generate graph's ModuleProto

[CtfGo]

这个模板函数有在哪里用到吗

[thisjiang]

暂时并没有用到。。。

[CtfGo]

符号执行的子步骤单测需要测下吗

[thisjiang]

这些子步骤都写在了.cc文件里，单独拎出来单测有点麻烦。。。目前拓扑排序和RunCompile我是通过Compile函数里往GetCompileOrder添加op_type来测试的，这方面可以完善下。CreateInputOperand是通过ModuleProto的值来判断，这个了解不多，应该也能完善

[wzzju]

为什么拓扑排序要写的如此麻烦，不能使用一个std::vector<Node*, size_t>维护入度吗？

[thisjiang]

不能啊。由于op Node的inputs是var Node，而不同var Node的inputs可能会有重叠，如果用std::vector<Node*, size_t>的话，这重叠的op Node就被重复计算，但后面topo遍历时却只会减一次。

[thisjiang]

形象点说就是这样：

op1   op2
|   /   |
var1    var2
 \    /
  op3

如果用vector，计算op3的入度时，op2会被统计两次。但topo遍历时，op2只遍历了一次，op3的入度也就只减了一次。

[wzzju]

我的理解，即使你说的这种case，下面这种写法也是正确的。

void TopologicSortGraph(const GraphNodeVec& cluster,
                        GraphNodeVec* cluster_sorted) {

  std::unordered_set<Node*> cluster_set(cluster.cbegin(), cluster.cend());
  std::unordered_map<Node*, std::vector<Node*>> adj_list;
  std::unordered_map<Node*, size_t> in_degree;
  std::queue<Node*> queue;

  for (auto* n : cluster) {
    PADDLE_ENFORCE_EQ(n->IsOp(), true,
                  platform::errors::InvalidArgument(
                      "Cluster Sub-Graph all should be op"));
    // the op's input is var
    for (auto* in_var : n->inputs) {
      // the var's input is op
      for (auto* in_op : in_var->inputs) {
        if (cluster_set.count(in_op) != 0) {
          adj_list[in_op].emplace_back(n);
          in_degree[n]++;
        }
      }
    }
  }

  // find topology entries
  for (auto* n : cluster) {
    if (!in_degree[n]) {
      queue.push(n);
    }
  }

  // topological sorting
  while (!queue.empty()) {
    auto* cur_op = queue.front();
    queue.pop();

    cluster_sorted->emplace_back(cur_op);
    for (auto* adj : adj_list[cur_op]) {
      in_degree[adj]--;

      if (!in_degree[adj]) {
        queue.push(adj);
      }
    }
  }

  PADDLE_ENFORCE_EQ(cluster_sorted.size(), cluster.size(),
              platform::errors::PreconditionNotMet(
                  "Cluster Sub-Graph shouldn't contain cycle."));

}

[thisjiang]

这样写的确可以哈，就是adj_list会有重复node。但在重复结点数目不多的情况下，相比unordered_set耗时会短点，我改下

[wzzju]

将std::unordered_map<Node*, std::unordered_set<Node*>> out_ops;替换为std::unordered_map<Node*, size_t>多数情况下也能减少空间吧，另外，重复node出现的概率不会太高且重复的次数一般也较少。这样的代码可读性个人感觉好一些。

[thisjiang]

已修改~

[wzzju]

如果想去重的话，可以试试下面的写法：

void TopologicSortGraph(const GraphNodeVec& cluster,
                        GraphNodeVec* cluster_sorted) {

  std::unordered_set<Node*> cluster_set(cluster.cbegin(), cluster.cend());
  std::unordered_map<Node*, std::unordered_map<Node*, size_t>> adj_list;
  std::unordered_map<Node*, size_t> indegree;
  std::queue<Node*> queue;

  for (auto* n : cluster) {
    PADDLE_ENFORCE_EQ(n->IsOp(), true,
                  platform::errors::InvalidArgument(
                      "Cluster Sub-Graph all should be op"));
    // the op's input is var
    for (auto* in_var : n->inputs) {
      // the var's input is op
      for (auto* in_op : in_var->inputs) {
        if (cluster_set.count(in_op) != 0) {
          ++adj_list[in_op][n];
          ++indegree[n];
        }
      }
    }
  }

  // find topology entries
  for (auto* n : cluster) {
    if (!indegree[n]) {
      queue.push(n);
    }
  }

  // topological sorting
  while (!queue.empty()) {
    auto* cur_op = queue.front();
    queue.pop();

    cluster_sorted->emplace_back(cur_op);
    for(auto it = adj_list[cur_op].begin(); it != adj_list[cur_op].end(); it++){
      indegree[it->first] -= it->second;

      if (!indegree[it->first]) {
        queue.push(it->first);
      }
    }
  }

  PADDLE_ENFORCE_EQ(cluster_sorted.size(), cluster.size(),
              platform::errors::PreconditionNotMet(
                  "Cluster Sub-Graph shouldn't contain cycle."));

}

[wzzju]

这些赋值其实并没有什么必要。

[thisjiang]

in_ops.emplace(n, 0);这个有必要吧？unordered_map对于内置类型不会赋初始值0吧？

[wzzju]

会初始化为0的，参考 https://en.cppreference.com/w/cpp/container/unordered_map/operator_at

[thisjiang]

done

[wzzju]

建议使用indegree和adj_list命名，便于理解。

[thisjiang]

好的，我改下

[wzzju]

LGTM.

[CtfGo]

LGTM

[wzzju]

头文件中定义static 函数并不建议，而且你的map也是static的。如果被多个源文件包含，那么每个源文件会有一份独立的函数和变量。如果多个.cc文件引用这个头文件，会导致link变慢。不建议这么写。

[thisjiang]

Done，已修改为extern形式

[wzzju]

同上。

[thisjiang]

Done

[wzzju]

感觉VarType2NoteType和GetVarDataType的定义可以拿出来放在vartype_utils.cc中，并将vartype2notetype.h更改为vartype_utils.h，让后将VarType2NoteType和GetVarDataType的声明放在vartype_utils.h。GetVarDataType放在类中作为static函数，但是又感觉与PianoGraphExecutor无关。

[thisjiang]

Done

[wzzju]

1）将CreateInputOperand声明为PianoScope CreateInputOperand(symbolization::NoteBuilder* builder)，scope作为返回值。
2）TopologicSortCluster改名为SortInternalCluster并将签名换为GraphNodeVec SortInternalCluster()。
以上两点建议是不是更好呢？

详见：https://zh-google-styleguide.readthedocs.io/en/latest/google-cpp-styleguide/classes/

[thisjiang]

PianoScope是DISABLE_COPY_AND_ASSIGN的，所以不能直接返回，改为返回std::unique_ptr<PianoScope>。另外SortInternalCluster改为返回GraphNodeVec并在调用处通过auto&&避免拷贝。

[wzzju]

这是在想通过右值引用延长变量的生命周期吗？其实个人感觉 auto scope = CreateInputOperand(&builder);即可。

[thisjiang]

Done

[wzzju]

这个写成auto cluster_sorted = SortInternalCluster();也是几乎没有开销的，调用的是vector的移动构造函数。

[thisjiang]

Done

[wzzju]

这里不需要写move，直接return scope;即可。

[thisjiang]

Done

[wzzju]

LGTM.

[CtfGo]

LGTM