From c4c7ce4a160172d07fbd870f43e1149646f97195 Mon Sep 17 00:00:00 2001 From: Sayantan Sarkar Date: Mon, 20 Aug 2018 19:48:18 -0700 Subject: [PATCH 01/32] Stub builder1 --- src/CMakeLists.txt | 1 + src/ngraph_builder1.cc | 287 +++++++++++++++++++++++++++++++++++ src/ngraph_builder1.h | 168 ++++++++++++++++++++ src/ngraph_encapsulate_op.cc | 27 ++++ 4 files changed, 483 insertions(+) create mode 100644 src/ngraph_builder1.cc create mode 100644 src/ngraph_builder1.h diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 2b816508..caf1fdd0 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -43,6 +43,7 @@ add_dependencies(ngraph_lib ext_ngraph) set(SRC ngraph_assign_clusters.cc ngraph_builder.cc + ngraph_builder1.cc ngraph_capture_variables.cc ngraph_cluster_manager.cc ngraph_deassign_clusters.cc diff --git a/src/ngraph_builder1.cc b/src/ngraph_builder1.cc new file mode 100644 index 00000000..96567c6e --- /dev/null +++ b/src/ngraph_builder1.cc @@ -0,0 +1,287 @@ +/******************************************************************************* + * Copyright 2017-2018 Intel Corporation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + *******************************************************************************/ + +#include "ngraph_builder1.h" +//#include "ngraph_conversions.h" +#include "ngraph_log.h" +#include "ngraph_utils.h" + +#include "ngraph/builder/autobroadcast.hpp" +#include "ngraph/builder/numpy_transpose.hpp" + +#include "tensorflow/core/framework/tensor.pb.h" +#include "tensorflow/core/framework/tensor_shape.pb.h" +#include "tensorflow/core/framework/tensor_shape.pb_text.h" +#include "tensorflow/core/graph/algorithm.h" +#include "tensorflow/core/graph/edgeset.h" +#include "tensorflow/core/lib/core/errors.h" + +using namespace std; +namespace ng = ngraph; + +namespace tensorflow { + +namespace ngraph_bridge { + +Status Builder1::TranslateGraph( + const std::vector& inputs, OpKernelContext* ctx, + std::shared_ptr& ng_function) { + // TODO: confirm that static_input_map cannot be constructed in constructor. + // It could be different everytime right? + std::vector + static_input_map; // TODO: use this instead of passing it around + + static_input_map.resize(ctx->num_inputs()); + for (int i = 0; i < ctx->num_inputs(); i++) { + const Tensor& input_tensor = ctx->input(i); + if (m_input_is_static[i]) { + static_input_map[i] = &input_tensor; + } + } + // TODO: pass static_input_map to translate_each_op + + vector> ng_parameter_list; + TF_RETURN_IF_ERROR(get_input_params(inputs, tf_params, &ng_parameter_list)); + + TF_RETURN_IF_ERROR(translate_each_op(tf_ops)); + + vector> ng_result_list; + TF_RETURN_IF_ERROR(get_output_nodes(tf_ops, ng_result_list)); + + // Create the nGraph function. + ng_function = make_shared(ng_result_list, ng_parameter_list); + return Status::OK(); // TODO +} + +Status Builder1::translate_each_op(const vector& tf_ops) { + // Create the nGraph ops from TensorFlow ops. + + for (auto op : tf_ops) { + NGRAPH_VLOG(2) << "Constructing op " << op->name() << " which is " + << op->type_string(); + + try { + // TODO.....TODO....TODO + // TF_RETURN_IF_ERROR(TRANSLATE_OP_MAP.at(op->type_string())( + // op, static_input_map, ng_op_map)); + } catch (const std::out_of_range&) { + // ----------------------------- + // Catch-all for unsupported ops + // ----------------------------- + NGRAPH_VLOG(3) << "Unsupported Op: " << op->name() << " (" + << op->type_string() << ")"; + NGRAPH_VLOG(3) << op->def().DebugString(); + return errors::InvalidArgument("Unsupported Op: ", op->name(), " (", + op->type_string(), ")"); + } + } + return Status::OK(); +} + +Status Builder1::classify_nodes(const vector& ordered, + vector& tf_params, + vector& tf_ret_vals, + vector& tf_ops) { + // Split ops into params, retvals, and all others. + + for (const auto n : ordered) { + if (n->IsSink() || n->IsSource()) { + continue; + } + + if (n->IsControlFlow()) { + return errors::Unimplemented( + "Encountered a control flow op in the nGraph bridge: ", + n->DebugString()); + } + + if (n->type_string() == "_Arg") { + tf_params.push_back(n); + } else if (n->type_string() == "_Retval") { + tf_ret_vals.push_back(n); + } else { + tf_ops.push_back(n); + } + } + return Status::OK(); +} + +Status Builder1::get_input_params( + const std::vector& inputs, vector tf_params, + vector>* ng_parameter_list) { + // Populate the parameter list, and also put parameters into the op map. + + ng_parameter_list->resize(tf_params.size()); + + for (auto parm : tf_params) { + DataType dtype; + if (GetNodeAttr(parm->attrs(), "T", &dtype) != Status::OK()) { + return errors::InvalidArgument("No data type defined for _Arg"); + } + int index; + if (GetNodeAttr(parm->attrs(), "index", &index) != Status::OK()) { + return errors::InvalidArgument("No index defined for _Arg"); + } + + ng::element::Type ng_et; + TF_RETURN_IF_ERROR(TFDataTypeToNGraphElementType(dtype, &ng_et)); + + ng::Shape ng_shape; + TF_RETURN_IF_ERROR(TFTensorShapeToNGraphShape(inputs[index], &ng_shape)); + + auto ng_param = make_shared(ng_et, ng_shape); + SaveNgOp(parm->name(), ng_param); + (*ng_parameter_list)[index] = ng_param; + } + return Status::OK(); +} + +Status Builder1::get_output_nodes( + const vector& tf_ret_vals, + vector>& ng_result_list) { + // Populate the result list. + + ng_result_list.resize(tf_ret_vals.size()); + + for (auto n : tf_ret_vals) { + // Make sure that this _Retval only has one input node. + if (n->num_inputs() != 1) { + return errors::InvalidArgument("_Retval has ", n->num_inputs(), + " inputs, should have 1"); + } + + int index; + if (GetNodeAttr(n->attrs(), "index", &index) != Status::OK()) { + return errors::InvalidArgument("No index defined for _Retval"); + } + + shared_ptr result; + // TF_RETURN_IF_ERROR(GetInputNode(ng_op_map, n, 0, &result)); //TODO...TODO + + ng_result_list[index] = result; + } + return Status::OK(); +} + +// TODO: combine the 2 MakePaddings together? with default args? +// would want the default args to be at the end of the paramlist, +// but 'outputs' (like ng_padding_below, ng_padding_above) are usually at the +// end of the param list +// TODO: why does MakePadding handle only 2 dimensions... generalize it? +// TODO: add unit tests for 1D conv. 1d pooling etc. check if MakePadding works +// in that case +template +void MakePadding(const std::string& tf_padding_type, + const ngraph::Shape& ng_image_shape, + const ngraph::Shape& ng_kernel_shape, + const ngraph::Strides& ng_strides, + const ngraph::Shape& ng_dilations, T& ng_padding_below, + T& ng_padding_above) { + ngraph::Shape ng_dilation_kernel_shape{ + (ng_kernel_shape[0] - 1) * ng_dilations[0] + 1, + (ng_kernel_shape[1] - 1) * ng_dilations[1] + 1}; + + MakePadding(tf_padding_type, ng_image_shape, ng_dilation_kernel_shape, + ng_strides, ng_padding_below, ng_padding_above); +} + +template +void MakePadding(const std::string& tf_padding_type, + const ngraph::Shape& ng_image_shape, + const ngraph::Shape& ng_kernel_shape, + const ngraph::Strides& ng_strides, T& ng_padding_below, + T& ng_padding_above) { + if (tf_padding_type == "SAME") { + for (size_t i = 0; i < 2; i++) { + /* + size_t image_size = ng_image_shape[i]; + size_t filter_shape = ng_kernel_shape[i]; + size_t filter_stride = ng_strides[i]; + + int64 padding_needed; + if (image_size % filter_stride == 0) { + padding_needed = filter_shape - filter_stride; + } else { + padding_needed = filter_shape - (image_size % filter_stride); + } + if (padding_needed < 0) { + padding_needed = 0; + } + */ + + // TODO: check this:. This formula is documented well documented here. + // So I prefer this, compared to the one above (though both are exactly + // the same) + // https://www.tensorflow.org/api_guides/python/nn#Notes_on_SAME_Convolution_Padding + int64 out_shape = ceil(ng_image_shape[i] / ng_strides[i]); + int64 padding_needed = ng_strides[i] * (out_shape - 1) + + ng_kernel_shape[i] - ng_image_shape[i]; + padding_needed = padding_needed < 0 ? 0 : padding_needed; + + size_t padding_lhs = padding_needed / 2; + size_t padding_rhs = padding_needed - padding_lhs; + ng_padding_below[i] = padding_lhs; + ng_padding_above[i] = padding_rhs; + } + } + + NGRAPH_VLOG(3) << "ng_padding_below: " << ngraph::join(ng_padding_below); + NGRAPH_VLOG(3) << "ng_padding_above: " << ngraph::join(ng_padding_above); +} + +Status Builder1::ValidateInputCount(const Node* op, size_t count) { + if (op->num_inputs() != count) { + return errors::InvalidArgument("\"", op->name(), "\" requires ", count, + " input(s), got ", op->num_inputs(), + " instead"); + } + return Status::OK(); +} + +Status Builder1::ValidateInputCountMin(const Node* op, size_t count) { + if (op->num_inputs() < count) { + return errors::InvalidArgument("\"", op->name(), "\" requires at least ", + count, " input(s), got ", op->num_inputs(), + " instead"); + } + return Status::OK(); +} + +void Builder1::SaveNgOp(const std::string& op_name, + const shared_ptr& output_node) { + // no need to try-catch, map[key] will create vector object + // if not exists + ng_op_map[op_name].push_back(output_node); +} + +Status Builder1::init() { + if (is_init) { + // + // We will visit ops in topological order. + // + // ought to be `const Node*`, but GetReversePostOrder doesn't use `const` + + GetReversePostOrder(tf_graph, &ordered); + + TF_RETURN_IF_ERROR(classify_nodes(ordered, tf_params, tf_ret_vals, tf_ops)); + } + is_init = true; + return Status::OK(); +} + +} // namespace ngraph_bridge + +} // namespace tensorflow diff --git a/src/ngraph_builder1.h b/src/ngraph_builder1.h new file mode 100644 index 00000000..da71676f --- /dev/null +++ b/src/ngraph_builder1.h @@ -0,0 +1,168 @@ +/******************************************************************************* + * Copyright 2017-2018 Intel Corporation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + *******************************************************************************/ +#ifndef NGRAPH_TF_BRIDGE_BUILDER1_H_ +#define NGRAPH_TF_BRIDGE_BUILDER1_H_ + +#include +#include + +#include "ngraph/ngraph.hpp" + +#include "ngraph_log.h" +#include "ngraph_mark_for_clustering.h" + +#include "tensorflow/core/framework/op.h" +#include "tensorflow/core/framework/op_kernel.h" +#include "tensorflow/core/framework/tensor_shape.h" +#include "tensorflow/core/graph/algorithm.h" +#include "tensorflow/core/graph/graph.h" + +using namespace std; +namespace ng = ngraph; +namespace tensorflow { + +namespace ngraph_bridge { + +// TODO: namespace detail? overload certain functions vs default args? + +class Builder1 { + using OpMap = std::unordered_map>>; + + private: + OpKernelConstruction* ctx; + bool is_init = false; + const Graph& tf_graph; + std::vector m_input_is_static; + vector ordered; + vector tf_params, tf_ret_vals, tf_ops; + + Status ValidateInputCount(const Node* op, size_t count); + Status ValidateInputCountMin(const Node* op, size_t count); + + // + // The op map holds a mapping from TensorFlow op names (strings) to + // vector of generated nGraph nodes. + // + Builder1::OpMap ng_op_map; + + // helper function for populating ng_op_map + void SaveNgOp(const std::string& op_name, + const shared_ptr& output_node); + + // TODO: write description + Status get_input_params(const std::vector&, vector, + vector>*); + Status classify_nodes(const vector&, vector&, + vector&, vector&); + Status translate_each_op(const vector&); + Status get_output_nodes(const vector&, + vector>&); + + template + void MakePadding(const std::string& tf_padding_type, + const ngraph::Shape& ng_image_shape, + const ngraph::Shape& ng_kernel_shape, + const ngraph::Strides& ng_strides, + const ngraph::Shape& ng_dilations, T& ng_padding_below, + T& ng_padding_above); + + template + void MakePadding(const std::string& tf_padding_type, + const ngraph::Shape& ng_image_shape, + const ngraph::Shape& ng_kernel_shape, + const ngraph::Strides& ng_strides, T& ng_padding_below, + T& ng_padding_above); + + public: + // Note: we need a separate init function for this. because we cant return + // values from constructor, + // we cannot wrap 'classify_nodes' in 'TF_RETURN_IF_ERROR' if we did this part + // in the constructor. + Status init(); + + // TODO: move constructor body to .cc + Builder1(const Graph& tf_graph, OpKernelConstruction* ctx) + : tf_graph(tf_graph), ctx(ctx) { + // TODO: move m_input_is_static construction to init?? + + // + // Initialize the "m_input_is_static" vector as follows: + // (1) create m_input_is_static with n+1 elements, where n is the max arg + // index + // (2) for each _Arg node n, set m_input_is_static[n.index] to true if n + // is driving any static input; else set it to false. + // + + // Create the vector. + int32 max_arg_index = -1; + std::vector arg_nodes; + + for (auto node : tf_graph.nodes()) { + if (node->type_string() == "_Arg") { + arg_nodes.push_back(node); + + int32 index; + // TODO: do we need the ctx here. can we not use it? + // macro defn: + // https://github.com/petewarden/tensorflow_makefile/blob/master/tensorflow/core/framework/op_kernel.h#L1265 + // For now, requiring builder to have access to ctx + OP_REQUIRES_OK(ctx, GetNodeAttr(node->attrs(), "index", &index)); + if (index > max_arg_index) max_arg_index = index; + } + } + + m_input_is_static = std::vector(max_arg_index + 1, false); + + // Fill the vector. + for (auto node : arg_nodes) { + int32 index; + OP_REQUIRES_OK(ctx, GetNodeAttr(node->attrs(), "index", &index)); + + // bool is_static = false; + for (auto edge : node->out_edges()) { + if (edge->IsControlEdge() || !edge->dst()->IsOp()) { + continue; + } + + NGRAPH_VLOG(5) << "For arg " << index << " checking edge " + << edge->DebugString(); + + if (InputIsStatic(edge->dst(), edge->dst_input())) { + NGRAPH_VLOG(5) << "Marking edge static: " << edge->DebugString(); + // is_static = true; + m_input_is_static[index] = true; + break; + } + } + + // NGRAPH_VLOG(5) << "Marking arg " << index << " is_static: " << + // is_static; + // m_input_is_static[index] = is_static; + NGRAPH_VLOG(5) << "Marking arg " << index + << " is static: " << m_input_is_static[index]; + } + } + Status TranslateGraph(const std::vector& inputs, + OpKernelContext* ctx, + std::shared_ptr& ng_function); +}; + +} // namespace ngraph_bridge + +} // namespace tensorflow + +#endif diff --git a/src/ngraph_encapsulate_op.cc b/src/ngraph_encapsulate_op.cc index 8de5c638..3b2c02ef 100644 --- a/src/ngraph_encapsulate_op.cc +++ b/src/ngraph_encapsulate_op.cc @@ -29,6 +29,7 @@ #include "ngraph/serializer.hpp" #include "ngraph_builder.h" +#include "ngraph_builder1.h" #include "ngraph_cluster_manager.h" #include "ngraph_freshness_tracker.h" #include "ngraph_log.h" @@ -37,6 +38,8 @@ #include "ngraph/runtime/interpreter/int_backend.hpp" +#define BUILDER1 + using namespace std; namespace ng = ngraph; @@ -59,10 +62,19 @@ REGISTER_OP("NGraphEncapsulate") .Doc("nGraph Encapsulation Op. For use by the nGraph JIT only."); class NGraphEncapsulateOp : public OpKernel { +#ifdef BUILDER1 + // TODO: remove "m_input_is_static" etc from constructor of + // NGraphEncapsulateOp. + private: + Builder1 ng_builder; +#endif public: explicit NGraphEncapsulateOp(OpKernelConstruction* ctx) : OpKernel(ctx), m_graph(OpRegistry::Global()), +#ifdef BUILDER1 + ng_builder(m_graph, ctx), +#endif m_freshness_tracker(nullptr) { GraphDef* graph_def; @@ -73,6 +85,10 @@ class NGraphEncapsulateOp : public OpKernel { opts.allow_internal_ops = true; OP_REQUIRES_OK(ctx, ConvertGraphDefToGraph(opts, *graph_def, &m_graph)); +#ifdef BUILDER1 + ng_builder.init(); +#else + // // Initialize the "m_input_is_static" vector as follows: // (1) create m_input_is_static with n+1 elements, where n is the max arg @@ -121,6 +137,7 @@ class NGraphEncapsulateOp : public OpKernel { NGRAPH_VLOG(5) << "Marking arg " << index << " is_static: " << is_static; m_input_is_static[index] = is_static; } +#endif // Create the backend if (m_ng_backend == nullptr) { @@ -240,11 +257,15 @@ class NGraphEncapsulateOp : public OpKernel { signature_ss << "/"; +#ifndef BUILDER1 std::vector static_input_map(ctx->num_inputs()); +#endif for (int i = 0; i < ctx->num_inputs(); i++) { const Tensor& input_tensor = ctx->input(i); if (m_input_is_static[i]) { +#ifndef BUILDER1 static_input_map[i] = &input_tensor; +#endif OP_REQUIRES_OK(ctx, TensorToStream(signature_ss, input_tensor)); signature_ss << ";"; } @@ -267,9 +288,15 @@ class NGraphEncapsulateOp : public OpKernel { // TODO(amprocte): Investigate performance of the compilation cache. if (it == m_ng_functions.end()) { NGRAPH_VLOG(1) << "Compilation cache miss: " << ctx->op_kernel().name(); +#ifndef BUILDER1 + OP_REQUIRES_OK( ctx, Builder::TranslateGraph(input_shapes, static_input_map, &m_graph, ng_function)); +#else + OP_REQUIRES_OK(ctx, + ng_builder.TranslateGraph(input_shapes, ctx, ng_function)); +#endif // Serialize to nGraph if needed if (std::getenv("NGRAPH_ENABLE_SERIALIZE") != nullptr) { From 22b0dc7f125f83ef09cc0938877604299f6d197b Mon Sep 17 00:00:00 2001 From: Sayantan Sarkar Date: Mon, 20 Aug 2018 19:55:52 -0700 Subject: [PATCH 02/32] Compute input shapes inside TranslateGraph --- src/ngraph_builder1.cc | 6 ++++-- src/ngraph_builder1.h | 3 +-- src/ngraph_encapsulate_op.cc | 3 +-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/ngraph_builder1.cc b/src/ngraph_builder1.cc index 96567c6e..3b04a094 100644 --- a/src/ngraph_builder1.cc +++ b/src/ngraph_builder1.cc @@ -37,19 +37,21 @@ namespace tensorflow { namespace ngraph_bridge { Status Builder1::TranslateGraph( - const std::vector& inputs, OpKernelContext* ctx, - std::shared_ptr& ng_function) { + OpKernelContext* ctx, std::shared_ptr& ng_function) { // TODO: confirm that static_input_map cannot be constructed in constructor. // It could be different everytime right? std::vector static_input_map; // TODO: use this instead of passing it around + std::vector inputs(ctx->num_inputs()); + static_input_map.resize(ctx->num_inputs()); for (int i = 0; i < ctx->num_inputs(); i++) { const Tensor& input_tensor = ctx->input(i); if (m_input_is_static[i]) { static_input_map[i] = &input_tensor; } + inputs[i] = input_tensor.shape(); } // TODO: pass static_input_map to translate_each_op diff --git a/src/ngraph_builder1.h b/src/ngraph_builder1.h index da71676f..9afa96d9 100644 --- a/src/ngraph_builder1.h +++ b/src/ngraph_builder1.h @@ -156,8 +156,7 @@ class Builder1 { << " is static: " << m_input_is_static[index]; } } - Status TranslateGraph(const std::vector& inputs, - OpKernelContext* ctx, + Status TranslateGraph(OpKernelContext* ctx, std::shared_ptr& ng_function); }; diff --git a/src/ngraph_encapsulate_op.cc b/src/ngraph_encapsulate_op.cc index 3b2c02ef..cd1e466b 100644 --- a/src/ngraph_encapsulate_op.cc +++ b/src/ngraph_encapsulate_op.cc @@ -294,8 +294,7 @@ class NGraphEncapsulateOp : public OpKernel { ctx, Builder::TranslateGraph(input_shapes, static_input_map, &m_graph, ng_function)); #else - OP_REQUIRES_OK(ctx, - ng_builder.TranslateGraph(input_shapes, ctx, ng_function)); + OP_REQUIRES_OK(ctx, ng_builder.TranslateGraph(ctx, ng_function)); #endif // Serialize to nGraph if needed From 3fa9ed0a2ec1d83ab6bfbe441c2f0aa44d5277bc Mon Sep 17 00:00:00 2001 From: Sayantan Sarkar Date: Tue, 21 Aug 2018 12:05:40 -0700 Subject: [PATCH 03/32] Added and building with a few utility functions --- src/ngraph_builder1.cc | 40 +++++++++++++- src/ngraph_builder1.h | 115 ++++++++++++++++++++++++++++++++++++++--- 2 files changed, 147 insertions(+), 8 deletions(-) diff --git a/src/ngraph_builder1.cc b/src/ngraph_builder1.cc index 3b04a094..4a80470d 100644 --- a/src/ngraph_builder1.cc +++ b/src/ngraph_builder1.cc @@ -40,8 +40,10 @@ Status Builder1::TranslateGraph( OpKernelContext* ctx, std::shared_ptr& ng_function) { // TODO: confirm that static_input_map cannot be constructed in constructor. // It could be different everytime right? + // TODO: static_input_map can't be a class variable right? Its different everytime TranslateGraph is called, + // so it must be passed around? std::vector - static_input_map; // TODO: use this instead of passing it around + static_input_map; std::vector inputs(ctx->num_inputs()); @@ -284,6 +286,42 @@ Status Builder1::init() { return Status::OK(); } + + + +//TODO: move translate ops to a different file +//TODO: make TranslateOps not static? +Status TranslateFloorDivOp1( + const Node* op, const std::vector& static_input_map) { + auto ng_floordiv = [](std::shared_ptr ng_input1, + std::shared_ptr ng_input2) { + return std::make_shared( + std::make_shared(ng_input1, ng_input2)); + }; + //TODO + //return TranslateBinaryOp(op, static_input_map, ng_op_map, ng_floordiv); +} + +Status TranslateFloorModOp1( + const Node* op, const std::vector& static_input_map) { + auto ng_floormod = [](std::shared_ptr ng_input1, + std::shared_ptr ng_input2) { + auto floordiv = std::make_shared( + std::make_shared(ng_input1, ng_input2)); + return std::make_shared( + ng_input1, std::make_shared(floordiv, ng_input2)); + }; + //TODO + //return TranslateBinaryOp(op, static_input_map, ng_op_map, ng_floormod); +} + +const std::map< + const string, + const function&)>> + Builder1::TRANSLATE_OP_MAP{ + {"FloorDiv", TranslateFloorDivOp1}, + {"FloorMod", TranslateFloorModOp1}}; + } // namespace ngraph_bridge } // namespace tensorflow diff --git a/src/ngraph_builder1.h b/src/ngraph_builder1.h index 9afa96d9..d4a91a05 100644 --- a/src/ngraph_builder1.h +++ b/src/ngraph_builder1.h @@ -37,14 +37,118 @@ namespace tensorflow { namespace ngraph_bridge { // TODO: namespace detail? overload certain functions vs default args? +//TODO: make sure all comments from old builder are copied correctly. + + + + + + + + +///////////////// class Builder1 { using OpMap = std::unordered_map>>; private: - OpKernelConstruction* ctx; - bool is_init = false; + // + // The op map holds a mapping from TensorFlow op names (strings) to + // vector of generated nGraph nodes. + // + Builder1::OpMap ng_op_map; + + + + //TODO: move GetInputNode(s) body to .cc + Status GetInputNode(const Node* op, + size_t input_idx, shared_ptr* result) { + // input op may have resulted in more than one ng::Node (eg. Split) + // we need to look at Edge to check index of the input op + std::vector edges; + TF_RETURN_IF_ERROR(op->input_edges(&edges)); + size_t src_output_idx; + try { + src_output_idx = edges.at(input_idx)->src_output(); + } catch (const out_of_range&) { + return Status(tensorflow::error::NOT_FOUND, "Edge not found"); + } + + Node* tf_input; + TF_RETURN_IF_ERROR(op->input_node(input_idx, &tf_input)); + try { + *result = ng_op_map.at(tf_input->name()).at(src_output_idx); + } catch (const out_of_range&) { + return Status(tensorflow::error::NOT_FOUND, "Input node not found"); + } + return Status::OK(); +} + +//TODO: enable detail namespace? +//Note: namespace details prevents template recursion error during compilation +//cannot use namespace in class, so using different names +//namespace detail { + Status detail_GetInputNodes(const Node* op, + size_t index) { + return Status::OK(); +} + +template + Status detail_GetInputNodes(const Node* op, + size_t index, shared_ptr* result, + Arguments&&... remaining) { + if (result != nullptr) { + TF_RETURN_IF_ERROR(GetInputNode(op, index, result)); + } + return detail_GetInputNodes(op, index + 1, remaining...); +} +//} // namespace detail + +template + Status GetInputNodes(const Node* op, Arguments&&... remaining) { + constexpr size_t args_len = sizeof...(Arguments); + TF_RETURN_IF_ERROR(ValidateInputCount(op, args_len)); + //return detail::GetInputNodes(ng_op_map, op, 0, remaining...); //TODO.. detail namespace + return detail_GetInputNodes(op, 0, remaining...); +} + + + //TODO: reconsider TranslateBinaryOp + Status TranslateBinaryOp( + const Node* op, const std::vector& static_input_map, + std::function(std::shared_ptr, + std::shared_ptr)> + create_binary_op) { + std::shared_ptr ng_lhs, ng_rhs; + TF_RETURN_IF_ERROR(GetInputNodes(op, &ng_lhs, &ng_rhs)); + + std::tie(ng_lhs, ng_rhs) = + ng::builder::numpy_broadcast(std::make_pair(ng_lhs, ng_rhs)); + + //TODO: enable this + //SaveNgOp(ng_op_map, op->name(), create_binary_op(ng_lhs, ng_rhs)); + + return Status::OK(); +} + +template +Status TranslateBinaryOp( + const Node* op, const std::vector& static_input_map) { + return TranslateBinaryOp( + op, static_input_map, + [](std::shared_ptr ng_lhs, std::shared_ptr ng_rhs) { + return make_shared(ng_lhs, ng_rhs); + }); +} + + const static std::map< + const string, + const function&)>> + TRANSLATE_OP_MAP; + + OpKernelConstruction* ctx; //TODO: do we need to save it? + bool is_init = false; //Prevent init from running twice const Graph& tf_graph; std::vector m_input_is_static; vector ordered; @@ -53,11 +157,7 @@ class Builder1 { Status ValidateInputCount(const Node* op, size_t count); Status ValidateInputCountMin(const Node* op, size_t count); - // - // The op map holds a mapping from TensorFlow op names (strings) to - // vector of generated nGraph nodes. - // - Builder1::OpMap ng_op_map; + // helper function for populating ng_op_map void SaveNgOp(const std::string& op_name, @@ -97,6 +197,7 @@ class Builder1 { // TODO: move constructor body to .cc Builder1(const Graph& tf_graph, OpKernelConstruction* ctx) : tf_graph(tf_graph), ctx(ctx) { + //TODO: maybe we do not need to copy ctx into a pvt variable., as we do not use it later // TODO: move m_input_is_static construction to init?? // From 75cba5a3c6fc229e772eecc79b32cf5820f1f4de Mon Sep 17 00:00:00 2001 From: Sayantan Sarkar Date: Tue, 21 Aug 2018 13:23:09 -0700 Subject: [PATCH 04/32] Some cleanup --- src/ngraph_builder1.cc | 75 +++++++++++++--- src/ngraph_builder1.h | 160 +++++++++++++---------------------- src/ngraph_encapsulate_op.cc | 2 +- 3 files changed, 121 insertions(+), 116 deletions(-) diff --git a/src/ngraph_builder1.cc b/src/ngraph_builder1.cc index 4a80470d..2ed18d29 100644 --- a/src/ngraph_builder1.cc +++ b/src/ngraph_builder1.cc @@ -40,10 +40,13 @@ Status Builder1::TranslateGraph( OpKernelContext* ctx, std::shared_ptr& ng_function) { // TODO: confirm that static_input_map cannot be constructed in constructor. // It could be different everytime right? - // TODO: static_input_map can't be a class variable right? Its different everytime TranslateGraph is called, + // TODO: static_input_map can't be a class variable right? Its different + // everytime TranslateGraph is called, // so it must be passed around? - std::vector - static_input_map; + + if (!is_init) init(); + + std::vector static_input_map; std::vector inputs(ctx->num_inputs()); @@ -81,6 +84,10 @@ Status Builder1::translate_each_op(const vector& tf_ops) { // TODO.....TODO....TODO // TF_RETURN_IF_ERROR(TRANSLATE_OP_MAP.at(op->type_string())( // op, static_input_map, ng_op_map)); + // required_ng_nodes = get_ng_nodes[op->type_string()](); //TODO add error + // check + // out_ng_nodes = get_ng_function[op->type_string()](required_nodes); + } catch (const std::out_of_range&) { // ----------------------------- // Catch-all for unsupported ops @@ -286,11 +293,56 @@ Status Builder1::init() { return Status::OK(); } +Status Builder1::GetInputNode(const Node* op, size_t input_idx, + shared_ptr* result) { + // input op may have resulted in more than one ng::Node (eg. Split) + // we need to look at Edge to check index of the input op + std::vector edges; + TF_RETURN_IF_ERROR(op->input_edges(&edges)); + size_t src_output_idx; + try { + src_output_idx = edges.at(input_idx)->src_output(); + } catch (const out_of_range&) { + return Status(tensorflow::error::NOT_FOUND, "Edge not found"); + } + Node* tf_input; + TF_RETURN_IF_ERROR(op->input_node(input_idx, &tf_input)); + try { + *result = ng_op_map.at(tf_input->name()).at(src_output_idx); + } catch (const out_of_range&) { + return Status(tensorflow::error::NOT_FOUND, "Input node not found"); + } + return Status::OK(); +} +// namespace detail { +Status Builder1::detail_GetInputNodes(const Node* op, size_t index) { + return Status::OK(); +} + +template +Status Builder1::detail_GetInputNodes(const Node* op, size_t index, + shared_ptr* result, + Arguments&&... remaining) { + if (result != nullptr) { + TF_RETURN_IF_ERROR(GetInputNode(op, index, result)); + } + return Builder1::detail_GetInputNodes(op, index + 1, remaining...); +} +//} // namespace detail + +template +Status Builder1::GetInputNodes(const Node* op, Arguments&&... remaining) { + constexpr size_t args_len = sizeof...(Arguments); + TF_RETURN_IF_ERROR(ValidateInputCount(op, args_len)); + // return detail::GetInputNodes(ng_op_map, op, 0, remaining...); //TODO.. + // detail namespace + return detail_GetInputNodes(op, 0, remaining...); +} -//TODO: move translate ops to a different file -//TODO: make TranslateOps not static? +// TODO: move translate ops to a different file +// TODO: make TranslateOps not static? Status TranslateFloorDivOp1( const Node* op, const std::vector& static_input_map) { auto ng_floordiv = [](std::shared_ptr ng_input1, @@ -298,8 +350,8 @@ Status TranslateFloorDivOp1( return std::make_shared( std::make_shared(ng_input1, ng_input2)); }; - //TODO - //return TranslateBinaryOp(op, static_input_map, ng_op_map, ng_floordiv); + // TODO + // return TranslateBinaryOp(op, static_input_map, ng_op_map, ng_floordiv); } Status TranslateFloorModOp1( @@ -311,16 +363,15 @@ Status TranslateFloorModOp1( return std::make_shared( ng_input1, std::make_shared(floordiv, ng_input2)); }; - //TODO - //return TranslateBinaryOp(op, static_input_map, ng_op_map, ng_floormod); + // TODO + // return TranslateBinaryOp(op, static_input_map, ng_op_map, ng_floormod); } const std::map< const string, const function&)>> - Builder1::TRANSLATE_OP_MAP{ - {"FloorDiv", TranslateFloorDivOp1}, - {"FloorMod", TranslateFloorModOp1}}; + Builder1::TRANSLATE_OP_MAP{{"FloorDiv", TranslateFloorDivOp1}, + {"FloorMod", TranslateFloorModOp1}}; } // namespace ngraph_bridge diff --git a/src/ngraph_builder1.h b/src/ngraph_builder1.h index d4a91a05..ecb969fa 100644 --- a/src/ngraph_builder1.h +++ b/src/ngraph_builder1.h @@ -37,14 +37,7 @@ namespace tensorflow { namespace ngraph_bridge { // TODO: namespace detail? overload certain functions vs default args? -//TODO: make sure all comments from old builder are copied correctly. - - - - - - - +// TODO: make sure all comments from old builder are copied correctly. ///////////////// @@ -53,112 +46,70 @@ class Builder1 { std::vector>>; private: - // + // // The op map holds a mapping from TensorFlow op names (strings) to // vector of generated nGraph nodes. // Builder1::OpMap ng_op_map; - - - - //TODO: move GetInputNode(s) body to .cc - Status GetInputNode(const Node* op, - size_t input_idx, shared_ptr* result) { - // input op may have resulted in more than one ng::Node (eg. Split) - // we need to look at Edge to check index of the input op - std::vector edges; - TF_RETURN_IF_ERROR(op->input_edges(&edges)); - size_t src_output_idx; - try { - src_output_idx = edges.at(input_idx)->src_output(); - } catch (const out_of_range&) { - return Status(tensorflow::error::NOT_FOUND, "Edge not found"); - } - Node* tf_input; - TF_RETURN_IF_ERROR(op->input_node(input_idx, &tf_input)); - try { - *result = ng_op_map.at(tf_input->name()).at(src_output_idx); - } catch (const out_of_range&) { - return Status(tensorflow::error::NOT_FOUND, "Input node not found"); - } - return Status::OK(); -} - -//TODO: enable detail namespace? -//Note: namespace details prevents template recursion error during compilation -//cannot use namespace in class, so using different names -//namespace detail { - Status detail_GetInputNodes(const Node* op, - size_t index) { - return Status::OK(); -} - -template - Status detail_GetInputNodes(const Node* op, - size_t index, shared_ptr* result, - Arguments&&... remaining) { - if (result != nullptr) { - TF_RETURN_IF_ERROR(GetInputNode(op, index, result)); - } - return detail_GetInputNodes(op, index + 1, remaining...); -} -//} // namespace detail - -template - Status GetInputNodes(const Node* op, Arguments&&... remaining) { - constexpr size_t args_len = sizeof...(Arguments); - TF_RETURN_IF_ERROR(ValidateInputCount(op, args_len)); - //return detail::GetInputNodes(ng_op_map, op, 0, remaining...); //TODO.. detail namespace - return detail_GetInputNodes(op, 0, remaining...); -} - - - //TODO: reconsider TranslateBinaryOp - Status TranslateBinaryOp( - const Node* op, const std::vector& static_input_map, - std::function(std::shared_ptr, - std::shared_ptr)> - create_binary_op) { - std::shared_ptr ng_lhs, ng_rhs; - TF_RETURN_IF_ERROR(GetInputNodes(op, &ng_lhs, &ng_rhs)); - - std::tie(ng_lhs, ng_rhs) = - ng::builder::numpy_broadcast(std::make_pair(ng_lhs, ng_rhs)); - - //TODO: enable this - //SaveNgOp(ng_op_map, op->name(), create_binary_op(ng_lhs, ng_rhs)); - - return Status::OK(); -} - -template -Status TranslateBinaryOp( - const Node* op, const std::vector& static_input_map) { - return TranslateBinaryOp( - op, static_input_map, - [](std::shared_ptr ng_lhs, std::shared_ptr ng_rhs) { - return make_shared(ng_lhs, ng_rhs); - }); -} - - const static std::map< - const string, - const function&)>> - TRANSLATE_OP_MAP; - - OpKernelConstruction* ctx; //TODO: do we need to save it? - bool is_init = false; //Prevent init from running twice + const static std::map< + const string, + const function&)>> + TRANSLATE_OP_MAP; + + OpKernelConstruction* ctx; // TODO: do we need to save it? + bool is_init = false; // Prevent init from running twice const Graph& tf_graph; std::vector m_input_is_static; vector ordered; vector tf_params, tf_ret_vals, tf_ops; + Status GetInputNode(const Node*, size_t, shared_ptr*); + + // TODO: enable detail namespace? + // Note: namespace details prevents template recursion error during + // compilation + // cannot use namespace in class, so using different names + Status detail_GetInputNodes(const Node* op, size_t index); + + template + Status detail_GetInputNodes(const Node*, size_t, shared_ptr*, + Arguments&&...); + + template + Status GetInputNodes(const Node*, Arguments&&...); + + // TODO: reconsider TranslateBinaryOp + Status TranslateBinaryOp( + const Node* op, const std::vector& static_input_map, + std::function(std::shared_ptr, + std::shared_ptr)> + create_binary_op) { + std::shared_ptr ng_lhs, ng_rhs; + TF_RETURN_IF_ERROR(GetInputNodes(op, &ng_lhs, &ng_rhs)); + + std::tie(ng_lhs, ng_rhs) = + ng::builder::numpy_broadcast(std::make_pair(ng_lhs, ng_rhs)); + + // TODO: enable this + // SaveNgOp(ng_op_map, op->name(), create_binary_op(ng_lhs, ng_rhs)); + + return Status::OK(); + } + + template + Status TranslateBinaryOp(const Node* op, + const std::vector& static_input_map) { + return TranslateBinaryOp( + op, static_input_map, + [](std::shared_ptr ng_lhs, std::shared_ptr ng_rhs) { + return make_shared(ng_lhs, ng_rhs); + }); + } + Status ValidateInputCount(const Node* op, size_t count); Status ValidateInputCountMin(const Node* op, size_t count); - - // helper function for populating ng_op_map void SaveNgOp(const std::string& op_name, const shared_ptr& output_node); @@ -187,17 +138,20 @@ Status TranslateBinaryOp( const ngraph::Strides& ng_strides, T& ng_padding_below, T& ng_padding_above); - public: // Note: we need a separate init function for this. because we cant return // values from constructor, // we cannot wrap 'classify_nodes' in 'TF_RETURN_IF_ERROR' if we did this part // in the constructor. Status init(); + public: // TODO: move constructor body to .cc Builder1(const Graph& tf_graph, OpKernelConstruction* ctx) : tf_graph(tf_graph), ctx(ctx) { - //TODO: maybe we do not need to copy ctx into a pvt variable., as we do not use it later + // TODO: since we have an init anyway, why not move this stuff there + + // TODO: maybe we do not need to copy ctx into a pvt variable., as we do not + // use it later // TODO: move m_input_is_static construction to init?? // diff --git a/src/ngraph_encapsulate_op.cc b/src/ngraph_encapsulate_op.cc index cd1e466b..de8df320 100644 --- a/src/ngraph_encapsulate_op.cc +++ b/src/ngraph_encapsulate_op.cc @@ -86,7 +86,7 @@ class NGraphEncapsulateOp : public OpKernel { OP_REQUIRES_OK(ctx, ConvertGraphDefToGraph(opts, *graph_def, &m_graph)); #ifdef BUILDER1 - ng_builder.init(); +// ng_builder.init(); //Now TranslateGraph will call init() if needed #else // From c074b4bb51928993e3fd992718ec1285ba461e16 Mon Sep 17 00:00:00 2001 From: Sayantan Sarkar Date: Tue, 21 Aug 2018 16:48:14 -0700 Subject: [PATCH 05/32] Saving for the comments. Does not build --- src/ngraph_builder1.cc | 46 +++++++++++++++++++++++++++++++++++++++--- 1 file changed, 43 insertions(+), 3 deletions(-) diff --git a/src/ngraph_builder1.cc b/src/ngraph_builder1.cc index 2ed18d29..f2641270 100644 --- a/src/ngraph_builder1.cc +++ b/src/ngraph_builder1.cc @@ -84,9 +84,48 @@ Status Builder1::translate_each_op(const vector& tf_ops) { // TODO.....TODO....TODO // TF_RETURN_IF_ERROR(TRANSLATE_OP_MAP.at(op->type_string())( // op, static_input_map, ng_op_map)); - // required_ng_nodes = get_ng_nodes[op->type_string()](); //TODO add error - // check - // out_ng_nodes = get_ng_function[op->type_string()](required_nodes); + required_ng_inputs = getInputNodes(op); + required_ng_arguments = get_ng_nodes[op->type_string()](); //TODO add error check + //Note: required_ng_nodes could be a map. string->ngnode. + // string would be "arguments" in ng ops like 'shape' and 'one_hot_axis' for OneHot: + // https://ngraph.nervanasys.com/index.html/ops/one_hot.html + // A map is preferable to a vector, as the map would have some semantic meaning for the arguments + // nodes['shape'] is more informative than nodes[0]. + out_ng_nodes = get_ng_function[op->type_string()](required_nodes); + + /* + consider this avgpool function. It accepts an input node and a bunch of arguments + one of the arguments is not a ng node but a boolean. + How to pass the bool? Had assumed all ng ops accept only ngnodes + another example is concat, which accepts an int + another example is convert, which accepts an ngraph::element::type + */ + + //Need to scope the ops... maybe all non-ng-node type ops are constant, and does not need passing. + // But that is bad design probably. + // union? ugh... + + + // We can ATLEAST extract out SaveNgOp right? + // but then the translateops would have to return a ngnode, rather than a status.... statusor?, pointer-to-ngnode as input? + + //Looks like it is pretty intricately linked. may not be easy/feasible/readable to decompose + // TranslateOp to compute(getinp(op)). The gains do not seem too much for all the trouble? + // Why not have + + //The static_input_map thing: + //some ops have static input, not everyone needs this input. + //we can make static_input_map a class data member, and set it to nullptr once the TranslateGraph is done + // That way, we do not pass it around, and make sure we are not reusing stale static_input_map + + + + i = getinputs() + o = compute(i) ..i ng nodes + + std::shared_ptr ng_avgpool = + make_shared(ng_input, ng_kernel_shape, ng_strides, + ng_padding_below, ng_padding_above, false); } catch (const std::out_of_range&) { // ----------------------------- @@ -316,6 +355,7 @@ Status Builder1::GetInputNode(const Node* op, size_t input_idx, return Status::OK(); } +//TODO: inner class? // namespace detail { Status Builder1::detail_GetInputNodes(const Node* op, size_t index) { return Status::OK(); From 0ab350c9b4fb213f277a89c7082aa0bb70e6a02a Mon Sep 17 00:00:00 2001 From: Sayantan Sarkar Date: Thu, 23 Aug 2018 11:07:09 -0700 Subject: [PATCH 06/32] Backend plan 1: pass translate functions the ng nodes --- src/ngraph_builder1.cc | 244 +++++++++++++++++++++++++++++------------ src/ngraph_builder1.h | 107 +++++------------- 2 files changed, 201 insertions(+), 150 deletions(-) diff --git a/src/ngraph_builder1.cc b/src/ngraph_builder1.cc index f2641270..172c37e8 100644 --- a/src/ngraph_builder1.cc +++ b/src/ngraph_builder1.cc @@ -44,7 +44,7 @@ Status Builder1::TranslateGraph( // everytime TranslateGraph is called, // so it must be passed around? - if (!is_init) init(); + if (!is_initialized) Initialize(); std::vector static_input_map; @@ -61,19 +61,20 @@ Status Builder1::TranslateGraph( // TODO: pass static_input_map to translate_each_op vector> ng_parameter_list; - TF_RETURN_IF_ERROR(get_input_params(inputs, tf_params, &ng_parameter_list)); + TF_RETURN_IF_ERROR(GetInputParams(inputs, tf_params, &ng_parameter_list)); - TF_RETURN_IF_ERROR(translate_each_op(tf_ops)); + TF_RETURN_IF_ERROR(TranslateEachOp(tf_ops, static_input_map)); vector> ng_result_list; - TF_RETURN_IF_ERROR(get_output_nodes(tf_ops, ng_result_list)); + TF_RETURN_IF_ERROR(GetOutputNodes(tf_ops, ng_result_list)); // Create the nGraph function. ng_function = make_shared(ng_result_list, ng_parameter_list); return Status::OK(); // TODO } -Status Builder1::translate_each_op(const vector& tf_ops) { +//TODO, is static_input_map const? +Status Builder1::TranslateEachOp(const vector& tf_ops, const std::vector& static_input_map) { // Create the nGraph ops from TensorFlow ops. for (auto op : tf_ops) { @@ -81,53 +82,62 @@ Status Builder1::translate_each_op(const vector& tf_ops) { << op->type_string(); try { + //extract out parent node finding and savengops + // have the translateops as a list of pluggable functions (pimpl?) + // TODO.....TODO....TODO // TF_RETURN_IF_ERROR(TRANSLATE_OP_MAP.at(op->type_string())( // op, static_input_map, ng_op_map)); - required_ng_inputs = getInputNodes(op); - required_ng_arguments = get_ng_nodes[op->type_string()](); //TODO add error check - //Note: required_ng_nodes could be a map. string->ngnode. - // string would be "arguments" in ng ops like 'shape' and 'one_hot_axis' for OneHot: - // https://ngraph.nervanasys.com/index.html/ops/one_hot.html - // A map is preferable to a vector, as the map would have some semantic meaning for the arguments - // nodes['shape'] is more informative than nodes[0]. - out_ng_nodes = get_ng_function[op->type_string()](required_nodes); - - /* - consider this avgpool function. It accepts an input node and a bunch of arguments - one of the arguments is not a ng node but a boolean. - How to pass the bool? Had assumed all ng ops accept only ngnodes - another example is concat, which accepts an int - another example is convert, which accepts an ngraph::element::type - */ - - //Need to scope the ops... maybe all non-ng-node type ops are constant, and does not need passing. - // But that is bad design probably. - // union? ugh... - - - // We can ATLEAST extract out SaveNgOp right? - // but then the translateops would have to return a ngnode, rather than a status.... statusor?, pointer-to-ngnode as input? - - //Looks like it is pretty intricately linked. may not be easy/feasible/readable to decompose - // TranslateOp to compute(getinp(op)). The gains do not seem too much for all the trouble? - // Why not have - + //The static_input_map thing: //some ops have static input, not everyone needs this input. //we can make static_input_map a class data member, and set it to nullptr once the TranslateGraph is done // That way, we do not pass it around, and make sure we are not reusing stale static_input_map + //Note: abstracting parents = GetInputNodes(): + // unknown number of nodes for each op + //std::shared_ptr ng_lhs, ng_rhs; + //TF_RETURN_IF_ERROR(GetInputNodes(op, &ng_lhs, &ng_rhs)); + + vector> subgraph_out_nodes; + //TF_RETURN_IF_ERROR(TRANSLATE_OP_MAP.at(op->type_string())(op, static_input_map)); + auto iter = TRANSLATE_OP_MAP.find(op->type_string()); + if (iter != TRANSLATE_OP_MAP.end()){ + Builder1::TranslatorFn translate_fn = iter->second.first; + vector input_idxs = iter->second.second; + //std::tie(translate_fn, input_idxs) = iter->second; + // input_idxs can be size 0 (to indicate/handle variadic inputs nodes like Addn) + bool variadic_input = input_idxs.size()==0; + int num_inputs = variadic_input ? op->num_inputs() : input_idxs.size(); + std::vector> ng_arg_vec(num_inputs); + for (int idx = 0; idx < num_inputs; idx++){ + TF_RETURN_IF_ERROR(GetInputNode(op, (variadic_input ? idx : input_idxs[idx]), &ng_arg_vec[idx])); + } + TF_RETURN_IF_ERROR(iter->second.first(op, ng_arg_vec, static_input_map, subgraph_out_nodes)); + } + else{ + //TODO TODO::: if-else or try-catch + // ----------------------------- + // Catch-all for unsupported ops + // ----------------------------- + NGRAPH_VLOG(3) << "Unsupported Op: " << op->name() << " (" + << op->type_string() << ")"; + NGRAPH_VLOG(3) << op->def().DebugString(); + return errors::InvalidArgument("Unsupported Op: ", op->name(), " (", + op->type_string(), ")"); + } - i = getinputs() - o = compute(i) ..i ng nodes + + //for (auto ng_node : subgraph_out_nodes) + // SaveNgOp(ng_op_map, op->name(), ng_node); + ng_op_map[op->name()] = subgraph_out_nodes; //SaveNgOp - std::shared_ptr ng_avgpool = - make_shared(ng_input, ng_kernel_shape, ng_strides, - ng_padding_below, ng_padding_above, false); + //TranslateBinaryOp(op, static_input_map, ng_op_map, ng_floormod) - } catch (const std::out_of_range&) { + } + //TODO: catching the error above. do we keep the try catch or use if-else? + catch (const std::out_of_range&) { // ----------------------------- // Catch-all for unsupported ops // ----------------------------- @@ -141,7 +151,7 @@ Status Builder1::translate_each_op(const vector& tf_ops) { return Status::OK(); } -Status Builder1::classify_nodes(const vector& ordered, +Status Builder1::ClassifyNodes(const vector& ordered, vector& tf_params, vector& tf_ret_vals, vector& tf_ops) { @@ -169,7 +179,7 @@ Status Builder1::classify_nodes(const vector& ordered, return Status::OK(); } -Status Builder1::get_input_params( +Status Builder1::GetInputParams( const std::vector& inputs, vector tf_params, vector>* ng_parameter_list) { // Populate the parameter list, and also put parameters into the op map. @@ -199,7 +209,7 @@ Status Builder1::get_input_params( return Status::OK(); } -Status Builder1::get_output_nodes( +Status Builder1::GetOutputNodes( const vector& tf_ret_vals, vector>& ng_result_list) { // Populate the result list. @@ -317,8 +327,8 @@ void Builder1::SaveNgOp(const std::string& op_name, ng_op_map[op_name].push_back(output_node); } -Status Builder1::init() { - if (is_init) { +Status Builder1::Initialize() { + if (is_initialized) { // // We will visit ops in topological order. // @@ -326,9 +336,75 @@ Status Builder1::init() { GetReversePostOrder(tf_graph, &ordered); - TF_RETURN_IF_ERROR(classify_nodes(ordered, tf_params, tf_ret_vals, tf_ops)); + TF_RETURN_IF_ERROR(ClassifyNodes(ordered, tf_params, tf_ret_vals, tf_ops)); + + // TODO: since we have an init anyway, why not move this stuff there + + // TODO: maybe we do not need to copy ctx into a pvt variable., as we do not + // use it later + // TODO: move m_input_is_static construction to init?? + + // + // Initialize the "m_input_is_static" vector as follows: + // (1) create m_input_is_static with n+1 elements, where n is the max arg + // index + // (2) for each _Arg node n, set m_input_is_static[n.index] to true if n + // is driving any static input; else set it to false. + // + + // Create the vector. + int32 max_arg_index = -1; + std::vector arg_nodes; + + for (auto node : tf_graph.nodes()) { + if (node->type_string() == "_Arg") { + arg_nodes.push_back(node); + + int32 index; + // macro defn: + // https://github.com/petewarden/tensorflow_makefile/blob/master/tensorflow/core/framework/op_kernel.h#L1265 + + //TODO check : removing OP_REQUIRES_OK for now + //// OP_REQUIRES_OK(ctx, GetNodeAttr(node->attrs(), "index", &index)); + TF_RETURN_IF_ERROR(GetNodeAttr(node->attrs(), "index", &index)); + if (index > max_arg_index) max_arg_index = index; + } + } + + m_input_is_static = std::vector(max_arg_index + 1, false); + + // Fill the vector. + for (auto node : arg_nodes) { + int32 index; + //// OP_REQUIRES_OK(ctx, GetNodeAttr(node->attrs(), "index", &index)); + TF_RETURN_IF_ERROR(GetNodeAttr(node->attrs(), "index", &index)); + + // bool is_static = false; + for (auto edge : node->out_edges()) { + if (edge->IsControlEdge() || !edge->dst()->IsOp()) { + continue; + } + + NGRAPH_VLOG(5) << "For arg " << index << " checking edge " + << edge->DebugString(); + + if (InputIsStatic(edge->dst(), edge->dst_input())) { + NGRAPH_VLOG(5) << "Marking edge static: " << edge->DebugString(); + // is_static = true; + m_input_is_static[index] = true; + break; + } + } + + // NGRAPH_VLOG(5) << "Marking arg " << index << " is_static: " << + // is_static; + // m_input_is_static[index] = is_static; + NGRAPH_VLOG(5) << "Marking arg " << index + << " is static: " << m_input_is_static[index]; + } + + is_initialized = true; } - is_init = true; return Status::OK(); } @@ -355,6 +431,7 @@ Status Builder1::GetInputNode(const Node* op, size_t input_idx, return Status::OK(); } + //TODO: inner class? // namespace detail { Status Builder1::detail_GetInputNodes(const Node* op, size_t index) { @@ -372,6 +449,7 @@ Status Builder1::detail_GetInputNodes(const Node* op, size_t index, } //} // namespace detail + template Status Builder1::GetInputNodes(const Node* op, Arguments&&... remaining) { constexpr size_t args_len = sizeof...(Arguments); @@ -383,35 +461,59 @@ Status Builder1::GetInputNodes(const Node* op, Arguments&&... remaining) { // TODO: move translate ops to a different file // TODO: make TranslateOps not static? -Status TranslateFloorDivOp1( - const Node* op, const std::vector& static_input_map) { - auto ng_floordiv = [](std::shared_ptr ng_input1, - std::shared_ptr ng_input2) { - return std::make_shared( - std::make_shared(ng_input1, ng_input2)); - }; - // TODO - // return TranslateBinaryOp(op, static_input_map, ng_op_map, ng_floordiv); +Status TranslateFloorDivOp1(const Node* op, const std::vector>& ng_arg_vec, const std::vector& static_input_map, vector>& subgraph_out_nodes) { + subgraph_out_nodes[0] = std::make_shared(ng_arg_vec[0] / ng_arg_vec[1]); + return Status::OK(); } -Status TranslateFloorModOp1( - const Node* op, const std::vector& static_input_map) { - auto ng_floormod = [](std::shared_ptr ng_input1, - std::shared_ptr ng_input2) { - auto floordiv = std::make_shared( - std::make_shared(ng_input1, ng_input2)); - return std::make_shared( - ng_input1, std::make_shared(floordiv, ng_input2)); - }; - // TODO - // return TranslateBinaryOp(op, static_input_map, ng_op_map, ng_floormod); +Status TranslateFloorModOp1(const Node* op, const std::vector>& ng_arg_vec, const std::vector& static_input_map, vector>& subgraph_out_nodes) { + auto floordiv = std::make_shared(ng_arg_vec[0] / ng_arg_vec[1]); + subgraph_out_nodes[0] = ng_arg_vec[0] - (floordiv * ng_arg_vec[1]); + return Status::OK(); } + +static Status TranslateBinaryOp( + const Node* op, const std::vector& static_input_map, + vector>& subgraph_out_nodes, + std::function(std::shared_ptr, + std::shared_ptr)> + create_binary_op) { + std::shared_ptr ng_lhs, ng_rhs; + + std::tie(ng_lhs, ng_rhs) = + ng::builder::numpy_broadcast(std::make_pair(ng_lhs, ng_rhs)); + return Status::OK(); +} + +// Helper function to translate a binary op in cases where there is a one-to-one +// mapping from TensorFlow ops to nGraph ops. +// +// Example usage: +// +// if (n->type_string == "Add") { +// TF_RETURN_IF_ERROR(TranslateBinaryOp(op, static_input_map, +// ng_op_map)); +// } +// +template +static Status TranslateBinaryOp( + const Node* op, const std::vector& static_input_map, + vector>& subgraph_out_nodes) { + return TranslateBinaryOp( + op, static_input_map, + [](std::shared_ptr ng_lhs, std::shared_ptr ng_rhs) { + return make_shared(ng_lhs, ng_rhs); + }); +} + + const std::map< - const string, - const function&)>> - Builder1::TRANSLATE_OP_MAP{{"FloorDiv", TranslateFloorDivOp1}, - {"FloorMod", TranslateFloorModOp1}}; + const string, std::pair> + > + Builder1::TRANSLATE_OP_MAP{{"FloorDiv", {TranslateFloorDivOp1, {0,1}}}, + {"FloorMod", {TranslateFloorModOp1, {0,1}}}}; + } // namespace ngraph_bridge diff --git a/src/ngraph_builder1.h b/src/ngraph_builder1.h index ecb969fa..404b05ba 100644 --- a/src/ngraph_builder1.h +++ b/src/ngraph_builder1.h @@ -30,6 +30,7 @@ #include "tensorflow/core/graph/algorithm.h" #include "tensorflow/core/graph/graph.h" + using namespace std; namespace ng = ngraph; namespace tensorflow { @@ -38,6 +39,8 @@ namespace ngraph_bridge { // TODO: namespace detail? overload certain functions vs default args? // TODO: make sure all comments from old builder are copied correctly. +// TODO: use camelcase, snakecase appropriately +// TODO add TF_RETURN_IF_ERROR where necessary ///////////////// @@ -45,6 +48,8 @@ class Builder1 { using OpMap = std::unordered_map>>; +typedef const function>&, const std::vector&, vector>&)> TranslatorFn; + private: // // The op map holds a mapping from TensorFlow op names (strings) to @@ -52,13 +57,17 @@ class Builder1 { // Builder1::OpMap ng_op_map; - const static std::map< - const string, - const function&)>> - TRANSLATE_OP_MAP; + struct detail; - OpKernelConstruction* ctx; // TODO: do we need to save it? - bool is_init = false; // Prevent init from running twice + //const static std::map< + //const string, + //const function&, + // vector>&)>> + //TRANSLATE_OP_MAP; + const static std::map>> TRANSLATE_OP_MAP; + + OpKernelConstruction* ctx; // TODO: do we need to save it? + bool is_initialized = false; // Prevent init from running twice const Graph& tf_graph; std::vector m_input_is_static; vector ordered; @@ -115,13 +124,14 @@ class Builder1 { const shared_ptr& output_node); // TODO: write description - Status get_input_params(const std::vector&, vector, - vector>*); - Status classify_nodes(const vector&, vector&, - vector&, vector&); - Status translate_each_op(const vector&); - Status get_output_nodes(const vector&, - vector>&); + Status GetInputParams(const std::vector&, vector, + vector>*); + Status ClassifyNodes(const vector&, vector&, + vector&, vector&); + Status TranslateEachOp(const vector&, + const std::vector&); + Status GetOutputNodes(const vector&, + vector>&); template void MakePadding(const std::string& tf_padding_type, @@ -142,75 +152,14 @@ class Builder1 { // values from constructor, // we cannot wrap 'classify_nodes' in 'TF_RETURN_IF_ERROR' if we did this part // in the constructor. - Status init(); + Status Initialize(); public: // TODO: move constructor body to .cc - Builder1(const Graph& tf_graph, OpKernelConstruction* ctx) - : tf_graph(tf_graph), ctx(ctx) { - // TODO: since we have an init anyway, why not move this stuff there - - // TODO: maybe we do not need to copy ctx into a pvt variable., as we do not - // use it later - // TODO: move m_input_is_static construction to init?? - - // - // Initialize the "m_input_is_static" vector as follows: - // (1) create m_input_is_static with n+1 elements, where n is the max arg - // index - // (2) for each _Arg node n, set m_input_is_static[n.index] to true if n - // is driving any static input; else set it to false. - // - - // Create the vector. - int32 max_arg_index = -1; - std::vector arg_nodes; - - for (auto node : tf_graph.nodes()) { - if (node->type_string() == "_Arg") { - arg_nodes.push_back(node); - - int32 index; - // TODO: do we need the ctx here. can we not use it? - // macro defn: - // https://github.com/petewarden/tensorflow_makefile/blob/master/tensorflow/core/framework/op_kernel.h#L1265 - // For now, requiring builder to have access to ctx - OP_REQUIRES_OK(ctx, GetNodeAttr(node->attrs(), "index", &index)); - if (index > max_arg_index) max_arg_index = index; - } - } - - m_input_is_static = std::vector(max_arg_index + 1, false); - - // Fill the vector. - for (auto node : arg_nodes) { - int32 index; - OP_REQUIRES_OK(ctx, GetNodeAttr(node->attrs(), "index", &index)); - - // bool is_static = false; - for (auto edge : node->out_edges()) { - if (edge->IsControlEdge() || !edge->dst()->IsOp()) { - continue; - } - - NGRAPH_VLOG(5) << "For arg " << index << " checking edge " - << edge->DebugString(); - - if (InputIsStatic(edge->dst(), edge->dst_input())) { - NGRAPH_VLOG(5) << "Marking edge static: " << edge->DebugString(); - // is_static = true; - m_input_is_static[index] = true; - break; - } - } - - // NGRAPH_VLOG(5) << "Marking arg " << index << " is_static: " << - // is_static; - // m_input_is_static[index] = is_static; - NGRAPH_VLOG(5) << "Marking arg " << index - << " is static: " << m_input_is_static[index]; - } - } + Builder1(const Graph& tf_graph, + OpKernelConstruction* ctx) // TODO make ctx const? + : tf_graph(tf_graph), + ctx(ctx) {} Status TranslateGraph(OpKernelContext* ctx, std::shared_ptr& ng_function); }; From 7909eef4802f55b6931607f99f392c0d319542e7 Mon Sep 17 00:00:00 2001 From: Sayantan Sarkar Date: Thu, 23 Aug 2018 17:11:50 -0700 Subject: [PATCH 07/32] Add unary and binary functions --- src/ngraph_builder1.cc | 200 ++++++++++++++++++++++++++--------------- src/ngraph_builder1.h | 28 ++++-- 2 files changed, 146 insertions(+), 82 deletions(-) diff --git a/src/ngraph_builder1.cc b/src/ngraph_builder1.cc index 172c37e8..c188cf2c 100644 --- a/src/ngraph_builder1.cc +++ b/src/ngraph_builder1.cc @@ -73,8 +73,10 @@ Status Builder1::TranslateGraph( return Status::OK(); // TODO } -//TODO, is static_input_map const? -Status Builder1::TranslateEachOp(const vector& tf_ops, const std::vector& static_input_map) { +// TODO, is static_input_map const? +Status Builder1::TranslateEachOp( + const vector& tf_ops, + const std::vector& static_input_map) { // Create the nGraph ops from TensorFlow ops. for (auto op : tf_ops) { @@ -82,61 +84,68 @@ Status Builder1::TranslateEachOp(const vector& tf_ops, const std::v << op->type_string(); try { - //extract out parent node finding and savengops + // extract out parent node finding and savengops // have the translateops as a list of pluggable functions (pimpl?) // TODO.....TODO....TODO // TF_RETURN_IF_ERROR(TRANSLATE_OP_MAP.at(op->type_string())( // op, static_input_map, ng_op_map)); - - //The static_input_map thing: - //some ops have static input, not everyone needs this input. - //we can make static_input_map a class data member, and set it to nullptr once the TranslateGraph is done - // That way, we do not pass it around, and make sure we are not reusing stale static_input_map + // The static_input_map thing: + // some ops have static input, not everyone needs this input. + // we can make static_input_map a class data member, and set it to nullptr + // once the TranslateGraph is done + // That way, we do not pass it around, and make sure we are not reusing + // stale static_input_map - //Note: abstracting parents = GetInputNodes(): + // Note: abstracting parents = GetInputNodes(): // unknown number of nodes for each op - //std::shared_ptr ng_lhs, ng_rhs; - //TF_RETURN_IF_ERROR(GetInputNodes(op, &ng_lhs, &ng_rhs)); + // std::shared_ptr ng_lhs, ng_rhs; + // TF_RETURN_IF_ERROR(GetInputNodes(op, &ng_lhs, &ng_rhs)); vector> subgraph_out_nodes; - //TF_RETURN_IF_ERROR(TRANSLATE_OP_MAP.at(op->type_string())(op, static_input_map)); + // TF_RETURN_IF_ERROR(TRANSLATE_OP_MAP.at(op->type_string())(op, + // static_input_map)); auto iter = TRANSLATE_OP_MAP.find(op->type_string()); - if (iter != TRANSLATE_OP_MAP.end()){ + if (iter != TRANSLATE_OP_MAP.end()) { Builder1::TranslatorFn translate_fn = iter->second.first; vector input_idxs = iter->second.second; - //std::tie(translate_fn, input_idxs) = iter->second; - // input_idxs can be size 0 (to indicate/handle variadic inputs nodes like Addn) - bool variadic_input = input_idxs.size()==0; + // std::tie(translate_fn, input_idxs) = iter->second; + // input_idxs can be size 0 (to indicate/handle variadic inputs nodes + // like Addn) + bool variadic_input = input_idxs.size() == 0; int num_inputs = variadic_input ? op->num_inputs() : input_idxs.size(); std::vector> ng_arg_vec(num_inputs); - for (int idx = 0; idx < num_inputs; idx++){ - TF_RETURN_IF_ERROR(GetInputNode(op, (variadic_input ? idx : input_idxs[idx]), &ng_arg_vec[idx])); + for (int idx = 0; idx < num_inputs; idx++) { + TF_RETURN_IF_ERROR(GetInputNode( + op, (variadic_input ? idx : input_idxs[idx]), &ng_arg_vec[idx])); } - TF_RETURN_IF_ERROR(iter->second.first(op, ng_arg_vec, static_input_map, subgraph_out_nodes)); - } - else{ - //TODO TODO::: if-else or try-catch + // TODO: instead of pass static_input_map, use GetStaticInputVector and + // pass the vector + // Then we'd have to pass a vector of vectors, in case a node has >1 + // static inputs + TF_RETURN_IF_ERROR(iter->second.first(op, ng_arg_vec, static_input_map, + subgraph_out_nodes)); + } else { + // TODO TODO::: if-else or try-catch // ----------------------------- // Catch-all for unsupported ops // ----------------------------- NGRAPH_VLOG(3) << "Unsupported Op: " << op->name() << " (" - << op->type_string() << ")"; + << op->type_string() << ")"; NGRAPH_VLOG(3) << op->def().DebugString(); return errors::InvalidArgument("Unsupported Op: ", op->name(), " (", - op->type_string(), ")"); + op->type_string(), ")"); } - - //for (auto ng_node : subgraph_out_nodes) + // for (auto ng_node : subgraph_out_nodes) // SaveNgOp(ng_op_map, op->name(), ng_node); - ng_op_map[op->name()] = subgraph_out_nodes; //SaveNgOp + ng_op_map[op->name()] = subgraph_out_nodes; // SaveNgOp - //TranslateBinaryOp(op, static_input_map, ng_op_map, ng_floormod) + // TranslateBinaryOp(op, static_input_map, ng_op_map, ng_floormod) - } - //TODO: catching the error above. do we keep the try catch or use if-else? + } + // TODO: catching the error above. do we keep the try catch or use if-else? catch (const std::out_of_range&) { // ----------------------------- // Catch-all for unsupported ops @@ -152,9 +161,9 @@ Status Builder1::TranslateEachOp(const vector& tf_ops, const std::v } Status Builder1::ClassifyNodes(const vector& ordered, - vector& tf_params, - vector& tf_ret_vals, - vector& tf_ops) { + vector& tf_params, + vector& tf_ret_vals, + vector& tf_ops) { // Split ops into params, retvals, and all others. for (const auto n : ordered) { @@ -209,9 +218,8 @@ Status Builder1::GetInputParams( return Status::OK(); } -Status Builder1::GetOutputNodes( - const vector& tf_ret_vals, - vector>& ng_result_list) { +Status Builder1::GetOutputNodes(const vector& tf_ret_vals, + vector>& ng_result_list) { // Populate the result list. ng_result_list.resize(tf_ret_vals.size()); @@ -337,8 +345,8 @@ Status Builder1::Initialize() { GetReversePostOrder(tf_graph, &ordered); TF_RETURN_IF_ERROR(ClassifyNodes(ordered, tf_params, tf_ret_vals, tf_ops)); - - // TODO: since we have an init anyway, why not move this stuff there + + // TODO: since we have an init anyway, why not move this stuff there // TODO: maybe we do not need to copy ctx into a pvt variable., as we do not // use it later @@ -363,8 +371,8 @@ Status Builder1::Initialize() { int32 index; // macro defn: // https://github.com/petewarden/tensorflow_makefile/blob/master/tensorflow/core/framework/op_kernel.h#L1265 - - //TODO check : removing OP_REQUIRES_OK for now + + // TODO check : removing OP_REQUIRES_OK for now //// OP_REQUIRES_OK(ctx, GetNodeAttr(node->attrs(), "index", &index)); TF_RETURN_IF_ERROR(GetNodeAttr(node->attrs(), "index", &index)); if (index > max_arg_index) max_arg_index = index; @@ -403,7 +411,7 @@ Status Builder1::Initialize() { << " is static: " << m_input_is_static[index]; } - is_initialized = true; + is_initialized = true; } return Status::OK(); } @@ -431,8 +439,7 @@ Status Builder1::GetInputNode(const Node* op, size_t input_idx, return Status::OK(); } - -//TODO: inner class? +// TODO: inner class? // namespace detail { Status Builder1::detail_GetInputNodes(const Node* op, size_t index) { return Status::OK(); @@ -449,7 +456,6 @@ Status Builder1::detail_GetInputNodes(const Node* op, size_t index, } //} // namespace detail - template Status Builder1::GetInputNodes(const Node* op, Arguments&&... remaining) { constexpr size_t args_len = sizeof...(Arguments); @@ -461,28 +467,27 @@ Status Builder1::GetInputNodes(const Node* op, Arguments&&... remaining) { // TODO: move translate ops to a different file // TODO: make TranslateOps not static? -Status TranslateFloorDivOp1(const Node* op, const std::vector>& ng_arg_vec, const std::vector& static_input_map, vector>& subgraph_out_nodes) { - subgraph_out_nodes[0] = std::make_shared(ng_arg_vec[0] / ng_arg_vec[1]); +Status TranslateFloorDivOp1(const Node* op, + const std::vector>& ng_arg_vec, + const std::vector& static_input_map, + vector>& subgraph_out_nodes) { + subgraph_out_nodes[0] = + std::make_shared(ng_arg_vec[0] / ng_arg_vec[1]); return Status::OK(); } -Status TranslateFloorModOp1(const Node* op, const std::vector>& ng_arg_vec, const std::vector& static_input_map, vector>& subgraph_out_nodes) { - auto floordiv = std::make_shared(ng_arg_vec[0] / ng_arg_vec[1]); +Status TranslateFloorModOp1(const Node* op, + const std::vector>& ng_arg_vec, + const std::vector& static_input_map, + vector>& subgraph_out_nodes) { + auto floordiv = + std::make_shared(ng_arg_vec[0] / ng_arg_vec[1]); subgraph_out_nodes[0] = ng_arg_vec[0] - (floordiv * ng_arg_vec[1]); - return Status::OK(); -} - -static Status TranslateBinaryOp( - const Node* op, const std::vector& static_input_map, - vector>& subgraph_out_nodes, - std::function(std::shared_ptr, - std::shared_ptr)> - create_binary_op) { - std::shared_ptr ng_lhs, ng_rhs; - - std::tie(ng_lhs, ng_rhs) = - ng::builder::numpy_broadcast(std::make_pair(ng_lhs, ng_rhs)); + // Possible reuse of floordiv: + // TranslateFloorDivOp1(op, ng_arg_vec, static_input_map, subgraph_out_nodes); + // subgraph_out_nodes[0] = ng_arg_vec[0] - (subgraph_out_nodes[0] * + // ng_arg_vec[1]); return Status::OK(); } @@ -496,24 +501,73 @@ static Status TranslateBinaryOp( // ng_op_map)); // } // + +// Note: Earlier TranslateBinary had 2 forms: templated (to conver a ng:Node) +// into a function +// and non-templated, which converted an arbitrary function that accepts 2 nodes +// and returns 1 node. +// In current implementation, we do not need the non-templated version, because +// TranslateBinary will be +// part of the class, and the TranslateOps will not have access to it. +/* template -static Status TranslateBinaryOp( - const Node* op, const std::vector& static_input_map, - vector>& subgraph_out_nodes) { - return TranslateBinaryOp( - op, static_input_map, - [](std::shared_ptr ng_lhs, std::shared_ptr ng_rhs) { - return make_shared(ng_lhs, ng_rhs); +Status TranslateBinary( + const Node* op, const std::vector>& ng_arg_vec, + const std::vector& static_input_map, + vector>& subgraph_out_nodes, + std::function(std::shared_ptr, + std::shared_ptr)> + create_op) { + // TODO: assert subgraph_out_nodes.size()==1, ng_arg_vec.size()==2 + std::tie(ng_arg_vec[0], ng_arg_vec[0]) = ng::builder::numpy_broadcast( + std::make_pair(ng_arg_vec[0], ng_arg_vec[1])); + subgraph_out_nodes[0] = create_op(ng_arg_vec[0], ng_arg_vec[1]); + return Status::OK(); +} + +template +Status TranslateBinary(const Node* op, + const std::vector>& ng_arg_vec, + const std::vector& static_input_map, + vector>& subgraph_out_nodes) { + return TranslateBinary( + op, ng_arg_vec, static_input_map, subgraph_out_nodes, + [](std::shared_ptr n1, std::shared_ptr n2) { + return make_shared(n1, n2); }); } +*/ +template +Status TranslateBinary(const Node* op, + const std::vector>& ng_arg_vec, + const std::vector& static_input_map, + vector>& subgraph_out_nodes) { + // TODO: assert subgraph_out_nodes.size()==1, ng_arg_vec.size()==2 + auto node_pair = ng::builder::numpy_broadcast( + std::make_pair(ng_arg_vec[0], ng_arg_vec[1])); + subgraph_out_nodes[0] = make_shared(node_pair.first, node_pair.second); + return Status::OK(); +} + +template +Status TranslateUnary(const Node* op, + const std::vector>& ng_arg_vec, + const std::vector& static_input_map, + vector>& subgraph_out_nodes) { + // TODO: assert subgraph_out_nodes.size()==1, ng_arg_vec.size()==1 + subgraph_out_nodes[0] = make_shared(ng_arg_vec[0]); + return Status::OK(); +} -const std::map< - const string, std::pair> - > - Builder1::TRANSLATE_OP_MAP{{"FloorDiv", {TranslateFloorDivOp1, {0,1}}}, - {"FloorMod", {TranslateFloorModOp1, {0,1}}}}; +const std::map>> + Builder1::TRANSLATE_OP_MAP{ + {"Abs", {TranslateUnary, {0}}}, + {"Add", {TranslateBinary, {0, 1}}}, + {"FloorDiv", {TranslateFloorDivOp1, {0, 1}}}, + {"FloorMod", {TranslateFloorModOp1, {0, 1}}}}; +// std::function f3 = [](int... x){return x+1;}; } // namespace ngraph_bridge diff --git a/src/ngraph_builder1.h b/src/ngraph_builder1.h index 404b05ba..0e27fe8e 100644 --- a/src/ngraph_builder1.h +++ b/src/ngraph_builder1.h @@ -30,7 +30,6 @@ #include "tensorflow/core/graph/algorithm.h" #include "tensorflow/core/graph/graph.h" - using namespace std; namespace ng = ngraph; namespace tensorflow { @@ -48,7 +47,16 @@ class Builder1 { using OpMap = std::unordered_map>>; -typedef const function>&, const std::vector&, vector>&)> TranslatorFn; + /* + typedef const function>&, + const std::vector&, vector>&)> + TranslatorFn; + */ + + using TranslatorFn = function>&, + const std::vector&, vector>&)>; private: // @@ -59,12 +67,14 @@ typedef const function&, - // vector>&)>> - //TRANSLATE_OP_MAP; - const static std::map>> TRANSLATE_OP_MAP; + // const static std::map< + // const string, + // const function&, + // vector>&)>> + // TRANSLATE_OP_MAP; + const static std::map>> + TRANSLATE_OP_MAP; OpKernelConstruction* ctx; // TODO: do we need to save it? bool is_initialized = false; // Prevent init from running twice @@ -157,7 +167,7 @@ typedef const function Date: Thu, 23 Aug 2018 17:42:50 -0700 Subject: [PATCH 08/32] Some cleanup --- src/ngraph_builder1.cc | 107 +++-------------------------------------- src/ngraph_builder1.h | 38 ++------------- 2 files changed, 13 insertions(+), 132 deletions(-) diff --git a/src/ngraph_builder1.cc b/src/ngraph_builder1.cc index c188cf2c..552f0c1e 100644 --- a/src/ngraph_builder1.cc +++ b/src/ngraph_builder1.cc @@ -38,13 +38,7 @@ namespace ngraph_bridge { Status Builder1::TranslateGraph( OpKernelContext* ctx, std::shared_ptr& ng_function) { - // TODO: confirm that static_input_map cannot be constructed in constructor. - // It could be different everytime right? - // TODO: static_input_map can't be a class variable right? Its different - // everytime TranslateGraph is called, - // so it must be passed around? - - if (!is_initialized) Initialize(); + TF_RETURN_IF_ERROR(Initialize()); std::vector static_input_map; @@ -58,7 +52,7 @@ Status Builder1::TranslateGraph( } inputs[i] = input_tensor.shape(); } - // TODO: pass static_input_map to translate_each_op + // TODO: pass static_input_map to translate_each_op... or pass the vector ? vector> ng_parameter_list; TF_RETURN_IF_ERROR(GetInputParams(inputs, tf_params, &ng_parameter_list)); @@ -70,10 +64,9 @@ Status Builder1::TranslateGraph( // Create the nGraph function. ng_function = make_shared(ng_result_list, ng_parameter_list); - return Status::OK(); // TODO + return Status::OK(); } -// TODO, is static_input_map const? Status Builder1::TranslateEachOp( const vector& tf_ops, const std::vector& static_input_map) { @@ -84,33 +77,11 @@ Status Builder1::TranslateEachOp( << op->type_string(); try { - // extract out parent node finding and savengops - // have the translateops as a list of pluggable functions (pimpl?) - - // TODO.....TODO....TODO - // TF_RETURN_IF_ERROR(TRANSLATE_OP_MAP.at(op->type_string())( - // op, static_input_map, ng_op_map)); - - // The static_input_map thing: - // some ops have static input, not everyone needs this input. - // we can make static_input_map a class data member, and set it to nullptr - // once the TranslateGraph is done - // That way, we do not pass it around, and make sure we are not reusing - // stale static_input_map - - // Note: abstracting parents = GetInputNodes(): - // unknown number of nodes for each op - // std::shared_ptr ng_lhs, ng_rhs; - // TF_RETURN_IF_ERROR(GetInputNodes(op, &ng_lhs, &ng_rhs)); - vector> subgraph_out_nodes; - // TF_RETURN_IF_ERROR(TRANSLATE_OP_MAP.at(op->type_string())(op, - // static_input_map)); auto iter = TRANSLATE_OP_MAP.find(op->type_string()); if (iter != TRANSLATE_OP_MAP.end()) { Builder1::TranslatorFn translate_fn = iter->second.first; vector input_idxs = iter->second.second; - // std::tie(translate_fn, input_idxs) = iter->second; // input_idxs can be size 0 (to indicate/handle variadic inputs nodes // like Addn) bool variadic_input = input_idxs.size() == 0; @@ -127,7 +98,7 @@ Status Builder1::TranslateEachOp( TF_RETURN_IF_ERROR(iter->second.first(op, ng_arg_vec, static_input_map, subgraph_out_nodes)); } else { - // TODO TODO::: if-else or try-catch + // TODO::: if-else or try-catch // ----------------------------- // Catch-all for unsupported ops // ----------------------------- @@ -237,7 +208,7 @@ Status Builder1::GetOutputNodes(const vector& tf_ret_vals, } shared_ptr result; - // TF_RETURN_IF_ERROR(GetInputNode(ng_op_map, n, 0, &result)); //TODO...TODO + TF_RETURN_IF_ERROR(GetInputNode(n, 0, &result)); ng_result_list[index] = result; } @@ -310,7 +281,7 @@ void MakePadding(const std::string& tf_padding_type, NGRAPH_VLOG(3) << "ng_padding_above: " << ngraph::join(ng_padding_above); } -Status Builder1::ValidateInputCount(const Node* op, size_t count) { +Status ValidateInputCount(const Node* op, size_t count) { if (op->num_inputs() != count) { return errors::InvalidArgument("\"", op->name(), "\" requires ", count, " input(s), got ", op->num_inputs(), @@ -319,7 +290,7 @@ Status Builder1::ValidateInputCount(const Node* op, size_t count) { return Status::OK(); } -Status Builder1::ValidateInputCountMin(const Node* op, size_t count) { +Status ValidateInputCountMin(const Node* op, size_t count) { if (op->num_inputs() < count) { return errors::InvalidArgument("\"", op->name(), "\" requires at least ", count, " input(s), got ", op->num_inputs(), @@ -345,13 +316,6 @@ Status Builder1::Initialize() { GetReversePostOrder(tf_graph, &ordered); TF_RETURN_IF_ERROR(ClassifyNodes(ordered, tf_params, tf_ret_vals, tf_ops)); - - // TODO: since we have an init anyway, why not move this stuff there - - // TODO: maybe we do not need to copy ctx into a pvt variable., as we do not - // use it later - // TODO: move m_input_is_static construction to init?? - // // Initialize the "m_input_is_static" vector as follows: // (1) create m_input_is_static with n+1 elements, where n is the max arg @@ -384,6 +348,7 @@ Status Builder1::Initialize() { // Fill the vector. for (auto node : arg_nodes) { int32 index; + //TODO: OP_REQUIRES_OK or TF_RETURN_IF_ERROR //// OP_REQUIRES_OK(ctx, GetNodeAttr(node->attrs(), "index", &index)); TF_RETURN_IF_ERROR(GetNodeAttr(node->attrs(), "index", &index)); @@ -439,34 +404,7 @@ Status Builder1::GetInputNode(const Node* op, size_t input_idx, return Status::OK(); } -// TODO: inner class? -// namespace detail { -Status Builder1::detail_GetInputNodes(const Node* op, size_t index) { - return Status::OK(); -} - -template -Status Builder1::detail_GetInputNodes(const Node* op, size_t index, - shared_ptr* result, - Arguments&&... remaining) { - if (result != nullptr) { - TF_RETURN_IF_ERROR(GetInputNode(op, index, result)); - } - return Builder1::detail_GetInputNodes(op, index + 1, remaining...); -} -//} // namespace detail - -template -Status Builder1::GetInputNodes(const Node* op, Arguments&&... remaining) { - constexpr size_t args_len = sizeof...(Arguments); - TF_RETURN_IF_ERROR(ValidateInputCount(op, args_len)); - // return detail::GetInputNodes(ng_op_map, op, 0, remaining...); //TODO.. - // detail namespace - return detail_GetInputNodes(op, 0, remaining...); -} - // TODO: move translate ops to a different file -// TODO: make TranslateOps not static? Status TranslateFloorDivOp1(const Node* op, const std::vector>& ng_arg_vec, const std::vector& static_input_map, @@ -509,35 +447,6 @@ Status TranslateFloorModOp1(const Node* op, // In current implementation, we do not need the non-templated version, because // TranslateBinary will be // part of the class, and the TranslateOps will not have access to it. -/* -template -Status TranslateBinary( - const Node* op, const std::vector>& ng_arg_vec, - const std::vector& static_input_map, - vector>& subgraph_out_nodes, - std::function(std::shared_ptr, - std::shared_ptr)> - create_op) { - // TODO: assert subgraph_out_nodes.size()==1, ng_arg_vec.size()==2 - std::tie(ng_arg_vec[0], ng_arg_vec[0]) = ng::builder::numpy_broadcast( - std::make_pair(ng_arg_vec[0], ng_arg_vec[1])); - subgraph_out_nodes[0] = create_op(ng_arg_vec[0], ng_arg_vec[1]); - return Status::OK(); -} - -template -Status TranslateBinary(const Node* op, - const std::vector>& ng_arg_vec, - const std::vector& static_input_map, - vector>& subgraph_out_nodes) { - return TranslateBinary( - op, ng_arg_vec, static_input_map, subgraph_out_nodes, - [](std::shared_ptr n1, std::shared_ptr n2) { - return make_shared(n1, n2); - }); -} -*/ - template Status TranslateBinary(const Node* op, const std::vector>& ng_arg_vec, diff --git a/src/ngraph_builder1.h b/src/ngraph_builder1.h index 0e27fe8e..0c86e442 100644 --- a/src/ngraph_builder1.h +++ b/src/ngraph_builder1.h @@ -36,7 +36,7 @@ namespace tensorflow { namespace ngraph_bridge { -// TODO: namespace detail? overload certain functions vs default args? +// TODO: overload certain functions (makepadding) vs default args? // TODO: make sure all comments from old builder are copied correctly. // TODO: use camelcase, snakecase appropriately // TODO add TF_RETURN_IF_ERROR where necessary @@ -76,7 +76,7 @@ class Builder1 { std::pair>> TRANSLATE_OP_MAP; - OpKernelConstruction* ctx; // TODO: do we need to save it? + OpKernelConstruction* ctx; //TODO: is this required? required in OP_REQUIRES_OK is used instead of TF_RETURN_IF_ERROR in Initialize bool is_initialized = false; // Prevent init from running twice const Graph& tf_graph; std::vector m_input_is_static; @@ -98,36 +98,9 @@ class Builder1 { template Status GetInputNodes(const Node*, Arguments&&...); - // TODO: reconsider TranslateBinaryOp - Status TranslateBinaryOp( - const Node* op, const std::vector& static_input_map, - std::function(std::shared_ptr, - std::shared_ptr)> - create_binary_op) { - std::shared_ptr ng_lhs, ng_rhs; - TF_RETURN_IF_ERROR(GetInputNodes(op, &ng_lhs, &ng_rhs)); - - std::tie(ng_lhs, ng_rhs) = - ng::builder::numpy_broadcast(std::make_pair(ng_lhs, ng_rhs)); - - // TODO: enable this - // SaveNgOp(ng_op_map, op->name(), create_binary_op(ng_lhs, ng_rhs)); - - return Status::OK(); - } - - template - Status TranslateBinaryOp(const Node* op, - const std::vector& static_input_map) { - return TranslateBinaryOp( - op, static_input_map, - [](std::shared_ptr ng_lhs, std::shared_ptr ng_rhs) { - return make_shared(ng_lhs, ng_rhs); - }); - } - - Status ValidateInputCount(const Node* op, size_t count); - Status ValidateInputCountMin(const Node* op, size_t count); + // TODO: move these out of class + //Status ValidateInputCount(const Node* op, size_t count); + //Status ValidateInputCountMin(const Node* op, size_t count); // helper function for populating ng_op_map void SaveNgOp(const std::string& op_name, @@ -165,7 +138,6 @@ class Builder1 { Status Initialize(); public: - // TODO: move constructor body to .cc Builder1(const Graph& tf_graph, OpKernelConstruction* ctx) // TODO make ctx const? : tf_graph(tf_graph), From 86fc0b7dbed2335ac46ef2f2b603c0288c4afa95 Mon Sep 17 00:00:00 2001 From: Sayantan Sarkar Date: Thu, 23 Aug 2018 19:07:32 -0700 Subject: [PATCH 09/32] Add AddN --- src/ngraph_builder1.cc | 46 +++++++++++++++++++++++++----------------- src/ngraph_builder1.h | 45 +++++++---------------------------------- 2 files changed, 35 insertions(+), 56 deletions(-) diff --git a/src/ngraph_builder1.cc b/src/ngraph_builder1.cc index 552f0c1e..4ade7def 100644 --- a/src/ngraph_builder1.cc +++ b/src/ngraph_builder1.cc @@ -52,7 +52,8 @@ Status Builder1::TranslateGraph( } inputs[i] = input_tensor.shape(); } - // TODO: pass static_input_map to translate_each_op... or pass the vector ? + // TODO: pass static_input_map to translate_each_op... or pass the vector + // ? vector> ng_parameter_list; TF_RETURN_IF_ERROR(GetInputParams(inputs, tf_params, &ng_parameter_list)); @@ -348,7 +349,7 @@ Status Builder1::Initialize() { // Fill the vector. for (auto node : arg_nodes) { int32 index; - //TODO: OP_REQUIRES_OK or TF_RETURN_IF_ERROR + // TODO: OP_REQUIRES_OK or TF_RETURN_IF_ERROR //// OP_REQUIRES_OK(ctx, GetNodeAttr(node->attrs(), "index", &index)); TF_RETURN_IF_ERROR(GetNodeAttr(node->attrs(), "index", &index)); @@ -405,19 +406,19 @@ Status Builder1::GetInputNode(const Node* op, size_t input_idx, } // TODO: move translate ops to a different file -Status TranslateFloorDivOp1(const Node* op, - const std::vector>& ng_arg_vec, - const std::vector& static_input_map, - vector>& subgraph_out_nodes) { +Status TranslateFloorDivOp(const Node* op, + const std::vector>& ng_arg_vec, + const std::vector& static_input_map, + vector>& subgraph_out_nodes) { subgraph_out_nodes[0] = std::make_shared(ng_arg_vec[0] / ng_arg_vec[1]); return Status::OK(); } -Status TranslateFloorModOp1(const Node* op, - const std::vector>& ng_arg_vec, - const std::vector& static_input_map, - vector>& subgraph_out_nodes) { +Status TranslateFloorModOp(const Node* op, + const std::vector>& ng_arg_vec, + const std::vector& static_input_map, + vector>& subgraph_out_nodes) { auto floordiv = std::make_shared(ng_arg_vec[0] / ng_arg_vec[1]); subgraph_out_nodes[0] = ng_arg_vec[0] - (floordiv * ng_arg_vec[1]); @@ -429,6 +430,17 @@ Status TranslateFloorModOp1(const Node* op, return Status::OK(); } +Status TranslateAddNOp(const Node* op, + const std::vector>& ng_arg_vec, + const std::vector& static_input_map, + vector>& subgraph_out_nodes) { + subgraph_out_nodes[0] = + std::accumulate(std::next(ng_arg_vec.begin()), ng_arg_vec.end(), + ng_arg_vec.at(0)); // accumulation: start with first + // element. default op is addition + return Status::OK(); +} + // Helper function to translate a binary op in cases where there is a one-to-one // mapping from TensorFlow ops to nGraph ops. // @@ -469,14 +481,12 @@ Status TranslateUnary(const Node* op, return Status::OK(); } -const std::map>> - Builder1::TRANSLATE_OP_MAP{ - {"Abs", {TranslateUnary, {0}}}, - {"Add", {TranslateBinary, {0, 1}}}, - {"FloorDiv", {TranslateFloorDivOp1, {0, 1}}}, - {"FloorMod", {TranslateFloorModOp1, {0, 1}}}}; - -// std::function f3 = [](int... x){return x+1;}; +Builder1::DispatchTable Builder1::TRANSLATE_OP_MAP{ + {"Abs", {TranslateUnary, {0}}}, + {"Add", {TranslateBinary, {0, 1}}}, + {"AddN", {TranslateAddNOp, {}}}, + {"FloorDiv", {TranslateFloorDivOp, {0, 1}}}, + {"FloorMod", {TranslateFloorModOp, {0, 1}}}}; } // namespace ngraph_bridge diff --git a/src/ngraph_builder1.h b/src/ngraph_builder1.h index 0c86e442..62dbc7a2 100644 --- a/src/ngraph_builder1.h +++ b/src/ngraph_builder1.h @@ -46,17 +46,12 @@ namespace ngraph_bridge { class Builder1 { using OpMap = std::unordered_map>>; - - /* - typedef const function>&, - const std::vector&, vector>&)> - TranslatorFn; - */ - using TranslatorFn = function>&, const std::vector&, vector>&)>; + using DispatchTable = + const std::map>>; private: // @@ -64,19 +59,13 @@ class Builder1 { // vector of generated nGraph nodes. // Builder1::OpMap ng_op_map; - - struct detail; - - // const static std::map< - // const string, - // const function&, - // vector>&)>> - // TRANSLATE_OP_MAP; const static std::map>> TRANSLATE_OP_MAP; - OpKernelConstruction* ctx; //TODO: is this required? required in OP_REQUIRES_OK is used instead of TF_RETURN_IF_ERROR in Initialize + OpKernelConstruction* ctx; // TODO: is this required? required in + // OP_REQUIRES_OK is used instead of + // TF_RETURN_IF_ERROR in Initialize bool is_initialized = false; // Prevent init from running twice const Graph& tf_graph; std::vector m_input_is_static; @@ -85,24 +74,8 @@ class Builder1 { Status GetInputNode(const Node*, size_t, shared_ptr*); - // TODO: enable detail namespace? - // Note: namespace details prevents template recursion error during - // compilation - // cannot use namespace in class, so using different names - Status detail_GetInputNodes(const Node* op, size_t index); - - template - Status detail_GetInputNodes(const Node*, size_t, shared_ptr*, - Arguments&&...); - - template - Status GetInputNodes(const Node*, Arguments&&...); - - // TODO: move these out of class - //Status ValidateInputCount(const Node* op, size_t count); - //Status ValidateInputCountMin(const Node* op, size_t count); - // helper function for populating ng_op_map + // TODO: no one to use it (except 1 place). delete? void SaveNgOp(const std::string& op_name, const shared_ptr& output_node); @@ -131,10 +104,6 @@ class Builder1 { const ngraph::Strides& ng_strides, T& ng_padding_below, T& ng_padding_above); - // Note: we need a separate init function for this. because we cant return - // values from constructor, - // we cannot wrap 'classify_nodes' in 'TF_RETURN_IF_ERROR' if we did this part - // in the constructor. Status Initialize(); public: From 671dcfe49cc3b196debef1ec8e4ab78886ef095d Mon Sep 17 00:00:00 2001 From: Sayantan Sarkar Date: Thu, 23 Aug 2018 19:24:17 -0700 Subject: [PATCH 10/32] Add a typedef --- src/ngraph_builder1.cc | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/ngraph_builder1.cc b/src/ngraph_builder1.cc index 4ade7def..e639dc38 100644 --- a/src/ngraph_builder1.cc +++ b/src/ngraph_builder1.cc @@ -28,6 +28,7 @@ #include "tensorflow/core/graph/algorithm.h" #include "tensorflow/core/graph/edgeset.h" #include "tensorflow/core/lib/core/errors.h" +#include "tensorflow/stream_executor/lib/statusor.h" using namespace std; namespace ng = ngraph; @@ -405,11 +406,14 @@ Status Builder1::GetInputNode(const Node* op, size_t input_idx, return Status::OK(); } + +using VectNg = std::vector>; + // TODO: move translate ops to a different file Status TranslateFloorDivOp(const Node* op, - const std::vector>& ng_arg_vec, + const VectNg& ng_arg_vec, const std::vector& static_input_map, - vector>& subgraph_out_nodes) { + VectNg& subgraph_out_nodes) { subgraph_out_nodes[0] = std::make_shared(ng_arg_vec[0] / ng_arg_vec[1]); return Status::OK(); @@ -418,7 +422,7 @@ Status TranslateFloorDivOp(const Node* op, Status TranslateFloorModOp(const Node* op, const std::vector>& ng_arg_vec, const std::vector& static_input_map, - vector>& subgraph_out_nodes) { + VectNg& subgraph_out_nodes) { auto floordiv = std::make_shared(ng_arg_vec[0] / ng_arg_vec[1]); subgraph_out_nodes[0] = ng_arg_vec[0] - (floordiv * ng_arg_vec[1]); @@ -431,9 +435,9 @@ Status TranslateFloorModOp(const Node* op, } Status TranslateAddNOp(const Node* op, - const std::vector>& ng_arg_vec, + const VectNg &ng_arg_vec, const std::vector& static_input_map, - vector>& subgraph_out_nodes) { + VectNg& subgraph_out_nodes) { subgraph_out_nodes[0] = std::accumulate(std::next(ng_arg_vec.begin()), ng_arg_vec.end(), ng_arg_vec.at(0)); // accumulation: start with first From 89116630a0a50e9414e28ebd45028dd283e1893f Mon Sep 17 00:00:00 2001 From: Sayantan Sarkar Date: Thu, 23 Aug 2018 23:09:34 -0700 Subject: [PATCH 11/32] AddN test runs with Builder, not with Builder1 --- test/tf_exec.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/test/tf_exec.cpp b/test/tf_exec.cpp index b4e7ee7c..f4df9cd4 100644 --- a/test/tf_exec.cpp +++ b/test/tf_exec.cpp @@ -834,8 +834,9 @@ TEST(tf_exec, Op_Negate) { TEST(tf_exec, Op_FloorDiv) { Scope scope_cpu = Scope::NewRootScope(); - Scope scope_ng = scope_cpu.WithDevice("/device:NGRAPH:0"); + Scope scope_ng = scope_cpu; //scope_cpu.WithDevice("/device:NGRAPH:0"); + DeactivateNGraph(); // ngraph execution auto A_ng = ops::Const(scope_ng, {{5.f, 6.f, 7.5f, -1.f, 2.f, -3.f}, {1.3f, 1.f, -5.f, -3.f, 0.f, -2.f}}); @@ -848,11 +849,14 @@ TEST(tf_exec, Op_FloorDiv) { std::vector outputs_ng; ClientSession session_ng(scope_ng); + cout << "XXXXX1\n"; ASSERT_OK(session_ng.Run({r0_ng, r1_ng}, &outputs_ng)); + cout << "XXXXX2\n"; ASSERT_EQ(outputs_ng[0].shape(), TensorShape({2, 6})); ASSERT_EQ(outputs_ng[1].shape(), TensorShape({2, 6})); + DeactivateNGraph(); // reference CPU execution auto A_cpu = ops::Const(scope_cpu, {{5.f, 6.f, 7.5f, -1.f, 2.f, -3.f}, {1.3f, 1.f, -5.f, -3.f, 0.f, -2.f}}); @@ -916,8 +920,9 @@ TEST(tf_exec, Op_FloorMod) { TEST(tf_exec, Op_AddN) { Scope scope_cpu = Scope::NewRootScope(); - Scope scope_ng = scope_cpu.WithDevice("/device:NGRAPH:0"); + Scope scope_ng = scope_cpu; //scope_cpu.WithDevice("/device:NGRAPH:0"); + ActivateNGraph(); // ngraph execution auto A_ng = ops::Const(scope_ng, {{256.f, 16.f}, {4.f, 64.f}}); auto B_ng = ops::Const(scope_ng, {{1.f, 2.f}, {3.f, 4.f}}); @@ -933,6 +938,7 @@ TEST(tf_exec, Op_AddN) { ASSERT_EQ(outputs_ng[0].shape(), TensorShape({2, 2})); + DeactivateNGraph(); // reference CPU execution auto A_cpu = ops::Const(scope_cpu, {{256.f, 16.f}, {4.f, 64.f}}); auto B_cpu = ops::Const(scope_cpu, {{1.f, 2.f}, {3.f, 4.f}}); From 483af0e1e1fa09e77bb062cc2552f430f6955856 Mon Sep 17 00:00:00 2001 From: Sayantan Sarkar Date: Fri, 24 Aug 2018 13:17:46 -0700 Subject: [PATCH 12/32] Trying a few tests. Constant folding seems to happen even with L0 --- src/ngraph_builder.cc | 11 +++++++++++ src/ngraph_builder1.cc | 21 +++++++++++++++------ src/ngraph_encapsulate_op.cc | 2 +- test/tf_exec.cpp | 23 ++++++++++++++++++----- 4 files changed, 45 insertions(+), 12 deletions(-) diff --git a/src/ngraph_builder.cc b/src/ngraph_builder.cc index bf9234ba..c416a0c5 100644 --- a/src/ngraph_builder.cc +++ b/src/ngraph_builder.cc @@ -2686,6 +2686,7 @@ Status Builder::TranslateGraph( // ought to be `const Node*`, but GetReversePostOrder doesn't use `const` vector ordered; GetReversePostOrder(*input_graph, &ordered); + cout << "input_graph: " << input_graph->num_node_ids() << "\n"; // // Split ops into params, retvals, and all others. @@ -2695,6 +2696,16 @@ Status Builder::TranslateGraph( vector tf_ops; for (const auto n : ordered) { + cout << "XXXXX " << n->type_string() << "\n"; + if (n->type_string() == "Const"){ + Tensor result; + vector vect; + result.FromProto(n->def().attr().at("value").tensor()); + TensorDataToVector(result, &vect); + for (auto vv : vect) + cout << " " << vv ; + cout << "\n"; + } if (n->IsSink() || n->IsSource()) { continue; } diff --git a/src/ngraph_builder1.cc b/src/ngraph_builder1.cc index e639dc38..5f617eee 100644 --- a/src/ngraph_builder1.cc +++ b/src/ngraph_builder1.cc @@ -39,7 +39,10 @@ namespace ngraph_bridge { Status Builder1::TranslateGraph( OpKernelContext* ctx, std::shared_ptr& ng_function) { + cout << "XXX1\n"; TF_RETURN_IF_ERROR(Initialize()); + cout << "XXX2 " << ordered.size() << "\n"; + cout << tf_params.size() << " " << tf_ops.size() << " " << tf_ret_vals.size() << "\n"; std::vector static_input_map; @@ -53,16 +56,20 @@ Status Builder1::TranslateGraph( } inputs[i] = input_tensor.shape(); } + cout << "XXX3\n"; // TODO: pass static_input_map to translate_each_op... or pass the vector // ? vector> ng_parameter_list; TF_RETURN_IF_ERROR(GetInputParams(inputs, tf_params, &ng_parameter_list)); + cout << "XXX4 " << tf_params.size() << " " << ng_parameter_list.size() << "\n"; TF_RETURN_IF_ERROR(TranslateEachOp(tf_ops, static_input_map)); + cout << "XXX5 " << tf_ops.size() << "\n"; vector> ng_result_list; - TF_RETURN_IF_ERROR(GetOutputNodes(tf_ops, ng_result_list)); + TF_RETURN_IF_ERROR(GetOutputNodes(tf_ret_vals, ng_result_list)); + cout << "XXX6 " << tf_ret_vals.size() << " " << ng_result_list.size() << "\n"; // Create the nGraph function. ng_function = make_shared(ng_result_list, ng_parameter_list); @@ -73,7 +80,7 @@ Status Builder1::TranslateEachOp( const vector& tf_ops, const std::vector& static_input_map) { // Create the nGraph ops from TensorFlow ops. - +cout << "TEO0 " << tf_ops.size() << "\n"; for (auto op : tf_ops) { NGRAPH_VLOG(2) << "Constructing op " << op->name() << " which is " << op->type_string(); @@ -138,8 +145,9 @@ Status Builder1::ClassifyNodes(const vector& ordered, vector& tf_ret_vals, vector& tf_ops) { // Split ops into params, retvals, and all others. - + cout << "Size of ordered: " << ordered.size() << "\n"; for (const auto n : ordered) { + cout << "XXXXXXXX " << n->type_string() << "\n"; if (n->IsSink() || n->IsSource()) { continue; } @@ -149,7 +157,7 @@ Status Builder1::ClassifyNodes(const vector& ordered, "Encountered a control flow op in the nGraph bridge: ", n->DebugString()); } - + cout << n->type_string() << "\n"; if (n->type_string() == "_Arg") { tf_params.push_back(n); } else if (n->type_string() == "_Retval") { @@ -309,12 +317,13 @@ void Builder1::SaveNgOp(const std::string& op_name, } Status Builder1::Initialize() { - if (is_initialized) { + if (!is_initialized) { // // We will visit ops in topological order. // // ought to be `const Node*`, but GetReversePostOrder doesn't use `const` + cout << "b4 revpostorder " << tf_graph.num_node_ids() << "\n"; GetReversePostOrder(tf_graph, &ordered); TF_RETURN_IF_ERROR(ClassifyNodes(ordered, tf_params, tf_ret_vals, tf_ops)); @@ -488,7 +497,7 @@ Status TranslateUnary(const Node* op, Builder1::DispatchTable Builder1::TRANSLATE_OP_MAP{ {"Abs", {TranslateUnary, {0}}}, {"Add", {TranslateBinary, {0, 1}}}, - {"AddN", {TranslateAddNOp, {}}}, + {"AddN", {TranslateAddNOp, {}}}, //TODO: document {} {"FloorDiv", {TranslateFloorDivOp, {0, 1}}}, {"FloorMod", {TranslateFloorModOp, {0, 1}}}}; diff --git a/src/ngraph_encapsulate_op.cc b/src/ngraph_encapsulate_op.cc index de8df320..5a380d56 100644 --- a/src/ngraph_encapsulate_op.cc +++ b/src/ngraph_encapsulate_op.cc @@ -38,7 +38,7 @@ #include "ngraph/runtime/interpreter/int_backend.hpp" -#define BUILDER1 +//#define BUILDER1 using namespace std; namespace ng = ngraph; diff --git a/test/tf_exec.cpp b/test/tf_exec.cpp index f4df9cd4..fb389872 100644 --- a/test/tf_exec.cpp +++ b/test/tf_exec.cpp @@ -807,24 +807,31 @@ TEST(tf_exec, Op_Rsqrt) { TEST(tf_exec, Op_Negate) { Scope scope_cpu = Scope::NewRootScope(); - Scope scope_ng = scope_cpu.WithDevice("/device:NGRAPH:0"); + Scope scope_ng = scope_cpu; //scope_cpu.WithDevice("/device:NGRAPH:0"); + ActivateNGraph(); // ngraph execution auto A_ng = ops::Const(scope_ng, {{-256.f, 16.5f}, {0.f, 64.f}}); auto r_ng = ops::Negate(scope_ng.WithOpName("r"), A_ng); std::vector outputs_ng; - ClientSession session_ng(scope_ng); + + SessionOptions options; + options.config.mutable_graph_options() + ->mutable_optimizer_options() + ->set_opt_level(OptimizerOptions_Level_L0); + ClientSession session_ng(scope_ng, options); ASSERT_OK(session_ng.Run({r_ng}, &outputs_ng)); ASSERT_EQ(outputs_ng[0].shape(), TensorShape({2, 2})); + DeactivateNGraph(); // reference CPU execution auto A_cpu = ops::Const(scope_cpu, {{-256.f, 16.5f}, {0.f, 64.f}}); auto r_cpu = ops::Negate(scope_cpu.WithOpName("r"), A_cpu); std::vector outputs_cpu; - ClientSession session_cpu(scope_cpu); + ClientSession session_cpu(scope_cpu, options); ASSERT_OK(session_cpu.Run({r_cpu}, &outputs_cpu)); ASSERT_EQ(outputs_cpu[0].shape(), TensorShape({2, 2})); @@ -932,8 +939,14 @@ TEST(tf_exec, Op_AddN) { // No broadcast test needed since AddN does not support it: // https://github.com/tensorflow/tensorflow/blob/master/tensorflow/core/ops/math_ops.cc#L355 + SessionOptions options; + options.config.mutable_graph_options() + ->mutable_optimizer_options() + ->set_opt_level(OptimizerOptions_Level_L0); + ClientSession session_ng(scope_ng, options); + std::vector outputs_ng; - ClientSession session_ng(scope_ng); + //ClientSession session_ng(scope_ng); ASSERT_OK(session_ng.Run({r_ng}, &outputs_ng)); ASSERT_EQ(outputs_ng[0].shape(), TensorShape({2, 2})); @@ -947,7 +960,7 @@ TEST(tf_exec, Op_AddN) { ops::AddN(scope_cpu.WithOpName("r"), {A_cpu, C_cpu, B_cpu, A_cpu, A_cpu}); std::vector outputs_cpu; - ClientSession session_cpu(scope_cpu); + ClientSession session_cpu(scope_cpu, options); ASSERT_OK(session_cpu.Run({r_cpu}, &outputs_cpu)); ASSERT_EQ(outputs_cpu[0].shape(), TensorShape({2, 2})); From 0de4d8bb59bd89ee7344079f98e180b6a20f341e Mon Sep 17 00:00:00 2001 From: Sayantan Sarkar Date: Fri, 24 Aug 2018 16:19:35 -0700 Subject: [PATCH 13/32] Adding Const and NoOp --- src/ngraph_builder1.cc | 82 ++++++++++++++++++++++++++++++++++-- src/ngraph_encapsulate_op.cc | 2 +- 2 files changed, 80 insertions(+), 4 deletions(-) diff --git a/src/ngraph_builder1.cc b/src/ngraph_builder1.cc index 5f617eee..80fd26f9 100644 --- a/src/ngraph_builder1.cc +++ b/src/ngraph_builder1.cc @@ -72,6 +72,7 @@ Status Builder1::TranslateGraph( cout << "XXX6 " << tf_ret_vals.size() << " " << ng_result_list.size() << "\n"; // Create the nGraph function. + cout << "XXX7 " << ng_result_list.size() << " " << ng_parameter_list.size() << "\n"; ng_function = make_shared(ng_result_list, ng_parameter_list); return Status::OK(); } @@ -82,6 +83,7 @@ Status Builder1::TranslateEachOp( // Create the nGraph ops from TensorFlow ops. cout << "TEO0 " << tf_ops.size() << "\n"; for (auto op : tf_ops) { + cout << "TEO:: " << op->type_string() << "\n"; NGRAPH_VLOG(2) << "Constructing op " << op->name() << " which is " << op->type_string(); @@ -96,6 +98,7 @@ cout << "TEO0 " << tf_ops.size() << "\n"; bool variadic_input = input_idxs.size() == 0; int num_inputs = variadic_input ? op->num_inputs() : input_idxs.size(); std::vector> ng_arg_vec(num_inputs); + cout << "TEO3\n"; for (int idx = 0; idx < num_inputs; idx++) { TF_RETURN_IF_ERROR(GetInputNode( op, (variadic_input ? idx : input_idxs[idx]), &ng_arg_vec[idx])); @@ -397,18 +400,23 @@ Status Builder1::GetInputNode(const Node* op, size_t input_idx, // input op may have resulted in more than one ng::Node (eg. Split) // we need to look at Edge to check index of the input op std::vector edges; + cout << op->type_string(); + cout << " GIN0\n"; TF_RETURN_IF_ERROR(op->input_edges(&edges)); size_t src_output_idx; try { + cout << "GIN1 "<src_output(); } catch (const out_of_range&) { return Status(tensorflow::error::NOT_FOUND, "Edge not found"); } - + cout << "GIN2\n"; Node* tf_input; TF_RETURN_IF_ERROR(op->input_node(input_idx, &tf_input)); + cout << "GIN3\n"; try { *result = ng_op_map.at(tf_input->name()).at(src_output_idx); + cout << "GIN4\n"; } catch (const out_of_range&) { return Status(tensorflow::error::NOT_FOUND, "Input node not found"); } @@ -494,12 +502,80 @@ Status TranslateUnary(const Node* op, return Status::OK(); } +template +Status MakeConstOp(const Node* op, ng::element::Type et, + std::shared_ptr* ng_node) { + vector const_values; + TensorShapeProto shape_proto; + + TF_RETURN_IF_ERROR( + ValuesFromConstNode(op->def(), &shape_proto, &const_values)); + + TensorShape const_shape(shape_proto); + ng::Shape ng_shape; + TF_RETURN_IF_ERROR(TFTensorShapeToNGraphShape(const_shape, &ng_shape)); + + *ng_node = make_shared(et, ng_shape, const_values); + return Status::OK(); +} + +static Status TranslateConstOp(const Node* op, + const std::vector>& ng_arg_vec, + const std::vector& static_input_map, + vector>& subgraph_out_nodes) { + + const static std::map< + const DataType, + const std::pair*)>, + const ngraph::element::Type>> + TF_NGRAPH_CONST_MAP = { + {DataType::DT_FLOAT, make_pair(MakeConstOp, ng::element::f32)}, + {DataType::DT_DOUBLE, make_pair(MakeConstOp, ng::element::f64)}, + {DataType::DT_INT8, make_pair(MakeConstOp, ng::element::i8)}, + {DataType::DT_INT16, make_pair(MakeConstOp, ng::element::i16)}, + {DataType::DT_INT32, make_pair(MakeConstOp, ng::element::i32)}, + {DataType::DT_INT64, make_pair(MakeConstOp, ng::element::i64)}, + {DataType::DT_UINT8, make_pair(MakeConstOp, ng::element::u8)}, + {DataType::DT_UINT16, make_pair(MakeConstOp, ng::element::u16)}, + {DataType::DT_BOOL, + make_pair(MakeConstOp, ng::element::boolean)}}; + + DataType dtype; + TF_RETURN_IF_ERROR(GetNodeAttr(op->attrs(), "dtype", &dtype)); + // For some reason the following do not work (no specialization of + // tensorflow::checkpoint::SavedTypeTraits...) + // case DataType::DT_UINT32: + // TF_RETURN_IF_ERROR(MakeConstOp(op, ng::element::u32, + // &ng_node)); + // break; + // case DataType::DT_UINT64: + // TF_RETURN_IF_ERROR(MakeConstOp(op, ng::element::u64, + // &ng_node)); + // break; + try { + const auto& func_param = TF_NGRAPH_CONST_MAP.at(dtype); + TF_RETURN_IF_ERROR(func_param.first(op, func_param.second, &subgraph_out_nodes[0])); + } catch (const std::out_of_range&) { + return errors::Unimplemented("Unsupported TensorFlow data type: ", + DataType_Name(dtype)); + } + return Status::OK(); +} + Builder1::DispatchTable Builder1::TRANSLATE_OP_MAP{ {"Abs", {TranslateUnary, {0}}}, {"Add", {TranslateBinary, {0, 1}}}, - {"AddN", {TranslateAddNOp, {}}}, //TODO: document {} + {"AddN", {TranslateAddNOp, {}}}, //TODO: document {} (variadic input ops) + {"Const", {TranslateConstOp, {0}}}, {"FloorDiv", {TranslateFloorDivOp, {0, 1}}}, - {"FloorMod", {TranslateFloorModOp, {0, 1}}}}; + {"FloorMod", {TranslateFloorModOp, {0, 1}}}, + {"Neg",{TranslateUnary, {0}}}, + {"NoOp", {[](const Node* op, const std::vector>& ng_arg_vec, const std::vector& static_input_map, + vector>& subgraph_out_nodes) { return Status::OK(); }, {}}} + }; + + } // namespace ngraph_bridge diff --git a/src/ngraph_encapsulate_op.cc b/src/ngraph_encapsulate_op.cc index 5a380d56..de8df320 100644 --- a/src/ngraph_encapsulate_op.cc +++ b/src/ngraph_encapsulate_op.cc @@ -38,7 +38,7 @@ #include "ngraph/runtime/interpreter/int_backend.hpp" -//#define BUILDER1 +#define BUILDER1 using namespace std; namespace ng = ngraph; From 07d3b8e01964ba5b3a40fd71c47b48b087734382 Mon Sep 17 00:00:00 2001 From: Sayantan Sarkar Date: Fri, 24 Aug 2018 17:52:38 -0700 Subject: [PATCH 14/32] Negate test runs, which is basically Const test (because of constant folding) --- src/ngraph_builder1.cc | 57 +++++++++++++++++++++--------------------- src/ngraph_builder1.h | 2 ++ 2 files changed, 31 insertions(+), 28 deletions(-) diff --git a/src/ngraph_builder1.cc b/src/ngraph_builder1.cc index 80fd26f9..a0b4aae5 100644 --- a/src/ngraph_builder1.cc +++ b/src/ngraph_builder1.cc @@ -39,10 +39,7 @@ namespace ngraph_bridge { Status Builder1::TranslateGraph( OpKernelContext* ctx, std::shared_ptr& ng_function) { - cout << "XXX1\n"; TF_RETURN_IF_ERROR(Initialize()); - cout << "XXX2 " << ordered.size() << "\n"; - cout << tf_params.size() << " " << tf_ops.size() << " " << tf_ret_vals.size() << "\n"; std::vector static_input_map; @@ -56,23 +53,18 @@ Status Builder1::TranslateGraph( } inputs[i] = input_tensor.shape(); } - cout << "XXX3\n"; // TODO: pass static_input_map to translate_each_op... or pass the vector // ? vector> ng_parameter_list; TF_RETURN_IF_ERROR(GetInputParams(inputs, tf_params, &ng_parameter_list)); - cout << "XXX4 " << tf_params.size() << " " << ng_parameter_list.size() << "\n"; TF_RETURN_IF_ERROR(TranslateEachOp(tf_ops, static_input_map)); - cout << "XXX5 " << tf_ops.size() << "\n"; vector> ng_result_list; TF_RETURN_IF_ERROR(GetOutputNodes(tf_ret_vals, ng_result_list)); - cout << "XXX6 " << tf_ret_vals.size() << " " << ng_result_list.size() << "\n"; // Create the nGraph function. - cout << "XXX7 " << ng_result_list.size() << " " << ng_parameter_list.size() << "\n"; ng_function = make_shared(ng_result_list, ng_parameter_list); return Status::OK(); } @@ -81,33 +73,33 @@ Status Builder1::TranslateEachOp( const vector& tf_ops, const std::vector& static_input_map) { // Create the nGraph ops from TensorFlow ops. -cout << "TEO0 " << tf_ops.size() << "\n"; for (auto op : tf_ops) { - cout << "TEO:: " << op->type_string() << "\n"; NGRAPH_VLOG(2) << "Constructing op " << op->name() << " which is " << op->type_string(); try { - vector> subgraph_out_nodes; + vector> subgraph_out_nodes(op->num_outputs()); //TODOOOO: assign subgraph_out_nodes enough space... it size should be equal to number of outputs expected form the node auto iter = TRANSLATE_OP_MAP.find(op->type_string()); if (iter != TRANSLATE_OP_MAP.end()) { - Builder1::TranslatorFn translate_fn = iter->second.first; - vector input_idxs = iter->second.second; + Builder1::TranslatorFn translate_fn; + vector input_idxs; + std::tie(translate_fn, input_idxs) = iter->second; // input_idxs can be size 0 (to indicate/handle variadic inputs nodes // like Addn) bool variadic_input = input_idxs.size() == 0; int num_inputs = variadic_input ? op->num_inputs() : input_idxs.size(); std::vector> ng_arg_vec(num_inputs); - cout << "TEO3\n"; - for (int idx = 0; idx < num_inputs; idx++) { - TF_RETURN_IF_ERROR(GetInputNode( - op, (variadic_input ? idx : input_idxs[idx]), &ng_arg_vec[idx])); + if (op->type_string() != "Const"){ + for (int idx = 0; idx < num_inputs; idx++) { + TF_RETURN_IF_ERROR(GetInputNode( + op, (variadic_input ? idx : input_idxs[idx]), &ng_arg_vec[idx])); + } } // TODO: instead of pass static_input_map, use GetStaticInputVector and // pass the vector // Then we'd have to pass a vector of vectors, in case a node has >1 // static inputs - TF_RETURN_IF_ERROR(iter->second.first(op, ng_arg_vec, static_input_map, + TF_RETURN_IF_ERROR(translate_fn(op, ng_arg_vec, static_input_map, subgraph_out_nodes)); } else { // TODO::: if-else or try-catch @@ -148,9 +140,7 @@ Status Builder1::ClassifyNodes(const vector& ordered, vector& tf_ret_vals, vector& tf_ops) { // Split ops into params, retvals, and all others. - cout << "Size of ordered: " << ordered.size() << "\n"; for (const auto n : ordered) { - cout << "XXXXXXXX " << n->type_string() << "\n"; if (n->IsSink() || n->IsSource()) { continue; } @@ -160,7 +150,6 @@ Status Builder1::ClassifyNodes(const vector& ordered, "Encountered a control flow op in the nGraph bridge: ", n->DebugString()); } - cout << n->type_string() << "\n"; if (n->type_string() == "_Arg") { tf_params.push_back(n); } else if (n->type_string() == "_Retval") { @@ -326,7 +315,6 @@ Status Builder1::Initialize() { // // ought to be `const Node*`, but GetReversePostOrder doesn't use `const` - cout << "b4 revpostorder " << tf_graph.num_node_ids() << "\n"; GetReversePostOrder(tf_graph, &ordered); TF_RETURN_IF_ERROR(ClassifyNodes(ordered, tf_params, tf_ret_vals, tf_ops)); @@ -400,23 +388,17 @@ Status Builder1::GetInputNode(const Node* op, size_t input_idx, // input op may have resulted in more than one ng::Node (eg. Split) // we need to look at Edge to check index of the input op std::vector edges; - cout << op->type_string(); - cout << " GIN0\n"; TF_RETURN_IF_ERROR(op->input_edges(&edges)); size_t src_output_idx; try { - cout << "GIN1 "<src_output(); } catch (const out_of_range&) { return Status(tensorflow::error::NOT_FOUND, "Edge not found"); } - cout << "GIN2\n"; Node* tf_input; TF_RETURN_IF_ERROR(op->input_node(input_idx, &tf_input)); - cout << "GIN3\n"; try { *result = ng_op_map.at(tf_input->name()).at(src_output_idx); - cout << "GIN4\n"; } catch (const out_of_range&) { return Status(tensorflow::error::NOT_FOUND, "Input node not found"); } @@ -516,6 +498,7 @@ Status MakeConstOp(const Node* op, ng::element::Type et, TF_RETURN_IF_ERROR(TFTensorShapeToNGraphShape(const_shape, &ng_shape)); *ng_node = make_shared(et, ng_shape, const_values); + return Status::OK(); } @@ -575,6 +558,24 @@ Builder1::DispatchTable Builder1::TRANSLATE_OP_MAP{ vector>& subgraph_out_nodes) { return Status::OK(); }, {}}} }; +//Just pass it the op. we can read its name inside. +//Also if #inputs, #outputs are not specified, we can construct them here +Status Builder1::GetOpTranslationRequirements(){ + //auto fn = TRANSLATE_OP_MAP[op_type]; + + //TODO: this function wraps TRANSLATE_OP_MAP. + //It returns a translate function, input indexes, and number of outputs + //The translate function MUST be present in TRANSLATE_OP_MAP + // input_idx and num outputs may not be present, or inferred from op + //Note: op itself may specify the number of outputs... so maybe we dont need to specify that. + //Is there a case we ask for less outputs than what TF provides? + + //For input idxs, by default we should return {0,1, ..., (op->num_inputs)-1}...unless otherwise specified. + + + + return Status::OK(); +} } // namespace ngraph_bridge diff --git a/src/ngraph_builder1.h b/src/ngraph_builder1.h index 62dbc7a2..298ff028 100644 --- a/src/ngraph_builder1.h +++ b/src/ngraph_builder1.h @@ -54,6 +54,8 @@ class Builder1 { std::pair>>; private: + + Status GetOpTranslationRequirements(); // // The op map holds a mapping from TensorFlow op names (strings) to // vector of generated nGraph nodes. From b0523e01b3a1810015b6ff9b5b92d1908e45f108 Mon Sep 17 00:00:00 2001 From: Sayantan Sarkar Date: Mon, 27 Aug 2018 18:40:19 -0700 Subject: [PATCH 15/32] Rearrange the TRANSLATE_OP_MAP portion --- src/ngraph_builder1.cc | 111 ++++++++++++++++++++++------------------- src/ngraph_builder1.h | 20 +++++--- 2 files changed, 74 insertions(+), 57 deletions(-) diff --git a/src/ngraph_builder1.cc b/src/ngraph_builder1.cc index a0b4aae5..8313bfb9 100644 --- a/src/ngraph_builder1.cc +++ b/src/ngraph_builder1.cc @@ -77,22 +77,21 @@ Status Builder1::TranslateEachOp( NGRAPH_VLOG(2) << "Constructing op " << op->name() << " which is " << op->type_string(); - try { - vector> subgraph_out_nodes(op->num_outputs()); //TODOOOO: assign subgraph_out_nodes enough space... it size should be equal to number of outputs expected form the node - auto iter = TRANSLATE_OP_MAP.find(op->type_string()); - if (iter != TRANSLATE_OP_MAP.end()) { - Builder1::TranslatorFn translate_fn; - vector input_idxs; - std::tie(translate_fn, input_idxs) = iter->second; - // input_idxs can be size 0 (to indicate/handle variadic inputs nodes - // like Addn) - bool variadic_input = input_idxs.size() == 0; - int num_inputs = variadic_input ? op->num_inputs() : input_idxs.size(); + Builder1::TranslatorFn translate_fn; + vector input_indexes; + TF_RETURN_IF_ERROR(GetOpTranslationRequirements(op, translate_fn, input_indexes)); + // input_idxs can be size 0 (to indicate/handle variadic inputs nodes + // like Addn) + vector> subgraph_out_nodes(op->num_outputs()); + + + bool variadic_input = input_indexes.size() == 0; + int num_inputs = variadic_input ? op->num_inputs() : input_indexes.size(); std::vector> ng_arg_vec(num_inputs); if (op->type_string() != "Const"){ for (int idx = 0; idx < num_inputs; idx++) { TF_RETURN_IF_ERROR(GetInputNode( - op, (variadic_input ? idx : input_idxs[idx]), &ng_arg_vec[idx])); + op, (variadic_input ? idx : input_indexes[idx]), &ng_arg_vec[idx])); } } // TODO: instead of pass static_input_map, use GetStaticInputVector and @@ -101,36 +100,12 @@ Status Builder1::TranslateEachOp( // static inputs TF_RETURN_IF_ERROR(translate_fn(op, ng_arg_vec, static_input_map, subgraph_out_nodes)); - } else { - // TODO::: if-else or try-catch - // ----------------------------- - // Catch-all for unsupported ops - // ----------------------------- - NGRAPH_VLOG(3) << "Unsupported Op: " << op->name() << " (" - << op->type_string() << ")"; - NGRAPH_VLOG(3) << op->def().DebugString(); - return errors::InvalidArgument("Unsupported Op: ", op->name(), " (", - op->type_string(), ")"); - } + // for (auto ng_node : subgraph_out_nodes) // SaveNgOp(ng_op_map, op->name(), ng_node); ng_op_map[op->name()] = subgraph_out_nodes; // SaveNgOp - // TranslateBinaryOp(op, static_input_map, ng_op_map, ng_floormod) - - } - // TODO: catching the error above. do we keep the try catch or use if-else? - catch (const std::out_of_range&) { - // ----------------------------- - // Catch-all for unsupported ops - // ----------------------------- - NGRAPH_VLOG(3) << "Unsupported Op: " << op->name() << " (" - << op->type_string() << ")"; - NGRAPH_VLOG(3) << op->def().DebugString(); - return errors::InvalidArgument("Unsupported Op: ", op->name(), " (", - op->type_string(), ")"); - } } return Status::OK(); } @@ -301,6 +276,7 @@ Status ValidateInputCountMin(const Node* op, size_t count) { return Status::OK(); } +//TODO: remove if not needed void Builder1::SaveNgOp(const std::string& op_name, const shared_ptr& output_node) { // no need to try-catch, map[key] will create vector object @@ -546,33 +522,68 @@ static Status TranslateConstOp(const Node* op, return Status::OK(); } -Builder1::DispatchTable Builder1::TRANSLATE_OP_MAP{ - {"Abs", {TranslateUnary, {0}}}, - {"Add", {TranslateBinary, {0, 1}}}, - {"AddN", {TranslateAddNOp, {}}}, //TODO: document {} (variadic input ops) - {"Const", {TranslateConstOp, {0}}}, - {"FloorDiv", {TranslateFloorDivOp, {0, 1}}}, - {"FloorMod", {TranslateFloorModOp, {0, 1}}}, - {"Neg",{TranslateUnary, {0}}}, - {"NoOp", {[](const Node* op, const std::vector>& ng_arg_vec, const std::vector& static_input_map, - vector>& subgraph_out_nodes) { return Status::OK(); }, {}}} +const std::map Builder1::TRANSLATE_OP_MAP{ + {"Abs", TranslateUnary}, + {"Add", TranslateBinary}, + {"AddN", TranslateAddNOp}, + {"Const", TranslateConstOp}, + {"FloorDiv", TranslateFloorDivOp}, + {"FloorMod", TranslateFloorModOp}, + {"Neg", TranslateUnary}, + {"NoOp", [](const Node* op, const std::vector>& ng_arg_vec, const std::vector& static_input_map, + vector>& subgraph_out_nodes) { return Status::OK();}} }; +const std::map> Builder1::INPUT_INDEX_MAP{}; + //Just pass it the op. we can read its name inside. //Also if #inputs, #outputs are not specified, we can construct them here -Status Builder1::GetOpTranslationRequirements(){ +Status Builder1::GetOpTranslationRequirements(const Node* op, Builder1::TranslatorFn& translate_fn, vector& input_indexes){ //auto fn = TRANSLATE_OP_MAP[op_type]; //TODO: this function wraps TRANSLATE_OP_MAP. - //It returns a translate function, input indexes, and number of outputs + //It returns a translate function and input indexes //The translate function MUST be present in TRANSLATE_OP_MAP - // input_idx and num outputs may not be present, or inferred from op + // input_idx may not be present, since it can be inferred from op + + //about num_outputs: //Note: op itself may specify the number of outputs... so maybe we dont need to specify that. //Is there a case we ask for less outputs than what TF provides? //For input idxs, by default we should return {0,1, ..., (op->num_inputs)-1}...unless otherwise specified. + auto iter_fn = TRANSLATE_OP_MAP.find(op->type_string()); + if (iter_fn != TRANSLATE_OP_MAP.end()) { + translate_fn = iter_fn->second; + } else { + // TODO::: if-else or try-catch + // ----------------------------- + // Catch-all for unsupported ops + // ----------------------------- + NGRAPH_VLOG(3) << "Unsupported Op: " << op->name() << " (" + << op->type_string() << ")"; + NGRAPH_VLOG(3) << op->def().DebugString(); + return errors::InvalidArgument("Unsupported Op: ", op->name(), " (", + op->type_string(), ")"); + } + + auto iter_input_indexes = INPUT_INDEX_MAP.find(op->type_string()); + if (iter_input_indexes != INPUT_INDEX_MAP.end()){ + input_indexes = iter_input_indexes->second; + } else{ + input_indexes.resize(op->num_inputs()); + std::iota(std::begin(input_indexes), std::end(input_indexes), 0); //Populate with increasing integers 1,2... + } + //TODO: do we even need this? Activate this if there is an op that returns < num_outputs outputs + /* + auto iter_num_outputs = NUM_OUTPUTS_MAP.find(op->type_string()); + if (iter_num_outputs != NUM_OUTPUTS_MAP.end()){ + num_outputs = iter_num_outputs->second; + } else{ + num_outputs = op->num_outputs(); + } + */ return Status::OK(); } diff --git a/src/ngraph_builder1.h b/src/ngraph_builder1.h index 298ff028..fd0c3a89 100644 --- a/src/ngraph_builder1.h +++ b/src/ngraph_builder1.h @@ -44,26 +44,30 @@ namespace ngraph_bridge { ///////////////// class Builder1 { + //TODO: Do we need to typedef OpMap, as it is used only once using OpMap = std::unordered_map>>; using TranslatorFn = function>&, const std::vector&, vector>&)>; - using DispatchTable = - const std::map>>; + //using DispatchTable = + // const std::map>>; private: - Status GetOpTranslationRequirements(); // // The op map holds a mapping from TensorFlow op names (strings) to // vector of generated nGraph nodes. // Builder1::OpMap ng_op_map; - const static std::map>> - TRANSLATE_OP_MAP; + + + + + const static std::map TRANSLATE_OP_MAP; + + const static std::map> INPUT_INDEX_MAP; OpKernelConstruction* ctx; // TODO: is this required? required in // OP_REQUIRES_OK is used instead of @@ -74,6 +78,8 @@ class Builder1 { vector ordered; vector tf_params, tf_ret_vals, tf_ops; + Status GetOpTranslationRequirements(const Node*, Builder1::TranslatorFn&, vector&); + Status GetInputNode(const Node*, size_t, shared_ptr*); // helper function for populating ng_op_map From 91f7a41b2a038f22319719b57394a3aa60ec520c Mon Sep 17 00:00:00 2001 From: Sayantan Sarkar Date: Mon, 27 Aug 2018 19:38:01 -0700 Subject: [PATCH 16/32] Able to run Neg test with new framework --- src/ngraph_builder1.cc | 7 + test/CMakeLists.txt | 4 +- test/opexecuter.cpp | 330 ++++++++++++++++++++++++++++++++++++++++ test/opexecuter.h | 94 ++++++++++++ test/test_math_ops.cpp | 96 ++++++++++++ test/test_utilities.cpp | 77 ++++++++++ test/test_utilities.h | 37 +++++ 7 files changed, 644 insertions(+), 1 deletion(-) create mode 100644 test/opexecuter.cpp create mode 100644 test/opexecuter.h create mode 100644 test/test_math_ops.cpp create mode 100644 test/test_utilities.cpp create mode 100644 test/test_utilities.h diff --git a/src/ngraph_builder1.cc b/src/ngraph_builder1.cc index 8313bfb9..06285e4e 100644 --- a/src/ngraph_builder1.cc +++ b/src/ngraph_builder1.cc @@ -66,6 +66,13 @@ Status Builder1::TranslateGraph( // Create the nGraph function. ng_function = make_shared(ng_result_list, ng_parameter_list); + + // + // Request row-major layout on results. + // + for (auto result : ng_function->get_results()) { + result->set_needs_default_layout(true); + } return Status::OK(); } diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index aec5e45d..cf5bea0a 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -36,10 +36,12 @@ set_target_properties( set(SRC main.cpp graph_exec.cpp - tf_exec.cpp padding.cpp conversions.cpp graph_rewrites/assign_clusters.cc + test_utilities.cpp + test_math_ops.cpp + opexecuter.cpp ) set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11") diff --git a/test/opexecuter.cpp b/test/opexecuter.cpp new file mode 100644 index 00000000..cfd39cf7 --- /dev/null +++ b/test/opexecuter.cpp @@ -0,0 +1,330 @@ +/******************************************************************************* + * Copyright 2017-2018 Intel Corporation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + *******************************************************************************/ +#include "opexecuter.h" + +using namespace std; +namespace ng = ngraph; + +namespace tensorflow { + +namespace ngraph_bridge { + +namespace testing { + +// Utility Function to create NodeDef for _Arg and _Retval nodes +void OpExecuter::CreateNodeDef(const string op_type, + const string op_name_prefix, int index, + const DataType dt, NodeDef& node_def) { + string new_node_name = op_name_prefix + std::to_string(index); + node_def.set_name(new_node_name); + node_def.set_op(op_type); + SetAttrValue(dt, &((*(node_def.mutable_attr()))["T"])); + SetAttrValue(index, &((*(node_def.mutable_attr()))["index"])); +} + +// Update data structures for book keeping +// node_inedge_md : Map of +// key : Node* +// value : vector of pair{Node* src_incoming_edge, int +// src_output_index} +// node_outedge_md : Map of +// key : Node* +// value : vector of pair{Node* dst_outgoing_edge, int +// dst_input_index} +// node_outedges : Map of +// key : Node* +// value : vector of outgoing edges (Edge*) +// test_op : update pointer to test_op pointer +void OpExecuter::GetNodeData(Graph& graph, NodeMetaData& node_inedge_md, + NodeMetaData& node_outedge_md, + NodeOutEdges& node_outedges, Node** test_op) { + bool found_test_op = false; + for (const Edge* e : graph.edges()) { + if (!found_test_op) { + if (e->src()->IsOp() && (e->src()->type_string()) == test_op_type_) { + found_test_op = true; + *test_op = e->src(); + } + if (e->dst()->IsOp() && (e->dst()->type_string()) == test_op_type_) { + found_test_op = true; + *test_op = e->dst(); + } + } + NGRAPH_VLOG(5) << "Edge between, Src: " << e->src()->name() + << " Src op index " << e->src_output() + << " ,Dst: " << e->dst()->name() << " dst ip index " + << e->dst_input(); + // update src's outedge metadata + node_outedge_md[e->src()].push_back({e->dst(), e->dst_input()}); + node_inedge_md[e->dst()].push_back({e->src(), e->src_output()}); + node_outedges[e->src()].push_back(e); + } +} + +// Validate that the graph has N allowed_nodes and 1 test_op_type node +// Graph must look like this +// +// Const1 ConstN +// \ ... / +// \ / +// Test_Op +// +// TO_DO check for vector allowed_nodes +// when we allow other than "Const" node type as input +void OpExecuter::ValidateGraph(const Graph& graph, + const vector allowed_nodes) { + NGRAPH_VLOG(5) << "Validate graph"; + bool found_test_op = false; + for (Node* node : graph.nodes()) { + if (node->IsSource() || node->IsSink()) { + continue; + } else if (node->type_string() == test_op_type_) { + // only one node of type test_op + ASSERT_FALSE(found_test_op); + found_test_op = true; + } else { + ASSERT_TRUE(node->type_string() == allowed_nodes[0]) + << "Found Not allowed Op: " << node->type_string(); + } + } + NGRAPH_VLOG(5) << "Validate graph done"; +} + +// Constructor Function +// TO DO: Add support for ops that take static inputs +// currently static_input_map is empty +OpExecuter::OpExecuter(const Scope sc, const string test_op, + const vector& op_types, + const std::vector& sess_run_fetchops) + : tf_scope_(sc), + test_op_type_(test_op), + expected_output_datatypes_(op_types), + static_input_map_({}), + sess_run_fetchoutputs_(sess_run_fetchops) {} + +// Destructor +OpExecuter::~OpExecuter() {} + +// Uses tf_scope to execute on TF +void OpExecuter::ExecuteOnTF() { + DeactivateNGraph(); + ClientSession session(tf_scope_); + ASSERT_EQ(Status::OK(), session.Run(sess_run_fetchoutputs_, &tf_outputs_)); +} + +// Compares tf_outputs_ with ngraph_outputs_ +void OpExecuter::CompareNgraphAndTF() { + ASSERT_EQ(tf_outputs_.size(), ngraph_outputs_.size()); + for (int i = 0; i < tf_outputs_.size(); i++) { + AssertTensorEquals(tf_outputs_[i], ngraph_outputs_[i]); + } +} + +// This function does the following: +// 1. Validates the graph +// 2. Rewrites the graph to have _Arg and _Retval nodes +// +// _Arg1 _ArgN +// \ ... / +// \ / +// Test_Op +// / \ +// / ... \ +// _Retval1 _RetvalM +// +// 3. Gets Tensor values from Const Nodes and adds to input_tensors +// 4. Creates ng::Function +// 5. Executes ng::Function on CPU backend +// 6. Updates output of ng::Function into ngraph_output +// TO DO : Refactor +void OpExecuter::ExecuteOnNGraph() { + Graph graph(OpRegistry::Global()); + TF_CHECK_OK(tf_scope_.ToGraph(&graph)); + + // For debug + GraphToPbTextFile(&graph, "tf_graph.pbtxt"); + + ValidateGraph(graph, {"Const"}); + + NodeMetaData node_inedge_metadata; + NodeMetaData node_outedge_metadata; + NodeOutEdges node_out_edges; + Node* test_op; + + GetNodeData(graph, node_inedge_metadata, node_outedge_metadata, + node_out_edges, &test_op); + NGRAPH_VLOG(5) << "Got graph data. Found op " << test_op->type_string(); + + // Get Tensor input shapes and values from the const nodes + int number_of_inputs = test_op->num_inputs(); + vector input_shapes; + vector input_node; + + for (int i = 0; i < number_of_inputs; i++) { + Node* ip; + Tensor ip_tensor; + ASSERT_EQ(Status::OK(), test_op->input_node(i, &ip)); + input_node.push_back(ip); + ASSERT_EQ(Status::OK(), GetNodeAttr(ip->attrs(), "value", &ip_tensor)); + input_shapes.push_back(ip_tensor.shape()); + tf_inputs_.push_back(ip_tensor); + NGRAPH_VLOG(5) << " Extracted tensor from const " << i << " " + << tf_inputs_[i].DebugString(); + } + + NGRAPH_VLOG(5) << "Got input nodes and tensors"; + + // Replace the input nodes to Test_op with _Arg nodes + for (int i = 0; i < number_of_inputs; i++) { + Node* ip_node = input_node[i]; + NodeDef new_arg_node_def; + CreateNodeDef("_Arg", "arg_", i, tf_inputs_[i].dtype(), new_arg_node_def); + + // Add node to graph + Status status; + Node* arg_node = graph.AddNode(new_arg_node_def, &status); + ASSERT_EQ(Status::OK(), status); + + // Remove the Const Node + graph.RemoveNode(input_node[i]); + + // Add edge from SOURCE to _Arg + auto src_nodes_metadata = node_inedge_metadata[ip_node]; + for (int j = 0; j < src_nodes_metadata.size(); j++) { + graph.AddEdge(src_nodes_metadata[j].first, src_nodes_metadata[j].second, + arg_node, 0); + } + // Adds an edge from arg_node to test_op + graph.AddEdge(arg_node, 0, test_op, i); + } + + NGRAPH_VLOG(5) << "Replaced input nodes with _Arg"; + + // Add _Retval to graph + int number_of_outputs = expected_output_datatypes_.size(); + // For all the output edges from test_op (there should be only one, to SINK) + // get the dest node and the + // destination_input_index + // (TO DO : ) ADD ASSERT to check one? + auto dest_nodes_metadata = node_outedge_metadata[test_op]; + + // Remove edges from test_op to SINK (not removing might be also ok) + for (const Edge* e : node_out_edges[test_op]) { + graph.RemoveEdge(e); + } + + for (int i = 0; i < number_of_outputs; i++) { + // Add new retval_ node + NodeDef new_ret_node_def; + CreateNodeDef("_Retval", "retval_", i, expected_output_datatypes_[i], + new_ret_node_def); + Status status; + Node* ret_node = graph.AddNode(new_ret_node_def, &status); + ASSERT_EQ(Status::OK(), status); + + // Add edges from _Retval to sink + for (int j = 0; j < dest_nodes_metadata.size(); j++) { + graph.AddEdge(ret_node, 0, dest_nodes_metadata[j].first, + dest_nodes_metadata[j].second); + } + // Add edges from test_op to _Retval + graph.AddEdge(test_op, i, ret_node, 0); + } + + NGRAPH_VLOG(5) << "Added _Retval nodes "; + + NGRAPH_VLOG(5) << "After rewrite *** "; + for (const Edge* e : graph.edges()) { + NGRAPH_VLOG(5) << "Edge between, Src: " << e->src()->name() + << " ,Dst: " << e->dst()->name(); + } + // For debug + GraphToPbTextFile(&graph, "rewrite_ngraph.pbtxt"); + + // Create nGraph function + NGRAPH_VLOG(5) << " Create ng function "; + shared_ptr ng_function; + ASSERT_EQ(Status::OK(), + Builder::TranslateGraph(input_shapes, static_input_map_, &graph, + ng_function)); + + // ng function should get same number of outputs + ASSERT_EQ(expected_output_datatypes_.size(), ng_function->get_output_size()); + + // Create nGraph backend + // Create the nGraph backend + auto backend = ng::runtime::Backend::create("CPU"); + + // Allocate tensors for inputs + vector> ng_ip_tensors; + vector> ng_op_tensors; + + NGRAPH_VLOG(5) << " Creating ng inputs "; + for (int i = 0; i < number_of_inputs; i++) { + ng::Shape ng_shape; + ASSERT_EQ(Status::OK(), + TFTensorShapeToNGraphShape(tf_inputs_[i].shape(), &ng_shape)); + ng::element::Type ng_et; + ASSERT_EQ(Status::OK(), + TFDataTypeToNGraphElementType(tf_inputs_[i].dtype(), &ng_et)); + void* src_ptr = (void*)DMAHelper::base(&tf_inputs_[i]); + auto result = backend->create_tensor(ng_et, ng_shape, src_ptr); + ng_ip_tensors.push_back(result); + } + + NGRAPH_VLOG(5) << " Creating ng outputs "; + vector tf_op_shapes; + for (int i = 0; i < number_of_outputs; i++) { + auto ng_op_shape = ng_function->get_output_shape(i); + auto ng_op_type = ng_function->get_output_element_type(i); + + ng::element::Type ng_et_expected; + ASSERT_EQ(Status::OK(), + TFDataTypeToNGraphElementType(expected_output_datatypes_[i], + &ng_et_expected)); + + // Expected element type should match ng_op_type + ASSERT_EQ(ng_et_expected, ng_op_type); + vector dims; + for (auto dim : ng_op_shape) { + dims.push_back(dim); + } + TensorShape tf_shape(dims); + tf_op_shapes.push_back(tf_shape); + auto result = backend->create_tensor(ng_op_type, ng_op_shape); + ng_op_tensors.push_back(result); + } + + // Execute the nGraph + NGRAPH_VLOG(5) << " Executing on nGraph "; + backend->call(ng_function, ng_op_tensors, ng_ip_tensors); + NGRAPH_VLOG(5) << " Writing to Tensors "; + for (auto i = 0; i < ng_function->get_output_size(); i++) { + // Convert to tf tensor + Tensor output_tensor(expected_output_datatypes_[i], tf_op_shapes[i]); + void* dst_ptr = DMAHelper::base(&output_tensor); + ng_op_tensors[i]->read(dst_ptr, 0, output_tensor.TotalBytes()); + ngraph_outputs_.push_back(output_tensor); + // DumpNGTensor(cout, ng_function->get_output_op(i)->get_name(), + // ng_op_tensors[i]); + } + +} // ExecuteOnNGraph + +} // namespace testing +} // namespace ngraph_bridge + +} // namespace tensorflow diff --git a/test/opexecuter.h b/test/opexecuter.h new file mode 100644 index 00000000..473b9ae3 --- /dev/null +++ b/test/opexecuter.h @@ -0,0 +1,94 @@ +/******************************************************************************* + * Copyright 2017-2018 Intel Corporation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + *******************************************************************************/ +#ifndef NGRAPH_TF_BRIDGE_OPEXECUTER_H_ +#define NGRAPH_TF_BRIDGE_OPEXECUTER_H_ + +#include "ngraph/ngraph.hpp" +#include "ngraph_builder.h" +#include "ngraph_utils.h" +#include "test_utilities.h" +#include "tf_graph_writer.h" + +#include "tensorflow/core/common_runtime/dma_helper.h" +#include "tensorflow/core/framework/graph.pb.h" +#include "tensorflow/core/framework/op.h" +#include "tensorflow/core/framework/tensor_types.h" +#include "tensorflow/core/graph/algorithm.h" +#include "tensorflow/core/graph/graph.h" +#include "tensorflow/core/graph/graph_constructor.h" +#include "tensorflow/core/platform/env.h" + +#include "tensorflow/cc/client/client_session.h" +#include "tensorflow/cc/ops/standard_ops.h" +#include "tensorflow/core/framework/tensor.h" +#include "tensorflow/core/public/session.h" + +using namespace std; +namespace ng = ngraph; + +namespace tensorflow { + +namespace ngraph_bridge { + +namespace testing { + +class OpExecuter { + public: + using NodeMetaData = map>>; + using NodeOutEdges = map>; + + // Scope sc : TF Scope with execution graph + // string test_op : test_op_type e.g. "Add" + // vector& op_types : expected tf data types of the output e.g. + // {DT_FLOAT, DT_INT} const std::vector& fetch_ops : Output ops to be + // fetched, is passed to tf.session.run() + OpExecuter(const Scope sc, const string test_op, + const vector& op_types, + const std::vector& fetch_ops); + + ~OpExecuter(); + + void ExecuteOnNGraph(); + void ExecuteOnTF(); + void CompareNgraphAndTF(); + + private: + Scope tf_scope_; + const string test_op_type_; + vector tf_inputs_; + vector tf_outputs_; + vector ngraph_outputs_; + const vector expected_output_datatypes_; + const vector& static_input_map_; + + // To Do : For placeholder const FeedType sess_run_inputs_; + const std::vector sess_run_fetchoutputs_; + + void GetNodeData(Graph& graph, NodeMetaData& node_inedge_md, + NodeMetaData& node_outedge_md, NodeOutEdges& node_outedges, + Node** test_op); + void ValidateGraph(const Graph& graph, const vector allowed_nodes); + + void CreateNodeDef(const string op_type, const string op_name_prefix, + int index, const DataType dt, NodeDef& node_def); +}; + +} // namespace testing +} // namespace ngraph_bridge + +} // namespace tensorflow + +#endif // NGRAPH_TF_BRIDGE_OPEXECUTER_H_ diff --git a/test/test_math_ops.cpp b/test/test_math_ops.cpp new file mode 100644 index 00000000..77dbf8db --- /dev/null +++ b/test/test_math_ops.cpp @@ -0,0 +1,96 @@ +/******************************************************************************* + * Copyright 2017-2018 Intel Corporation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + *******************************************************************************/ +#include "gtest/gtest.h" +#include "opexecuter.h" +#include "test_utilities.h" +#include "ngraph_utils.h" +#include "tf_graph_writer.h" +#include "tensorflow/core/common_runtime/dma_helper.h" +#include "tensorflow/core/framework/graph.pb.h" +#include "tensorflow/core/framework/op.h" +#include "tensorflow/core/framework/tensor_types.h" +#include "tensorflow/core/graph/algorithm.h" +#include "tensorflow/core/graph/graph.h" +#include "tensorflow/core/graph/graph_constructor.h" +#include "tensorflow/core/platform/env.h" +#include "tensorflow/cc/client/client_session.h" +#include "tensorflow/cc/ops/standard_ops.h" +#include "tensorflow/core/framework/tensor.h" +#include "tensorflow/core/public/session.h" + using namespace std; +namespace ng = ngraph; + namespace tensorflow { + namespace ngraph_bridge { + namespace testing { + #define ASSERT_OK(x) ASSERT_EQ((x), ::tensorflow::Status::OK()); + // Test(TestCaseName, TestName) +// Please ensure +// Neither TestCaseName nor TestName should contain underscore +// https://github.com/google/googletest/blob/master/googletest/docs/primer.md +// Use only Tensors and ops::Const() to provide input to the test op +TEST(MathOps, Neg) { + // Create a tf graph + Scope root = Scope::NewRootScope(); + int dim1 = 2; + int dim2 = 2; + Tensor A(DT_FLOAT, TensorShape({dim1, dim2})); + AssignInputValues(A, -2.1f); + auto R = ops::Neg(root, A); + vector output_datatypes = {DT_FLOAT}; + std::vector sess_run_fetchoutputs = {R}; + OpExecuter opexecuter(root, "Neg", output_datatypes, sess_run_fetchoutputs); + opexecuter.ExecuteOnNGraph(); + opexecuter.ExecuteOnTF(); + opexecuter.CompareNgraphAndTF(); +} + + TEST(MathOps, Add) { + // Create a tf graph + Scope root = Scope::NewRootScope(); + int dim1 = 2; + int dim2 = 2; + Tensor A(DT_FLOAT, TensorShape({dim1, dim2})); + Tensor B(DT_FLOAT, TensorShape({dim1, dim2})); + AssignInputValues(A, 2.1f); + AssignInputValues(B, 4.1f); + auto R = ops::Add(root, A, B); + vector output_datatypes = {DT_FLOAT}; + std::vector sess_run_fetchoutputs = {R}; + OpExecuter opexecuter(root, "Add", output_datatypes, sess_run_fetchoutputs); + opexecuter.ExecuteOnNGraph(); + opexecuter.ExecuteOnTF(); + opexecuter.CompareNgraphAndTF(); +} + TEST(MathOps, RealDiv) { + Scope root = Scope::NewRootScope(); + int dim1 = 100; + int dim2 = 200; + Tensor A(DT_FLOAT, TensorShape({dim1, dim2})); + Tensor B(DT_FLOAT, TensorShape({dim1, dim2})); + AssignInputValues(A, 2.0f); + AssignInputValues(B, 7.0f); + auto R = ops::RealDiv(root, A, B); + vector output_datatypes = {DT_FLOAT}; + std::vector sess_run_fetchoutputs = {R}; + OpExecuter opexecuter(root, "RealDiv", output_datatypes, + sess_run_fetchoutputs); + opexecuter.ExecuteOnNGraph(); + opexecuter.ExecuteOnTF(); + opexecuter.CompareNgraphAndTF(); +} + } // namespace testing + } // namespace ngraph_bridge +} // namespace tensorflow diff --git a/test/test_utilities.cpp b/test/test_utilities.cpp new file mode 100644 index 00000000..90b30085 --- /dev/null +++ b/test/test_utilities.cpp @@ -0,0 +1,77 @@ +/******************************************************************************* + * Copyright 2017-2018 Intel Corporation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + *******************************************************************************/ + #include "test_utilities.h" + using namespace std; + namespace ng = ngraph; + namespace tensorflow { + namespace ngraph_bridge { + void ActivateNGraph() { + setenv("NGRAPH_TF_DISABLE_DEASSIGN_CLUSTERS", "1", 1); + unsetenv("NGRAPH_TF_DISABLE"); +} + void DeactivateNGraph() { + unsetenv("NGRAPH_TF_DISABLE_DEASSIGN_CLUSTERS"); + setenv("NGRAPH_TF_DISABLE", "1", 1); +} + void AssertTensorEquals(Tensor& T1, Tensor& T2) { + auto T_size = T1.flat().size(); + auto T1_data = T1.flat().data(); + auto T2_data = T2.flat().data(); + for (int k = 0; k < T_size; k++) { + auto a = T1_data[k]; + auto b = T2_data[k]; + EXPECT_FLOAT_EQ(a, b); + } +} + void AssignInputIntValues(Tensor& A, int maxval) { + auto A_flat = A.flat(); + auto A_flat_data = A_flat.data(); + int counter = 0; + for (int i = 0; i < A_flat.size(); i++) { + A_flat_data[i] = counter++; + if (counter == maxval) { + counter = 0; + } + } +} + void AssignInputValues(Tensor& A, float x) { + auto A_flat = A.flat(); + auto A_flat_data = A_flat.data(); + for (int i = 0; i < A_flat.size(); i++) { + A_flat_data[i] = x; + } +} + void PrintTensor(const Tensor& T1) { + LOG(INFO) << "print tensor values" << T1.DebugString(); +} + void ValidateTensorData(Tensor& T1, Tensor& T2, float tol) { + auto T_size = T1.flat().size(); + auto T1_data = T1.flat().data(); + auto T2_data = T2.flat().data(); + for (int k = 0; k < T_size; k++) { + auto a = T1_data[k]; + auto b = T2_data[k]; + if (a == 0) { + EXPECT_NEAR(a, b, tol); + } else { + auto rel = a - b; + auto rel_div = std::abs(rel / a); + EXPECT_TRUE(rel_div < tol); + } + } +} + } // namespace ngraph_bridge +} //namespace tensorflow diff --git a/test/test_utilities.h b/test/test_utilities.h new file mode 100644 index 00000000..749f6527 --- /dev/null +++ b/test/test_utilities.h @@ -0,0 +1,37 @@ +/******************************************************************************* + * Copyright 2017-2018 Intel Corporation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + *******************************************************************************/ +#ifndef NGRAPH_TF_BRIDGE_TESTUTILITIES_H_ +#define NGRAPH_TF_BRIDGE_TESTUTILITIES_H_ + #include "gtest/gtest.h" +#include "ngraph/ngraph.hpp" + #include "tensorflow/core/framework/tensor.h" +#include "tensorflow/core/framework/tensor_types.h" +#include "tensorflow/core/platform/env.h" + using namespace std; +namespace ng = ngraph; + namespace tensorflow { + namespace ngraph_bridge { +// some utility functions copied from tf_exec.cpp +void ActivateNGraph(); +void DeactivateNGraph(); +void AssertTensorEquals(Tensor& T1, Tensor& T2); +void AssignInputIntValues(Tensor& A, int maxval); +void AssignInputValues(Tensor& A, float x); +void PrintTensor(const Tensor& T1); +void ValidateTensorData(Tensor& T1, Tensor& T2, float tol); + } // namespace ngraph_bridge + } // namespace tensorflow + #endif // NGRAPH_TF_BRIDGE_TESTUTILITIES_H_ From b9fe4eccb5221c023e435b6fc33a1773e99e7061 Mon Sep 17 00:00:00 2001 From: Sayantan Sarkar Date: Tue, 28 Aug 2018 11:49:22 -0700 Subject: [PATCH 17/32] Neg, Add, RealDiv working --- src/ngraph_builder.cc | 1 - src/ngraph_builder1.cc | 46 +++++++++++++++++++++++------------- src/ngraph_builder1.h | 9 +++++++ src/ngraph_encapsulate_op.cc | 2 +- test/opexecuter.cpp | 7 ++++++ test/opexecuter.h | 1 + 6 files changed, 47 insertions(+), 19 deletions(-) diff --git a/src/ngraph_builder.cc b/src/ngraph_builder.cc index c416a0c5..1ccc0511 100644 --- a/src/ngraph_builder.cc +++ b/src/ngraph_builder.cc @@ -2686,7 +2686,6 @@ Status Builder::TranslateGraph( // ought to be `const Node*`, but GetReversePostOrder doesn't use `const` vector ordered; GetReversePostOrder(*input_graph, &ordered); - cout << "input_graph: " << input_graph->num_node_ids() << "\n"; // // Split ops into params, retvals, and all others. diff --git a/src/ngraph_builder1.cc b/src/ngraph_builder1.cc index 06285e4e..7504ceab 100644 --- a/src/ngraph_builder1.cc +++ b/src/ngraph_builder1.cc @@ -38,27 +38,17 @@ namespace tensorflow { namespace ngraph_bridge { Status Builder1::TranslateGraph( - OpKernelContext* ctx, std::shared_ptr& ng_function) { - TF_RETURN_IF_ERROR(Initialize()); - - std::vector static_input_map; - - std::vector inputs(ctx->num_inputs()); + const std::vector& inputs, + const std::vector& static_input_map, + shared_ptr& ng_function){ - static_input_map.resize(ctx->num_inputs()); - for (int i = 0; i < ctx->num_inputs(); i++) { - const Tensor& input_tensor = ctx->input(i); - if (m_input_is_static[i]) { - static_input_map[i] = &input_tensor; - } - inputs[i] = input_tensor.shape(); - } - // TODO: pass static_input_map to translate_each_op... or pass the vector - // ? + TF_RETURN_IF_ERROR(Initialize()); vector> ng_parameter_list; TF_RETURN_IF_ERROR(GetInputParams(inputs, tf_params, &ng_parameter_list)); + // TODO: pass static_input_map to translate_each_op... or pass the vector + // ? TF_RETURN_IF_ERROR(TranslateEachOp(tf_ops, static_input_map)); vector> ng_result_list; @@ -76,9 +66,30 @@ Status Builder1::TranslateGraph( return Status::OK(); } +Status Builder1::TranslateGraph( + OpKernelContext* ctx, std::shared_ptr& ng_function) { + TF_RETURN_IF_ERROR(Initialize()); + + std::vector static_input_map; + + std::vector inputs(ctx->num_inputs()); + + static_input_map.resize(ctx->num_inputs()); + for (int i = 0; i < ctx->num_inputs(); i++) { + const Tensor& input_tensor = ctx->input(i); + if (m_input_is_static[i]) { + static_input_map[i] = &input_tensor; + } + inputs[i] = input_tensor.shape(); + } + + return TranslateGraph(inputs, static_input_map, ng_function); +} + Status Builder1::TranslateEachOp( const vector& tf_ops, const std::vector& static_input_map) { + cout << "======TranslateEachOp======\n"; // Create the nGraph ops from TensorFlow ops. for (auto op : tf_ops) { NGRAPH_VLOG(2) << "Constructing op " << op->name() << " which is " @@ -538,7 +549,8 @@ const std::map Builder1::TRANSLATE_OP_MAP{ {"FloorMod", TranslateFloorModOp}, {"Neg", TranslateUnary}, {"NoOp", [](const Node* op, const std::vector>& ng_arg_vec, const std::vector& static_input_map, - vector>& subgraph_out_nodes) { return Status::OK();}} + vector>& subgraph_out_nodes) { return Status::OK();}}, + {"RealDiv", TranslateBinary} }; const std::map> Builder1::INPUT_INDEX_MAP{}; diff --git a/src/ngraph_builder1.h b/src/ngraph_builder1.h index fd0c3a89..0d6de1d5 100644 --- a/src/ngraph_builder1.h +++ b/src/ngraph_builder1.h @@ -115,10 +115,19 @@ class Builder1 { Status Initialize(); public: + Builder1(const Graph& tf_graph, OpKernelConstruction* ctx) // TODO make ctx const? : tf_graph(tf_graph), ctx(ctx) {} + + Builder1(const Graph& tf_graph) : tf_graph(tf_graph) {} + + Status TranslateGraph( + const std::vector&, + const std::vector&, + shared_ptr&); + Status TranslateGraph(OpKernelContext* ctx, std::shared_ptr& ng_function); }; diff --git a/src/ngraph_encapsulate_op.cc b/src/ngraph_encapsulate_op.cc index de8df320..88feeefc 100644 --- a/src/ngraph_encapsulate_op.cc +++ b/src/ngraph_encapsulate_op.cc @@ -73,7 +73,7 @@ class NGraphEncapsulateOp : public OpKernel { : OpKernel(ctx), m_graph(OpRegistry::Global()), #ifdef BUILDER1 - ng_builder(m_graph, ctx), + ng_builder(m_graph, ctx), //TODO: does ng_builder get the correct graph before ConvertGraphDefToGraph is called? #endif m_freshness_tracker(nullptr) { GraphDef* graph_def; diff --git a/test/opexecuter.cpp b/test/opexecuter.cpp index cfd39cf7..6f2f9ea0 100644 --- a/test/opexecuter.cpp +++ b/test/opexecuter.cpp @@ -257,9 +257,16 @@ void OpExecuter::ExecuteOnNGraph() { // Create nGraph function NGRAPH_VLOG(5) << " Create ng function "; shared_ptr ng_function; + if (false){ ASSERT_EQ(Status::OK(), Builder::TranslateGraph(input_shapes, static_input_map_, &graph, ng_function)); + } + else{ + auto builder = Builder1(graph); + ASSERT_EQ(Status::OK(), + builder.TranslateGraph(input_shapes, static_input_map_, ng_function)); + } // ng function should get same number of outputs ASSERT_EQ(expected_output_datatypes_.size(), ng_function->get_output_size()); diff --git a/test/opexecuter.h b/test/opexecuter.h index 473b9ae3..ca9311be 100644 --- a/test/opexecuter.h +++ b/test/opexecuter.h @@ -18,6 +18,7 @@ #include "ngraph/ngraph.hpp" #include "ngraph_builder.h" +#include "ngraph_builder1.h" #include "ngraph_utils.h" #include "test_utilities.h" #include "tf_graph_writer.h" From 0fa9307ee96d746cdeb091c40e4d178c771ceeef Mon Sep 17 00:00:00 2001 From: Sayantan Sarkar Date: Tue, 28 Aug 2018 14:31:18 -0700 Subject: [PATCH 18/32] Add AvgPool and conversions Split conversions into .cc and .h to add header guards So that it can be included in multiple places (builder and builder1) --- src/CMakeLists.txt | 1 + src/ngraph_builder1.cc | 95 ++++++++++++++++++++++++++++++++------- src/ngraph_builder1.h | 4 +- src/ngraph_conversions.cc | 47 +++++++++++++++++++ src/ngraph_conversions.h | 23 +++------- 5 files changed, 137 insertions(+), 33 deletions(-) create mode 100644 src/ngraph_conversions.cc diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index caf1fdd0..0f9825be 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -56,6 +56,7 @@ set(SRC ngraph_tracked_variable.cc ngraph_utils.cc tf_graphcycles.cc + ngraph_conversions.cc ) add_library(${LIB_NAME} SHARED ${SRC}) diff --git a/src/ngraph_builder1.cc b/src/ngraph_builder1.cc index 7504ceab..a75c3d5c 100644 --- a/src/ngraph_builder1.cc +++ b/src/ngraph_builder1.cc @@ -15,7 +15,7 @@ *******************************************************************************/ #include "ngraph_builder1.h" -//#include "ngraph_conversions.h" +#include "ngraph_conversions.h" #include "ngraph_log.h" #include "ngraph_utils.h" @@ -37,6 +37,9 @@ namespace tensorflow { namespace ngraph_bridge { +//TODO: move this to .h +using VectNg = std::vector>; + Status Builder1::TranslateGraph( const std::vector& inputs, const std::vector& static_input_map, @@ -89,7 +92,6 @@ Status Builder1::TranslateGraph( Status Builder1::TranslateEachOp( const vector& tf_ops, const std::vector& static_input_map) { - cout << "======TranslateEachOp======\n"; // Create the nGraph ops from TensorFlow ops. for (auto op : tf_ops) { NGRAPH_VLOG(2) << "Constructing op " << op->name() << " which is " @@ -100,12 +102,12 @@ Status Builder1::TranslateEachOp( TF_RETURN_IF_ERROR(GetOpTranslationRequirements(op, translate_fn, input_indexes)); // input_idxs can be size 0 (to indicate/handle variadic inputs nodes // like Addn) - vector> subgraph_out_nodes(op->num_outputs()); + VectNg subgraph_out_nodes(op->num_outputs()); bool variadic_input = input_indexes.size() == 0; int num_inputs = variadic_input ? op->num_inputs() : input_indexes.size(); - std::vector> ng_arg_vec(num_inputs); + VectNg ng_arg_vec(num_inputs); if (op->type_string() != "Const"){ for (int idx = 0; idx < num_inputs; idx++) { TF_RETURN_IF_ERROR(GetInputNode( @@ -399,12 +401,9 @@ Status Builder1::GetInputNode(const Node* op, size_t input_idx, return Status::OK(); } - -using VectNg = std::vector>; - // TODO: move translate ops to a different file Status TranslateFloorDivOp(const Node* op, - const VectNg& ng_arg_vec, + VectNg& ng_arg_vec, const std::vector& static_input_map, VectNg& subgraph_out_nodes) { subgraph_out_nodes[0] = @@ -413,7 +412,7 @@ Status TranslateFloorDivOp(const Node* op, } Status TranslateFloorModOp(const Node* op, - const std::vector>& ng_arg_vec, + VectNg& ng_arg_vec, const std::vector& static_input_map, VectNg& subgraph_out_nodes) { auto floordiv = @@ -428,7 +427,7 @@ Status TranslateFloorModOp(const Node* op, } Status TranslateAddNOp(const Node* op, - const VectNg &ng_arg_vec, + VectNg &ng_arg_vec, const std::vector& static_input_map, VectNg& subgraph_out_nodes) { subgraph_out_nodes[0] = @@ -458,9 +457,9 @@ Status TranslateAddNOp(const Node* op, // part of the class, and the TranslateOps will not have access to it. template Status TranslateBinary(const Node* op, - const std::vector>& ng_arg_vec, + VectNg& ng_arg_vec, const std::vector& static_input_map, - vector>& subgraph_out_nodes) { + VectNg& subgraph_out_nodes) { // TODO: assert subgraph_out_nodes.size()==1, ng_arg_vec.size()==2 auto node_pair = ng::builder::numpy_broadcast( std::make_pair(ng_arg_vec[0], ng_arg_vec[1])); @@ -497,9 +496,9 @@ Status MakeConstOp(const Node* op, ng::element::Type et, } static Status TranslateConstOp(const Node* op, - const std::vector>& ng_arg_vec, + VectNg& ng_arg_vec, const std::vector& static_input_map, - vector>& subgraph_out_nodes) { + VectNg& subgraph_out_nodes) { const static std::map< const DataType, @@ -540,16 +539,80 @@ static Status TranslateConstOp(const Node* op, return Status::OK(); } +//TODO: document why ng_arg_vec is not const. (BatchToNGraph changes it) +Status TranslateAvgPoolOp(const Node* op, + VectNg& ng_arg_vec, + const std::vector& static_input_map, + VectNg& subgraph_out_nodes){ + std::vector tf_strides; + std::vector tf_ksize; + std::string tf_padding_type; + std::string tf_data_format; + TF_RETURN_IF_ERROR(GetNodeAttr(op->attrs(), "strides", &tf_strides)); + TF_RETURN_IF_ERROR(GetNodeAttr(op->attrs(), "ksize", &tf_ksize)); + TF_RETURN_IF_ERROR(GetNodeAttr(op->attrs(), "padding", &tf_padding_type)); + TF_RETURN_IF_ERROR(GetNodeAttr(op->attrs(), "data_format", &tf_data_format)); + + if (tf_data_format != "NHWC" && tf_data_format != "NCHW") { + return errors::InvalidArgument( + "AvgPool data format is neither NHWC nor NCHW"); + } + + bool is_nhwc = (tf_data_format == "NHWC"); + + NGRAPH_VLOG(3) << ng::join(tf_strides); + NGRAPH_VLOG(3) << ng::join(tf_ksize); + NGRAPH_VLOG(3) << tf_padding_type; + NGRAPH_VLOG(3) << tf_data_format; + + ng::Strides ng_strides(2); + ng::Shape ng_image_shape(2); + ng::Shape ng_kernel_shape(2); + + //TODO: should these be in Builder instead of in ngraph_conversions.h? + BatchedOpParamToNGraph(is_nhwc, tf_strides, ng_strides); + BatchedOpParamToNGraph(is_nhwc, ng_arg_vec[0]->get_shape(), ng_image_shape); + BatchedOpParamToNGraph(is_nhwc, tf_ksize, ng_kernel_shape); + BatchToNGraph(is_nhwc, ng_arg_vec[0]); + NGRAPH_VLOG(3) << "ng_strides: " << ng::join(ng_strides); + NGRAPH_VLOG(3) << "ng_image_shape: " << ng::join(ng_image_shape); + NGRAPH_VLOG(3) << "ng_kernel_shape: " << ng::join(ng_kernel_shape); + + // TODO: change this once nGraph supports negative padding + // (CoordinateDiff) for AvgPool + // ng::CoordinateDiff ng_padding_below{0,0}; + // ng::CoordinateDiff ng_padding_above{0,0}; + ng::Shape ng_padding_below{0, 0}; + ng::Shape ng_padding_above{0, 0}; + + MakePadding(tf_padding_type, ng_image_shape, ng_kernel_shape, + ng_strides, ng_padding_below, ng_padding_above); + + std::shared_ptr ng_avgpool = + make_shared(ng_arg_vec[0], ng_kernel_shape, ng_strides, + ng_padding_below, ng_padding_above, false); + + BatchToTensorflow(is_nhwc, ng_avgpool); + NGRAPH_VLOG(3) << "avgpool outshape: {" << ng::join(ng_avgpool->get_shape()) + << "}"; + + subgraph_out_nodes[0] = ng_avgpool; + //SaveNgOp(ng_op_map, op->name(), ng_avgpool); + return Status::OK(); +} + + const std::map Builder1::TRANSLATE_OP_MAP{ {"Abs", TranslateUnary}, {"Add", TranslateBinary}, {"AddN", TranslateAddNOp}, + {"AvgPool", TranslateAvgPoolOp}, {"Const", TranslateConstOp}, {"FloorDiv", TranslateFloorDivOp}, {"FloorMod", TranslateFloorModOp}, {"Neg", TranslateUnary}, - {"NoOp", [](const Node* op, const std::vector>& ng_arg_vec, const std::vector& static_input_map, - vector>& subgraph_out_nodes) { return Status::OK();}}, + {"NoOp", [](const Node* op, VectNg& ng_arg_vec, const std::vector& static_input_map, + VectNg& subgraph_out_nodes) { return Status::OK();}}, {"RealDiv", TranslateBinary} }; diff --git a/src/ngraph_builder1.h b/src/ngraph_builder1.h index 0d6de1d5..2d2d5cd1 100644 --- a/src/ngraph_builder1.h +++ b/src/ngraph_builder1.h @@ -19,6 +19,8 @@ #include #include +//#include "ngraph_conversions.h" + #include "ngraph/ngraph.hpp" #include "ngraph_log.h" @@ -48,7 +50,7 @@ class Builder1 { using OpMap = std::unordered_map>>; using TranslatorFn = function>&, + const Node*, std::vector>&, const std::vector&, vector>&)>; //using DispatchTable = // const std::map& ng_node) { + Reshape<0, 3, 1, 2>(ng_node); +} +} + +void BatchToNGraph(bool is_nhwc, std::shared_ptr& ng_input) { + if (is_nhwc) { + detail::NhwcToNGraph(ng_input); + } +} + +void BatchToTensorflow(bool is_nhwc, std::shared_ptr& ng_node) { + if (!is_nhwc) { + return; + } + Reshape<0, 2, 3, 1>(ng_node); +} + +} // namespace ngraph_bridge + +} // namespace tensorflow diff --git a/src/ngraph_conversions.h b/src/ngraph_conversions.h index d54ea841..1b87b79b 100644 --- a/src/ngraph_conversions.h +++ b/src/ngraph_conversions.h @@ -14,6 +14,8 @@ * limitations under the License. *******************************************************************************/ +#ifndef NGRAPH_TF_BRIDGE_CONVERSIONS_ +#define NGRAPH_TF_BRIDGE_CONVERSIONS_ #pragma once #include "ngraph_builder.h" @@ -24,7 +26,7 @@ namespace tensorflow { namespace ngraph_bridge { template -void Reshape(std::shared_ptr& ng_node) { +void Reshape(std::shared_ptr& ng_node){ static_assert(a < 4 && b < 4 && c < 4 && d < 4, "Number of dimensions cannot exceed 4"); static_assert(a != b && a != c && a != d && b != c && b != d && c != d, @@ -43,9 +45,7 @@ void NhwcToNGraph(const std::vector& src, std::vector& dst) { dst[1] = src[2]; } -void NhwcToNGraph(std::shared_ptr& ng_node) { - Reshape<0, 3, 1, 2>(ng_node); -} +void NhwcToNGraph(std::shared_ptr& ng_node); template void NchwToNGraph(const std::vector& src, std::vector& dst) { @@ -62,11 +62,7 @@ void NhwcToNchw(const std::vector& src, std::vector& dst) { } } -void BatchToNGraph(bool is_nhwc, std::shared_ptr& ng_input) { - if (is_nhwc) { - detail::NhwcToNGraph(ng_input); - } -} +void BatchToNGraph(bool is_nhwc, std::shared_ptr& ng_input); template void BatchedOpParamToNGraph(bool is_nhwc, const std::vector& src, @@ -88,13 +84,8 @@ void BatchedOpParamReshape(bool is_nhwc, const std::vector& src, } } -void BatchToTensorflow(bool is_nhwc, std::shared_ptr& ng_node) { - if (!is_nhwc) { - return; - } - Reshape<0, 2, 3, 1>(ng_node); -} - +void BatchToTensorflow(bool is_nhwc, std::shared_ptr& ng_node); } // namespace ngraph_bridge } // namespace tensorflow +#endif \ No newline at end of file From 4504bdf3d97577f91d7b3953bd4d964b99f82aa8 Mon Sep 17 00:00:00 2001 From: Sayantan Sarkar Date: Tue, 28 Aug 2018 17:09:10 -0700 Subject: [PATCH 19/32] Move translateops functions to separate file --- src/ngraph_builder.cc | 5 +- src/ngraph_builder1.cc | 439 ++++++----------------------------- src/ngraph_builder1.h | 47 ++-- src/ngraph_conversions.cc | 1 - src/ngraph_conversions.h | 2 +- src/ngraph_encapsulate_op.cc | 4 +- src/ngraph_translateops.h | 271 +++++++++++++++++++++ test/opexecuter.cpp | 15 +- test/test_math_ops.cpp | 64 ++--- test/test_utilities.cpp | 28 +-- test/test_utilities.h | 16 +- test/tf_exec.cpp | 10 +- 12 files changed, 434 insertions(+), 468 deletions(-) create mode 100644 src/ngraph_translateops.h diff --git a/src/ngraph_builder.cc b/src/ngraph_builder.cc index 1ccc0511..93029aea 100644 --- a/src/ngraph_builder.cc +++ b/src/ngraph_builder.cc @@ -2696,13 +2696,12 @@ Status Builder::TranslateGraph( for (const auto n : ordered) { cout << "XXXXX " << n->type_string() << "\n"; - if (n->type_string() == "Const"){ + if (n->type_string() == "Const") { Tensor result; vector vect; result.FromProto(n->def().attr().at("value").tensor()); TensorDataToVector(result, &vect); - for (auto vv : vect) - cout << " " << vv ; + for (auto vv : vect) cout << " " << vv; cout << "\n"; } if (n->IsSink() || n->IsSource()) { diff --git a/src/ngraph_builder1.cc b/src/ngraph_builder1.cc index a75c3d5c..267910c1 100644 --- a/src/ngraph_builder1.cc +++ b/src/ngraph_builder1.cc @@ -15,20 +15,18 @@ *******************************************************************************/ #include "ngraph_builder1.h" -#include "ngraph_conversions.h" -#include "ngraph_log.h" -#include "ngraph_utils.h" +#include "ngraph_translateops.h" -#include "ngraph/builder/autobroadcast.hpp" -#include "ngraph/builder/numpy_transpose.hpp" +//#include "ngraph/builder/autobroadcast.hpp" +//#include "ngraph/builder/numpy_transpose.hpp" -#include "tensorflow/core/framework/tensor.pb.h" -#include "tensorflow/core/framework/tensor_shape.pb.h" -#include "tensorflow/core/framework/tensor_shape.pb_text.h" -#include "tensorflow/core/graph/algorithm.h" -#include "tensorflow/core/graph/edgeset.h" -#include "tensorflow/core/lib/core/errors.h" -#include "tensorflow/stream_executor/lib/statusor.h" +//TODO: remove the header files, if not needed +//#include "tensorflow/core/framework/tensor.pb.h" +//#include "tensorflow/core/framework/tensor_shape.pb.h" +//#include "tensorflow/core/framework/tensor_shape.pb_text.h" +//#include "tensorflow/core/graph/algorithm.h" +//#include "tensorflow/core/graph/edgeset.h" +//#include "tensorflow/core/lib/core/errors.h" using namespace std; namespace ng = ngraph; @@ -37,24 +35,18 @@ namespace tensorflow { namespace ngraph_bridge { -//TODO: move this to .h -using VectNg = std::vector>; - Status Builder1::TranslateGraph( const std::vector& inputs, const std::vector& static_input_map, - shared_ptr& ng_function){ - + shared_ptr& ng_function) { TF_RETURN_IF_ERROR(Initialize()); vector> ng_parameter_list; TF_RETURN_IF_ERROR(GetInputParams(inputs, tf_params, &ng_parameter_list)); - // TODO: pass static_input_map to translate_each_op... or pass the vector - // ? TF_RETURN_IF_ERROR(TranslateEachOp(tf_ops, static_input_map)); - vector> ng_result_list; + VectNg ng_result_list; TF_RETURN_IF_ERROR(GetOutputNodes(tf_ret_vals, ng_result_list)); // Create the nGraph function. @@ -85,7 +77,7 @@ Status Builder1::TranslateGraph( } inputs[i] = input_tensor.shape(); } - + return TranslateGraph(inputs, static_input_map, ng_function); } @@ -97,35 +89,27 @@ Status Builder1::TranslateEachOp( NGRAPH_VLOG(2) << "Constructing op " << op->name() << " which is " << op->type_string(); - Builder1::TranslatorFn translate_fn; - vector input_indexes; - TF_RETURN_IF_ERROR(GetOpTranslationRequirements(op, translate_fn, input_indexes)); - // input_idxs can be size 0 (to indicate/handle variadic inputs nodes - // like Addn) - VectNg subgraph_out_nodes(op->num_outputs()); - - - bool variadic_input = input_indexes.size() == 0; - int num_inputs = variadic_input ? op->num_inputs() : input_indexes.size(); - VectNg ng_arg_vec(num_inputs); - if (op->type_string() != "Const"){ - for (int idx = 0; idx < num_inputs; idx++) { - TF_RETURN_IF_ERROR(GetInputNode( - op, (variadic_input ? idx : input_indexes[idx]), &ng_arg_vec[idx])); - } - } - // TODO: instead of pass static_input_map, use GetStaticInputVector and - // pass the vector - // Then we'd have to pass a vector of vectors, in case a node has >1 - // static inputs - TF_RETURN_IF_ERROR(translate_fn(op, ng_arg_vec, static_input_map, - subgraph_out_nodes)); - - - // for (auto ng_node : subgraph_out_nodes) - // SaveNgOp(ng_op_map, op->name(), ng_node); - ng_op_map[op->name()] = subgraph_out_nodes; // SaveNgOp - // TranslateBinaryOp(op, static_input_map, ng_op_map, ng_floormod) + Builder1::TranslatorFn translate_fn; + vector input_indexes; + TF_RETURN_IF_ERROR( + GetOpTranslationRequirements(op, translate_fn, input_indexes)); + // input_indexes can be size 0 (to indicate/handle variadic inputs nodes + // like Addn) + VectNg subgraph_out_nodes(op->num_outputs()); + + bool variadic_input = input_indexes.size() == 0; + int num_inputs = variadic_input ? op->num_inputs() : input_indexes.size(); + VectNg ng_arg_vec(num_inputs); + if (op->type_string() != "Const") { + for (int idx = 0; idx < num_inputs; idx++) { + TF_RETURN_IF_ERROR(GetInputNode( + op, (variadic_input ? idx : input_indexes[idx]), &ng_arg_vec[idx])); + } + } + TF_RETURN_IF_ERROR( + translate_fn(op, ng_arg_vec, static_input_map, subgraph_out_nodes)); + + ng_op_map[op->name()] = subgraph_out_nodes; } return Status::OK(); } @@ -180,14 +164,14 @@ Status Builder1::GetInputParams( TF_RETURN_IF_ERROR(TFTensorShapeToNGraphShape(inputs[index], &ng_shape)); auto ng_param = make_shared(ng_et, ng_shape); - SaveNgOp(parm->name(), ng_param); + ng_op_map[parm->name()].push_back(ng_param); (*ng_parameter_list)[index] = ng_param; } return Status::OK(); } Status Builder1::GetOutputNodes(const vector& tf_ret_vals, - vector>& ng_result_list) { + VectNg& ng_result_list) { // Populate the result list. ng_result_list.resize(tf_ret_vals.size()); @@ -212,98 +196,6 @@ Status Builder1::GetOutputNodes(const vector& tf_ret_vals, return Status::OK(); } -// TODO: combine the 2 MakePaddings together? with default args? -// would want the default args to be at the end of the paramlist, -// but 'outputs' (like ng_padding_below, ng_padding_above) are usually at the -// end of the param list -// TODO: why does MakePadding handle only 2 dimensions... generalize it? -// TODO: add unit tests for 1D conv. 1d pooling etc. check if MakePadding works -// in that case -template -void MakePadding(const std::string& tf_padding_type, - const ngraph::Shape& ng_image_shape, - const ngraph::Shape& ng_kernel_shape, - const ngraph::Strides& ng_strides, - const ngraph::Shape& ng_dilations, T& ng_padding_below, - T& ng_padding_above) { - ngraph::Shape ng_dilation_kernel_shape{ - (ng_kernel_shape[0] - 1) * ng_dilations[0] + 1, - (ng_kernel_shape[1] - 1) * ng_dilations[1] + 1}; - - MakePadding(tf_padding_type, ng_image_shape, ng_dilation_kernel_shape, - ng_strides, ng_padding_below, ng_padding_above); -} - -template -void MakePadding(const std::string& tf_padding_type, - const ngraph::Shape& ng_image_shape, - const ngraph::Shape& ng_kernel_shape, - const ngraph::Strides& ng_strides, T& ng_padding_below, - T& ng_padding_above) { - if (tf_padding_type == "SAME") { - for (size_t i = 0; i < 2; i++) { - /* - size_t image_size = ng_image_shape[i]; - size_t filter_shape = ng_kernel_shape[i]; - size_t filter_stride = ng_strides[i]; - - int64 padding_needed; - if (image_size % filter_stride == 0) { - padding_needed = filter_shape - filter_stride; - } else { - padding_needed = filter_shape - (image_size % filter_stride); - } - if (padding_needed < 0) { - padding_needed = 0; - } - */ - - // TODO: check this:. This formula is documented well documented here. - // So I prefer this, compared to the one above (though both are exactly - // the same) - // https://www.tensorflow.org/api_guides/python/nn#Notes_on_SAME_Convolution_Padding - int64 out_shape = ceil(ng_image_shape[i] / ng_strides[i]); - int64 padding_needed = ng_strides[i] * (out_shape - 1) + - ng_kernel_shape[i] - ng_image_shape[i]; - padding_needed = padding_needed < 0 ? 0 : padding_needed; - - size_t padding_lhs = padding_needed / 2; - size_t padding_rhs = padding_needed - padding_lhs; - ng_padding_below[i] = padding_lhs; - ng_padding_above[i] = padding_rhs; - } - } - - NGRAPH_VLOG(3) << "ng_padding_below: " << ngraph::join(ng_padding_below); - NGRAPH_VLOG(3) << "ng_padding_above: " << ngraph::join(ng_padding_above); -} - -Status ValidateInputCount(const Node* op, size_t count) { - if (op->num_inputs() != count) { - return errors::InvalidArgument("\"", op->name(), "\" requires ", count, - " input(s), got ", op->num_inputs(), - " instead"); - } - return Status::OK(); -} - -Status ValidateInputCountMin(const Node* op, size_t count) { - if (op->num_inputs() < count) { - return errors::InvalidArgument("\"", op->name(), "\" requires at least ", - count, " input(s), got ", op->num_inputs(), - " instead"); - } - return Status::OK(); -} - -//TODO: remove if not needed -void Builder1::SaveNgOp(const std::string& op_name, - const shared_ptr& output_node) { - // no need to try-catch, map[key] will create vector object - // if not exists - ng_op_map[op_name].push_back(output_node); -} - Status Builder1::Initialize() { if (!is_initialized) { // @@ -335,7 +227,7 @@ Status Builder1::Initialize() { // https://github.com/petewarden/tensorflow_makefile/blob/master/tensorflow/core/framework/op_kernel.h#L1265 // TODO check : removing OP_REQUIRES_OK for now - //// OP_REQUIRES_OK(ctx, GetNodeAttr(node->attrs(), "index", &index)); + // OP_REQUIRES_OK(ctx, GetNodeAttr(node->attrs(), "index", &index)); TF_RETURN_IF_ERROR(GetNodeAttr(node->attrs(), "index", &index)); if (index > max_arg_index) max_arg_index = index; } @@ -361,19 +253,13 @@ Status Builder1::Initialize() { if (InputIsStatic(edge->dst(), edge->dst_input())) { NGRAPH_VLOG(5) << "Marking edge static: " << edge->DebugString(); - // is_static = true; m_input_is_static[index] = true; break; } } - - // NGRAPH_VLOG(5) << "Marking arg " << index << " is_static: " << - // is_static; - // m_input_is_static[index] = is_static; NGRAPH_VLOG(5) << "Marking arg " << index << " is static: " << m_input_is_static[index]; } - is_initialized = true; } return Status::OK(); @@ -401,66 +287,29 @@ Status Builder1::GetInputNode(const Node* op, size_t input_idx, return Status::OK(); } -// TODO: move translate ops to a different file -Status TranslateFloorDivOp(const Node* op, - VectNg& ng_arg_vec, - const std::vector& static_input_map, - VectNg& subgraph_out_nodes) { - subgraph_out_nodes[0] = - std::make_shared(ng_arg_vec[0] / ng_arg_vec[1]); - return Status::OK(); -} - -Status TranslateFloorModOp(const Node* op, - VectNg& ng_arg_vec, - const std::vector& static_input_map, - VectNg& subgraph_out_nodes) { - auto floordiv = - std::make_shared(ng_arg_vec[0] / ng_arg_vec[1]); - subgraph_out_nodes[0] = ng_arg_vec[0] - (floordiv * ng_arg_vec[1]); - - // Possible reuse of floordiv: - // TranslateFloorDivOp1(op, ng_arg_vec, static_input_map, subgraph_out_nodes); - // subgraph_out_nodes[0] = ng_arg_vec[0] - (subgraph_out_nodes[0] * - // ng_arg_vec[1]); - return Status::OK(); -} - -Status TranslateAddNOp(const Node* op, - VectNg &ng_arg_vec, - const std::vector& static_input_map, - VectNg& subgraph_out_nodes) { - subgraph_out_nodes[0] = - std::accumulate(std::next(ng_arg_vec.begin()), ng_arg_vec.end(), - ng_arg_vec.at(0)); // accumulation: start with first - // element. default op is addition - return Status::OK(); -} - // Helper function to translate a binary op in cases where there is a one-to-one // mapping from TensorFlow ops to nGraph ops. // // Example usage: // // if (n->type_string == "Add") { -// TF_RETURN_IF_ERROR(TranslateBinaryOp(op, static_input_map, -// ng_op_map)); +// TF_RETURN_IF_ERROR(TranslateBinary(op, ng_arg_vec, +// static_input_map, +// subgraph_out_nodes)); // } // - -// Note: Earlier TranslateBinary had 2 forms: templated (to conver a ng:Node) -// into a function -// and non-templated, which converted an arbitrary function that accepts 2 nodes -// and returns 1 node. -// In current implementation, we do not need the non-templated version, because -// TranslateBinary will be -// part of the class, and the TranslateOps will not have access to it. template -Status TranslateBinary(const Node* op, - VectNg& ng_arg_vec, +Status TranslateBinary(const Node* op, VectNg& ng_arg_vec, const std::vector& static_input_map, VectNg& subgraph_out_nodes) { - // TODO: assert subgraph_out_nodes.size()==1, ng_arg_vec.size()==2 + if (subgraph_out_nodes.size() != 1) + return errors::InvalidArgument( + "TranslateBinary call for ", op->type_string(), " expects ", + subgraph_out_nodes.size(), " outputs. Should have been 1"); + if (ng_arg_vec.size() != 2) + return errors::InvalidArgument("TranslateBinary call for ", + op->type_string(), " expects 2 inputs. Got ", + ng_arg_vec.size(), "."); auto node_pair = ng::builder::numpy_broadcast( std::make_pair(ng_arg_vec[0], ng_arg_vec[1])); subgraph_out_nodes[0] = make_shared(node_pair.first, node_pair.second); @@ -468,140 +317,22 @@ Status TranslateBinary(const Node* op, } template -Status TranslateUnary(const Node* op, - const std::vector>& ng_arg_vec, - const std::vector& static_input_map, - vector>& subgraph_out_nodes) { - // TODO: assert subgraph_out_nodes.size()==1, ng_arg_vec.size()==1 - subgraph_out_nodes[0] = make_shared(ng_arg_vec[0]); - return Status::OK(); -} - -template -Status MakeConstOp(const Node* op, ng::element::Type et, - std::shared_ptr* ng_node) { - vector const_values; - TensorShapeProto shape_proto; - - TF_RETURN_IF_ERROR( - ValuesFromConstNode(op->def(), &shape_proto, &const_values)); - - TensorShape const_shape(shape_proto); - ng::Shape ng_shape; - TF_RETURN_IF_ERROR(TFTensorShapeToNGraphShape(const_shape, &ng_shape)); - - *ng_node = make_shared(et, ng_shape, const_values); - - return Status::OK(); -} - -static Status TranslateConstOp(const Node* op, - VectNg& ng_arg_vec, +Status TranslateUnary(const Node* op, const VectNg& ng_arg_vec, const std::vector& static_input_map, VectNg& subgraph_out_nodes) { - - const static std::map< - const DataType, - const std::pair*)>, - const ngraph::element::Type>> - TF_NGRAPH_CONST_MAP = { - {DataType::DT_FLOAT, make_pair(MakeConstOp, ng::element::f32)}, - {DataType::DT_DOUBLE, make_pair(MakeConstOp, ng::element::f64)}, - {DataType::DT_INT8, make_pair(MakeConstOp, ng::element::i8)}, - {DataType::DT_INT16, make_pair(MakeConstOp, ng::element::i16)}, - {DataType::DT_INT32, make_pair(MakeConstOp, ng::element::i32)}, - {DataType::DT_INT64, make_pair(MakeConstOp, ng::element::i64)}, - {DataType::DT_UINT8, make_pair(MakeConstOp, ng::element::u8)}, - {DataType::DT_UINT16, make_pair(MakeConstOp, ng::element::u16)}, - {DataType::DT_BOOL, - make_pair(MakeConstOp, ng::element::boolean)}}; - - DataType dtype; - TF_RETURN_IF_ERROR(GetNodeAttr(op->attrs(), "dtype", &dtype)); - // For some reason the following do not work (no specialization of - // tensorflow::checkpoint::SavedTypeTraits...) - // case DataType::DT_UINT32: - // TF_RETURN_IF_ERROR(MakeConstOp(op, ng::element::u32, - // &ng_node)); - // break; - // case DataType::DT_UINT64: - // TF_RETURN_IF_ERROR(MakeConstOp(op, ng::element::u64, - // &ng_node)); - // break; - try { - const auto& func_param = TF_NGRAPH_CONST_MAP.at(dtype); - TF_RETURN_IF_ERROR(func_param.first(op, func_param.second, &subgraph_out_nodes[0])); - } catch (const std::out_of_range&) { - return errors::Unimplemented("Unsupported TensorFlow data type: ", - DataType_Name(dtype)); - } - return Status::OK(); -} - -//TODO: document why ng_arg_vec is not const. (BatchToNGraph changes it) -Status TranslateAvgPoolOp(const Node* op, - VectNg& ng_arg_vec, - const std::vector& static_input_map, - VectNg& subgraph_out_nodes){ - std::vector tf_strides; - std::vector tf_ksize; - std::string tf_padding_type; - std::string tf_data_format; - TF_RETURN_IF_ERROR(GetNodeAttr(op->attrs(), "strides", &tf_strides)); - TF_RETURN_IF_ERROR(GetNodeAttr(op->attrs(), "ksize", &tf_ksize)); - TF_RETURN_IF_ERROR(GetNodeAttr(op->attrs(), "padding", &tf_padding_type)); - TF_RETURN_IF_ERROR(GetNodeAttr(op->attrs(), "data_format", &tf_data_format)); - - if (tf_data_format != "NHWC" && tf_data_format != "NCHW") { + if (subgraph_out_nodes.size() != 1) return errors::InvalidArgument( - "AvgPool data format is neither NHWC nor NCHW"); - } + "TranslateUnary call for ", op->type_string(), " expects ", + subgraph_out_nodes.size(), " outputs. Should have been 1"); + if (ng_arg_vec.size() != 1) + return errors::InvalidArgument("TranslateUnary call for ", + op->type_string(), " expects 1 inputs. Got ", + ng_arg_vec.size(), "."); - bool is_nhwc = (tf_data_format == "NHWC"); - - NGRAPH_VLOG(3) << ng::join(tf_strides); - NGRAPH_VLOG(3) << ng::join(tf_ksize); - NGRAPH_VLOG(3) << tf_padding_type; - NGRAPH_VLOG(3) << tf_data_format; - - ng::Strides ng_strides(2); - ng::Shape ng_image_shape(2); - ng::Shape ng_kernel_shape(2); - - //TODO: should these be in Builder instead of in ngraph_conversions.h? - BatchedOpParamToNGraph(is_nhwc, tf_strides, ng_strides); - BatchedOpParamToNGraph(is_nhwc, ng_arg_vec[0]->get_shape(), ng_image_shape); - BatchedOpParamToNGraph(is_nhwc, tf_ksize, ng_kernel_shape); - BatchToNGraph(is_nhwc, ng_arg_vec[0]); - NGRAPH_VLOG(3) << "ng_strides: " << ng::join(ng_strides); - NGRAPH_VLOG(3) << "ng_image_shape: " << ng::join(ng_image_shape); - NGRAPH_VLOG(3) << "ng_kernel_shape: " << ng::join(ng_kernel_shape); - - // TODO: change this once nGraph supports negative padding - // (CoordinateDiff) for AvgPool - // ng::CoordinateDiff ng_padding_below{0,0}; - // ng::CoordinateDiff ng_padding_above{0,0}; - ng::Shape ng_padding_below{0, 0}; - ng::Shape ng_padding_above{0, 0}; - - MakePadding(tf_padding_type, ng_image_shape, ng_kernel_shape, - ng_strides, ng_padding_below, ng_padding_above); - - std::shared_ptr ng_avgpool = - make_shared(ng_arg_vec[0], ng_kernel_shape, ng_strides, - ng_padding_below, ng_padding_above, false); - - BatchToTensorflow(is_nhwc, ng_avgpool); - NGRAPH_VLOG(3) << "avgpool outshape: {" << ng::join(ng_avgpool->get_shape()) - << "}"; - - subgraph_out_nodes[0] = ng_avgpool; - //SaveNgOp(ng_op_map, op->name(), ng_avgpool); + subgraph_out_nodes[0] = make_shared(ng_arg_vec[0]); return Status::OK(); } - const std::map Builder1::TRANSLATE_OP_MAP{ {"Abs", TranslateUnary}, {"Add", TranslateBinary}, @@ -611,29 +342,20 @@ const std::map Builder1::TRANSLATE_OP_MAP{ {"FloorDiv", TranslateFloorDivOp}, {"FloorMod", TranslateFloorModOp}, {"Neg", TranslateUnary}, - {"NoOp", [](const Node* op, VectNg& ng_arg_vec, const std::vector& static_input_map, - VectNg& subgraph_out_nodes) { return Status::OK();}}, - {"RealDiv", TranslateBinary} - }; + {"NoOp", [](const Node* op, VectNg& ng_arg_vec, + const std::vector& static_input_map, + VectNg& subgraph_out_nodes) { return Status::OK(); }}, + {"RealDiv", TranslateBinary}}; const std::map> Builder1::INPUT_INDEX_MAP{}; -//Just pass it the op. we can read its name inside. -//Also if #inputs, #outputs are not specified, we can construct them here -Status Builder1::GetOpTranslationRequirements(const Node* op, Builder1::TranslatorFn& translate_fn, vector& input_indexes){ - //auto fn = TRANSLATE_OP_MAP[op_type]; - - //TODO: this function wraps TRANSLATE_OP_MAP. - //It returns a translate function and input indexes - //The translate function MUST be present in TRANSLATE_OP_MAP +Status Builder1::GetOpTranslationRequirements( + const Node* op, Builder1::TranslatorFn& translate_fn, + vector& input_indexes) { + // This function wraps TRANSLATE_OP_MAP. + // It returns a translate function and input indexes + // The translate function MUST be present in TRANSLATE_OP_MAP // input_idx may not be present, since it can be inferred from op - - //about num_outputs: - //Note: op itself may specify the number of outputs... so maybe we dont need to specify that. - //Is there a case we ask for less outputs than what TF provides? - - //For input idxs, by default we should return {0,1, ..., (op->num_inputs)-1}...unless otherwise specified. - auto iter_fn = TRANSLATE_OP_MAP.find(op->type_string()); if (iter_fn != TRANSLATE_OP_MAP.end()) { translate_fn = iter_fn->second; @@ -643,34 +365,25 @@ Status Builder1::GetOpTranslationRequirements(const Node* op, Builder1::Translat // Catch-all for unsupported ops // ----------------------------- NGRAPH_VLOG(3) << "Unsupported Op: " << op->name() << " (" - << op->type_string() << ")"; + << op->type_string() << ")"; NGRAPH_VLOG(3) << op->def().DebugString(); return errors::InvalidArgument("Unsupported Op: ", op->name(), " (", - op->type_string(), ")"); + op->type_string(), ")"); } auto iter_input_indexes = INPUT_INDEX_MAP.find(op->type_string()); - if (iter_input_indexes != INPUT_INDEX_MAP.end()){ + if (iter_input_indexes != INPUT_INDEX_MAP.end()) { input_indexes = iter_input_indexes->second; - } else{ + } else { + // By default we should return {0,1, ..., (op->num_inputs)-1}...unless + // otherwise specified. input_indexes.resize(op->num_inputs()); - std::iota(std::begin(input_indexes), std::end(input_indexes), 0); //Populate with increasing integers 1,2... + std::iota(std::begin(input_indexes), std::end(input_indexes), + 0); // Populate with increasing integers 1,2... } - - //TODO: do we even need this? Activate this if there is an op that returns < num_outputs outputs - /* - auto iter_num_outputs = NUM_OUTPUTS_MAP.find(op->type_string()); - if (iter_num_outputs != NUM_OUTPUTS_MAP.end()){ - num_outputs = iter_num_outputs->second; - } else{ - num_outputs = op->num_outputs(); - } - */ - return Status::OK(); } - } // namespace ngraph_bridge } // namespace tensorflow diff --git a/src/ngraph_builder1.h b/src/ngraph_builder1.h index 2d2d5cd1..9c20fc8c 100644 --- a/src/ngraph_builder1.h +++ b/src/ngraph_builder1.h @@ -26,11 +26,12 @@ #include "ngraph_log.h" #include "ngraph_mark_for_clustering.h" -#include "tensorflow/core/framework/op.h" +//TODO: remove headers if not needed +//#include "tensorflow/core/framework/op.h" #include "tensorflow/core/framework/op_kernel.h" #include "tensorflow/core/framework/tensor_shape.h" #include "tensorflow/core/graph/algorithm.h" -#include "tensorflow/core/graph/graph.h" +//#include "tensorflow/core/graph/graph.h" using namespace std; namespace ng = ngraph; @@ -38,7 +39,6 @@ namespace tensorflow { namespace ngraph_bridge { -// TODO: overload certain functions (makepadding) vs default args? // TODO: make sure all comments from old builder are copied correctly. // TODO: use camelcase, snakecase appropriately // TODO add TF_RETURN_IF_ERROR where necessary @@ -46,49 +46,35 @@ namespace ngraph_bridge { ///////////////// class Builder1 { - //TODO: Do we need to typedef OpMap, as it is used only once - using OpMap = std::unordered_map>>; + using VectNg = std::vector>; using TranslatorFn = function>&, - const std::vector&, vector>&)>; - //using DispatchTable = - // const std::map>>; + const Node*, VectNg&, const std::vector&, VectNg&)>; private: - // // The op map holds a mapping from TensorFlow op names (strings) to // vector of generated nGraph nodes. // - Builder1::OpMap ng_op_map; - - - + std::unordered_map ng_op_map; const static std::map TRANSLATE_OP_MAP; const static std::map> INPUT_INDEX_MAP; - OpKernelConstruction* ctx; // TODO: is this required? required in - // OP_REQUIRES_OK is used instead of - // TF_RETURN_IF_ERROR in Initialize + OpKernelConstruction* ctx; // TODO: is this required? required in + // OP_REQUIRES_OK is used instead of + // TF_RETURN_IF_ERROR in Initialize bool is_initialized = false; // Prevent init from running twice const Graph& tf_graph; std::vector m_input_is_static; vector ordered; vector tf_params, tf_ret_vals, tf_ops; - Status GetOpTranslationRequirements(const Node*, Builder1::TranslatorFn&, vector&); + Status GetOpTranslationRequirements(const Node*, Builder1::TranslatorFn&, + vector&); Status GetInputNode(const Node*, size_t, shared_ptr*); - // helper function for populating ng_op_map - // TODO: no one to use it (except 1 place). delete? - void SaveNgOp(const std::string& op_name, - const shared_ptr& output_node); - // TODO: write description Status GetInputParams(const std::vector&, vector, vector>*); @@ -96,8 +82,7 @@ class Builder1 { vector&, vector&); Status TranslateEachOp(const vector&, const std::vector&); - Status GetOutputNodes(const vector&, - vector>&); + Status GetOutputNodes(const vector&, VectNg&); template void MakePadding(const std::string& tf_padding_type, @@ -117,7 +102,6 @@ class Builder1 { Status Initialize(); public: - Builder1(const Graph& tf_graph, OpKernelConstruction* ctx) // TODO make ctx const? : tf_graph(tf_graph), @@ -125,10 +109,9 @@ class Builder1 { Builder1(const Graph& tf_graph) : tf_graph(tf_graph) {} - Status TranslateGraph( - const std::vector&, - const std::vector&, - shared_ptr&); + Status TranslateGraph(const std::vector&, + const std::vector&, + shared_ptr&); Status TranslateGraph(OpKernelContext* ctx, std::shared_ptr& ng_function); diff --git a/src/ngraph_conversions.cc b/src/ngraph_conversions.cc index 58a6af3f..80bc8a09 100644 --- a/src/ngraph_conversions.cc +++ b/src/ngraph_conversions.cc @@ -14,7 +14,6 @@ * limitations under the License. *******************************************************************************/ - #include "ngraph_conversions.h" #include "ngraph_builder.h" #include "ngraph_log.h" diff --git a/src/ngraph_conversions.h b/src/ngraph_conversions.h index 1b87b79b..d3be027f 100644 --- a/src/ngraph_conversions.h +++ b/src/ngraph_conversions.h @@ -26,7 +26,7 @@ namespace tensorflow { namespace ngraph_bridge { template -void Reshape(std::shared_ptr& ng_node){ +void Reshape(std::shared_ptr& ng_node) { static_assert(a < 4 && b < 4 && c < 4 && d < 4, "Number of dimensions cannot exceed 4"); static_assert(a != b && a != c && a != d && b != c && b != d && c != d, diff --git a/src/ngraph_encapsulate_op.cc b/src/ngraph_encapsulate_op.cc index 88feeefc..e68e1356 100644 --- a/src/ngraph_encapsulate_op.cc +++ b/src/ngraph_encapsulate_op.cc @@ -73,7 +73,9 @@ class NGraphEncapsulateOp : public OpKernel { : OpKernel(ctx), m_graph(OpRegistry::Global()), #ifdef BUILDER1 - ng_builder(m_graph, ctx), //TODO: does ng_builder get the correct graph before ConvertGraphDefToGraph is called? + ng_builder(m_graph, ctx), // TODO: does ng_builder get the correct + // graph before ConvertGraphDefToGraph is + // called? #endif m_freshness_tracker(nullptr) { GraphDef* graph_def; diff --git a/src/ngraph_translateops.h b/src/ngraph_translateops.h new file mode 100644 index 00000000..587219c2 --- /dev/null +++ b/src/ngraph_translateops.h @@ -0,0 +1,271 @@ +/******************************************************************************* + * Copyright 2017-2018 Intel Corporation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + *******************************************************************************/ + +#include "ngraph_conversions.h" +#include "ngraph_utils.h" + +using namespace std; +namespace ng = ngraph; + +namespace tensorflow { + +namespace ngraph_bridge { + +using VectNg = std::vector>; + +// TODO (sarkars): combine the 2 MakePaddings together? with default args? +// would want the default args to be at the end of the paramlist, +// but 'outputs' (like ng_padding_below, ng_padding_above) are usually at the +// end of the param list +// TODO (sarkars): why does MakePadding handle only 2 dimensions... generalize +// it? +// TODO (sarkars): add unit tests for 1D conv. 1d pooling etc. check if +// MakePadding works +// in that case +template +void MakePadding(const std::string& tf_padding_type, + const ngraph::Shape& ng_image_shape, + const ngraph::Shape& ng_kernel_shape, + const ngraph::Strides& ng_strides, + const ngraph::Shape& ng_dilations, T& ng_padding_below, + T& ng_padding_above) { + ngraph::Shape ng_dilation_kernel_shape{ + (ng_kernel_shape[0] - 1) * ng_dilations[0] + 1, + (ng_kernel_shape[1] - 1) * ng_dilations[1] + 1}; + + MakePadding(tf_padding_type, ng_image_shape, ng_dilation_kernel_shape, + ng_strides, ng_padding_below, ng_padding_above); +} + +template +void MakePadding(const std::string& tf_padding_type, + const ngraph::Shape& ng_image_shape, + const ngraph::Shape& ng_kernel_shape, + const ngraph::Strides& ng_strides, T& ng_padding_below, + T& ng_padding_above) { + if (tf_padding_type == "SAME") { + for (size_t i = 0; i < 2; i++) { + /* + size_t image_size = ng_image_shape[i]; + size_t filter_shape = ng_kernel_shape[i]; + size_t filter_stride = ng_strides[i]; + + int64 padding_needed; + if (image_size % filter_stride == 0) { + padding_needed = filter_shape - filter_stride; + } else { + padding_needed = filter_shape - (image_size % filter_stride); + } + if (padding_needed < 0) { + padding_needed = 0; + } + */ + + // TODO: check this:. This formula is documented well documented here. + // So I prefer this, compared to the one above (though both are exactly + // the same) + // https://www.tensorflow.org/api_guides/python/nn#Notes_on_SAME_Convolution_Padding + int64 out_shape = ceil(ng_image_shape[i] / ng_strides[i]); + int64 padding_needed = ng_strides[i] * (out_shape - 1) + + ng_kernel_shape[i] - ng_image_shape[i]; + padding_needed = padding_needed < 0 ? 0 : padding_needed; + + size_t padding_lhs = padding_needed / 2; + size_t padding_rhs = padding_needed - padding_lhs; + ng_padding_below[i] = padding_lhs; + ng_padding_above[i] = padding_rhs; + } + } + + NGRAPH_VLOG(3) << "ng_padding_below: " << ngraph::join(ng_padding_below); + NGRAPH_VLOG(3) << "ng_padding_above: " << ngraph::join(ng_padding_above); +} + +Status ValidateInputCount(const Node* op, size_t count) { + if (op->num_inputs() != count) { + return errors::InvalidArgument("\"", op->name(), "\" requires ", count, + " input(s), got ", op->num_inputs(), + " instead"); + } + return Status::OK(); +} + +Status ValidateInputCountMin(const Node* op, size_t count) { + if (op->num_inputs() < count) { + return errors::InvalidArgument("\"", op->name(), "\" requires at least ", + count, " input(s), got ", op->num_inputs(), + " instead"); + } + return Status::OK(); +} + +Status TranslateAddNOp(const Node* op, VectNg& ng_arg_vec, + const std::vector& static_input_map, + VectNg& subgraph_out_nodes) { + subgraph_out_nodes[0] = + std::accumulate(std::next(ng_arg_vec.begin()), ng_arg_vec.end(), + ng_arg_vec.at(0)); // accumulation: start with first + // element. default op is addition + return Status::OK(); +} + +// TODO: document why ng_arg_vec is not const. (BatchToNGraph changes it) +Status TranslateAvgPoolOp(const Node* op, VectNg& ng_arg_vec, + const std::vector& static_input_map, + VectNg& subgraph_out_nodes) { + std::vector tf_strides; + std::vector tf_ksize; + std::string tf_padding_type; + std::string tf_data_format; + TF_RETURN_IF_ERROR(GetNodeAttr(op->attrs(), "strides", &tf_strides)); + TF_RETURN_IF_ERROR(GetNodeAttr(op->attrs(), "ksize", &tf_ksize)); + TF_RETURN_IF_ERROR(GetNodeAttr(op->attrs(), "padding", &tf_padding_type)); + TF_RETURN_IF_ERROR(GetNodeAttr(op->attrs(), "data_format", &tf_data_format)); + + if (tf_data_format != "NHWC" && tf_data_format != "NCHW") { + return errors::InvalidArgument( + "AvgPool data format is neither NHWC nor NCHW"); + } + + bool is_nhwc = (tf_data_format == "NHWC"); + + NGRAPH_VLOG(3) << ng::join(tf_strides); + NGRAPH_VLOG(3) << ng::join(tf_ksize); + NGRAPH_VLOG(3) << tf_padding_type; + NGRAPH_VLOG(3) << tf_data_format; + + ng::Strides ng_strides(2); + ng::Shape ng_image_shape(2); + ng::Shape ng_kernel_shape(2); + + // TODO: should these be in Builder instead of in ngraph_conversions.h? + BatchedOpParamToNGraph(is_nhwc, tf_strides, ng_strides); + BatchedOpParamToNGraph(is_nhwc, ng_arg_vec[0]->get_shape(), ng_image_shape); + BatchedOpParamToNGraph(is_nhwc, tf_ksize, ng_kernel_shape); + BatchToNGraph(is_nhwc, ng_arg_vec[0]); + NGRAPH_VLOG(3) << "ng_strides: " << ng::join(ng_strides); + NGRAPH_VLOG(3) << "ng_image_shape: " << ng::join(ng_image_shape); + NGRAPH_VLOG(3) << "ng_kernel_shape: " << ng::join(ng_kernel_shape); + + // TODO: change this once nGraph supports negative padding + // (CoordinateDiff) for AvgPool + // ng::CoordinateDiff ng_padding_below{0,0}; + // ng::CoordinateDiff ng_padding_above{0,0}; + ng::Shape ng_padding_below{0, 0}; + ng::Shape ng_padding_above{0, 0}; + + MakePadding(tf_padding_type, ng_image_shape, ng_kernel_shape, ng_strides, + ng_padding_below, ng_padding_above); + + std::shared_ptr ng_avgpool = + make_shared(ng_arg_vec[0], ng_kernel_shape, ng_strides, + ng_padding_below, ng_padding_above, false); + + BatchToTensorflow(is_nhwc, ng_avgpool); + NGRAPH_VLOG(3) << "avgpool outshape: {" << ng::join(ng_avgpool->get_shape()) + << "}"; + + subgraph_out_nodes[0] = ng_avgpool; + return Status::OK(); +} + +template +Status MakeConstOp(const Node* op, ng::element::Type et, + std::shared_ptr* ng_node) { + vector const_values; + TensorShapeProto shape_proto; + + TF_RETURN_IF_ERROR( + ValuesFromConstNode(op->def(), &shape_proto, &const_values)); + + TensorShape const_shape(shape_proto); + ng::Shape ng_shape; + TF_RETURN_IF_ERROR(TFTensorShapeToNGraphShape(const_shape, &ng_shape)); + + *ng_node = make_shared(et, ng_shape, const_values); + + return Status::OK(); +} + +static Status TranslateConstOp( + const Node* op, VectNg& ng_arg_vec, + const std::vector& static_input_map, + VectNg& subgraph_out_nodes) { + const static std::map< + const DataType, + const std::pair*)>, + const ngraph::element::Type>> + TF_NGRAPH_CONST_MAP = { + {DataType::DT_FLOAT, make_pair(MakeConstOp, ng::element::f32)}, + {DataType::DT_DOUBLE, + make_pair(MakeConstOp, ng::element::f64)}, + {DataType::DT_INT8, make_pair(MakeConstOp, ng::element::i8)}, + {DataType::DT_INT16, make_pair(MakeConstOp, ng::element::i16)}, + {DataType::DT_INT32, make_pair(MakeConstOp, ng::element::i32)}, + {DataType::DT_INT64, make_pair(MakeConstOp, ng::element::i64)}, + {DataType::DT_UINT8, make_pair(MakeConstOp, ng::element::u8)}, + {DataType::DT_UINT16, + make_pair(MakeConstOp, ng::element::u16)}, + {DataType::DT_BOOL, + make_pair(MakeConstOp, ng::element::boolean)}}; + + DataType dtype; + TF_RETURN_IF_ERROR(GetNodeAttr(op->attrs(), "dtype", &dtype)); + // For some reason the following do not work (no specialization of + // tensorflow::checkpoint::SavedTypeTraits...) + // case DataType::DT_UINT32: + // TF_RETURN_IF_ERROR(MakeConstOp(op, ng::element::u32, + // &ng_node)); + // break; + // case DataType::DT_UINT64: + // TF_RETURN_IF_ERROR(MakeConstOp(op, ng::element::u64, + // &ng_node)); + // break; + try { + const auto& func_param = TF_NGRAPH_CONST_MAP.at(dtype); + TF_RETURN_IF_ERROR( + func_param.first(op, func_param.second, &subgraph_out_nodes[0])); + } catch (const std::out_of_range&) { + return errors::Unimplemented("Unsupported TensorFlow data type: ", + DataType_Name(dtype)); + } + return Status::OK(); +} + +Status TranslateFloorDivOp(const Node* op, VectNg& ng_arg_vec, + const std::vector& static_input_map, + VectNg& subgraph_out_nodes) { + subgraph_out_nodes[0] = + std::make_shared(ng_arg_vec[0] / ng_arg_vec[1]); + return Status::OK(); +} + +Status TranslateFloorModOp(const Node* op, VectNg& ng_arg_vec, + const std::vector& static_input_map, + VectNg& subgraph_out_nodes) { + auto floordiv = + std::make_shared(ng_arg_vec[0] / ng_arg_vec[1]); + subgraph_out_nodes[0] = ng_arg_vec[0] - (floordiv * ng_arg_vec[1]); + + // Possible reuse of floordiv: + // TranslateFloorDivOp1(op, ng_arg_vec, static_input_map, subgraph_out_nodes); + // subgraph_out_nodes[0] = ng_arg_vec[0] - (subgraph_out_nodes[0] * + // ng_arg_vec[1]); + return Status::OK(); +} +} +} \ No newline at end of file diff --git a/test/opexecuter.cpp b/test/opexecuter.cpp index 6f2f9ea0..8b02827a 100644 --- a/test/opexecuter.cpp +++ b/test/opexecuter.cpp @@ -257,15 +257,14 @@ void OpExecuter::ExecuteOnNGraph() { // Create nGraph function NGRAPH_VLOG(5) << " Create ng function "; shared_ptr ng_function; - if (false){ - ASSERT_EQ(Status::OK(), - Builder::TranslateGraph(input_shapes, static_input_map_, &graph, - ng_function)); - } - else{ - auto builder = Builder1(graph); + if (false) { ASSERT_EQ(Status::OK(), - builder.TranslateGraph(input_shapes, static_input_map_, ng_function)); + Builder::TranslateGraph(input_shapes, static_input_map_, &graph, + ng_function)); + } else { + auto builder = Builder1(graph); + ASSERT_EQ(Status::OK(), builder.TranslateGraph( + input_shapes, static_input_map_, ng_function)); } // ng function should get same number of outputs diff --git a/test/test_math_ops.cpp b/test/test_math_ops.cpp index 77dbf8db..20630561 100644 --- a/test/test_math_ops.cpp +++ b/test/test_math_ops.cpp @@ -14,29 +14,29 @@ * limitations under the License. *******************************************************************************/ #include "gtest/gtest.h" -#include "opexecuter.h" -#include "test_utilities.h" #include "ngraph_utils.h" -#include "tf_graph_writer.h" +#include "opexecuter.h" +#include "tensorflow/cc/client/client_session.h" +#include "tensorflow/cc/ops/standard_ops.h" #include "tensorflow/core/common_runtime/dma_helper.h" #include "tensorflow/core/framework/graph.pb.h" #include "tensorflow/core/framework/op.h" +#include "tensorflow/core/framework/tensor.h" #include "tensorflow/core/framework/tensor_types.h" #include "tensorflow/core/graph/algorithm.h" #include "tensorflow/core/graph/graph.h" #include "tensorflow/core/graph/graph_constructor.h" #include "tensorflow/core/platform/env.h" -#include "tensorflow/cc/client/client_session.h" -#include "tensorflow/cc/ops/standard_ops.h" -#include "tensorflow/core/framework/tensor.h" #include "tensorflow/core/public/session.h" - using namespace std; +#include "test_utilities.h" +#include "tf_graph_writer.h" +using namespace std; namespace ng = ngraph; - namespace tensorflow { - namespace ngraph_bridge { - namespace testing { - #define ASSERT_OK(x) ASSERT_EQ((x), ::tensorflow::Status::OK()); - // Test(TestCaseName, TestName) +namespace tensorflow { +namespace ngraph_bridge { +namespace testing { +#define ASSERT_OK(x) ASSERT_EQ((x), ::tensorflow::Status::OK()); +// Test(TestCaseName, TestName) // Please ensure // Neither TestCaseName nor TestName should contain underscore // https://github.com/google/googletest/blob/master/googletest/docs/primer.md @@ -46,51 +46,51 @@ TEST(MathOps, Neg) { Scope root = Scope::NewRootScope(); int dim1 = 2; int dim2 = 2; - Tensor A(DT_FLOAT, TensorShape({dim1, dim2})); - AssignInputValues(A, -2.1f); - auto R = ops::Neg(root, A); - vector output_datatypes = {DT_FLOAT}; + Tensor A(DT_FLOAT, TensorShape({dim1, dim2})); + AssignInputValues(A, -2.1f); + auto R = ops::Neg(root, A); + vector output_datatypes = {DT_FLOAT}; std::vector sess_run_fetchoutputs = {R}; OpExecuter opexecuter(root, "Neg", output_datatypes, sess_run_fetchoutputs); - opexecuter.ExecuteOnNGraph(); + opexecuter.ExecuteOnNGraph(); opexecuter.ExecuteOnTF(); opexecuter.CompareNgraphAndTF(); } - TEST(MathOps, Add) { +TEST(MathOps, Add) { // Create a tf graph Scope root = Scope::NewRootScope(); int dim1 = 2; int dim2 = 2; - Tensor A(DT_FLOAT, TensorShape({dim1, dim2})); + Tensor A(DT_FLOAT, TensorShape({dim1, dim2})); Tensor B(DT_FLOAT, TensorShape({dim1, dim2})); - AssignInputValues(A, 2.1f); + AssignInputValues(A, 2.1f); AssignInputValues(B, 4.1f); - auto R = ops::Add(root, A, B); - vector output_datatypes = {DT_FLOAT}; + auto R = ops::Add(root, A, B); + vector output_datatypes = {DT_FLOAT}; std::vector sess_run_fetchoutputs = {R}; OpExecuter opexecuter(root, "Add", output_datatypes, sess_run_fetchoutputs); - opexecuter.ExecuteOnNGraph(); + opexecuter.ExecuteOnNGraph(); opexecuter.ExecuteOnTF(); opexecuter.CompareNgraphAndTF(); } - TEST(MathOps, RealDiv) { +TEST(MathOps, RealDiv) { Scope root = Scope::NewRootScope(); int dim1 = 100; int dim2 = 200; - Tensor A(DT_FLOAT, TensorShape({dim1, dim2})); + Tensor A(DT_FLOAT, TensorShape({dim1, dim2})); Tensor B(DT_FLOAT, TensorShape({dim1, dim2})); - AssignInputValues(A, 2.0f); + AssignInputValues(A, 2.0f); AssignInputValues(B, 7.0f); - auto R = ops::RealDiv(root, A, B); - vector output_datatypes = {DT_FLOAT}; - std::vector sess_run_fetchoutputs = {R}; + auto R = ops::RealDiv(root, A, B); + vector output_datatypes = {DT_FLOAT}; + std::vector sess_run_fetchoutputs = {R}; OpExecuter opexecuter(root, "RealDiv", output_datatypes, sess_run_fetchoutputs); - opexecuter.ExecuteOnNGraph(); + opexecuter.ExecuteOnNGraph(); opexecuter.ExecuteOnTF(); opexecuter.CompareNgraphAndTF(); } - } // namespace testing - } // namespace ngraph_bridge +} // namespace testing +} // namespace ngraph_bridge } // namespace tensorflow diff --git a/test/test_utilities.cpp b/test/test_utilities.cpp index 90b30085..2c9cb0e4 100644 --- a/test/test_utilities.cpp +++ b/test/test_utilities.cpp @@ -13,20 +13,20 @@ * See the License for the specific language governing permissions and * limitations under the License. *******************************************************************************/ - #include "test_utilities.h" - using namespace std; - namespace ng = ngraph; - namespace tensorflow { - namespace ngraph_bridge { - void ActivateNGraph() { +#include "test_utilities.h" +using namespace std; +namespace ng = ngraph; +namespace tensorflow { +namespace ngraph_bridge { +void ActivateNGraph() { setenv("NGRAPH_TF_DISABLE_DEASSIGN_CLUSTERS", "1", 1); unsetenv("NGRAPH_TF_DISABLE"); } - void DeactivateNGraph() { +void DeactivateNGraph() { unsetenv("NGRAPH_TF_DISABLE_DEASSIGN_CLUSTERS"); setenv("NGRAPH_TF_DISABLE", "1", 1); } - void AssertTensorEquals(Tensor& T1, Tensor& T2) { +void AssertTensorEquals(Tensor& T1, Tensor& T2) { auto T_size = T1.flat().size(); auto T1_data = T1.flat().data(); auto T2_data = T2.flat().data(); @@ -36,7 +36,7 @@ EXPECT_FLOAT_EQ(a, b); } } - void AssignInputIntValues(Tensor& A, int maxval) { +void AssignInputIntValues(Tensor& A, int maxval) { auto A_flat = A.flat(); auto A_flat_data = A_flat.data(); int counter = 0; @@ -47,17 +47,17 @@ } } } - void AssignInputValues(Tensor& A, float x) { +void AssignInputValues(Tensor& A, float x) { auto A_flat = A.flat(); auto A_flat_data = A_flat.data(); for (int i = 0; i < A_flat.size(); i++) { A_flat_data[i] = x; } } - void PrintTensor(const Tensor& T1) { +void PrintTensor(const Tensor& T1) { LOG(INFO) << "print tensor values" << T1.DebugString(); } - void ValidateTensorData(Tensor& T1, Tensor& T2, float tol) { +void ValidateTensorData(Tensor& T1, Tensor& T2, float tol) { auto T_size = T1.flat().size(); auto T1_data = T1.flat().data(); auto T2_data = T2.flat().data(); @@ -73,5 +73,5 @@ } } } - } // namespace ngraph_bridge -} //namespace tensorflow +} // namespace ngraph_bridge +} // namespace tensorflow diff --git a/test/test_utilities.h b/test/test_utilities.h index 749f6527..588f700f 100644 --- a/test/test_utilities.h +++ b/test/test_utilities.h @@ -15,15 +15,15 @@ *******************************************************************************/ #ifndef NGRAPH_TF_BRIDGE_TESTUTILITIES_H_ #define NGRAPH_TF_BRIDGE_TESTUTILITIES_H_ - #include "gtest/gtest.h" +#include "gtest/gtest.h" #include "ngraph/ngraph.hpp" - #include "tensorflow/core/framework/tensor.h" +#include "tensorflow/core/framework/tensor.h" #include "tensorflow/core/framework/tensor_types.h" #include "tensorflow/core/platform/env.h" - using namespace std; +using namespace std; namespace ng = ngraph; - namespace tensorflow { - namespace ngraph_bridge { +namespace tensorflow { +namespace ngraph_bridge { // some utility functions copied from tf_exec.cpp void ActivateNGraph(); void DeactivateNGraph(); @@ -32,6 +32,6 @@ void AssignInputIntValues(Tensor& A, int maxval); void AssignInputValues(Tensor& A, float x); void PrintTensor(const Tensor& T1); void ValidateTensorData(Tensor& T1, Tensor& T2, float tol); - } // namespace ngraph_bridge - } // namespace tensorflow - #endif // NGRAPH_TF_BRIDGE_TESTUTILITIES_H_ +} // namespace ngraph_bridge +} // namespace tensorflow +#endif // NGRAPH_TF_BRIDGE_TESTUTILITIES_H_ diff --git a/test/tf_exec.cpp b/test/tf_exec.cpp index fb389872..9a860576 100644 --- a/test/tf_exec.cpp +++ b/test/tf_exec.cpp @@ -807,7 +807,7 @@ TEST(tf_exec, Op_Rsqrt) { TEST(tf_exec, Op_Negate) { Scope scope_cpu = Scope::NewRootScope(); - Scope scope_ng = scope_cpu; //scope_cpu.WithDevice("/device:NGRAPH:0"); + Scope scope_ng = scope_cpu; // scope_cpu.WithDevice("/device:NGRAPH:0"); ActivateNGraph(); // ngraph execution @@ -815,7 +815,7 @@ TEST(tf_exec, Op_Negate) { auto r_ng = ops::Negate(scope_ng.WithOpName("r"), A_ng); std::vector outputs_ng; - + SessionOptions options; options.config.mutable_graph_options() ->mutable_optimizer_options() @@ -841,7 +841,7 @@ TEST(tf_exec, Op_Negate) { TEST(tf_exec, Op_FloorDiv) { Scope scope_cpu = Scope::NewRootScope(); - Scope scope_ng = scope_cpu; //scope_cpu.WithDevice("/device:NGRAPH:0"); + Scope scope_ng = scope_cpu; // scope_cpu.WithDevice("/device:NGRAPH:0"); DeactivateNGraph(); // ngraph execution @@ -927,7 +927,7 @@ TEST(tf_exec, Op_FloorMod) { TEST(tf_exec, Op_AddN) { Scope scope_cpu = Scope::NewRootScope(); - Scope scope_ng = scope_cpu; //scope_cpu.WithDevice("/device:NGRAPH:0"); + Scope scope_ng = scope_cpu; // scope_cpu.WithDevice("/device:NGRAPH:0"); ActivateNGraph(); // ngraph execution @@ -946,7 +946,7 @@ TEST(tf_exec, Op_AddN) { ClientSession session_ng(scope_ng, options); std::vector outputs_ng; - //ClientSession session_ng(scope_ng); + // ClientSession session_ng(scope_ng); ASSERT_OK(session_ng.Run({r_ng}, &outputs_ng)); ASSERT_EQ(outputs_ng[0].shape(), TensorShape({2, 2})); From cf7fcdda9f36b7ab130abd0a617ad69407dcb55f Mon Sep 17 00:00:00 2001 From: Sayantan Sarkar Date: Wed, 29 Aug 2018 10:25:47 -0700 Subject: [PATCH 20/32] Some cleanup --- src/ngraph_builder1.cc | 19 ++++--------- src/ngraph_builder1.h | 64 +++++++++++++++++++++++++----------------- 2 files changed, 44 insertions(+), 39 deletions(-) diff --git a/src/ngraph_builder1.cc b/src/ngraph_builder1.cc index 267910c1..9414edaf 100644 --- a/src/ngraph_builder1.cc +++ b/src/ngraph_builder1.cc @@ -20,7 +20,7 @@ //#include "ngraph/builder/autobroadcast.hpp" //#include "ngraph/builder/numpy_transpose.hpp" -//TODO: remove the header files, if not needed +// TODO: remove the header files, if not needed //#include "tensorflow/core/framework/tensor.pb.h" //#include "tensorflow/core/framework/tensor_shape.pb.h" //#include "tensorflow/core/framework/tensor_shape.pb_text.h" @@ -42,7 +42,7 @@ Status Builder1::TranslateGraph( TF_RETURN_IF_ERROR(Initialize()); vector> ng_parameter_list; - TF_RETURN_IF_ERROR(GetInputParams(inputs, tf_params, &ng_parameter_list)); + TF_RETURN_IF_ERROR(GetInputParams(inputs, tf_params, ng_parameter_list)); TF_RETURN_IF_ERROR(TranslateEachOp(tf_ops, static_input_map)); @@ -142,10 +142,10 @@ Status Builder1::ClassifyNodes(const vector& ordered, Status Builder1::GetInputParams( const std::vector& inputs, vector tf_params, - vector>* ng_parameter_list) { + vector>& ng_parameter_list) { // Populate the parameter list, and also put parameters into the op map. - ng_parameter_list->resize(tf_params.size()); + ng_parameter_list.resize(tf_params.size()); for (auto parm : tf_params) { DataType dtype; @@ -165,7 +165,7 @@ Status Builder1::GetInputParams( auto ng_param = make_shared(ng_et, ng_shape); ng_op_map[parm->name()].push_back(ng_param); - (*ng_parameter_list)[index] = ng_param; + ng_parameter_list[index] = ng_param; } return Status::OK(); } @@ -221,13 +221,7 @@ Status Builder1::Initialize() { for (auto node : tf_graph.nodes()) { if (node->type_string() == "_Arg") { arg_nodes.push_back(node); - int32 index; - // macro defn: - // https://github.com/petewarden/tensorflow_makefile/blob/master/tensorflow/core/framework/op_kernel.h#L1265 - - // TODO check : removing OP_REQUIRES_OK for now - // OP_REQUIRES_OK(ctx, GetNodeAttr(node->attrs(), "index", &index)); TF_RETURN_IF_ERROR(GetNodeAttr(node->attrs(), "index", &index)); if (index > max_arg_index) max_arg_index = index; } @@ -238,11 +232,8 @@ Status Builder1::Initialize() { // Fill the vector. for (auto node : arg_nodes) { int32 index; - // TODO: OP_REQUIRES_OK or TF_RETURN_IF_ERROR - //// OP_REQUIRES_OK(ctx, GetNodeAttr(node->attrs(), "index", &index)); TF_RETURN_IF_ERROR(GetNodeAttr(node->attrs(), "index", &index)); - // bool is_static = false; for (auto edge : node->out_edges()) { if (edge->IsControlEdge() || !edge->dst()->IsOp()) { continue; diff --git a/src/ngraph_builder1.h b/src/ngraph_builder1.h index 9c20fc8c..a7cff8f3 100644 --- a/src/ngraph_builder1.h +++ b/src/ngraph_builder1.h @@ -26,7 +26,7 @@ #include "ngraph_log.h" #include "ngraph_mark_for_clustering.h" -//TODO: remove headers if not needed +// TODO: remove headers if not needed //#include "tensorflow/core/framework/op.h" #include "tensorflow/core/framework/op_kernel.h" #include "tensorflow/core/framework/tensor_shape.h" @@ -50,6 +50,28 @@ class Builder1 { using TranslatorFn = function&, VectNg&)>; + public: + // Note: in case we want to get rid of Initialize, + // pass a OpKernelConstruction* ctx to the constructor, + // and use OP_REQUIRES_OK instead of TF_RETURN_IF_ERROR + // This gets rid of Initialize() as it can handle errors in construction + // But in the case of overloaded constructor that does not accept a ctx, + // which is used for OpExecuter test class, we cannot handle error during + // construction. + // Hence keeping the Initialize() function + Builder1(const Graph& tf_graph, + OpKernelConstruction* ctx) // TODO make ctx const? + : tf_graph(tf_graph) {} + + Builder1(const Graph& tf_graph) : Builder1(tf_graph, nullptr) {} + + Status TranslateGraph(const std::vector&, + const std::vector&, + shared_ptr&); + + Status TranslateGraph(OpKernelContext* ctx, + std::shared_ptr& ng_function); + private: // // The op map holds a mapping from TensorFlow op names (strings) to @@ -57,31 +79,38 @@ class Builder1 { // std::unordered_map ng_op_map; - const static std::map TRANSLATE_OP_MAP; - const static std::map> INPUT_INDEX_MAP; - OpKernelConstruction* ctx; // TODO: is this required? required in - // OP_REQUIRES_OK is used instead of - // TF_RETURN_IF_ERROR in Initialize - bool is_initialized = false; // Prevent init from running twice + bool is_initialized = false; // Prevent Initialize from running twice const Graph& tf_graph; std::vector m_input_is_static; vector ordered; vector tf_params, tf_ret_vals, tf_ops; + // A map from Tf op type_string to a TranslateOp + const static std::map TRANSLATE_OP_MAP; + // Given a TF node, return its corresponding TranslateOp function and required input indexes + // A wrapper for TRANSLATE_OP_MAP Status GetOpTranslationRequirements(const Node*, Builder1::TranslatorFn&, vector&); Status GetInputNode(const Node*, size_t, shared_ptr*); - // TODO: write description - Status GetInputParams(const std::vector&, vector, - vector>*); + // Classify a list of TF nodes into _Arg (input), _Retval (output) and other + // nodes Status ClassifyNodes(const vector&, vector&, vector&, vector&); + + // Given the input shapes and a list of TF _Arg nodes, create corresponding + // nGraph parameters. Also populate the ng_op_map + Status GetInputParams(const std::vector&, vector, + vector>&); + // Given a TF node, retrieve its corresponding nGraph nodes (using ng_op_map), + // then call the appropriate TranslateOp function Status TranslateEachOp(const vector&, const std::vector&); + // After each TF op has been translated, find the nGraph nodes corresponding + // to the _Retval nodes Status GetOutputNodes(const vector&, VectNg&); template @@ -100,21 +129,6 @@ class Builder1 { T& ng_padding_above); Status Initialize(); - - public: - Builder1(const Graph& tf_graph, - OpKernelConstruction* ctx) // TODO make ctx const? - : tf_graph(tf_graph), - ctx(ctx) {} - - Builder1(const Graph& tf_graph) : tf_graph(tf_graph) {} - - Status TranslateGraph(const std::vector&, - const std::vector&, - shared_ptr&); - - Status TranslateGraph(OpKernelContext* ctx, - std::shared_ptr& ng_function); }; } // namespace ngraph_bridge From 415c35058fa4ec3410a1b0974fee696a1ba04cec Mon Sep 17 00:00:00 2001 From: Sayantan Sarkar Date: Wed, 29 Aug 2018 12:50:51 -0700 Subject: [PATCH 21/32] More cleanup --- src/ngraph_builder1.cc | 28 ++++-------- src/ngraph_builder1.h | 91 ++++++++++++++++++++------------------- src/ngraph_translateops.h | 6 +-- 3 files changed, 58 insertions(+), 67 deletions(-) diff --git a/src/ngraph_builder1.cc b/src/ngraph_builder1.cc index 9414edaf..c78bdce8 100644 --- a/src/ngraph_builder1.cc +++ b/src/ngraph_builder1.cc @@ -17,17 +17,6 @@ #include "ngraph_builder1.h" #include "ngraph_translateops.h" -//#include "ngraph/builder/autobroadcast.hpp" -//#include "ngraph/builder/numpy_transpose.hpp" - -// TODO: remove the header files, if not needed -//#include "tensorflow/core/framework/tensor.pb.h" -//#include "tensorflow/core/framework/tensor_shape.pb.h" -//#include "tensorflow/core/framework/tensor_shape.pb_text.h" -//#include "tensorflow/core/graph/algorithm.h" -//#include "tensorflow/core/graph/edgeset.h" -//#include "tensorflow/core/lib/core/errors.h" - using namespace std; namespace ng = ngraph; @@ -50,7 +39,11 @@ Status Builder1::TranslateGraph( TF_RETURN_IF_ERROR(GetOutputNodes(tf_ret_vals, ng_result_list)); // Create the nGraph function. - ng_function = make_shared(ng_result_list, ng_parameter_list); + try { + ng_function = make_shared(ng_result_list, ng_parameter_list); + } catch (...) { + return errors::Unimplemented("Unable to create nGraph function"); + } // // Request row-major layout on results. @@ -114,10 +107,7 @@ Status Builder1::TranslateEachOp( return Status::OK(); } -Status Builder1::ClassifyNodes(const vector& ordered, - vector& tf_params, - vector& tf_ret_vals, - vector& tf_ops) { +Status Builder1::ClassifyNodes(const vector& ordered) { // Split ops into params, retvals, and all others. for (const auto n : ordered) { if (n->IsSink() || n->IsSource()) { @@ -203,9 +193,10 @@ Status Builder1::Initialize() { // // ought to be `const Node*`, but GetReversePostOrder doesn't use `const` + vector ordered; GetReversePostOrder(tf_graph, &ordered); - TF_RETURN_IF_ERROR(ClassifyNodes(ordered, tf_params, tf_ret_vals, tf_ops)); + TF_RETURN_IF_ERROR(ClassifyNodes(ordered)); // // Initialize the "m_input_is_static" vector as follows: // (1) create m_input_is_static with n+1 elements, where n is the max arg @@ -351,7 +342,6 @@ Status Builder1::GetOpTranslationRequirements( if (iter_fn != TRANSLATE_OP_MAP.end()) { translate_fn = iter_fn->second; } else { - // TODO::: if-else or try-catch // ----------------------------- // Catch-all for unsupported ops // ----------------------------- @@ -370,7 +360,7 @@ Status Builder1::GetOpTranslationRequirements( // otherwise specified. input_indexes.resize(op->num_inputs()); std::iota(std::begin(input_indexes), std::end(input_indexes), - 0); // Populate with increasing integers 1,2... + 0); // iota: Populate with increasing integers 1,2... } return Status::OK(); } diff --git a/src/ngraph_builder1.h b/src/ngraph_builder1.h index a7cff8f3..5ccc7c78 100644 --- a/src/ngraph_builder1.h +++ b/src/ngraph_builder1.h @@ -19,19 +19,13 @@ #include #include -//#include "ngraph_conversions.h" - #include "ngraph/ngraph.hpp" - #include "ngraph_log.h" #include "ngraph_mark_for_clustering.h" -// TODO: remove headers if not needed -//#include "tensorflow/core/framework/op.h" #include "tensorflow/core/framework/op_kernel.h" #include "tensorflow/core/framework/tensor_shape.h" #include "tensorflow/core/graph/algorithm.h" -//#include "tensorflow/core/graph/graph.h" using namespace std; namespace ng = ngraph; @@ -39,11 +33,19 @@ namespace tensorflow { namespace ngraph_bridge { -// TODO: make sure all comments from old builder are copied correctly. +// TODO (sarkars): make sure all comments from old builder are copied correctly. // TODO: use camelcase, snakecase appropriately // TODO add TF_RETURN_IF_ERROR where necessary -///////////////// + +// The purpose of Builder is to accept a TF graph and convert it to a +// equivalent ngraph fucntion. To that end, it acts as an interface to +// a library of 'TranslateOps' functions. These functions are defined +// in ngraph_translateops.h. Builder just goes through each TF op and +// calls the appropriate 'TranslatorFn'. It does not implement the per-op +// translation, the heavy-lifting being done by ngraph_translateops.h; +// It merely does some bookkeeping, such as process the nodes in correct +// order, keep track of and retrieve parent nGraph nodes etc. class Builder1 { using VectNg = std::vector>; @@ -51,83 +53,84 @@ class Builder1 { const Node*, VectNg&, const std::vector&, VectNg&)>; public: - // Note: in case we want to get rid of Initialize, - // pass a OpKernelConstruction* ctx to the constructor, - // and use OP_REQUIRES_OK instead of TF_RETURN_IF_ERROR - // This gets rid of Initialize() as it can handle errors in construction + // Note: in case we want to get rid of Initialize, pass an + // OpKernelConstruction* ctx to the constructor, and use OP_REQUIRES_OK + // instead of TF_RETURN_IF_ERROR. This gets rid of Initialize() as it + // can handle errors in construction. // But in the case of overloaded constructor that does not accept a ctx, // which is used for OpExecuter test class, we cannot handle error during // construction. // Hence keeping the Initialize() function - Builder1(const Graph& tf_graph, - OpKernelConstruction* ctx) // TODO make ctx const? + + // This constructor is for actual use (encapsulate_op) + Builder1(const Graph& tf_graph, OpKernelConstruction* ctx) : tf_graph(tf_graph) {} + // And this version is for tests (OpExecuter) Builder1(const Graph& tf_graph) : Builder1(tf_graph, nullptr) {} + // TranslateGraph is overloaded. This is for actual use (encapsulate_op) + Status TranslateGraph(OpKernelContext* ctx, + std::shared_ptr& ng_function); + + // And this version is for tests (OpExecuter) Status TranslateGraph(const std::vector&, const std::vector&, shared_ptr&); - Status TranslateGraph(OpKernelContext* ctx, - std::shared_ptr& ng_function); - private: - // // The op map holds a mapping from TensorFlow op names (strings) to - // vector of generated nGraph nodes. - // + // vector of generated nGraph nodes. Since we process nodes in toposort + // order, it is guaranteed when processing a TF node, its parents + // will already have been processed and safely stowed in ng_op_map std::unordered_map ng_op_map; - const static std::map> INPUT_INDEX_MAP; - - bool is_initialized = false; // Prevent Initialize from running twice + // Prevent Initialize from running twice + bool is_initialized = false; const Graph& tf_graph; + + // An array that will be useful in creating the static_input_map std::vector m_input_is_static; - vector ordered; + + // Vectors containing TF nodes in 3 bins: inputs, outputs, actual ops vector tf_params, tf_ret_vals, tf_ops; + // This map tells us which inputs to read for a particular node. If no + // information is present explicitly in the map, we read all inputs + // from 0 to num_inputs-1 + const static std::map> INPUT_INDEX_MAP; + // A map from Tf op type_string to a TranslateOp const static std::map TRANSLATE_OP_MAP; - // Given a TF node, return its corresponding TranslateOp function and required input indexes - // A wrapper for TRANSLATE_OP_MAP + + // Given a TF node, return its corresponding TranslateOp function and required + // input indexes. A wrapper for TRANSLATE_OP_MAP and INPUT_INDEX_MAP Status GetOpTranslationRequirements(const Node*, Builder1::TranslatorFn&, vector&); + // Given a TF node, an index i, it returns the ith nGraph input Status GetInputNode(const Node*, size_t, shared_ptr*); // Classify a list of TF nodes into _Arg (input), _Retval (output) and other - // nodes - Status ClassifyNodes(const vector&, vector&, - vector&, vector&); + // nodes. Used to populate tf_params, tf_ret_vals, tf_ops + Status ClassifyNodes(const vector&); // Given the input shapes and a list of TF _Arg nodes, create corresponding // nGraph parameters. Also populate the ng_op_map Status GetInputParams(const std::vector&, vector, vector>&); + // Given a TF node, retrieve its corresponding nGraph nodes (using ng_op_map), // then call the appropriate TranslateOp function Status TranslateEachOp(const vector&, const std::vector&); + // After each TF op has been translated, find the nGraph nodes corresponding // to the _Retval nodes Status GetOutputNodes(const vector&, VectNg&); - template - void MakePadding(const std::string& tf_padding_type, - const ngraph::Shape& ng_image_shape, - const ngraph::Shape& ng_kernel_shape, - const ngraph::Strides& ng_strides, - const ngraph::Shape& ng_dilations, T& ng_padding_below, - T& ng_padding_above); - - template - void MakePadding(const std::string& tf_padding_type, - const ngraph::Shape& ng_image_shape, - const ngraph::Shape& ng_kernel_shape, - const ngraph::Strides& ng_strides, T& ng_padding_below, - T& ng_padding_above); - + // Since the constructor does not return Status, delegating its job to a + // separate function that is evaluated lazily and only once. Status Initialize(); }; diff --git a/src/ngraph_translateops.h b/src/ngraph_translateops.h index 587219c2..d3242cc4 100644 --- a/src/ngraph_translateops.h +++ b/src/ngraph_translateops.h @@ -33,8 +33,7 @@ using VectNg = std::vector>; // TODO (sarkars): why does MakePadding handle only 2 dimensions... generalize // it? // TODO (sarkars): add unit tests for 1D conv. 1d pooling etc. check if -// MakePadding works -// in that case +// MakePadding works in that case template void MakePadding(const std::string& tf_padding_type, const ngraph::Shape& ng_image_shape, @@ -122,7 +121,7 @@ Status TranslateAddNOp(const Node* op, VectNg& ng_arg_vec, return Status::OK(); } -// TODO: document why ng_arg_vec is not const. (BatchToNGraph changes it) +// ng_arg_vec is not const. For example, BatchToNGraph changes it Status TranslateAvgPoolOp(const Node* op, VectNg& ng_arg_vec, const std::vector& static_input_map, VectNg& subgraph_out_nodes) { @@ -151,7 +150,6 @@ Status TranslateAvgPoolOp(const Node* op, VectNg& ng_arg_vec, ng::Shape ng_image_shape(2); ng::Shape ng_kernel_shape(2); - // TODO: should these be in Builder instead of in ngraph_conversions.h? BatchedOpParamToNGraph(is_nhwc, tf_strides, ng_strides); BatchedOpParamToNGraph(is_nhwc, ng_arg_vec[0]->get_shape(), ng_image_shape); BatchedOpParamToNGraph(is_nhwc, tf_ksize, ng_kernel_shape); From 43d319bf79f417c0e85b9f4d766905cef7f6d030 Mon Sep 17 00:00:00 2001 From: Sayantan Sarkar Date: Wed, 29 Aug 2018 13:35:00 -0700 Subject: [PATCH 22/32] More cleanup --- src/ngraph_builder.cc | 9 - src/ngraph_builder1.h | 2 - src/ngraph_encapsulate_op.cc | 16 +- test/CMakeLists.txt | 4 +- test/opexecuter.cpp | 336 ----------------------------------- test/opexecuter.h | 95 ---------- test/test_math_ops.cpp | 96 ---------- test/test_utilities.cpp | 77 -------- test/test_utilities.h | 37 ---- 9 files changed, 8 insertions(+), 664 deletions(-) delete mode 100644 test/opexecuter.cpp delete mode 100644 test/opexecuter.h delete mode 100644 test/test_math_ops.cpp delete mode 100644 test/test_utilities.cpp delete mode 100644 test/test_utilities.h diff --git a/src/ngraph_builder.cc b/src/ngraph_builder.cc index 93029aea..bf9234ba 100644 --- a/src/ngraph_builder.cc +++ b/src/ngraph_builder.cc @@ -2695,15 +2695,6 @@ Status Builder::TranslateGraph( vector tf_ops; for (const auto n : ordered) { - cout << "XXXXX " << n->type_string() << "\n"; - if (n->type_string() == "Const") { - Tensor result; - vector vect; - result.FromProto(n->def().attr().at("value").tensor()); - TensorDataToVector(result, &vect); - for (auto vv : vect) cout << " " << vv; - cout << "\n"; - } if (n->IsSink() || n->IsSource()) { continue; } diff --git a/src/ngraph_builder1.h b/src/ngraph_builder1.h index 5ccc7c78..c68aa0c2 100644 --- a/src/ngraph_builder1.h +++ b/src/ngraph_builder1.h @@ -34,8 +34,6 @@ namespace tensorflow { namespace ngraph_bridge { // TODO (sarkars): make sure all comments from old builder are copied correctly. -// TODO: use camelcase, snakecase appropriately -// TODO add TF_RETURN_IF_ERROR where necessary // The purpose of Builder is to accept a TF graph and convert it to a diff --git a/src/ngraph_encapsulate_op.cc b/src/ngraph_encapsulate_op.cc index e68e1356..56ba1bc2 100644 --- a/src/ngraph_encapsulate_op.cc +++ b/src/ngraph_encapsulate_op.cc @@ -29,7 +29,6 @@ #include "ngraph/serializer.hpp" #include "ngraph_builder.h" -#include "ngraph_builder1.h" #include "ngraph_cluster_manager.h" #include "ngraph_freshness_tracker.h" #include "ngraph_log.h" @@ -38,7 +37,11 @@ #include "ngraph/runtime/interpreter/int_backend.hpp" -#define BUILDER1 +#define BUILDER1 // (un)comment this line to (de)activate old builder + +#ifdef BUILDER1 +#include "ngraph_builder1.h" +#endif using namespace std; namespace ng = ngraph; @@ -63,8 +66,6 @@ REGISTER_OP("NGraphEncapsulate") class NGraphEncapsulateOp : public OpKernel { #ifdef BUILDER1 - // TODO: remove "m_input_is_static" etc from constructor of - // NGraphEncapsulateOp. private: Builder1 ng_builder; #endif @@ -87,10 +88,8 @@ class NGraphEncapsulateOp : public OpKernel { opts.allow_internal_ops = true; OP_REQUIRES_OK(ctx, ConvertGraphDefToGraph(opts, *graph_def, &m_graph)); -#ifdef BUILDER1 -// ng_builder.init(); //Now TranslateGraph will call init() if needed -#else - +#ifndef BUILDER1 + // TODO (sarkars): remove "m_input_is_static" etc from this constructor // // Initialize the "m_input_is_static" vector as follows: // (1) create m_input_is_static with n+1 elements, where n is the max arg @@ -291,7 +290,6 @@ class NGraphEncapsulateOp : public OpKernel { if (it == m_ng_functions.end()) { NGRAPH_VLOG(1) << "Compilation cache miss: " << ctx->op_kernel().name(); #ifndef BUILDER1 - OP_REQUIRES_OK( ctx, Builder::TranslateGraph(input_shapes, static_input_map, &m_graph, ng_function)); diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index cf5bea0a..aec5e45d 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -36,12 +36,10 @@ set_target_properties( set(SRC main.cpp graph_exec.cpp + tf_exec.cpp padding.cpp conversions.cpp graph_rewrites/assign_clusters.cc - test_utilities.cpp - test_math_ops.cpp - opexecuter.cpp ) set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11") diff --git a/test/opexecuter.cpp b/test/opexecuter.cpp deleted file mode 100644 index 8b02827a..00000000 --- a/test/opexecuter.cpp +++ /dev/null @@ -1,336 +0,0 @@ -/******************************************************************************* - * Copyright 2017-2018 Intel Corporation - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - *******************************************************************************/ -#include "opexecuter.h" - -using namespace std; -namespace ng = ngraph; - -namespace tensorflow { - -namespace ngraph_bridge { - -namespace testing { - -// Utility Function to create NodeDef for _Arg and _Retval nodes -void OpExecuter::CreateNodeDef(const string op_type, - const string op_name_prefix, int index, - const DataType dt, NodeDef& node_def) { - string new_node_name = op_name_prefix + std::to_string(index); - node_def.set_name(new_node_name); - node_def.set_op(op_type); - SetAttrValue(dt, &((*(node_def.mutable_attr()))["T"])); - SetAttrValue(index, &((*(node_def.mutable_attr()))["index"])); -} - -// Update data structures for book keeping -// node_inedge_md : Map of -// key : Node* -// value : vector of pair{Node* src_incoming_edge, int -// src_output_index} -// node_outedge_md : Map of -// key : Node* -// value : vector of pair{Node* dst_outgoing_edge, int -// dst_input_index} -// node_outedges : Map of -// key : Node* -// value : vector of outgoing edges (Edge*) -// test_op : update pointer to test_op pointer -void OpExecuter::GetNodeData(Graph& graph, NodeMetaData& node_inedge_md, - NodeMetaData& node_outedge_md, - NodeOutEdges& node_outedges, Node** test_op) { - bool found_test_op = false; - for (const Edge* e : graph.edges()) { - if (!found_test_op) { - if (e->src()->IsOp() && (e->src()->type_string()) == test_op_type_) { - found_test_op = true; - *test_op = e->src(); - } - if (e->dst()->IsOp() && (e->dst()->type_string()) == test_op_type_) { - found_test_op = true; - *test_op = e->dst(); - } - } - NGRAPH_VLOG(5) << "Edge between, Src: " << e->src()->name() - << " Src op index " << e->src_output() - << " ,Dst: " << e->dst()->name() << " dst ip index " - << e->dst_input(); - // update src's outedge metadata - node_outedge_md[e->src()].push_back({e->dst(), e->dst_input()}); - node_inedge_md[e->dst()].push_back({e->src(), e->src_output()}); - node_outedges[e->src()].push_back(e); - } -} - -// Validate that the graph has N allowed_nodes and 1 test_op_type node -// Graph must look like this -// -// Const1 ConstN -// \ ... / -// \ / -// Test_Op -// -// TO_DO check for vector allowed_nodes -// when we allow other than "Const" node type as input -void OpExecuter::ValidateGraph(const Graph& graph, - const vector allowed_nodes) { - NGRAPH_VLOG(5) << "Validate graph"; - bool found_test_op = false; - for (Node* node : graph.nodes()) { - if (node->IsSource() || node->IsSink()) { - continue; - } else if (node->type_string() == test_op_type_) { - // only one node of type test_op - ASSERT_FALSE(found_test_op); - found_test_op = true; - } else { - ASSERT_TRUE(node->type_string() == allowed_nodes[0]) - << "Found Not allowed Op: " << node->type_string(); - } - } - NGRAPH_VLOG(5) << "Validate graph done"; -} - -// Constructor Function -// TO DO: Add support for ops that take static inputs -// currently static_input_map is empty -OpExecuter::OpExecuter(const Scope sc, const string test_op, - const vector& op_types, - const std::vector& sess_run_fetchops) - : tf_scope_(sc), - test_op_type_(test_op), - expected_output_datatypes_(op_types), - static_input_map_({}), - sess_run_fetchoutputs_(sess_run_fetchops) {} - -// Destructor -OpExecuter::~OpExecuter() {} - -// Uses tf_scope to execute on TF -void OpExecuter::ExecuteOnTF() { - DeactivateNGraph(); - ClientSession session(tf_scope_); - ASSERT_EQ(Status::OK(), session.Run(sess_run_fetchoutputs_, &tf_outputs_)); -} - -// Compares tf_outputs_ with ngraph_outputs_ -void OpExecuter::CompareNgraphAndTF() { - ASSERT_EQ(tf_outputs_.size(), ngraph_outputs_.size()); - for (int i = 0; i < tf_outputs_.size(); i++) { - AssertTensorEquals(tf_outputs_[i], ngraph_outputs_[i]); - } -} - -// This function does the following: -// 1. Validates the graph -// 2. Rewrites the graph to have _Arg and _Retval nodes -// -// _Arg1 _ArgN -// \ ... / -// \ / -// Test_Op -// / \ -// / ... \ -// _Retval1 _RetvalM -// -// 3. Gets Tensor values from Const Nodes and adds to input_tensors -// 4. Creates ng::Function -// 5. Executes ng::Function on CPU backend -// 6. Updates output of ng::Function into ngraph_output -// TO DO : Refactor -void OpExecuter::ExecuteOnNGraph() { - Graph graph(OpRegistry::Global()); - TF_CHECK_OK(tf_scope_.ToGraph(&graph)); - - // For debug - GraphToPbTextFile(&graph, "tf_graph.pbtxt"); - - ValidateGraph(graph, {"Const"}); - - NodeMetaData node_inedge_metadata; - NodeMetaData node_outedge_metadata; - NodeOutEdges node_out_edges; - Node* test_op; - - GetNodeData(graph, node_inedge_metadata, node_outedge_metadata, - node_out_edges, &test_op); - NGRAPH_VLOG(5) << "Got graph data. Found op " << test_op->type_string(); - - // Get Tensor input shapes and values from the const nodes - int number_of_inputs = test_op->num_inputs(); - vector input_shapes; - vector input_node; - - for (int i = 0; i < number_of_inputs; i++) { - Node* ip; - Tensor ip_tensor; - ASSERT_EQ(Status::OK(), test_op->input_node(i, &ip)); - input_node.push_back(ip); - ASSERT_EQ(Status::OK(), GetNodeAttr(ip->attrs(), "value", &ip_tensor)); - input_shapes.push_back(ip_tensor.shape()); - tf_inputs_.push_back(ip_tensor); - NGRAPH_VLOG(5) << " Extracted tensor from const " << i << " " - << tf_inputs_[i].DebugString(); - } - - NGRAPH_VLOG(5) << "Got input nodes and tensors"; - - // Replace the input nodes to Test_op with _Arg nodes - for (int i = 0; i < number_of_inputs; i++) { - Node* ip_node = input_node[i]; - NodeDef new_arg_node_def; - CreateNodeDef("_Arg", "arg_", i, tf_inputs_[i].dtype(), new_arg_node_def); - - // Add node to graph - Status status; - Node* arg_node = graph.AddNode(new_arg_node_def, &status); - ASSERT_EQ(Status::OK(), status); - - // Remove the Const Node - graph.RemoveNode(input_node[i]); - - // Add edge from SOURCE to _Arg - auto src_nodes_metadata = node_inedge_metadata[ip_node]; - for (int j = 0; j < src_nodes_metadata.size(); j++) { - graph.AddEdge(src_nodes_metadata[j].first, src_nodes_metadata[j].second, - arg_node, 0); - } - // Adds an edge from arg_node to test_op - graph.AddEdge(arg_node, 0, test_op, i); - } - - NGRAPH_VLOG(5) << "Replaced input nodes with _Arg"; - - // Add _Retval to graph - int number_of_outputs = expected_output_datatypes_.size(); - // For all the output edges from test_op (there should be only one, to SINK) - // get the dest node and the - // destination_input_index - // (TO DO : ) ADD ASSERT to check one? - auto dest_nodes_metadata = node_outedge_metadata[test_op]; - - // Remove edges from test_op to SINK (not removing might be also ok) - for (const Edge* e : node_out_edges[test_op]) { - graph.RemoveEdge(e); - } - - for (int i = 0; i < number_of_outputs; i++) { - // Add new retval_ node - NodeDef new_ret_node_def; - CreateNodeDef("_Retval", "retval_", i, expected_output_datatypes_[i], - new_ret_node_def); - Status status; - Node* ret_node = graph.AddNode(new_ret_node_def, &status); - ASSERT_EQ(Status::OK(), status); - - // Add edges from _Retval to sink - for (int j = 0; j < dest_nodes_metadata.size(); j++) { - graph.AddEdge(ret_node, 0, dest_nodes_metadata[j].first, - dest_nodes_metadata[j].second); - } - // Add edges from test_op to _Retval - graph.AddEdge(test_op, i, ret_node, 0); - } - - NGRAPH_VLOG(5) << "Added _Retval nodes "; - - NGRAPH_VLOG(5) << "After rewrite *** "; - for (const Edge* e : graph.edges()) { - NGRAPH_VLOG(5) << "Edge between, Src: " << e->src()->name() - << " ,Dst: " << e->dst()->name(); - } - // For debug - GraphToPbTextFile(&graph, "rewrite_ngraph.pbtxt"); - - // Create nGraph function - NGRAPH_VLOG(5) << " Create ng function "; - shared_ptr ng_function; - if (false) { - ASSERT_EQ(Status::OK(), - Builder::TranslateGraph(input_shapes, static_input_map_, &graph, - ng_function)); - } else { - auto builder = Builder1(graph); - ASSERT_EQ(Status::OK(), builder.TranslateGraph( - input_shapes, static_input_map_, ng_function)); - } - - // ng function should get same number of outputs - ASSERT_EQ(expected_output_datatypes_.size(), ng_function->get_output_size()); - - // Create nGraph backend - // Create the nGraph backend - auto backend = ng::runtime::Backend::create("CPU"); - - // Allocate tensors for inputs - vector> ng_ip_tensors; - vector> ng_op_tensors; - - NGRAPH_VLOG(5) << " Creating ng inputs "; - for (int i = 0; i < number_of_inputs; i++) { - ng::Shape ng_shape; - ASSERT_EQ(Status::OK(), - TFTensorShapeToNGraphShape(tf_inputs_[i].shape(), &ng_shape)); - ng::element::Type ng_et; - ASSERT_EQ(Status::OK(), - TFDataTypeToNGraphElementType(tf_inputs_[i].dtype(), &ng_et)); - void* src_ptr = (void*)DMAHelper::base(&tf_inputs_[i]); - auto result = backend->create_tensor(ng_et, ng_shape, src_ptr); - ng_ip_tensors.push_back(result); - } - - NGRAPH_VLOG(5) << " Creating ng outputs "; - vector tf_op_shapes; - for (int i = 0; i < number_of_outputs; i++) { - auto ng_op_shape = ng_function->get_output_shape(i); - auto ng_op_type = ng_function->get_output_element_type(i); - - ng::element::Type ng_et_expected; - ASSERT_EQ(Status::OK(), - TFDataTypeToNGraphElementType(expected_output_datatypes_[i], - &ng_et_expected)); - - // Expected element type should match ng_op_type - ASSERT_EQ(ng_et_expected, ng_op_type); - vector dims; - for (auto dim : ng_op_shape) { - dims.push_back(dim); - } - TensorShape tf_shape(dims); - tf_op_shapes.push_back(tf_shape); - auto result = backend->create_tensor(ng_op_type, ng_op_shape); - ng_op_tensors.push_back(result); - } - - // Execute the nGraph - NGRAPH_VLOG(5) << " Executing on nGraph "; - backend->call(ng_function, ng_op_tensors, ng_ip_tensors); - NGRAPH_VLOG(5) << " Writing to Tensors "; - for (auto i = 0; i < ng_function->get_output_size(); i++) { - // Convert to tf tensor - Tensor output_tensor(expected_output_datatypes_[i], tf_op_shapes[i]); - void* dst_ptr = DMAHelper::base(&output_tensor); - ng_op_tensors[i]->read(dst_ptr, 0, output_tensor.TotalBytes()); - ngraph_outputs_.push_back(output_tensor); - // DumpNGTensor(cout, ng_function->get_output_op(i)->get_name(), - // ng_op_tensors[i]); - } - -} // ExecuteOnNGraph - -} // namespace testing -} // namespace ngraph_bridge - -} // namespace tensorflow diff --git a/test/opexecuter.h b/test/opexecuter.h deleted file mode 100644 index ca9311be..00000000 --- a/test/opexecuter.h +++ /dev/null @@ -1,95 +0,0 @@ -/******************************************************************************* - * Copyright 2017-2018 Intel Corporation - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - *******************************************************************************/ -#ifndef NGRAPH_TF_BRIDGE_OPEXECUTER_H_ -#define NGRAPH_TF_BRIDGE_OPEXECUTER_H_ - -#include "ngraph/ngraph.hpp" -#include "ngraph_builder.h" -#include "ngraph_builder1.h" -#include "ngraph_utils.h" -#include "test_utilities.h" -#include "tf_graph_writer.h" - -#include "tensorflow/core/common_runtime/dma_helper.h" -#include "tensorflow/core/framework/graph.pb.h" -#include "tensorflow/core/framework/op.h" -#include "tensorflow/core/framework/tensor_types.h" -#include "tensorflow/core/graph/algorithm.h" -#include "tensorflow/core/graph/graph.h" -#include "tensorflow/core/graph/graph_constructor.h" -#include "tensorflow/core/platform/env.h" - -#include "tensorflow/cc/client/client_session.h" -#include "tensorflow/cc/ops/standard_ops.h" -#include "tensorflow/core/framework/tensor.h" -#include "tensorflow/core/public/session.h" - -using namespace std; -namespace ng = ngraph; - -namespace tensorflow { - -namespace ngraph_bridge { - -namespace testing { - -class OpExecuter { - public: - using NodeMetaData = map>>; - using NodeOutEdges = map>; - - // Scope sc : TF Scope with execution graph - // string test_op : test_op_type e.g. "Add" - // vector& op_types : expected tf data types of the output e.g. - // {DT_FLOAT, DT_INT} const std::vector& fetch_ops : Output ops to be - // fetched, is passed to tf.session.run() - OpExecuter(const Scope sc, const string test_op, - const vector& op_types, - const std::vector& fetch_ops); - - ~OpExecuter(); - - void ExecuteOnNGraph(); - void ExecuteOnTF(); - void CompareNgraphAndTF(); - - private: - Scope tf_scope_; - const string test_op_type_; - vector tf_inputs_; - vector tf_outputs_; - vector ngraph_outputs_; - const vector expected_output_datatypes_; - const vector& static_input_map_; - - // To Do : For placeholder const FeedType sess_run_inputs_; - const std::vector sess_run_fetchoutputs_; - - void GetNodeData(Graph& graph, NodeMetaData& node_inedge_md, - NodeMetaData& node_outedge_md, NodeOutEdges& node_outedges, - Node** test_op); - void ValidateGraph(const Graph& graph, const vector allowed_nodes); - - void CreateNodeDef(const string op_type, const string op_name_prefix, - int index, const DataType dt, NodeDef& node_def); -}; - -} // namespace testing -} // namespace ngraph_bridge - -} // namespace tensorflow - -#endif // NGRAPH_TF_BRIDGE_OPEXECUTER_H_ diff --git a/test/test_math_ops.cpp b/test/test_math_ops.cpp deleted file mode 100644 index 20630561..00000000 --- a/test/test_math_ops.cpp +++ /dev/null @@ -1,96 +0,0 @@ -/******************************************************************************* - * Copyright 2017-2018 Intel Corporation - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - *******************************************************************************/ -#include "gtest/gtest.h" -#include "ngraph_utils.h" -#include "opexecuter.h" -#include "tensorflow/cc/client/client_session.h" -#include "tensorflow/cc/ops/standard_ops.h" -#include "tensorflow/core/common_runtime/dma_helper.h" -#include "tensorflow/core/framework/graph.pb.h" -#include "tensorflow/core/framework/op.h" -#include "tensorflow/core/framework/tensor.h" -#include "tensorflow/core/framework/tensor_types.h" -#include "tensorflow/core/graph/algorithm.h" -#include "tensorflow/core/graph/graph.h" -#include "tensorflow/core/graph/graph_constructor.h" -#include "tensorflow/core/platform/env.h" -#include "tensorflow/core/public/session.h" -#include "test_utilities.h" -#include "tf_graph_writer.h" -using namespace std; -namespace ng = ngraph; -namespace tensorflow { -namespace ngraph_bridge { -namespace testing { -#define ASSERT_OK(x) ASSERT_EQ((x), ::tensorflow::Status::OK()); -// Test(TestCaseName, TestName) -// Please ensure -// Neither TestCaseName nor TestName should contain underscore -// https://github.com/google/googletest/blob/master/googletest/docs/primer.md -// Use only Tensors and ops::Const() to provide input to the test op -TEST(MathOps, Neg) { - // Create a tf graph - Scope root = Scope::NewRootScope(); - int dim1 = 2; - int dim2 = 2; - Tensor A(DT_FLOAT, TensorShape({dim1, dim2})); - AssignInputValues(A, -2.1f); - auto R = ops::Neg(root, A); - vector output_datatypes = {DT_FLOAT}; - std::vector sess_run_fetchoutputs = {R}; - OpExecuter opexecuter(root, "Neg", output_datatypes, sess_run_fetchoutputs); - opexecuter.ExecuteOnNGraph(); - opexecuter.ExecuteOnTF(); - opexecuter.CompareNgraphAndTF(); -} - -TEST(MathOps, Add) { - // Create a tf graph - Scope root = Scope::NewRootScope(); - int dim1 = 2; - int dim2 = 2; - Tensor A(DT_FLOAT, TensorShape({dim1, dim2})); - Tensor B(DT_FLOAT, TensorShape({dim1, dim2})); - AssignInputValues(A, 2.1f); - AssignInputValues(B, 4.1f); - auto R = ops::Add(root, A, B); - vector output_datatypes = {DT_FLOAT}; - std::vector sess_run_fetchoutputs = {R}; - OpExecuter opexecuter(root, "Add", output_datatypes, sess_run_fetchoutputs); - opexecuter.ExecuteOnNGraph(); - opexecuter.ExecuteOnTF(); - opexecuter.CompareNgraphAndTF(); -} -TEST(MathOps, RealDiv) { - Scope root = Scope::NewRootScope(); - int dim1 = 100; - int dim2 = 200; - Tensor A(DT_FLOAT, TensorShape({dim1, dim2})); - Tensor B(DT_FLOAT, TensorShape({dim1, dim2})); - AssignInputValues(A, 2.0f); - AssignInputValues(B, 7.0f); - auto R = ops::RealDiv(root, A, B); - vector output_datatypes = {DT_FLOAT}; - std::vector sess_run_fetchoutputs = {R}; - OpExecuter opexecuter(root, "RealDiv", output_datatypes, - sess_run_fetchoutputs); - opexecuter.ExecuteOnNGraph(); - opexecuter.ExecuteOnTF(); - opexecuter.CompareNgraphAndTF(); -} -} // namespace testing -} // namespace ngraph_bridge -} // namespace tensorflow diff --git a/test/test_utilities.cpp b/test/test_utilities.cpp deleted file mode 100644 index 2c9cb0e4..00000000 --- a/test/test_utilities.cpp +++ /dev/null @@ -1,77 +0,0 @@ -/******************************************************************************* - * Copyright 2017-2018 Intel Corporation - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - *******************************************************************************/ -#include "test_utilities.h" -using namespace std; -namespace ng = ngraph; -namespace tensorflow { -namespace ngraph_bridge { -void ActivateNGraph() { - setenv("NGRAPH_TF_DISABLE_DEASSIGN_CLUSTERS", "1", 1); - unsetenv("NGRAPH_TF_DISABLE"); -} -void DeactivateNGraph() { - unsetenv("NGRAPH_TF_DISABLE_DEASSIGN_CLUSTERS"); - setenv("NGRAPH_TF_DISABLE", "1", 1); -} -void AssertTensorEquals(Tensor& T1, Tensor& T2) { - auto T_size = T1.flat().size(); - auto T1_data = T1.flat().data(); - auto T2_data = T2.flat().data(); - for (int k = 0; k < T_size; k++) { - auto a = T1_data[k]; - auto b = T2_data[k]; - EXPECT_FLOAT_EQ(a, b); - } -} -void AssignInputIntValues(Tensor& A, int maxval) { - auto A_flat = A.flat(); - auto A_flat_data = A_flat.data(); - int counter = 0; - for (int i = 0; i < A_flat.size(); i++) { - A_flat_data[i] = counter++; - if (counter == maxval) { - counter = 0; - } - } -} -void AssignInputValues(Tensor& A, float x) { - auto A_flat = A.flat(); - auto A_flat_data = A_flat.data(); - for (int i = 0; i < A_flat.size(); i++) { - A_flat_data[i] = x; - } -} -void PrintTensor(const Tensor& T1) { - LOG(INFO) << "print tensor values" << T1.DebugString(); -} -void ValidateTensorData(Tensor& T1, Tensor& T2, float tol) { - auto T_size = T1.flat().size(); - auto T1_data = T1.flat().data(); - auto T2_data = T2.flat().data(); - for (int k = 0; k < T_size; k++) { - auto a = T1_data[k]; - auto b = T2_data[k]; - if (a == 0) { - EXPECT_NEAR(a, b, tol); - } else { - auto rel = a - b; - auto rel_div = std::abs(rel / a); - EXPECT_TRUE(rel_div < tol); - } - } -} -} // namespace ngraph_bridge -} // namespace tensorflow diff --git a/test/test_utilities.h b/test/test_utilities.h deleted file mode 100644 index 588f700f..00000000 --- a/test/test_utilities.h +++ /dev/null @@ -1,37 +0,0 @@ -/******************************************************************************* - * Copyright 2017-2018 Intel Corporation - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - *******************************************************************************/ -#ifndef NGRAPH_TF_BRIDGE_TESTUTILITIES_H_ -#define NGRAPH_TF_BRIDGE_TESTUTILITIES_H_ -#include "gtest/gtest.h" -#include "ngraph/ngraph.hpp" -#include "tensorflow/core/framework/tensor.h" -#include "tensorflow/core/framework/tensor_types.h" -#include "tensorflow/core/platform/env.h" -using namespace std; -namespace ng = ngraph; -namespace tensorflow { -namespace ngraph_bridge { -// some utility functions copied from tf_exec.cpp -void ActivateNGraph(); -void DeactivateNGraph(); -void AssertTensorEquals(Tensor& T1, Tensor& T2); -void AssignInputIntValues(Tensor& A, int maxval); -void AssignInputValues(Tensor& A, float x); -void PrintTensor(const Tensor& T1); -void ValidateTensorData(Tensor& T1, Tensor& T2, float tol); -} // namespace ngraph_bridge -} // namespace tensorflow -#endif // NGRAPH_TF_BRIDGE_TESTUTILITIES_H_ From dd39a79adff057ed95c2377fb03dcc56cd0949c0 Mon Sep 17 00:00:00 2001 From: Sayantan Sarkar Date: Wed, 29 Aug 2018 13:42:37 -0700 Subject: [PATCH 23/32] More cleanup --- test/tf_exec.cpp | 33 +++++++-------------------------- 1 file changed, 7 insertions(+), 26 deletions(-) diff --git a/test/tf_exec.cpp b/test/tf_exec.cpp index 9a860576..b4e7ee7c 100644 --- a/test/tf_exec.cpp +++ b/test/tf_exec.cpp @@ -807,31 +807,24 @@ TEST(tf_exec, Op_Rsqrt) { TEST(tf_exec, Op_Negate) { Scope scope_cpu = Scope::NewRootScope(); - Scope scope_ng = scope_cpu; // scope_cpu.WithDevice("/device:NGRAPH:0"); + Scope scope_ng = scope_cpu.WithDevice("/device:NGRAPH:0"); - ActivateNGraph(); // ngraph execution auto A_ng = ops::Const(scope_ng, {{-256.f, 16.5f}, {0.f, 64.f}}); auto r_ng = ops::Negate(scope_ng.WithOpName("r"), A_ng); std::vector outputs_ng; - - SessionOptions options; - options.config.mutable_graph_options() - ->mutable_optimizer_options() - ->set_opt_level(OptimizerOptions_Level_L0); - ClientSession session_ng(scope_ng, options); + ClientSession session_ng(scope_ng); ASSERT_OK(session_ng.Run({r_ng}, &outputs_ng)); ASSERT_EQ(outputs_ng[0].shape(), TensorShape({2, 2})); - DeactivateNGraph(); // reference CPU execution auto A_cpu = ops::Const(scope_cpu, {{-256.f, 16.5f}, {0.f, 64.f}}); auto r_cpu = ops::Negate(scope_cpu.WithOpName("r"), A_cpu); std::vector outputs_cpu; - ClientSession session_cpu(scope_cpu, options); + ClientSession session_cpu(scope_cpu); ASSERT_OK(session_cpu.Run({r_cpu}, &outputs_cpu)); ASSERT_EQ(outputs_cpu[0].shape(), TensorShape({2, 2})); @@ -841,9 +834,8 @@ TEST(tf_exec, Op_Negate) { TEST(tf_exec, Op_FloorDiv) { Scope scope_cpu = Scope::NewRootScope(); - Scope scope_ng = scope_cpu; // scope_cpu.WithDevice("/device:NGRAPH:0"); + Scope scope_ng = scope_cpu.WithDevice("/device:NGRAPH:0"); - DeactivateNGraph(); // ngraph execution auto A_ng = ops::Const(scope_ng, {{5.f, 6.f, 7.5f, -1.f, 2.f, -3.f}, {1.3f, 1.f, -5.f, -3.f, 0.f, -2.f}}); @@ -856,14 +848,11 @@ TEST(tf_exec, Op_FloorDiv) { std::vector outputs_ng; ClientSession session_ng(scope_ng); - cout << "XXXXX1\n"; ASSERT_OK(session_ng.Run({r0_ng, r1_ng}, &outputs_ng)); - cout << "XXXXX2\n"; ASSERT_EQ(outputs_ng[0].shape(), TensorShape({2, 6})); ASSERT_EQ(outputs_ng[1].shape(), TensorShape({2, 6})); - DeactivateNGraph(); // reference CPU execution auto A_cpu = ops::Const(scope_cpu, {{5.f, 6.f, 7.5f, -1.f, 2.f, -3.f}, {1.3f, 1.f, -5.f, -3.f, 0.f, -2.f}}); @@ -927,9 +916,8 @@ TEST(tf_exec, Op_FloorMod) { TEST(tf_exec, Op_AddN) { Scope scope_cpu = Scope::NewRootScope(); - Scope scope_ng = scope_cpu; // scope_cpu.WithDevice("/device:NGRAPH:0"); + Scope scope_ng = scope_cpu.WithDevice("/device:NGRAPH:0"); - ActivateNGraph(); // ngraph execution auto A_ng = ops::Const(scope_ng, {{256.f, 16.f}, {4.f, 64.f}}); auto B_ng = ops::Const(scope_ng, {{1.f, 2.f}, {3.f, 4.f}}); @@ -939,19 +927,12 @@ TEST(tf_exec, Op_AddN) { // No broadcast test needed since AddN does not support it: // https://github.com/tensorflow/tensorflow/blob/master/tensorflow/core/ops/math_ops.cc#L355 - SessionOptions options; - options.config.mutable_graph_options() - ->mutable_optimizer_options() - ->set_opt_level(OptimizerOptions_Level_L0); - ClientSession session_ng(scope_ng, options); - std::vector outputs_ng; - // ClientSession session_ng(scope_ng); + ClientSession session_ng(scope_ng); ASSERT_OK(session_ng.Run({r_ng}, &outputs_ng)); ASSERT_EQ(outputs_ng[0].shape(), TensorShape({2, 2})); - DeactivateNGraph(); // reference CPU execution auto A_cpu = ops::Const(scope_cpu, {{256.f, 16.f}, {4.f, 64.f}}); auto B_cpu = ops::Const(scope_cpu, {{1.f, 2.f}, {3.f, 4.f}}); @@ -960,7 +941,7 @@ TEST(tf_exec, Op_AddN) { ops::AddN(scope_cpu.WithOpName("r"), {A_cpu, C_cpu, B_cpu, A_cpu, A_cpu}); std::vector outputs_cpu; - ClientSession session_cpu(scope_cpu, options); + ClientSession session_cpu(scope_cpu); ASSERT_OK(session_cpu.Run({r_cpu}, &outputs_cpu)); ASSERT_EQ(outputs_cpu[0].shape(), TensorShape({2, 2})); From 9852f7d1699454e587ec591a9adffbe947346d27 Mon Sep 17 00:00:00 2001 From: Sayantan Sarkar Date: Wed, 29 Aug 2018 14:11:49 -0700 Subject: [PATCH 24/32] Going back to old builder --- src/ngraph_encapsulate_op.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ngraph_encapsulate_op.cc b/src/ngraph_encapsulate_op.cc index 56ba1bc2..ef1459e8 100644 --- a/src/ngraph_encapsulate_op.cc +++ b/src/ngraph_encapsulate_op.cc @@ -37,7 +37,7 @@ #include "ngraph/runtime/interpreter/int_backend.hpp" -#define BUILDER1 // (un)comment this line to (de)activate old builder +//#define BUILDER1 // (un)comment this line to (de)activate old builder #ifdef BUILDER1 #include "ngraph_builder1.h" From 59deb18f8e0e56db3f3c5146882a90031f755e69 Mon Sep 17 00:00:00 2001 From: Sayantan Sarkar Date: Wed, 29 Aug 2018 14:29:36 -0700 Subject: [PATCH 25/32] Applying style --- src/ngraph_builder1.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/ngraph_builder1.h b/src/ngraph_builder1.h index c68aa0c2..7c9b7496 100644 --- a/src/ngraph_builder1.h +++ b/src/ngraph_builder1.h @@ -35,7 +35,6 @@ namespace ngraph_bridge { // TODO (sarkars): make sure all comments from old builder are copied correctly. - // The purpose of Builder is to accept a TF graph and convert it to a // equivalent ngraph fucntion. To that end, it acts as an interface to // a library of 'TranslateOps' functions. These functions are defined From fb00018edd013d4ce47644953e49eaf53f7eb30c Mon Sep 17 00:00:00 2001 From: Sayantan Sarkar Date: Wed, 29 Aug 2018 15:42:47 -0700 Subject: [PATCH 26/32] Low hanging fruits --- src/ngraph_builder1.cc | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/src/ngraph_builder1.cc b/src/ngraph_builder1.cc index c78bdce8..ff1ba7b9 100644 --- a/src/ngraph_builder1.cc +++ b/src/ngraph_builder1.cc @@ -321,13 +321,32 @@ const std::map Builder1::TRANSLATE_OP_MAP{ {"AddN", TranslateAddNOp}, {"AvgPool", TranslateAvgPoolOp}, {"Const", TranslateConstOp}, + {"Equal", TranslateBinary}, + {"Exp", TranslateUnary}, + {"Floor", TranslateUnary}, {"FloorDiv", TranslateFloorDivOp}, {"FloorMod", TranslateFloorModOp}, + {"Greater", TranslateBinary}, + {"GreaterEqual", TranslateBinary}, + {"Less", TranslateBinary}, + {"LessEqual", TranslateBinary}, + {"Log", TranslateUnary}, + {"LogicalAnd", TranslateBinary}, + {"LogicalNot", TranslateUnary}, + {"Maximum", TranslateBinary}, + {"Minimum", TranslateBinary}, + {"Mul", TranslateBinary}, + {"Neg", TranslateUnary}, {"Neg", TranslateUnary}, {"NoOp", [](const Node* op, VectNg& ng_arg_vec, const std::vector& static_input_map, VectNg& subgraph_out_nodes) { return Status::OK(); }}, - {"RealDiv", TranslateBinary}}; + {"Pow", TranslateBinary}, + {"RealDiv", TranslateBinary}, + {"Sign", TranslateUnary}, + {"Sqrt", TranslateUnary}, + {"Sub", TranslateBinary}, + {"Tanh", TranslateUnary}}; const std::map> Builder1::INPUT_INDEX_MAP{}; From b258f06ae13b369880913b168b950be4477ddb77 Mon Sep 17 00:00:00 2001 From: Sayantan Sarkar Date: Thu, 30 Aug 2018 23:43:01 -0700 Subject: [PATCH 27/32] Resolving some of the review comments --- src/ngraph_builder1.cc | 24 +++++++++++++----------- src/ngraph_translateops.h | 11 ----------- 2 files changed, 13 insertions(+), 22 deletions(-) diff --git a/src/ngraph_builder1.cc b/src/ngraph_builder1.cc index ff1ba7b9..38b0316d 100644 --- a/src/ngraph_builder1.cc +++ b/src/ngraph_builder1.cc @@ -25,13 +25,13 @@ namespace tensorflow { namespace ngraph_bridge { Status Builder1::TranslateGraph( - const std::vector& inputs, + const std::vector& input_shapes, const std::vector& static_input_map, shared_ptr& ng_function) { TF_RETURN_IF_ERROR(Initialize()); vector> ng_parameter_list; - TF_RETURN_IF_ERROR(GetInputParams(inputs, tf_params, ng_parameter_list)); + TF_RETURN_IF_ERROR(GetInputParams(input_shapes, tf_params, ng_parameter_list)); TF_RETURN_IF_ERROR(TranslateEachOp(tf_ops, static_input_map)); @@ -41,8 +41,12 @@ Status Builder1::TranslateGraph( // Create the nGraph function. try { ng_function = make_shared(ng_result_list, ng_parameter_list); + } catch (const std::exception& e) { + return errors::Internal( + "Exception thrown while constructing nGraph function: ", e.what()); } catch (...) { - return errors::Unimplemented("Unable to create nGraph function"); + return errors::Internal( + "Exception of unknown type thrown while constructing nGraph function"); } // @@ -58,20 +62,18 @@ Status Builder1::TranslateGraph( OpKernelContext* ctx, std::shared_ptr& ng_function) { TF_RETURN_IF_ERROR(Initialize()); - std::vector static_input_map; + std::vector static_input_map(ctx->num_inputs()); + std::vector input_shapes(ctx->num_inputs()); - std::vector inputs(ctx->num_inputs()); - - static_input_map.resize(ctx->num_inputs()); for (int i = 0; i < ctx->num_inputs(); i++) { const Tensor& input_tensor = ctx->input(i); if (m_input_is_static[i]) { static_input_map[i] = &input_tensor; } - inputs[i] = input_tensor.shape(); + input_shapes[i] = input_tensor.shape(); } - return TranslateGraph(inputs, static_input_map, ng_function); + return TranslateGraph(input_shapes, static_input_map, ng_function); } Status Builder1::TranslateEachOp( @@ -131,7 +133,7 @@ Status Builder1::ClassifyNodes(const vector& ordered) { } Status Builder1::GetInputParams( - const std::vector& inputs, vector tf_params, + const std::vector& input_shapes, vector tf_params, vector>& ng_parameter_list) { // Populate the parameter list, and also put parameters into the op map. @@ -151,7 +153,7 @@ Status Builder1::GetInputParams( TF_RETURN_IF_ERROR(TFDataTypeToNGraphElementType(dtype, &ng_et)); ng::Shape ng_shape; - TF_RETURN_IF_ERROR(TFTensorShapeToNGraphShape(inputs[index], &ng_shape)); + TF_RETURN_IF_ERROR(TFTensorShapeToNGraphShape(input_shapes[index], &ng_shape)); auto ng_param = make_shared(ng_et, ng_shape); ng_op_map[parm->name()].push_back(ng_param); diff --git a/src/ngraph_translateops.h b/src/ngraph_translateops.h index d3242cc4..4ce4d573 100644 --- a/src/ngraph_translateops.h +++ b/src/ngraph_translateops.h @@ -57,7 +57,6 @@ void MakePadding(const std::string& tf_padding_type, T& ng_padding_above) { if (tf_padding_type == "SAME") { for (size_t i = 0; i < 2; i++) { - /* size_t image_size = ng_image_shape[i]; size_t filter_shape = ng_kernel_shape[i]; size_t filter_stride = ng_strides[i]; @@ -71,16 +70,6 @@ void MakePadding(const std::string& tf_padding_type, if (padding_needed < 0) { padding_needed = 0; } - */ - - // TODO: check this:. This formula is documented well documented here. - // So I prefer this, compared to the one above (though both are exactly - // the same) - // https://www.tensorflow.org/api_guides/python/nn#Notes_on_SAME_Convolution_Padding - int64 out_shape = ceil(ng_image_shape[i] / ng_strides[i]); - int64 padding_needed = ng_strides[i] * (out_shape - 1) + - ng_kernel_shape[i] - ng_image_shape[i]; - padding_needed = padding_needed < 0 ? 0 : padding_needed; size_t padding_lhs = padding_needed / 2; size_t padding_rhs = padding_needed - padding_lhs; From 1888cc7eab182db083212adbbca73e16f65bec98 Mon Sep 17 00:00:00 2001 From: Sayantan Sarkar Date: Thu, 30 Aug 2018 23:43:53 -0700 Subject: [PATCH 28/32] Apply style --- src/ngraph_builder1.cc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/ngraph_builder1.cc b/src/ngraph_builder1.cc index 38b0316d..6ba62d56 100644 --- a/src/ngraph_builder1.cc +++ b/src/ngraph_builder1.cc @@ -31,7 +31,8 @@ Status Builder1::TranslateGraph( TF_RETURN_IF_ERROR(Initialize()); vector> ng_parameter_list; - TF_RETURN_IF_ERROR(GetInputParams(input_shapes, tf_params, ng_parameter_list)); + TF_RETURN_IF_ERROR( + GetInputParams(input_shapes, tf_params, ng_parameter_list)); TF_RETURN_IF_ERROR(TranslateEachOp(tf_ops, static_input_map)); @@ -153,7 +154,8 @@ Status Builder1::GetInputParams( TF_RETURN_IF_ERROR(TFDataTypeToNGraphElementType(dtype, &ng_et)); ng::Shape ng_shape; - TF_RETURN_IF_ERROR(TFTensorShapeToNGraphShape(input_shapes[index], &ng_shape)); + TF_RETURN_IF_ERROR( + TFTensorShapeToNGraphShape(input_shapes[index], &ng_shape)); auto ng_param = make_shared(ng_et, ng_shape); ng_op_map[parm->name()].push_back(ng_param); From 09309e89725034de686c67cd2abc7282d7e5abc0 Mon Sep 17 00:00:00 2001 From: Sayantan Sarkar Date: Fri, 31 Aug 2018 00:22:25 -0700 Subject: [PATCH 29/32] Adding namespace comments and initializing bool in ctor --- src/ngraph_builder1.h | 4 ++-- src/ngraph_translateops.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/ngraph_builder1.h b/src/ngraph_builder1.h index 7c9b7496..bd309412 100644 --- a/src/ngraph_builder1.h +++ b/src/ngraph_builder1.h @@ -61,7 +61,7 @@ class Builder1 { // This constructor is for actual use (encapsulate_op) Builder1(const Graph& tf_graph, OpKernelConstruction* ctx) - : tf_graph(tf_graph) {} + : tf_graph(tf_graph), is_initialized(false) {} // And this version is for tests (OpExecuter) Builder1(const Graph& tf_graph) : Builder1(tf_graph, nullptr) {} @@ -83,7 +83,7 @@ class Builder1 { std::unordered_map ng_op_map; // Prevent Initialize from running twice - bool is_initialized = false; + bool is_initialized; const Graph& tf_graph; // An array that will be useful in creating the static_input_map diff --git a/src/ngraph_translateops.h b/src/ngraph_translateops.h index 4ce4d573..8947981d 100644 --- a/src/ngraph_translateops.h +++ b/src/ngraph_translateops.h @@ -254,5 +254,5 @@ Status TranslateFloorModOp(const Node* op, VectNg& ng_arg_vec, // ng_arg_vec[1]); return Status::OK(); } -} -} \ No newline at end of file +} // namespace ngraph_bridge +} // namespace tensorflow \ No newline at end of file From efffaac04b95f3a6fefdb6e1bf820572283883e4 Mon Sep 17 00:00:00 2001 From: Sayantan Sarkar Date: Fri, 31 Aug 2018 11:13:53 -0700 Subject: [PATCH 30/32] Adding more review comments, not passing some class members in methods --- src/ngraph_builder1.cc | 68 +++++++++++++++++++-------------------- src/ngraph_builder1.h | 23 +++++++------ src/ngraph_translateops.h | 30 +++++++++-------- 3 files changed, 61 insertions(+), 60 deletions(-) diff --git a/src/ngraph_builder1.cc b/src/ngraph_builder1.cc index 6ba62d56..20a02e93 100644 --- a/src/ngraph_builder1.cc +++ b/src/ngraph_builder1.cc @@ -31,13 +31,12 @@ Status Builder1::TranslateGraph( TF_RETURN_IF_ERROR(Initialize()); vector> ng_parameter_list; - TF_RETURN_IF_ERROR( - GetInputParams(input_shapes, tf_params, ng_parameter_list)); + TF_RETURN_IF_ERROR(GetInputParams(input_shapes, ng_parameter_list)); - TF_RETURN_IF_ERROR(TranslateEachOp(tf_ops, static_input_map)); + TF_RETURN_IF_ERROR(TranslateEachOp(static_input_map)); - VectNg ng_result_list; - TF_RETURN_IF_ERROR(GetOutputNodes(tf_ret_vals, ng_result_list)); + vector> ng_result_list; + TF_RETURN_IF_ERROR(GetOutputNodes(ng_result_list)); // Create the nGraph function. try { @@ -78,10 +77,9 @@ Status Builder1::TranslateGraph( } Status Builder1::TranslateEachOp( - const vector& tf_ops, const std::vector& static_input_map) { // Create the nGraph ops from TensorFlow ops. - for (auto op : tf_ops) { + for (auto op : m_tf_ops) { NGRAPH_VLOG(2) << "Constructing op " << op->name() << " which is " << op->type_string(); @@ -91,11 +89,11 @@ Status Builder1::TranslateEachOp( GetOpTranslationRequirements(op, translate_fn, input_indexes)); // input_indexes can be size 0 (to indicate/handle variadic inputs nodes // like Addn) - VectNg subgraph_out_nodes(op->num_outputs()); + vector> subgraph_out_nodes(op->num_outputs()); bool variadic_input = input_indexes.size() == 0; int num_inputs = variadic_input ? op->num_inputs() : input_indexes.size(); - VectNg ng_arg_vec(num_inputs); + vector> ng_arg_vec(num_inputs); if (op->type_string() != "Const") { for (int idx = 0; idx < num_inputs; idx++) { TF_RETURN_IF_ERROR(GetInputNode( @@ -105,7 +103,7 @@ Status Builder1::TranslateEachOp( TF_RETURN_IF_ERROR( translate_fn(op, ng_arg_vec, static_input_map, subgraph_out_nodes)); - ng_op_map[op->name()] = subgraph_out_nodes; + m_ng_op_map[op->name()] = subgraph_out_nodes; } return Status::OK(); } @@ -123,24 +121,23 @@ Status Builder1::ClassifyNodes(const vector& ordered) { n->DebugString()); } if (n->type_string() == "_Arg") { - tf_params.push_back(n); + m_tf_params.push_back(n); } else if (n->type_string() == "_Retval") { - tf_ret_vals.push_back(n); + m_tf_ret_vals.push_back(n); } else { - tf_ops.push_back(n); + m_tf_ops.push_back(n); } } return Status::OK(); } Status Builder1::GetInputParams( - const std::vector& input_shapes, vector tf_params, + const std::vector& input_shapes, vector>& ng_parameter_list) { // Populate the parameter list, and also put parameters into the op map. + ng_parameter_list.resize(m_tf_params.size()); - ng_parameter_list.resize(tf_params.size()); - - for (auto parm : tf_params) { + for (auto parm : m_tf_params) { DataType dtype; if (GetNodeAttr(parm->attrs(), "T", &dtype) != Status::OK()) { return errors::InvalidArgument("No data type defined for _Arg"); @@ -158,19 +155,18 @@ Status Builder1::GetInputParams( TFTensorShapeToNGraphShape(input_shapes[index], &ng_shape)); auto ng_param = make_shared(ng_et, ng_shape); - ng_op_map[parm->name()].push_back(ng_param); + m_ng_op_map[parm->name()].push_back(ng_param); ng_parameter_list[index] = ng_param; } return Status::OK(); } -Status Builder1::GetOutputNodes(const vector& tf_ret_vals, - VectNg& ng_result_list) { +Status Builder1::GetOutputNodes(vector>& ng_result_list) { // Populate the result list. - ng_result_list.resize(tf_ret_vals.size()); + ng_result_list.resize(m_tf_ret_vals.size()); - for (auto n : tf_ret_vals) { + for (auto n : m_tf_ret_vals) { // Make sure that this _Retval only has one input node. if (n->num_inputs() != 1) { return errors::InvalidArgument("_Retval has ", n->num_inputs(), @@ -191,14 +187,14 @@ Status Builder1::GetOutputNodes(const vector& tf_ret_vals, } Status Builder1::Initialize() { - if (!is_initialized) { + if (!m_is_initialized) { // // We will visit ops in topological order. // // ought to be `const Node*`, but GetReversePostOrder doesn't use `const` vector ordered; - GetReversePostOrder(tf_graph, &ordered); + GetReversePostOrder(m_tf_graph, &ordered); TF_RETURN_IF_ERROR(ClassifyNodes(ordered)); // @@ -213,7 +209,7 @@ Status Builder1::Initialize() { int32 max_arg_index = -1; std::vector arg_nodes; - for (auto node : tf_graph.nodes()) { + for (auto node : m_tf_graph.nodes()) { if (node->type_string() == "_Arg") { arg_nodes.push_back(node); int32 index; @@ -246,7 +242,7 @@ Status Builder1::Initialize() { NGRAPH_VLOG(5) << "Marking arg " << index << " is static: " << m_input_is_static[index]; } - is_initialized = true; + m_is_initialized = true; } return Status::OK(); } @@ -266,7 +262,7 @@ Status Builder1::GetInputNode(const Node* op, size_t input_idx, Node* tf_input; TF_RETURN_IF_ERROR(op->input_node(input_idx, &tf_input)); try { - *result = ng_op_map.at(tf_input->name()).at(src_output_idx); + *result = m_ng_op_map.at(tf_input->name()).at(src_output_idx); } catch (const out_of_range&) { return Status(tensorflow::error::NOT_FOUND, "Input node not found"); } @@ -285,9 +281,9 @@ Status Builder1::GetInputNode(const Node* op, size_t input_idx, // } // template -Status TranslateBinary(const Node* op, VectNg& ng_arg_vec, +Status TranslateBinary(const Node* op, vector>& ng_arg_vec, const std::vector& static_input_map, - VectNg& subgraph_out_nodes) { + vector>& subgraph_out_nodes) { if (subgraph_out_nodes.size() != 1) return errors::InvalidArgument( "TranslateBinary call for ", op->type_string(), " expects ", @@ -303,9 +299,10 @@ Status TranslateBinary(const Node* op, VectNg& ng_arg_vec, } template -Status TranslateUnary(const Node* op, const VectNg& ng_arg_vec, +Status TranslateUnary(const Node* op, + const vector>& ng_arg_vec, const std::vector& static_input_map, - VectNg& subgraph_out_nodes) { + vector>& subgraph_out_nodes) { if (subgraph_out_nodes.size() != 1) return errors::InvalidArgument( "TranslateUnary call for ", op->type_string(), " expects ", @@ -342,9 +339,12 @@ const std::map Builder1::TRANSLATE_OP_MAP{ {"Mul", TranslateBinary}, {"Neg", TranslateUnary}, {"Neg", TranslateUnary}, - {"NoOp", [](const Node* op, VectNg& ng_arg_vec, - const std::vector& static_input_map, - VectNg& subgraph_out_nodes) { return Status::OK(); }}, + {"NoOp", + [](const Node* op, vector>& ng_arg_vec, + const std::vector& static_input_map, + vector>& subgraph_out_nodes) { + return Status::OK(); + }}, {"Pow", TranslateBinary}, {"RealDiv", TranslateBinary}, {"Sign", TranslateUnary}, diff --git a/src/ngraph_builder1.h b/src/ngraph_builder1.h index bd309412..25291715 100644 --- a/src/ngraph_builder1.h +++ b/src/ngraph_builder1.h @@ -45,11 +45,11 @@ namespace ngraph_bridge { // order, keep track of and retrieve parent nGraph nodes etc. class Builder1 { - using VectNg = std::vector>; + public: using TranslatorFn = function&, VectNg&)>; + const Node*, vector>&, + const std::vector&, vector>&)>; - public: // Note: in case we want to get rid of Initialize, pass an // OpKernelConstruction* ctx to the constructor, and use OP_REQUIRES_OK // instead of TF_RETURN_IF_ERROR. This gets rid of Initialize() as it @@ -61,7 +61,7 @@ class Builder1 { // This constructor is for actual use (encapsulate_op) Builder1(const Graph& tf_graph, OpKernelConstruction* ctx) - : tf_graph(tf_graph), is_initialized(false) {} + : m_tf_graph(tf_graph), m_is_initialized(false) {} // And this version is for tests (OpExecuter) Builder1(const Graph& tf_graph) : Builder1(tf_graph, nullptr) {} @@ -80,17 +80,17 @@ class Builder1 { // vector of generated nGraph nodes. Since we process nodes in toposort // order, it is guaranteed when processing a TF node, its parents // will already have been processed and safely stowed in ng_op_map - std::unordered_map ng_op_map; + std::unordered_map>> m_ng_op_map; // Prevent Initialize from running twice - bool is_initialized; - const Graph& tf_graph; + bool m_is_initialized; + const Graph& m_tf_graph; // An array that will be useful in creating the static_input_map std::vector m_input_is_static; // Vectors containing TF nodes in 3 bins: inputs, outputs, actual ops - vector tf_params, tf_ret_vals, tf_ops; + vector m_tf_params, m_tf_ret_vals, m_tf_ops; // This map tells us which inputs to read for a particular node. If no // information is present explicitly in the map, we read all inputs @@ -114,17 +114,16 @@ class Builder1 { // Given the input shapes and a list of TF _Arg nodes, create corresponding // nGraph parameters. Also populate the ng_op_map - Status GetInputParams(const std::vector&, vector, + Status GetInputParams(const std::vector&, vector>&); // Given a TF node, retrieve its corresponding nGraph nodes (using ng_op_map), // then call the appropriate TranslateOp function - Status TranslateEachOp(const vector&, - const std::vector&); + Status TranslateEachOp(const std::vector&); // After each TF op has been translated, find the nGraph nodes corresponding // to the _Retval nodes - Status GetOutputNodes(const vector&, VectNg&); + Status GetOutputNodes(vector>&); // Since the constructor does not return Status, delegating its job to a // separate function that is evaluated lazily and only once. diff --git a/src/ngraph_translateops.h b/src/ngraph_translateops.h index 8947981d..65d3e32b 100644 --- a/src/ngraph_translateops.h +++ b/src/ngraph_translateops.h @@ -14,6 +14,7 @@ * limitations under the License. *******************************************************************************/ +#include "ngraph_builder1.h" #include "ngraph_conversions.h" #include "ngraph_utils.h" @@ -24,8 +25,6 @@ namespace tensorflow { namespace ngraph_bridge { -using VectNg = std::vector>; - // TODO (sarkars): combine the 2 MakePaddings together? with default args? // would want the default args to be at the end of the paramlist, // but 'outputs' (like ng_padding_below, ng_padding_above) are usually at the @@ -100,9 +99,9 @@ Status ValidateInputCountMin(const Node* op, size_t count) { return Status::OK(); } -Status TranslateAddNOp(const Node* op, VectNg& ng_arg_vec, +Status TranslateAddNOp(const Node* op, vector>& ng_arg_vec, const std::vector& static_input_map, - VectNg& subgraph_out_nodes) { + vector>& subgraph_out_nodes) { subgraph_out_nodes[0] = std::accumulate(std::next(ng_arg_vec.begin()), ng_arg_vec.end(), ng_arg_vec.at(0)); // accumulation: start with first @@ -111,9 +110,10 @@ Status TranslateAddNOp(const Node* op, VectNg& ng_arg_vec, } // ng_arg_vec is not const. For example, BatchToNGraph changes it -Status TranslateAvgPoolOp(const Node* op, VectNg& ng_arg_vec, +Status TranslateAvgPoolOp(const Node* op, + vector>& ng_arg_vec, const std::vector& static_input_map, - VectNg& subgraph_out_nodes) { + vector>& subgraph_out_nodes) { std::vector tf_strides; std::vector tf_ksize; std::string tf_padding_type; @@ -188,9 +188,9 @@ Status MakeConstOp(const Node* op, ng::element::Type et, } static Status TranslateConstOp( - const Node* op, VectNg& ng_arg_vec, + const Node* op, vector>& ng_arg_vec, const std::vector& static_input_map, - VectNg& subgraph_out_nodes) { + vector>& subgraph_out_nodes) { const static std::map< const DataType, const std::pair>& ng_arg_vec, const std::vector& static_input_map, - VectNg& subgraph_out_nodes) { + vector>& subgraph_out_nodes) { subgraph_out_nodes[0] = std::make_shared(ng_arg_vec[0] / ng_arg_vec[1]); return Status::OK(); } -Status TranslateFloorModOp(const Node* op, VectNg& ng_arg_vec, +Status TranslateFloorModOp(const Node* op, + vector>& ng_arg_vec, const std::vector& static_input_map, - VectNg& subgraph_out_nodes) { + vector>& subgraph_out_nodes) { auto floordiv = std::make_shared(ng_arg_vec[0] / ng_arg_vec[1]); subgraph_out_nodes[0] = ng_arg_vec[0] - (floordiv * ng_arg_vec[1]); @@ -254,5 +256,5 @@ Status TranslateFloorModOp(const Node* op, VectNg& ng_arg_vec, // ng_arg_vec[1]); return Status::OK(); } -} // namespace ngraph_bridge -} // namespace tensorflow \ No newline at end of file +} // namespace ngraph_bridge +} // namespace tensorflow \ No newline at end of file From f9568fc59a912a39828af5d18a0c5bf14dd7a62f Mon Sep 17 00:00:00 2001 From: Sayantan Sarkar Date: Fri, 31 Aug 2018 11:43:26 -0700 Subject: [PATCH 31/32] Incorporating more review comments --- src/ngraph_builder1.cc | 4 +++- src/ngraph_builder1.h | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/ngraph_builder1.cc b/src/ngraph_builder1.cc index 20a02e93..a14085db 100644 --- a/src/ngraph_builder1.cc +++ b/src/ngraph_builder1.cc @@ -100,6 +100,8 @@ Status Builder1::TranslateEachOp( op, (variadic_input ? idx : input_indexes[idx]), &ng_arg_vec[idx])); } } + // TODO (sarkars): Instead of passing static_input_map, + // pass extracted tensors TF_RETURN_IF_ERROR( translate_fn(op, ng_arg_vec, static_input_map, subgraph_out_nodes)); @@ -355,7 +357,7 @@ const std::map Builder1::TRANSLATE_OP_MAP{ const std::map> Builder1::INPUT_INDEX_MAP{}; Status Builder1::GetOpTranslationRequirements( - const Node* op, Builder1::TranslatorFn& translate_fn, + const Node*& op, Builder1::TranslatorFn& translate_fn, vector& input_indexes) { // This function wraps TRANSLATE_OP_MAP. // It returns a translate function and input indexes diff --git a/src/ngraph_builder1.h b/src/ngraph_builder1.h index 25291715..82fd8c75 100644 --- a/src/ngraph_builder1.h +++ b/src/ngraph_builder1.h @@ -102,7 +102,7 @@ class Builder1 { // Given a TF node, return its corresponding TranslateOp function and required // input indexes. A wrapper for TRANSLATE_OP_MAP and INPUT_INDEX_MAP - Status GetOpTranslationRequirements(const Node*, Builder1::TranslatorFn&, + Status GetOpTranslationRequirements(const Node*&, Builder1::TranslatorFn&, vector&); // Given a TF node, an index i, it returns the ith nGraph input From 407c2959787b5ef25b3c0a91bd6a511afa057968 Mon Sep 17 00:00:00 2001 From: Sayantan Sarkar Date: Fri, 31 Aug 2018 13:40:30 -0700 Subject: [PATCH 32/32] Lowercasing static vars --- src/ngraph_builder1.cc | 87 +++++++++++++++++++++--------------------- src/ngraph_builder1.h | 7 ++-- 2 files changed, 48 insertions(+), 46 deletions(-) diff --git a/src/ngraph_builder1.cc b/src/ngraph_builder1.cc index a14085db..345915f4 100644 --- a/src/ngraph_builder1.cc +++ b/src/ngraph_builder1.cc @@ -318,53 +318,54 @@ Status TranslateUnary(const Node* op, return Status::OK(); } -const std::map Builder1::TRANSLATE_OP_MAP{ - {"Abs", TranslateUnary}, - {"Add", TranslateBinary}, - {"AddN", TranslateAddNOp}, - {"AvgPool", TranslateAvgPoolOp}, - {"Const", TranslateConstOp}, - {"Equal", TranslateBinary}, - {"Exp", TranslateUnary}, - {"Floor", TranslateUnary}, - {"FloorDiv", TranslateFloorDivOp}, - {"FloorMod", TranslateFloorModOp}, - {"Greater", TranslateBinary}, - {"GreaterEqual", TranslateBinary}, - {"Less", TranslateBinary}, - {"LessEqual", TranslateBinary}, - {"Log", TranslateUnary}, - {"LogicalAnd", TranslateBinary}, - {"LogicalNot", TranslateUnary}, - {"Maximum", TranslateBinary}, - {"Minimum", TranslateBinary}, - {"Mul", TranslateBinary}, - {"Neg", TranslateUnary}, - {"Neg", TranslateUnary}, - {"NoOp", - [](const Node* op, vector>& ng_arg_vec, - const std::vector& static_input_map, - vector>& subgraph_out_nodes) { - return Status::OK(); - }}, - {"Pow", TranslateBinary}, - {"RealDiv", TranslateBinary}, - {"Sign", TranslateUnary}, - {"Sqrt", TranslateUnary}, - {"Sub", TranslateBinary}, - {"Tanh", TranslateUnary}}; - -const std::map> Builder1::INPUT_INDEX_MAP{}; +const std::map + Builder1::s_translate_op_map{ + {"Abs", TranslateUnary}, + {"Add", TranslateBinary}, + {"AddN", TranslateAddNOp}, + {"AvgPool", TranslateAvgPoolOp}, + {"Const", TranslateConstOp}, + {"Equal", TranslateBinary}, + {"Exp", TranslateUnary}, + {"Floor", TranslateUnary}, + {"FloorDiv", TranslateFloorDivOp}, + {"FloorMod", TranslateFloorModOp}, + {"Greater", TranslateBinary}, + {"GreaterEqual", TranslateBinary}, + {"Less", TranslateBinary}, + {"LessEqual", TranslateBinary}, + {"Log", TranslateUnary}, + {"LogicalAnd", TranslateBinary}, + {"LogicalNot", TranslateUnary}, + {"Maximum", TranslateBinary}, + {"Minimum", TranslateBinary}, + {"Mul", TranslateBinary}, + {"Neg", TranslateUnary}, + {"Neg", TranslateUnary}, + {"NoOp", + [](const Node* op, vector>& ng_arg_vec, + const std::vector& static_input_map, + vector>& subgraph_out_nodes) { + return Status::OK(); + }}, + {"Pow", TranslateBinary}, + {"RealDiv", TranslateBinary}, + {"Sign", TranslateUnary}, + {"Sqrt", TranslateUnary}, + {"Sub", TranslateBinary}, + {"Tanh", TranslateUnary}}; + +const std::map> Builder1::s_input_index_map{}; Status Builder1::GetOpTranslationRequirements( const Node*& op, Builder1::TranslatorFn& translate_fn, vector& input_indexes) { - // This function wraps TRANSLATE_OP_MAP. + // This function wraps s_translate_op_map. // It returns a translate function and input indexes - // The translate function MUST be present in TRANSLATE_OP_MAP + // The translate function MUST be present in s_translate_op_map // input_idx may not be present, since it can be inferred from op - auto iter_fn = TRANSLATE_OP_MAP.find(op->type_string()); - if (iter_fn != TRANSLATE_OP_MAP.end()) { + auto iter_fn = s_translate_op_map.find(op->type_string()); + if (iter_fn != s_translate_op_map.end()) { translate_fn = iter_fn->second; } else { // ----------------------------- @@ -377,8 +378,8 @@ Status Builder1::GetOpTranslationRequirements( op->type_string(), ")"); } - auto iter_input_indexes = INPUT_INDEX_MAP.find(op->type_string()); - if (iter_input_indexes != INPUT_INDEX_MAP.end()) { + auto iter_input_indexes = s_input_index_map.find(op->type_string()); + if (iter_input_indexes != s_input_index_map.end()) { input_indexes = iter_input_indexes->second; } else { // By default we should return {0,1, ..., (op->num_inputs)-1}...unless diff --git a/src/ngraph_builder1.h b/src/ngraph_builder1.h index 82fd8c75..d93587d5 100644 --- a/src/ngraph_builder1.h +++ b/src/ngraph_builder1.h @@ -95,13 +95,14 @@ class Builder1 { // This map tells us which inputs to read for a particular node. If no // information is present explicitly in the map, we read all inputs // from 0 to num_inputs-1 - const static std::map> INPUT_INDEX_MAP; + const static std::map> s_input_index_map; // A map from Tf op type_string to a TranslateOp - const static std::map TRANSLATE_OP_MAP; + const static std::map + s_translate_op_map; // Given a TF node, return its corresponding TranslateOp function and required - // input indexes. A wrapper for TRANSLATE_OP_MAP and INPUT_INDEX_MAP + // input indexes. A wrapper for s_translate_op_map and s_input_index_map Status GetOpTranslationRequirements(const Node*&, Builder1::TranslatorFn&, vector&);