From 9fd8350f64c1afdab38ae93b68459cc6f6778fbc Mon Sep 17 00:00:00 2001 From: Shrestha Malik Date: Fri, 12 Jul 2019 17:05:19 -0700 Subject: [PATCH 01/17] Made changes to NGraphAssignOp --- src/enable_variable_ops/ngraph_assign_op.cc | 26 ++++++++++----------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/enable_variable_ops/ngraph_assign_op.cc b/src/enable_variable_ops/ngraph_assign_op.cc index 3965f72eb..5553d2ec0 100644 --- a/src/enable_variable_ops/ngraph_assign_op.cc +++ b/src/enable_variable_ops/ngraph_assign_op.cc @@ -125,19 +125,19 @@ class NGraphAssignOp : public OpKernel { // DO NOT CARE ABOUT SYNCING AS WE ARE ALWAYS SETTING THE NGTENSOR // Get input[1] - string valkey = to_string(ng_graph_id_) + "_" + def().input(1); - bool valref_exists = NGraphCatalog::ExistsInEncapOutputTensorMap(valkey); - if (valref_exists) { - // Value is from encap - NGRAPH_VLOG(4) << "NGraphAssign::Getting from catalog: " << valkey; - auto ng_val = NGraphCatalog::GetTensorFromEncapOutputTensorMap(valkey); - var->update_ng_tensor(ng_val); - } else { - NGRAPH_VLOG(4) << "NGraphAssign::Getting from TF : " << valkey; - if (var->update_ng_tensor(rhs_tensor)) { - number_of_copies++; - copy_log_str << " COPY_INP_VAL[0]"; - } + + // input[1] cannot be from NGraphEncap Op + // No way to get input node and check its type + string input_1_name = def().input(1); + OP_REQUIRES( + context, input_1_name.find("ngraph_cluster") == -1, + errors::Internal( + "Caught exception: Input to NGAssign from Encapsulate Op.\n")); + + NGRAPH_VLOG(4) << "NGraphAssign:: Updating"; + if (var->update_ng_tensor(rhs_tensor)) { + number_of_copies++; + copy_log_str << " COPY_INP_VAL[0]"; } mutex_lock l(*context->input_ref_mutex(0)); From 8e5a8ea8f37af7710f04ceea9d5485c0d9563b0a Mon Sep 17 00:00:00 2001 From: Shrestha Malik Date: Mon, 15 Jul 2019 14:19:02 -0700 Subject: [PATCH 02/17] Added utilities to remove the enteries from the catalog maps. Removed entries in destructor of Encap Op --- src/enable_variable_ops/ngraph_catalog.cc | 8 ++++++++ src/enable_variable_ops/ngraph_catalog.h | 2 ++ .../ngraph_enter_in_catalog.h | 1 + src/ngraph_encapsulate_op.cc | 16 ++++++++++++++++ 4 files changed, 27 insertions(+) diff --git a/src/enable_variable_ops/ngraph_catalog.cc b/src/enable_variable_ops/ngraph_catalog.cc index ef1801828..1be782212 100644 --- a/src/enable_variable_ops/ngraph_catalog.cc +++ b/src/enable_variable_ops/ngraph_catalog.cc @@ -54,6 +54,10 @@ bool NGraphCatalog::EncapOutputIndexNeedsCopy(string key, int index) { return true; } +void NGraphCatalog::DeleteFromEncapOutputCopyIndexesMap(string key) { + NGraphCatalog::encap_output_copy_indexes_map_.erase(key); +} + string NGraphCatalog::CreateNodeKey(int graph_id, string node_name, int index) { if (index == 0) { return to_string(graph_id) + "_" + node_name; @@ -111,5 +115,9 @@ bool NGraphCatalog::ExistsInInputVariableSharedNameMap(int graphid, NGraphCatalog::CreateNodeKey(graphid, node_name, input_index)); } +void NGraphCatalog::DeleteFromInputVariableSharedNameMap(string key) { + NGraphCatalog::input_variable_sharedname_map_.erase(key); +} + } // ngraph_bridge } // tensorflow diff --git a/src/enable_variable_ops/ngraph_catalog.h b/src/enable_variable_ops/ngraph_catalog.h index 523e9c590..3fa74bbde 100644 --- a/src/enable_variable_ops/ngraph_catalog.h +++ b/src/enable_variable_ops/ngraph_catalog.h @@ -79,6 +79,7 @@ class NGraphCatalog { unordered_set val); static bool EncapOutputIndexNeedsCopy(string key, int index); static unordered_set GetEncapOutputIndexesThatNeedCopy(string key); + static void DeleteFromEncapOutputCopyIndexesMap(string key); // Functions for InputVariableSharedName Map static string GetInputVariableSharedName(int graphid, string node_name, @@ -89,6 +90,7 @@ class NGraphCatalog { static bool ExistsInInputVariableSharedNameMap(string key); static bool ExistsInInputVariableSharedNameMap(int graphid, string node_name, int input_index); + static void DeleteFromInputVariableSharedNameMap(string key); // Functions for EncapOutputTensorMap static void AddToEncapOutputTensorMap(string key, diff --git a/src/enable_variable_ops/ngraph_enter_in_catalog.h b/src/enable_variable_ops/ngraph_enter_in_catalog.h index ebb598daf..1a70f7c89 100644 --- a/src/enable_variable_ops/ngraph_enter_in_catalog.h +++ b/src/enable_variable_ops/ngraph_enter_in_catalog.h @@ -49,6 +49,7 @@ namespace ngraph_bridge { // We add mapping of {graphId_nodename_InputIndex : Shared_Name} to the // InputVariableSharedNameMap // +// TODO: Update the comments for EnacapOutputInfoMap // 2. If the output of NGraphEncapsulate Op is an input to NGraphVariableType // Op, we store this NG-Tensor // so that it can be directly accessed in compute call of NGraphVariableType. diff --git a/src/ngraph_encapsulate_op.cc b/src/ngraph_encapsulate_op.cc index f585aeb85..55de20b7b 100644 --- a/src/ngraph_encapsulate_op.cc +++ b/src/ngraph_encapsulate_op.cc @@ -225,13 +225,29 @@ class NGraphEncapsulateOp : public OpKernel { } #if defined(NGRAPH_TF_ENABLE_VARIABLES_AND_OPTIMIZERS) + // Remove Entries from Catalog + // Remove entries related to outputs for (int i = 0; i < m_number_outputs; i++) { string key = NGraphCatalog::CreateNodeKey(m_graph_id, name(), i); if (NGraphCatalog::ExistsInEncapOutputTensorMap(key)) { NGraphCatalog::DeleteFromEncapOutputTensorMap(key); NGRAPH_VLOG(2) << "Deleting from output tensor map " << key; } + if (NGraphCatalog::EncapOutputIndexNeedsCopy(name(), i)) { + NGraphCatalog::DeleteFromEncapOutputCopyIndexesMap(name()); + NGRAPH_VLOG(2) << "Deleting from Output Copy Index map " << name(); + } } + // Remove entries related to inputs + for (int i = 0; i < m_number_inputs; i++) { + string key = NGraphCatalog::CreateNodeKey(m_graph_id, name(), i); + if (NGraphCatalog::ExistsInInputVariableSharedNameMap(key)) { + NGraphCatalog::DeleteFromInputVariableSharedNameMap(key); + NGRAPH_VLOG(2) << "Deleting from input variable shared name map " + << key; + } + } + #endif // Release the backend From 168570957486e76d2726a9097a10fc5552241ac9 Mon Sep 17 00:00:00 2001 From: Shrestha Malik Date: Mon, 15 Jul 2019 14:41:50 -0700 Subject: [PATCH 03/17] Remove entries from Catalog in NGVariable Destructors. --- src/enable_variable_ops/ngraph_assign_op.cc | 8 +++++++- src/enable_variable_ops/ngraph_tracked_variable.cc | 2 ++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/enable_variable_ops/ngraph_assign_op.cc b/src/enable_variable_ops/ngraph_assign_op.cc index 5553d2ec0..d647a164f 100644 --- a/src/enable_variable_ops/ngraph_assign_op.cc +++ b/src/enable_variable_ops/ngraph_assign_op.cc @@ -59,7 +59,13 @@ class NGraphAssignOp : public OpKernel { // use_exclusive_lock_, validate_shape_, relax_constraints_; public: - ~NGraphAssignOp() { NGRAPH_VLOG(4) << "~NGraphAssignOp::" << name() << endl; } + ~NGraphAssignOp() { + NGRAPH_VLOG(4) << "~NGraphAssignOp::" << name() << endl; + // Delete from Input Variable Shared Name Map + string key = NGraphCatalog::CreateNodeKey(ng_graph_id_, name(), 0); + NGraphCatalog::DeleteFromInputVariableSharedNameMap(key); + } + explicit NGraphAssignOp(OpKernelConstruction* context) : OpKernel(context), is_tf_just_looking_(false), copy_to_tf_(false) { OP_REQUIRES_OK( diff --git a/src/enable_variable_ops/ngraph_tracked_variable.cc b/src/enable_variable_ops/ngraph_tracked_variable.cc index 9c31a81a8..9675b941d 100644 --- a/src/enable_variable_ops/ngraph_tracked_variable.cc +++ b/src/enable_variable_ops/ngraph_tracked_variable.cc @@ -113,6 +113,8 @@ NGraphVariableOp::NGraphVariableOp(OpKernelConstruction* context) NGraphVariableOp::~NGraphVariableOp() { NGRAPH_VLOG(4) << "~NGraphVariableOp:: " << name() << endl; + string node_key = NGraphCatalog::CreateNodeKey(ng_graph_id_, name(), 0); + NGraphCatalog::DeleteFromInputVariableSharedNameMap(key); tracker_->Unref(); } From 8c3dd8051756df816531ca2614e0fbcc7d804f5f Mon Sep 17 00:00:00 2001 From: Shrestha Malik Date: Mon, 15 Jul 2019 18:25:37 -0700 Subject: [PATCH 04/17] Modifications to catalog. Compiles --- src/enable_variable_ops/ngraph_catalog.cc | 15 +++++++++++---- src/enable_variable_ops/ngraph_catalog.h | 13 ++++++++----- .../ngraph_enter_in_catalog.cc | 2 +- .../ngraph_tracked_variable.cc | 3 ++- src/ngraph_encapsulate_op.cc | 14 +++++++++----- 5 files changed, 31 insertions(+), 16 deletions(-) diff --git a/src/enable_variable_ops/ngraph_catalog.cc b/src/enable_variable_ops/ngraph_catalog.cc index 1be782212..3c32dc818 100644 --- a/src/enable_variable_ops/ngraph_catalog.cc +++ b/src/enable_variable_ops/ngraph_catalog.cc @@ -34,17 +34,22 @@ unordered_map> NGraphCatalog::encap_output_copy_indexes_map_; // Functions for Encapsulate Output Copy Indexes Map -void NGraphCatalog::AddToEncapOutputCopyIndexesMap(string key, +void NGraphCatalog::AddToEncapOutputCopyIndexesMap(int graphid, + string node_name, unordered_set val) { + string key = graphid + "_" + node_name; NGraphCatalog::encap_output_copy_indexes_map_[key] = val; } unordered_set NGraphCatalog::GetEncapOutputIndexesThatNeedCopy( - string key) { + int graphid, string node_name) { + string key = graphid + "_" + node_name; return NGraphCatalog::encap_output_copy_indexes_map_[key]; } -bool NGraphCatalog::EncapOutputIndexNeedsCopy(string key, int index) { +bool NGraphCatalog::EncapOutputIndexNeedsCopy(int graphid, string node_name, + int index) { + string key = graphid + "_" + node_name; auto itr = NGraphCatalog::encap_output_copy_indexes_map_.find(key); if (itr != NGraphCatalog::encap_output_copy_indexes_map_.end()) { auto op_copy_indexes = itr->second; @@ -54,7 +59,9 @@ bool NGraphCatalog::EncapOutputIndexNeedsCopy(string key, int index) { return true; } -void NGraphCatalog::DeleteFromEncapOutputCopyIndexesMap(string key) { +void NGraphCatalog::DeleteFromEncapOutputCopyIndexesMap(int graphid, + string node_name) { + string key = graphid + "_" + node_name; NGraphCatalog::encap_output_copy_indexes_map_.erase(key); } diff --git a/src/enable_variable_ops/ngraph_catalog.h b/src/enable_variable_ops/ngraph_catalog.h index 3fa74bbde..49aefb042 100644 --- a/src/enable_variable_ops/ngraph_catalog.h +++ b/src/enable_variable_ops/ngraph_catalog.h @@ -67,7 +67,7 @@ class NGraphCatalog { // Will be used by NGraphEncapsulateOP // Map of // Key - // string : nodename (nGraphEncapsulateOp name) + // string : GraphId + _ + nodename // Value : Set of indices static unordered_map> encap_output_copy_indexes_map_; @@ -75,11 +75,14 @@ class NGraphCatalog { public: // Utility Functions for the data structures // Functions for EncapsulateOutputCopyIndexes Map - static void AddToEncapOutputCopyIndexesMap(string key, + static void AddToEncapOutputCopyIndexesMap(int graphid, string node_name, unordered_set val); - static bool EncapOutputIndexNeedsCopy(string key, int index); - static unordered_set GetEncapOutputIndexesThatNeedCopy(string key); - static void DeleteFromEncapOutputCopyIndexesMap(string key); + static bool EncapOutputIndexNeedsCopy(int graphid, string node_name, + int index); + static unordered_set GetEncapOutputIndexesThatNeedCopy(int graphid, + string node_name); + static void DeleteFromEncapOutputCopyIndexesMap(int graphid, + string node_name); // Functions for InputVariableSharedName Map static string GetInputVariableSharedName(int graphid, string node_name, diff --git a/src/enable_variable_ops/ngraph_enter_in_catalog.cc b/src/enable_variable_ops/ngraph_enter_in_catalog.cc index 3c05e8349..476d4c419 100644 --- a/src/enable_variable_ops/ngraph_enter_in_catalog.cc +++ b/src/enable_variable_ops/ngraph_enter_in_catalog.cc @@ -106,7 +106,7 @@ Status EnterInCatalog(Graph* graph, int graph_id) { op_index_to_copy.insert(edge->src_output()); } } - NGraphCatalog::AddToEncapOutputCopyIndexesMap(node->name(), + NGraphCatalog::AddToEncapOutputCopyIndexesMap(graph_id, node->name(), op_index_to_copy); } // end of node is type NGraphEncapsulate diff --git a/src/enable_variable_ops/ngraph_tracked_variable.cc b/src/enable_variable_ops/ngraph_tracked_variable.cc index 9675b941d..b8a758305 100644 --- a/src/enable_variable_ops/ngraph_tracked_variable.cc +++ b/src/enable_variable_ops/ngraph_tracked_variable.cc @@ -27,6 +27,7 @@ #include "ngraph_freshness_tracker.h" #include "ngraph_utils.h" #include "ngraph_var.h" +#include "ngraph_catalog.h" #include "ngraph/event_tracing.hpp" @@ -114,7 +115,7 @@ NGraphVariableOp::NGraphVariableOp(OpKernelConstruction* context) NGraphVariableOp::~NGraphVariableOp() { NGRAPH_VLOG(4) << "~NGraphVariableOp:: " << name() << endl; string node_key = NGraphCatalog::CreateNodeKey(ng_graph_id_, name(), 0); - NGraphCatalog::DeleteFromInputVariableSharedNameMap(key); + NGraphCatalog::DeleteFromInputVariableSharedNameMap(node_key); tracker_->Unref(); } diff --git a/src/ngraph_encapsulate_op.cc b/src/ngraph_encapsulate_op.cc index 55de20b7b..025c95883 100644 --- a/src/ngraph_encapsulate_op.cc +++ b/src/ngraph_encapsulate_op.cc @@ -233,11 +233,11 @@ class NGraphEncapsulateOp : public OpKernel { NGraphCatalog::DeleteFromEncapOutputTensorMap(key); NGRAPH_VLOG(2) << "Deleting from output tensor map " << key; } - if (NGraphCatalog::EncapOutputIndexNeedsCopy(name(), i)) { - NGraphCatalog::DeleteFromEncapOutputCopyIndexesMap(name()); - NGRAPH_VLOG(2) << "Deleting from Output Copy Index map " << name(); - } } + + NGRAPH_VLOG(2) << "Deleting from Output Copy Index map " << name(); + NGraphCatalog::DeleteFromEncapOutputCopyIndexesMap(m_graph_id, name()); + // Remove entries related to inputs for (int i = 0; i < m_number_inputs; i++) { string key = NGraphCatalog::CreateNodeKey(m_graph_id, name(), i); @@ -817,6 +817,8 @@ class NGraphEncapsulateOp : public OpKernel { if (m_number_outputs == -1) { NGRAPH_VLOG(4) << "Settig number of outputs for " << def().name(); m_number_outputs = output_caches.size(); + NGRAPH_VLOG(4) << "Settig number of inputs for " << def().name(); + m_number_inputs = ng_inputs.size(); } for (size_t i = 0; i < output_tensor_count; ++i) { string key = NGraphCatalog::CreateNodeKey(m_graph_id, def().name(), i); @@ -831,7 +833,8 @@ class NGraphEncapsulateOp : public OpKernel { } if (m_op_backend_name != "CPU" && - NGraphCatalog::EncapOutputIndexNeedsCopy(def().name(), i)) { + NGraphCatalog::EncapOutputIndexNeedsCopy(m_graph_id, def().name(), + i)) { number_of_copies++; copy_log_str << " COPY_OP_VAL[" << i << "]"; @@ -1009,6 +1012,7 @@ class NGraphEncapsulateOp : public OpKernel { static int s_instance_count; int my_instance_id{0}; int m_number_outputs = -1; + int m_number_inputs = -1; }; int NGraphEncapsulateOp::s_instance_count = 0; From 6c2dd52047155d61a866fa8e8e35cf32e10c6b9f Mon Sep 17 00:00:00 2001 From: Shrestha Malik Date: Fri, 19 Jul 2019 10:46:00 -0700 Subject: [PATCH 05/17] Code Format --- src/enable_variable_ops/ngraph_tracked_variable.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/enable_variable_ops/ngraph_tracked_variable.cc b/src/enable_variable_ops/ngraph_tracked_variable.cc index f96d59320..ac413d82c 100644 --- a/src/enable_variable_ops/ngraph_tracked_variable.cc +++ b/src/enable_variable_ops/ngraph_tracked_variable.cc @@ -23,10 +23,10 @@ #include "ngraph/runtime/backend.hpp" #include "ngraph_backend_manager.h" +#include "ngraph_catalog.h" #include "ngraph_freshness_tracker.h" #include "ngraph_utils.h" #include "ngraph_var.h" -#include "ngraph_catalog.h" #include "ngraph/event_tracing.hpp" From bc9b16a39b494da5cfd01cf7c541c199a4913871 Mon Sep 17 00:00:00 2001 From: Shrestha Malik Date: Mon, 22 Jul 2019 17:32:31 -0700 Subject: [PATCH 06/17] Not using RemoveEdge Api. Made changes to RewritePass --- .../ngraph_remove_ngraphassigns.cc | 7 ++-- .../ngraph_rewrite_pass.cc | 33 +++++++++++++++---- 2 files changed, 31 insertions(+), 9 deletions(-) diff --git a/ngraph_bridge/enable_variable_ops/ngraph_remove_ngraphassigns.cc b/ngraph_bridge/enable_variable_ops/ngraph_remove_ngraphassigns.cc index 9752248be..6bf6fb1a5 100644 --- a/ngraph_bridge/enable_variable_ops/ngraph_remove_ngraphassigns.cc +++ b/ngraph_bridge/enable_variable_ops/ngraph_remove_ngraphassigns.cc @@ -45,6 +45,11 @@ Status RemoveNGraphAssigns(Graph* graph) { input_1->type_string()); } + // Handle input and output edges + // Only adding the new edges, edges to and from the current node will be + // removed when + // node is removed + // https://github.com/tensorflow/tensorflow/blob/master/tensorflow/core/graph/graph.h#L495 // Handle input edges NGRAPH_VLOG(3) << "Handling input edges "; for (auto edge : node->in_edges()) { @@ -55,7 +60,6 @@ Status RemoveNGraphAssigns(Graph* graph) { if (edge->src() == input_1) continue; graph->AddEdge(edge->src(), edge->src_output(), input_1, edge->dst_input()); - graph->RemoveEdge(edge); } } @@ -80,7 +84,6 @@ Status RemoveNGraphAssigns(Graph* graph) { graph->AddEdge(input_1, Graph::kControlSlot, edge->dst(), Graph::kControlSlot); } - graph->RemoveEdge(edge); } // Add the node for removal diff --git a/ngraph_bridge/enable_variable_ops/ngraph_rewrite_pass.cc b/ngraph_bridge/enable_variable_ops/ngraph_rewrite_pass.cc index 98e1067f5..603dbc50f 100644 --- a/ngraph_bridge/enable_variable_ops/ngraph_rewrite_pass.cc +++ b/ngraph_bridge/enable_variable_ops/ngraph_rewrite_pass.cc @@ -22,6 +22,7 @@ #include "logging/ngraph_log.h" #include "logging/tf_graph_writer.h" #include "ngraph_bridge/enable_variable_ops/ngraph_enter_in_catalog.h" +#include "ngraph_bridge/enable_variable_ops/ngraph_remove_ngraphassigns.h" #include "ngraph_bridge/enable_variable_ops/ngraph_replace_variable_modifiers.h" #include "ngraph_bridge/ngraph_api.h" #include "ngraph_bridge/ngraph_assign_clusters.h" @@ -206,16 +207,22 @@ class NGraphVariableCapturePass : public NGraphRewritePass { // 2. Cluster Assignment [ngraph_assign_clusters.cc] // 3. Cluster Deassignment [ngraph_deassign_clusters.cc] // 4. Cluster Encapsulation [ngraph_encapsulate_clusters.cc] -// +// 5. Rewrite Variable Type Ops for Tracking [ngraph_rewrite_for_tracking.cc] +// 6. Enter In Catalog [ngraph_enter_in_catalog.cc] +// 7. Remove NGraphAssigns [ngraph_remove_ngraphassigns.cc] // Between phases, graph dumps (in both .dot and .pbtxt format) may be // requested by setting the following environment variables: // -// NGRAPH_TF_DUMP_UNMARKED_GRAPHS=1 dumps graphs before phase 1 -// NGRAPH_TF_DUMP_MARKED_GRAPHS=1 dumps graphs after phase 1 -// NGRAPH_TF_DUMP_CLUSTERED_GRAPHS=1 dumps graphs after phase 2 -// NGRAPH_TF_DUMP_DECLUSTERED_GRAPHS=1 dumps graphs after phase 3 -// NGRAPH_TF_DUMP_ENCAPSULATED_GRAPHS=1 dumps graphs after phase 4 -// NGRAPH_TF_DUMP_GRAPHS=1 all of the above +// NGRAPH_TF_DUMP_UNMARKED_GRAPHS=1 dumps graphs before phase 0 +// NGRAPH_TF_DUMP_REPLACEDMODIFIERS_GRAPHS=1 dumps graphs after phase 0 +// NGRAPH_TF_DUMP_MARKED_GRAPHS=1 dumps graphs after phase 1 +// NGRAPH_TF_DUMP_CLUSTERED_GRAPHS=1 dumps graphs after phase 2 +// NGRAPH_TF_DUMP_DECLUSTERED_GRAPHS=1 dumps graphs after phase 3 +// NGRAPH_TF_DUMP_ENCAPSULATED_GRAPHS=1 dumps graphs after phase 4 +// NGRAPH_TF_DUMP_TRACKED_GRAPHS=1 dumps graphs after phase 5 +// NGRAPH_TF_DUMP_CATALOGED_GRAPHS=1 dumps graphs after phase 6 +// NGRAPH_TF_DUMP_REMOVENGASSIGNS_GRAPHS=1 dumps graphs after phase 7 +// NGRAPH_TF_DUMP_GRAPHS=1 all of the above // class NGraphEncapsulationPass : public NGraphRewritePass { public: @@ -323,6 +330,13 @@ class NGraphEncapsulationPass : public NGraphRewritePass { "Graph with Variables Inputs Entered in Catalog"); } + // Remove Certain NGraphAssigns then. + TF_RETURN_IF_ERROR(RemoveNGraphAssigns(options.graph->get())); + if (DumpRemoveNGraphAssignsGraphs()) { + DumpGraphs(options, idx, "ngraphssigns_optimized", + "Graph with NGraphAssigns Optimized/Removed"); + } + return Status::OK(); } @@ -360,6 +374,11 @@ class NGraphEncapsulationPass : public NGraphRewritePass { return DumpAllGraphs() || std::getenv("NGRAPH_TF_DUMP_CATALOGED_GRAPHS") != nullptr; } + + static bool DumpRemoveNGraphAssignsGraphs() { + return DumpAllGraphs() || + std::getenv("NGRAPH_TF_DUMP_REMOVENGASSIGNS_GRAPHS") != nullptr; + } }; } // namespace ngraph_bridge From a2f13366e29e38413cc298560f8b0fcd25c5e165 Mon Sep 17 00:00:00 2001 From: Shrestha Malik Date: Mon, 22 Jul 2019 17:55:55 -0700 Subject: [PATCH 07/17] changes to get var before compute --- ngraph_bridge/ngraph_encapsulate_op.cc | 27 ++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/ngraph_bridge/ngraph_encapsulate_op.cc b/ngraph_bridge/ngraph_encapsulate_op.cc index 243229a90..6c49064c8 100644 --- a/ngraph_bridge/ngraph_encapsulate_op.cc +++ b/ngraph_bridge/ngraph_encapsulate_op.cc @@ -683,13 +683,32 @@ class NGraphEncapsulateOp : public OpKernel { output_caches[i].second; void* current_dst_ptr = DMAHelper::base(output_tensor); - std::shared_ptr current_ng_tensor = - GetCurrentNgTensor(current_dst_ptr, last_dst_ptr, last_ng_tensor, - true, ng_exec, op_backend, ng_element_type, - ng_shape); + std::shared_ptr current_ng_tensor = nullptr; +// if the output tensor is going to be assigned to a variable +// we ask nGraph to provide the output directly in the variable tensor +#if defined(NGRAPH_TF_ENABLE_VARIABLES_AND_OPTIMIZERS) + if (NGraphCatalog::ExistsInEncapOutputInfoMap(m_graph_id, name(), i)) { + string output_key = NGraphCatalog::CreatNodeKey(m_graph_id, name(), i); + string ref_var_name = + NGraphCatalog::GetVariableSharedNameFromEncapOutputInfoMap( + output_key); + NGraphVar* var; + OP_REQUIRES_OK(ctx, ctx->resource_manager()->Lookup( + ctx->resource_manager()->default_container(), + ref_var_name, &var)); + current_ng_tensor = var->ng_tensor(); + var->Unref(); + ng_outputs.push_back(current_ng_tensor); + continue; + } +#endif + current_ng_tensor = GetCurrentNgTensor( + current_dst_ptr, last_dst_ptr, last_ng_tensor, true, ng_exec, + op_backend, ng_element_type, ng_shape); current_ng_tensor->set_stale(true); output_caches[i] = std::make_pair(current_dst_ptr, current_ng_tensor); + ng_outputs.push_back(current_ng_tensor); } From 6f7543343509b4741f69101a6212721735b12f9e Mon Sep 17 00:00:00 2001 From: Shrestha Malik Date: Mon, 22 Jul 2019 19:12:01 -0700 Subject: [PATCH 08/17] Made changes to remove edges right way.Added syncing in encap --- .../ngraph_remove_ngraphassigns.cc | 7 ++++ ngraph_bridge/ngraph_encapsulate_op.cc | 42 +++++++++++++++---- 2 files changed, 41 insertions(+), 8 deletions(-) diff --git a/ngraph_bridge/enable_variable_ops/ngraph_remove_ngraphassigns.cc b/ngraph_bridge/enable_variable_ops/ngraph_remove_ngraphassigns.cc index 6bf6fb1a5..498e07f16 100644 --- a/ngraph_bridge/enable_variable_ops/ngraph_remove_ngraphassigns.cc +++ b/ngraph_bridge/enable_variable_ops/ngraph_remove_ngraphassigns.cc @@ -52,6 +52,7 @@ Status RemoveNGraphAssigns(Graph* graph) { // https://github.com/tensorflow/tensorflow/blob/master/tensorflow/core/graph/graph.h#L495 // Handle input edges NGRAPH_VLOG(3) << "Handling input edges "; + vector remove_edges; for (auto edge : node->in_edges()) { // attach incoming control edge to input_1, as that's where update // will happen @@ -61,6 +62,7 @@ Status RemoveNGraphAssigns(Graph* graph) { graph->AddEdge(edge->src(), edge->src_output(), input_1, edge->dst_input()); } + remove_edges.push_back(edge); } // Handle output edges @@ -84,6 +86,11 @@ Status RemoveNGraphAssigns(Graph* graph) { graph->AddEdge(input_1, Graph::kControlSlot, edge->dst(), Graph::kControlSlot); } + remove_edges.push_back(edge); + } + + for (auto edge : remove_edges) { + graph->RemoveEdge(edge); } // Add the node for removal diff --git a/ngraph_bridge/ngraph_encapsulate_op.cc b/ngraph_bridge/ngraph_encapsulate_op.cc index 6c49064c8..2d1f52921 100644 --- a/ngraph_bridge/ngraph_encapsulate_op.cc +++ b/ngraph_bridge/ngraph_encapsulate_op.cc @@ -688,7 +688,7 @@ class NGraphEncapsulateOp : public OpKernel { // we ask nGraph to provide the output directly in the variable tensor #if defined(NGRAPH_TF_ENABLE_VARIABLES_AND_OPTIMIZERS) if (NGraphCatalog::ExistsInEncapOutputInfoMap(m_graph_id, name(), i)) { - string output_key = NGraphCatalog::CreatNodeKey(m_graph_id, name(), i); + string output_key = NGraphCatalog::CreateNodeKey(m_graph_id, name(), i); string ref_var_name = NGraphCatalog::GetVariableSharedNameFromEncapOutputInfoMap( output_key); @@ -828,15 +828,41 @@ class NGraphEncapsulateOp : public OpKernel { m_number_inputs = ng_inputs.size(); } for (size_t i = 0; i < output_tensor_count; ++i) { - string key = NGraphCatalog::CreateNodeKey(m_graph_id, def().name(), i); - bool ref_exists = NGraphCatalog::ExistsInEncapOutputTensorMap(key); - void* dst_ptr; + // Sync the Var Tensor if required + string output_key = + NGraphCatalog::CreateNodeKey(m_graph_id, def().name(), i); + bool ref_exists = NGraphCatalog::ExistsInEncapOutputInfoMap(output_key); std::shared_ptr dst_ng_tensor; - std::tie(dst_ptr, dst_ng_tensor) = output_caches[i]; - + void* dst_ptr; if (ref_exists) { - NGRAPH_VLOG(4) << "Adding in output tensor map " << key; - NGraphCatalog::AddToEncapOutputTensorMap(key, dst_ng_tensor); + NGRAPH_VLOG(4) << "Syncing the output var tensor " << output_key; + + // Get var + string ref_var_name = + NGraphCatalog::GetVariableSharedNameFromEncapOutputInfoMap( + output_key); + NGraphVar* var; + OP_REQUIRES_OK(ctx, ctx->resource_manager()->Lookup( + ctx->resource_manager()->default_container(), + ref_var_name, &var)); + + if (NGraphCatalog::GetCopyToTFFromEncapOutputInfoMap(output_key)) { + if (var->copy_ng_to_tf()) { + number_of_copies++; + copy_log_str << " COPY_TF "; + } + if (!NGraphCatalog::GetIsTFJustLookingFromEncapOutputInfoMap( + output_key)) { + // Some tf op might update the ng-tensor value so mark it stale + copy_log_str << " SET_SYNC "; + var->set_sync_ng_tensor(true); + } + } + dst_ng_tensor = var->ng_tensor(); + var->Unref(); + dst_ptr = output_caches[i].first; + } else { + std::tie(dst_ptr, dst_ng_tensor) = output_caches[i]; } if (m_op_backend_name != "CPU" && From aeb8514c1769a6500b3b9d55a56a82dabb6b4d08 Mon Sep 17 00:00:00 2001 From: Shrestha Malik Date: Tue, 23 Jul 2019 17:23:33 -0700 Subject: [PATCH 09/17] Removed Enacap Output Tensor Map --- .../enable_variable_ops/ngraph_catalog.cc | 43 ++++--------------- .../enable_variable_ops/ngraph_catalog.h | 30 ++----------- .../ngraph_enter_in_catalog.cc | 26 +---------- ngraph_bridge/ngraph_encapsulate_op.cc | 6 +-- 4 files changed, 15 insertions(+), 90 deletions(-) diff --git a/ngraph_bridge/enable_variable_ops/ngraph_catalog.cc b/ngraph_bridge/enable_variable_ops/ngraph_catalog.cc index c7afb4e29..158d9cadb 100644 --- a/ngraph_bridge/enable_variable_ops/ngraph_catalog.cc +++ b/ngraph_bridge/enable_variable_ops/ngraph_catalog.cc @@ -29,13 +29,19 @@ namespace tensorflow { namespace ngraph_bridge { unordered_map NGraphCatalog::input_variable_sharedname_map_; -unordered_map> - NGraphCatalog::encap_output_tensor_map_; unordered_map> NGraphCatalog::encap_output_copy_indexes_map_; unordered_map> NGraphCatalog::encap_output_info_map_; +// Function to create the Node Key +string NGraphCatalog::CreateNodeKey(int graph_id, string node_name, int index) { + if (index == 0) { + return to_string(graph_id) + "_" + node_name; + } + return to_string(graph_id) + "_" + node_name + ":" + to_string(index); +} + // Functions for Encapsulate Output Copy Indexes Map void NGraphCatalog::AddToEncapOutputCopyIndexesMap(int graphid, string node_name, @@ -68,39 +74,6 @@ void NGraphCatalog::DeleteFromEncapOutputCopyIndexesMap(int graphid, NGraphCatalog::encap_output_copy_indexes_map_.erase(key); } -string NGraphCatalog::CreateNodeKey(int graph_id, string node_name, int index) { - if (index == 0) { - return to_string(graph_id) + "_" + node_name; - } - return to_string(graph_id) + "_" + node_name + ":" + to_string(index); -} - -// Functions for OutputTensorMap -void NGraphCatalog::AddToEncapOutputTensorMap( - string key, shared_ptr ng_val) { - NGraphCatalog::encap_output_tensor_map_[key] = ng_val; -} - -bool NGraphCatalog::ExistsInEncapOutputTensorMap(string key) { - auto itr = NGraphCatalog::encap_output_tensor_map_.find(key); - return itr != NGraphCatalog::encap_output_tensor_map_.end(); -} - -bool NGraphCatalog::ExistsInEncapOutputTensorMap(int graphid, string node_name, - int input_index) { - return NGraphCatalog::ExistsInEncapOutputTensorMap( - NGraphCatalog::CreateNodeKey(graphid, node_name, input_index)); -} - -shared_ptr -NGraphCatalog::GetTensorFromEncapOutputTensorMap(string key) { - return NGraphCatalog::encap_output_tensor_map_[key]; -} - -void NGraphCatalog::DeleteFromEncapOutputTensorMap(string key) { - NGraphCatalog::encap_output_tensor_map_.erase(key); -} - // Functions relating Input Variable Shared Name Map string NGraphCatalog::GetInputVariableSharedName(int graphid, string node_name, int input_index) { diff --git a/ngraph_bridge/enable_variable_ops/ngraph_catalog.h b/ngraph_bridge/enable_variable_ops/ngraph_catalog.h index 9b354f7c7..ce11bc530 100644 --- a/ngraph_bridge/enable_variable_ops/ngraph_catalog.h +++ b/ngraph_bridge/enable_variable_ops/ngraph_catalog.h @@ -50,19 +50,6 @@ class NGraphCatalog { // LOCK? static unordered_map input_variable_sharedname_map_; - // Map keeps track of nodes whose input is a tensor computed by NGraph - // For e.g. if the value to be assigned was computed by NGraphEncapsulate Op - // Will be used by Assign/Optimizers - // Map of - // Key - // when op index ==0 - // string : GraphId + _ + nodename - // otherwise - // string : GraphId + _ + nodename + : + output_index - // Value : shared_ptr - static unordered_map> - encap_output_tensor_map_; - // Map keeps track of output indexes of NGraphEncapsulate Op // that will be used by TF Nodes or other NGraphEncapsulate Op // Will be used by NGraphEncapsulateOP @@ -91,6 +78,9 @@ class NGraphCatalog { encap_output_info_map_; public: + // Utility to create key to query the maps + static string CreateNodeKey(int graph_id, string node_name, int index); + // Utility Functions for the data structures // Functions for EncapsulateOutputCopyIndexes Map static void AddToEncapOutputCopyIndexesMap(int graphid, string node_name, @@ -113,17 +103,6 @@ class NGraphCatalog { int input_index); static void DeleteFromInputVariableSharedNameMap(string key); - // Functions for EncapOutputTensorMap - static void AddToEncapOutputTensorMap(string key, - shared_ptr ng_val); - static bool ExistsInEncapOutputTensorMap(string key); - static bool ExistsInEncapOutputTensorMap(int graphid, string node_name, - int input_index); - - static shared_ptr GetTensorFromEncapOutputTensorMap( - string key); - static void DeleteFromEncapOutputTensorMap(string key); - // Functions for EncapOutputInfo Map static void AddToEncapOutputInfoMap(string key, tuple val); @@ -139,9 +118,6 @@ class NGraphCatalog { static void DeleteFromEncapOutputInfoMap(string key); static void ClearEncapOutputInfoMap(); static void PrintEncapOutputInfoMap(); - - // Utility to create key to query the maps - static string CreateNodeKey(int graph_id, string node_name, int index); }; } // ngraph_bridge diff --git a/ngraph_bridge/enable_variable_ops/ngraph_enter_in_catalog.cc b/ngraph_bridge/enable_variable_ops/ngraph_enter_in_catalog.cc index 1a209c05c..d1db4241d 100644 --- a/ngraph_bridge/enable_variable_ops/ngraph_enter_in_catalog.cc +++ b/ngraph_bridge/enable_variable_ops/ngraph_enter_in_catalog.cc @@ -98,8 +98,7 @@ Status EnterInCatalog(Graph* graph, int graph_id) { NGRAPH_VLOG(4) << "Value: " << get<0>(value) << " " << get<1>(value) << " " << get<2>(value); NGraphCatalog::AddToEncapOutputInfoMap(key, value); - // TODO: Uncomment the continue when all the tasks are integrated - // continue; + continue; } } // Update the input variable map @@ -145,29 +144,6 @@ Status EnterInCatalog(Graph* graph, int graph_id) { op_index_to_copy); } // end of node is type NGraphEncapsulate - - // Update the output tensor map - if (IsNGVariableType(node->type_string())) { - for (auto edge : node->in_edges()) { - if (!edge->src()->IsOp() || edge->IsControlEdge() || - IsRefType(edge->dst()->input_type(edge->dst_input())) || - edge->src()->type_string() != "NGraphEncapsulate") { - continue; - } - - NGRAPH_VLOG(4) << "Get " << node->type_string() - << " and input is from NGraphEncapsulate"; - - auto src = edge->src(); - int src_output = edge->src_output(); - string node_key = - NGraphCatalog::CreateNodeKey(graph_id, src->name(), src_output); - // Will be updated with real tensors in Encapsulate - NGraphCatalog::AddToEncapOutputTensorMap(node_key, nullptr); - NGRAPH_VLOG(4) << "Adding in Output Tensor Map"; - NGRAPH_VLOG(4) << "Key: " << node_key; - } - } // end of if node of type NGraphAssign } // enter in catalog NGRAPH_VLOG(4) << "Entered in Catalog"; diff --git a/ngraph_bridge/ngraph_encapsulate_op.cc b/ngraph_bridge/ngraph_encapsulate_op.cc index 2d1f52921..5a5236358 100644 --- a/ngraph_bridge/ngraph_encapsulate_op.cc +++ b/ngraph_bridge/ngraph_encapsulate_op.cc @@ -218,9 +218,9 @@ class NGraphEncapsulateOp : public OpKernel { // Remove entries related to outputs for (int i = 0; i < m_number_outputs; i++) { string key = NGraphCatalog::CreateNodeKey(m_graph_id, name(), i); - if (NGraphCatalog::ExistsInEncapOutputTensorMap(key)) { - NGraphCatalog::DeleteFromEncapOutputTensorMap(key); - NGRAPH_VLOG(2) << "Deleting from output tensor map " << key; + if (NGraphCatalog::ExistsInEncapOutputInfoMap(key)) { + NGraphCatalog::DeleteFromEncapOutputInfoMap(key); + NGRAPH_VLOG(2) << "Deleting from output info map " << key; } } From 90591541c98e12d0d141ff3d9aa0009f0929b402 Mon Sep 17 00:00:00 2001 From: Shrestha Malik Date: Tue, 23 Jul 2019 17:29:48 -0700 Subject: [PATCH 10/17] Updated Comments --- .../enable_variable_ops/ngraph_enter_in_catalog.h | 12 ++++++------ .../ngraph_remove_ngraphassigns.cc | 5 ----- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/ngraph_bridge/enable_variable_ops/ngraph_enter_in_catalog.h b/ngraph_bridge/enable_variable_ops/ngraph_enter_in_catalog.h index 5ddf7f4be..26eeb4e61 100644 --- a/ngraph_bridge/enable_variable_ops/ngraph_enter_in_catalog.h +++ b/ngraph_bridge/enable_variable_ops/ngraph_enter_in_catalog.h @@ -50,12 +50,12 @@ namespace ngraph_bridge { // We add mapping of {graphId_nodename_InputIndex : Shared_Name} to the // InputVariableSharedNameMap // -// TODO: Update the comments for EnacapOutputInfoMap -// 2. If the output of NGraphEncapsulate Op is an input to NGraphVariableType -// Op, we store this NG-Tensor -// so that it can be directly accessed in compute call of NGraphVariableType. -// We add mapping of {graphId_encapnodename_OutputIndex : NG-Tensor} to the -// EncapOutputTensorMap +// 2. If the input to NGraphAssign Op is from NGraphEncapsulate Op +// We add mapping of +// {graphId_encapnodename_OutputIndex : tuple:{Variable_Shared_Name, CopyToTF, +// IsTFJustLooking}} +// to the EncapOutputInfoMap +// We attach "_ngraph_remove" attribute to the NGraphAssign node // // 3. If the output of NGraphEncapsulate Op is not required by a TF Op or // NGraphEncapsulate Op, diff --git a/ngraph_bridge/enable_variable_ops/ngraph_remove_ngraphassigns.cc b/ngraph_bridge/enable_variable_ops/ngraph_remove_ngraphassigns.cc index 498e07f16..f18804978 100644 --- a/ngraph_bridge/enable_variable_ops/ngraph_remove_ngraphassigns.cc +++ b/ngraph_bridge/enable_variable_ops/ngraph_remove_ngraphassigns.cc @@ -45,11 +45,6 @@ Status RemoveNGraphAssigns(Graph* graph) { input_1->type_string()); } - // Handle input and output edges - // Only adding the new edges, edges to and from the current node will be - // removed when - // node is removed - // https://github.com/tensorflow/tensorflow/blob/master/tensorflow/core/graph/graph.h#L495 // Handle input edges NGRAPH_VLOG(3) << "Handling input edges "; vector remove_edges; From 48bf3fe6aecd11e31b502b3536d76c940cdb4096 Mon Sep 17 00:00:00 2001 From: Shrestha Malik Date: Tue, 23 Jul 2019 17:29:48 -0700 Subject: [PATCH 11/17] Updated Comments --- .../enable_variable_ops/ngraph_enter_in_catalog.cc | 3 +++ .../enable_variable_ops/ngraph_enter_in_catalog.h | 12 ++++++------ .../ngraph_remove_ngraphassigns.cc | 5 ----- 3 files changed, 9 insertions(+), 11 deletions(-) diff --git a/ngraph_bridge/enable_variable_ops/ngraph_enter_in_catalog.cc b/ngraph_bridge/enable_variable_ops/ngraph_enter_in_catalog.cc index d1db4241d..1145f4cac 100644 --- a/ngraph_bridge/enable_variable_ops/ngraph_enter_in_catalog.cc +++ b/ngraph_bridge/enable_variable_ops/ngraph_enter_in_catalog.cc @@ -98,9 +98,12 @@ Status EnterInCatalog(Graph* graph, int graph_id) { NGRAPH_VLOG(4) << "Value: " << get<0>(value) << " " << get<1>(value) << " " << get<2>(value); NGraphCatalog::AddToEncapOutputInfoMap(key, value); + // This NGraphAssign will be removed subsequently + // so we dont need to fill the rest of the catalog continue; } } + // Update the input variable map if (IsNGVariableType(node->type_string())) { string node_key = NGraphCatalog::CreateNodeKey(graph_id, node->name(), 0); diff --git a/ngraph_bridge/enable_variable_ops/ngraph_enter_in_catalog.h b/ngraph_bridge/enable_variable_ops/ngraph_enter_in_catalog.h index 5ddf7f4be..26eeb4e61 100644 --- a/ngraph_bridge/enable_variable_ops/ngraph_enter_in_catalog.h +++ b/ngraph_bridge/enable_variable_ops/ngraph_enter_in_catalog.h @@ -50,12 +50,12 @@ namespace ngraph_bridge { // We add mapping of {graphId_nodename_InputIndex : Shared_Name} to the // InputVariableSharedNameMap // -// TODO: Update the comments for EnacapOutputInfoMap -// 2. If the output of NGraphEncapsulate Op is an input to NGraphVariableType -// Op, we store this NG-Tensor -// so that it can be directly accessed in compute call of NGraphVariableType. -// We add mapping of {graphId_encapnodename_OutputIndex : NG-Tensor} to the -// EncapOutputTensorMap +// 2. If the input to NGraphAssign Op is from NGraphEncapsulate Op +// We add mapping of +// {graphId_encapnodename_OutputIndex : tuple:{Variable_Shared_Name, CopyToTF, +// IsTFJustLooking}} +// to the EncapOutputInfoMap +// We attach "_ngraph_remove" attribute to the NGraphAssign node // // 3. If the output of NGraphEncapsulate Op is not required by a TF Op or // NGraphEncapsulate Op, diff --git a/ngraph_bridge/enable_variable_ops/ngraph_remove_ngraphassigns.cc b/ngraph_bridge/enable_variable_ops/ngraph_remove_ngraphassigns.cc index 498e07f16..f18804978 100644 --- a/ngraph_bridge/enable_variable_ops/ngraph_remove_ngraphassigns.cc +++ b/ngraph_bridge/enable_variable_ops/ngraph_remove_ngraphassigns.cc @@ -45,11 +45,6 @@ Status RemoveNGraphAssigns(Graph* graph) { input_1->type_string()); } - // Handle input and output edges - // Only adding the new edges, edges to and from the current node will be - // removed when - // node is removed - // https://github.com/tensorflow/tensorflow/blob/master/tensorflow/core/graph/graph.h#L495 // Handle input edges NGRAPH_VLOG(3) << "Handling input edges "; vector remove_edges; From 4b1a475fc7fb76963168179302a914718f9f2457 Mon Sep 17 00:00:00 2001 From: Shrestha Malik Date: Wed, 24 Jul 2019 14:08:53 -0700 Subject: [PATCH 12/17] Integrate with output cache --- ngraph_bridge/ngraph_encapsulate_op.cc | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/ngraph_bridge/ngraph_encapsulate_op.cc b/ngraph_bridge/ngraph_encapsulate_op.cc index 5a5236358..438605654 100644 --- a/ngraph_bridge/ngraph_encapsulate_op.cc +++ b/ngraph_bridge/ngraph_encapsulate_op.cc @@ -697,6 +697,16 @@ class NGraphEncapsulateOp : public OpKernel { ctx->resource_manager()->default_container(), ref_var_name, &var)); current_ng_tensor = var->ng_tensor(); + + // There might be scenarios where the input and output tensors are the + // same. + // The staleness determined for the input tensor should be the final + // staleness + // for the given tensor. The staleness of output tensor should not + // matter + // as this tensor is meant to be overwritten with the computed value + // So not setting staleness here . + output_caches[i] = std::make_pair(current_dst_ptr, current_ng_tensor); var->Unref(); ng_outputs.push_back(current_ng_tensor); continue; @@ -832,8 +842,7 @@ class NGraphEncapsulateOp : public OpKernel { string output_key = NGraphCatalog::CreateNodeKey(m_graph_id, def().name(), i); bool ref_exists = NGraphCatalog::ExistsInEncapOutputInfoMap(output_key); - std::shared_ptr dst_ng_tensor; - void* dst_ptr; + if (ref_exists) { NGRAPH_VLOG(4) << "Syncing the output var tensor " << output_key; @@ -858,13 +867,13 @@ class NGraphEncapsulateOp : public OpKernel { var->set_sync_ng_tensor(true); } } - dst_ng_tensor = var->ng_tensor(); var->Unref(); - dst_ptr = output_caches[i].first; - } else { - std::tie(dst_ptr, dst_ng_tensor) = output_caches[i]; } + std::shared_ptr dst_ng_tensor; + void* dst_ptr; + std::tie(dst_ptr, dst_ng_tensor) = output_caches[i]; + if (m_op_backend_name != "CPU" && NGraphCatalog::EncapOutputIndexNeedsCopy(m_graph_id, def().name(), i)) { From 6e49e07078136c23b2165f47150db5ef9ac11c35 Mon Sep 17 00:00:00 2001 From: Shrestha Malik Date: Wed, 24 Jul 2019 14:13:35 -0700 Subject: [PATCH 13/17] Update ngraph_bridge/enable_variable_ops/ngraph_enter_in_catalog.h --- ngraph_bridge/enable_variable_ops/ngraph_enter_in_catalog.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ngraph_bridge/enable_variable_ops/ngraph_enter_in_catalog.h b/ngraph_bridge/enable_variable_ops/ngraph_enter_in_catalog.h index 26eeb4e61..1857446f1 100644 --- a/ngraph_bridge/enable_variable_ops/ngraph_enter_in_catalog.h +++ b/ngraph_bridge/enable_variable_ops/ngraph_enter_in_catalog.h @@ -55,7 +55,7 @@ namespace ngraph_bridge { // {graphId_encapnodename_OutputIndex : tuple:{Variable_Shared_Name, CopyToTF, // IsTFJustLooking}} // to the EncapOutputInfoMap -// We attach "_ngraph_remove" attribute to the NGraphAssign node +// We attach "_ngraph_remove" attribute to this NGraphAssign node // // 3. If the output of NGraphEncapsulate Op is not required by a TF Op or // NGraphEncapsulate Op, From c80ade72a872a7ac536bf07cb85d20e70e5accdf Mon Sep 17 00:00:00 2001 From: Shrestha Malik Date: Wed, 24 Jul 2019 14:21:52 -0700 Subject: [PATCH 14/17] Minor --- ngraph_bridge/ngraph_encapsulate_op.cc | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/ngraph_bridge/ngraph_encapsulate_op.cc b/ngraph_bridge/ngraph_encapsulate_op.cc index 438605654..ac4816910 100644 --- a/ngraph_bridge/ngraph_encapsulate_op.cc +++ b/ngraph_bridge/ngraph_encapsulate_op.cc @@ -699,13 +699,13 @@ class NGraphEncapsulateOp : public OpKernel { current_ng_tensor = var->ng_tensor(); // There might be scenarios where the input and output tensors are the - // same. - // The staleness determined for the input tensor should be the final - // staleness - // for the given tensor. The staleness of output tensor should not - // matter - // as this tensor is meant to be overwritten with the computed value - // So not setting staleness here . + // same.The staleness determined for the input tensor should be the + // final + // staleness for the given tensor. The staleness of output tensor should + // not + // matter as this tensor is meant to be overwritten with the computed + // value. + // So not setting staleness here. output_caches[i] = std::make_pair(current_dst_ptr, current_ng_tensor); var->Unref(); ng_outputs.push_back(current_ng_tensor); @@ -833,7 +833,7 @@ class NGraphEncapsulateOp : public OpKernel { #if defined(NGRAPH_TF_ENABLE_VARIABLES_AND_OPTIMIZERS) if (m_number_outputs == -1) { NGRAPH_VLOG(4) << "Settig number of outputs for " << def().name(); - m_number_outputs = output_caches.size(); + m_number_outputs = ng_outputs.size(); NGRAPH_VLOG(4) << "Settig number of inputs for " << def().name(); m_number_inputs = ng_inputs.size(); } @@ -1063,4 +1063,4 @@ int NGraphEncapsulateOp::s_instance_count = 0; REGISTER_KERNEL_BUILDER(Name("NGraphEncapsulate").Device(DEVICE_CPU), ngraph_bridge::NGraphEncapsulateOp); -} // namespace tensorflow \ No newline at end of file +} // namespace tensorflow \ No newline at end of file From d4b647c7e779f925dfa3269d2841f565a63dcf43 Mon Sep 17 00:00:00 2001 From: Shrestha Malik Date: Wed, 24 Jul 2019 14:24:46 -0700 Subject: [PATCH 15/17] minor --- ngraph_bridge/ngraph_encapsulate_op.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ngraph_bridge/ngraph_encapsulate_op.cc b/ngraph_bridge/ngraph_encapsulate_op.cc index ac4816910..b68e89bed 100644 --- a/ngraph_bridge/ngraph_encapsulate_op.cc +++ b/ngraph_bridge/ngraph_encapsulate_op.cc @@ -1063,4 +1063,4 @@ int NGraphEncapsulateOp::s_instance_count = 0; REGISTER_KERNEL_BUILDER(Name("NGraphEncapsulate").Device(DEVICE_CPU), ngraph_bridge::NGraphEncapsulateOp); -} // namespace tensorflow \ No newline at end of file +} // namespace tensorflow \ No newline at end of file From 19d1839f1e6e0b88dce95f6922a6bc19b80e0da1 Mon Sep 17 00:00:00 2001 From: Shrestha Malik Date: Wed, 24 Jul 2019 15:29:28 -0700 Subject: [PATCH 16/17] Formatted for formatting --- ngraph_bridge/ngraph_encapsulate_op.cc | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/ngraph_bridge/ngraph_encapsulate_op.cc b/ngraph_bridge/ngraph_encapsulate_op.cc index b68e89bed..2d63d6d18 100644 --- a/ngraph_bridge/ngraph_encapsulate_op.cc +++ b/ngraph_bridge/ngraph_encapsulate_op.cc @@ -700,11 +700,9 @@ class NGraphEncapsulateOp : public OpKernel { // There might be scenarios where the input and output tensors are the // same.The staleness determined for the input tensor should be the - // final - // staleness for the given tensor. The staleness of output tensor should - // not - // matter as this tensor is meant to be overwritten with the computed - // value. + // final staleness for the given tensor. The staleness of output + // tensor should not matter as this tensor is meant to be + // overwritten with the computed value. // So not setting staleness here. output_caches[i] = std::make_pair(current_dst_ptr, current_ng_tensor); var->Unref(); From b44cee05cc18f45994931cdc40917a95c46aca64 Mon Sep 17 00:00:00 2001 From: Shrestha Malik Date: Wed, 24 Jul 2019 16:47:57 -0700 Subject: [PATCH 17/17] Update ngraph_bridge/ngraph_encapsulate_op.cc Co-Authored-By: kanvi-nervana --- ngraph_bridge/ngraph_encapsulate_op.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ngraph_bridge/ngraph_encapsulate_op.cc b/ngraph_bridge/ngraph_encapsulate_op.cc index 2d63d6d18..ad801acae 100644 --- a/ngraph_bridge/ngraph_encapsulate_op.cc +++ b/ngraph_bridge/ngraph_encapsulate_op.cc @@ -832,7 +832,7 @@ class NGraphEncapsulateOp : public OpKernel { if (m_number_outputs == -1) { NGRAPH_VLOG(4) << "Settig number of outputs for " << def().name(); m_number_outputs = ng_outputs.size(); - NGRAPH_VLOG(4) << "Settig number of inputs for " << def().name(); + NGRAPH_VLOG(4) << "Setting number of inputs for " << def().name(); m_number_inputs = ng_inputs.size(); } for (size_t i = 0; i < output_tensor_count; ++i) { @@ -1061,4 +1061,4 @@ int NGraphEncapsulateOp::s_instance_count = 0; REGISTER_KERNEL_BUILDER(Name("NGraphEncapsulate").Device(DEVICE_CPU), ngraph_bridge::NGraphEncapsulateOp); -} // namespace tensorflow \ No newline at end of file +} // namespace tensorflow