diff options
author | Skye Wanderman-Milne <skyewm@google.com> | 2018-05-31 07:56:56 -0700 |
---|---|---|
committer | TensorFlower Gardener <gardener@tensorflow.org> | 2018-05-31 07:59:30 -0700 |
commit | a951093889128db4acf4ed80a286ebb2de813241 (patch) | |
tree | 20d0253a63605288919f038de0630d74aa0b370e | |
parent | 582f2e61c7219bfbbec21ce087bee9fde26bce7c (diff) |
Make GraphConstructor create nodes in the same order as the GraphDef.
While technically the order of the created nodes doesn't matter, this
makes viewing and debugging graphs more sensible.
Fixes #19594.
PiperOrigin-RevId: 198721173
-rw-r--r-- | tensorflow/compiler/jit/encapsulate_subgraphs_pass_test.cc | 8 | ||||
-rw-r--r-- | tensorflow/contrib/tensorrt/segment/segment_test.cc | 4 | ||||
-rw-r--r-- | tensorflow/core/common_runtime/function_test.cc | 2 | ||||
-rw-r--r-- | tensorflow/core/graph/algorithm_test.cc | 4 | ||||
-rw-r--r-- | tensorflow/core/graph/graph_constructor.cc | 15 | ||||
-rw-r--r-- | tensorflow/core/graph/graph_partition_test.cc | 16 | ||||
-rw-r--r-- | tensorflow/core/graph/optimizer_cse_test.cc | 32 |
7 files changed, 41 insertions, 40 deletions
diff --git a/tensorflow/compiler/jit/encapsulate_subgraphs_pass_test.cc b/tensorflow/compiler/jit/encapsulate_subgraphs_pass_test.cc index 5ec24d39a2..eef113a354 100644 --- a/tensorflow/compiler/jit/encapsulate_subgraphs_pass_test.cc +++ b/tensorflow/compiler/jit/encapsulate_subgraphs_pass_test.cc @@ -1050,7 +1050,7 @@ TEST(EncapsulateSubgraphsTest, OneFunctionTwoOutside) { .WithAttr("_outside", "O1")); Node* recv2 = RecvAtHost(ops::NodeOut(key_constant, 0), "F1", "O2", {DT_FLOAT, DT_FLOAT}, shape2.opts()); - Node* h = Binary(ops::NodeOut(recv2, 0), e, + Node* h = Binary(ops::NodeOut(recv2, 1), e, shape2.opts() .WithName("H") .WithAttr("_encapsulate", "F1") @@ -1075,7 +1075,7 @@ TEST(EncapsulateSubgraphsTest, OneFunctionTwoOutside) { {"outside_compilation_O1_host_compute"}}, {{"outside_compilation_O2_host_compute"}, "XlaHostCompute", - {"D:o:0", "F:o:0"}, + {"F:o:0", "D:o:0"}, {{"Tinputs", gtl::ArraySlice<DataType>({DT_FLOAT, DT_FLOAT})}, {"Toutputs", gtl::ArraySlice<DataType>({DT_FLOAT})}, {"ancestors", @@ -1123,13 +1123,13 @@ TEST(EncapsulateSubgraphsTest, OneFunctionTwoOutside) { Node* recv2 = RecvAtHost(ops::NodeOut(key_constant, 0), "F1", "O2", {DT_FLOAT, DT_FLOAT}, b2.opts()); - Node* g = Binary(e, ops::NodeOut(recv2, 1), + Node* g = Binary(e, ops::NodeOut(recv2, 0), b2.opts() .WithName("G") .WithControlInputs({recv2, e}) .WithAttr("_encapsulate", "F1") .WithAttr("_outside", "O2")); - Node* h = Binary(ops::NodeOut(recv2, 0), e, + Node* h = Binary(ops::NodeOut(recv2, 1), e, b2.opts() .WithName("H") .WithAttr("_encapsulate", "F1") diff --git a/tensorflow/contrib/tensorrt/segment/segment_test.cc b/tensorflow/contrib/tensorrt/segment/segment_test.cc index 2de3923b06..f5b2d258d7 100644 --- a/tensorflow/contrib/tensorrt/segment/segment_test.cc +++ b/tensorflow/contrib/tensorrt/segment/segment_test.cc @@ -275,13 +275,13 @@ TEST_F(SegmentTest, Multiple) { // Expect two subgraphs EXPECT_EQ(segments.size(), 2); - std::vector<string> expected0{"add0", "add1", "add2", "add3"}; + std::vector<string> expected0{"add6", "add8"}; for (const auto& ex : expected0) { EXPECT_TRUE(segments[0].first.find(ex) != segments[0].first.end()) << "Missing expected node " << ex; } - std::vector<string> expected1{"add6", "add8"}; + std::vector<string> expected1{"add0", "add1", "add2", "add3"}; for (const auto& ex : expected1) { EXPECT_TRUE(segments[1].first.find(ex) != segments[1].first.end()) << "Missing expected node " << ex; diff --git a/tensorflow/core/common_runtime/function_test.cc b/tensorflow/core/common_runtime/function_test.cc index 61b2f0e60f..f4f5198396 100644 --- a/tensorflow/core/common_runtime/function_test.cc +++ b/tensorflow/core/common_runtime/function_test.cc @@ -845,7 +845,7 @@ TEST_F(FunctionLibraryRuntimeTest, ManySwapsNodeDef) { ASSERT_TRUE(g != nullptr); OptimizeGraph(flr0_, &g); const char* e0 = R"P( -(n3:float, n2:float) -> (n3:float) { +(n2:float, n3:float) -> (n2:float) { } )P"; EXPECT_EQ(e0, DebugString(g.get())); diff --git a/tensorflow/core/graph/algorithm_test.cc b/tensorflow/core/graph/algorithm_test.cc index 99ced0c0f5..f67d5a2fd2 100644 --- a/tensorflow/core/graph/algorithm_test.cc +++ b/tensorflow/core/graph/algorithm_test.cc @@ -144,8 +144,8 @@ TEST(AlgorithmTest, ReversePostOrderStable) { std::vector<Node*> order; // Test reverse post order generates expected ordering. - GetReversePostOrder(g, &order, /*stable_comparator=*/NodeComparatorID()); - EXPECT_TRUE(ExpectBefore({{"t3", "t2"}}, order, &error)); + GetReversePostOrder(g, &order, /*stable_comparator=*/NodeComparatorName()); + EXPECT_TRUE(ExpectBefore({{"t2", "t3"}}, order, &error)); } } } // namespace diff --git a/tensorflow/core/graph/graph_constructor.cc b/tensorflow/core/graph/graph_constructor.cc index 2fd32c0bd4..0967492d92 100644 --- a/tensorflow/core/graph/graph_constructor.cc +++ b/tensorflow/core/graph/graph_constructor.cc @@ -278,8 +278,9 @@ class GraphConstructor { // name, the value is the new unique name. std::unordered_map<string, string> uniquified_names_; - // Index of NodeDefs in node_defs_ with all inputs already converted. - std::vector<int> ready_; + // Index of NodeDefs in node_defs_ with all inputs already converted. We use a + // (sorted) set so nodes are created in the order defined in the GraphDef. + std::set<int> ready_; // Mapping between index within node_defs_ and the number of inputs that // still need to be converted. @@ -520,7 +521,7 @@ Status GraphConstructor::InitFromEdges() { } } if (pending_count == 0) { - ready_.push_back(n); + ready_.insert(n); } pending_count_.push_back(pending_count); } @@ -884,12 +885,12 @@ namespace { void UpdatePendingCountAndReady( const std::vector<gtl::InlinedVector<int, 4>>& outputs, int o, - std::vector<int>* pending_count, std::vector<int>* ready) { + std::vector<int>* pending_count, std::set<int>* ready) { for (size_t i = 0; i < outputs[o].size(); ++i) { const int output = outputs[o][i]; (*pending_count)[output]--; if ((*pending_count)[output] == 0) { - ready->push_back(output); + ready->insert(output); } } } @@ -913,8 +914,8 @@ Status GraphConstructor::Convert() { // inputs, pending_counts_ with the number of inputs for each node and // outputs_ with the outputs of each node). while (!ready_.empty()) { - int o = ready_.back(); - ready_.pop_back(); + int o = *ready_.begin(); + ready_.erase(ready_.begin()); ++processed; inputs.clear(); bool has_data_back_edge = false; diff --git a/tensorflow/core/graph/graph_partition_test.cc b/tensorflow/core/graph/graph_partition_test.cc index 83b24cafe2..f44ed47a6e 100644 --- a/tensorflow/core/graph/graph_partition_test.cc +++ b/tensorflow/core/graph/graph_partition_test.cc @@ -329,11 +329,11 @@ TEST_F(GraphPartitionTest, CrossDeviceControl_MultiUse) { string b = "/job:a/replica:0/task:0/cpu:1"; a1 = FloatInput(scope_a_.WithOpName("A1")); auto c = Const(scope_a_.WithOpName("A1/_0").WithControlDependencies(a1), {}); - _Send(scope_a_.WithOpName("A1/_1"), c, "edge_1_A1", a, 82, b); + _Send(scope_a_.WithOpName("A1/_1"), c, "edge_3_A1", a, 82, b); ExpectMatchA(); auto recv = - _Recv(scope_b_.WithOpName("A1/_2"), DT_FLOAT, "edge_1_A1", a, 82, b); + _Recv(scope_b_.WithOpName("A1/_2"), DT_FLOAT, "edge_3_A1", a, 82, b); auto id = Identity(scope_b_.WithOpName("A1/_3"), recv); b1 = FloatInput(scope_b_.WithOpName("B1")); Combine(scope_b_.WithOpName("B2").WithControlDependencies(id), b1, b1); @@ -353,18 +353,18 @@ TEST_F(GraphPartitionTest, CrossDevice_DataControl) { string a = "/job:a/replica:0/task:0/cpu:0"; string b = "/job:a/replica:0/task:0/cpu:1"; a1 = FloatInput(scope_a_.WithOpName("A1")); - auto c = Const(scope_a_.WithOpName("A1/_0").WithControlDependencies(a1), {}); + _Send(scope_a_.WithOpName("A1/_0"), a1, "edge_1_A1", a, 82, b); + auto c = Const(scope_a_.WithOpName("A1/_2").WithControlDependencies(a1), {}); // NOTE: Send 0 A1/_1 -> A1/_2 is not necessarily needed. We could // use A1/_0 -> A1/_4 as the control as a minor optimization. - _Send(scope_a_.WithOpName("A1/_1"), c, "edge_1_A1", a, 82, b); - _Send(scope_a_.WithOpName("A1/_4"), a1, "edge_2_A1", a, 82, b); + _Send(scope_a_.WithOpName("A1/_3"), c, "edge_3_A1", a, 82, b); ExpectMatchA(); auto recv1 = - _Recv(scope_b_.WithOpName("A1/_2"), DT_FLOAT, "edge_1_A1", a, 82, b); - auto id1 = Identity(scope_b_.WithOpName("A1/_3"), recv1); + _Recv(scope_b_.WithOpName("A1/_4"), DT_FLOAT, "edge_3_A1", a, 82, b); + auto id1 = Identity(scope_b_.WithOpName("A1/_5"), recv1); auto recv2 = - _Recv(scope_b_.WithOpName("A1/_5"), DT_FLOAT, "edge_2_A1", a, 82, b); + _Recv(scope_b_.WithOpName("A1/_1"), DT_FLOAT, "edge_1_A1", a, 82, b); b1 = FloatInput(scope_b_.WithOpName("B1")); Combine(scope_b_.WithOpName("B2"), recv2, b1); FloatInput(scope_b_.WithOpName("B3").WithControlDependencies(id1)); diff --git a/tensorflow/core/graph/optimizer_cse_test.cc b/tensorflow/core/graph/optimizer_cse_test.cc index 21a63662cf..c1f93ce05a 100644 --- a/tensorflow/core/graph/optimizer_cse_test.cc +++ b/tensorflow/core/graph/optimizer_cse_test.cc @@ -115,8 +115,8 @@ TEST_F(OptimizerCSETest, Simple) { "node { name: 'D' op: 'Mul' attr { key: 'T' value { type: DT_FLOAT } }" " input: ['A', 'B'] }"); EXPECT_EQ(DoCSE(), - "A(Input);B(Input);D(Mul)|" - "A->D;B->D:1"); + "A(Input);B(Input);C(Mul)|" + "A->C;B->C:1"); } TEST_F(OptimizerCSETest, Simple_ThreeEquivalent) { @@ -130,8 +130,8 @@ TEST_F(OptimizerCSETest, Simple_ThreeEquivalent) { "node { name: 'E' op: 'Mul' attr { key: 'T' value { type: DT_FLOAT } }" " input: ['A', 'B'] }"); EXPECT_EQ(DoCSE(), - "A(Input);B(Input);E(Mul)|" - "A->E;B->E:1"); + "A(Input);B(Input);C(Mul)|" + "A->C;B->C:1"); } TEST_F(OptimizerCSETest, Simple_WithFixups) { @@ -145,8 +145,8 @@ TEST_F(OptimizerCSETest, Simple_WithFixups) { "node { name: 'E' op: 'Mul' attr { key: 'T' value { type: DT_FLOAT } }" " input: ['C', 'D'] }"); EXPECT_EQ(DoCSE(), - "A(Input);B(Input);D(Mul);E(Mul)|" - "A->D;B->D:1;D->E;D->E:1"); + "A(Input);B(Input);C(Mul);E(Mul)|" + "A->C;B->C:1;C->E;C->E:1"); } TEST_F(OptimizerCSETest, Simple_Commutative) { @@ -158,8 +158,8 @@ TEST_F(OptimizerCSETest, Simple_Commutative) { "node { name: 'D' op: 'Mul' attr { key: 'T' value { type: DT_FLOAT } }" " input: ['B', 'A'] }"); EXPECT_EQ(DoCSE(), - "A(Input);B(Input);D(Mul)|" - "A->D:1;B->D"); + "A(Input);B(Input);C(Mul)|" + "A->C;B->C:1"); } static bool IsNotMultiply(const Node* n) { return n->type_string() != "Mul"; } @@ -210,8 +210,8 @@ TEST_F(OptimizerCSETest, Simple_SameOps_SameAttrs1) { " input: ['A', 'B'] attr { key: 'shape'" " value { shape: { dim: { size: 37 name: 'SAME_NAME' } } } } }"); EXPECT_EQ(DoCSE(), - "A(Input);B(Input);D(Mul)|" - "A->D;B->D:1"); + "A(Input);B(Input);C(Mul)|" + "A->C;B->C:1"); } TEST_F(OptimizerCSETest, Simple_SameOps_SameAttrs2) { @@ -229,8 +229,8 @@ TEST_F(OptimizerCSETest, Simple_SameOps_SameAttrs2) { " attr { key: 't' value { type: DT_INT32 } }" " attr { key: 'a' value { i: 3 } } }"); EXPECT_EQ(DoCSE(), - "A(Input);B(Input);D(Mul)|" - "A->D;B->D:1"); + "A(Input);B(Input);C(Mul)|" + "A->C;B->C:1"); } TEST_F(OptimizerCSETest, SameConstants) { @@ -249,8 +249,8 @@ TEST_F(OptimizerCSETest, SameConstants) { "node { name: 'D' op: 'Mul' attr { key: 'T' value { type: DT_INT32 } }" " input: ['A', 'B'] }"); EXPECT_EQ(DoCSE(), - "B(Const);D(Mul)|" - "B->D;B->D:1"); + "A(Const);D(Mul)|" + "A->D;A->D:1"); } TEST_F(OptimizerCSETest, DifferentConstants) { @@ -338,8 +338,8 @@ TEST_F(OptimizerCSETest, Constant_Dedup) { "n/_0(Const);n/_1(Const);n/_2(Const);n/_3(Const);" "n/_4(Const);n/_5(Const);n/_6(Const);n/_7(Const)|"); // In theory, there are 2^4 possible correct output of CSE. In this - // test, it happens to eliminate the first 4 nodes. - EXPECT_EQ(DoCSE(), "n/_4(Const);n/_5(Const);n/_6(Const);n/_7(Const)|"); + // test, it happens to eliminate the last 4 nodes. + EXPECT_EQ(DoCSE(), "n/_0(Const);n/_1(Const);n/_2(Const);n/_3(Const)|"); } static void BM_CSE(int iters, int op_nodes) { |