diff --git a/build_ngtf.py b/build_ngtf.py index f279e42de..e7607f213 100755 --- a/build_ngtf.py +++ b/build_ngtf.py @@ -96,11 +96,6 @@ def main(): default='', action="store_true") - parser.add_argument( - '--use_grappler_optimizer', - help="Use Grappler optimizer instead of the optimization passes\n", - action="store_true") - parser.add_argument( '--artifacts_dir', type=str, @@ -387,11 +382,6 @@ def main(): "tensorflow") ]) - ngraph_tf_cmake_flags.extend([ - "-DNGRAPH_TF_USE_GRAPPLER_OPTIMIZER=" + - flag_string_map[arguments.use_grappler_optimizer] - ]) - # Now build the bridge ng_tf_whl = build_ngraph_tf(build_dir, artifacts_location, ngraph_tf_src_dir, venv_dir, @@ -442,13 +432,6 @@ def main(): install_ngraph_tf(tf_version, venv_dir, os.path.join(artifacts_location, ng_tf_whl)) - if arguments.use_grappler_optimizer: - import tensorflow as tf - import ngraph_bridge - if not ngraph_bridge.is_grappler_enabled(): - raise Exception( - "Build failed: 'use_grappler_optimizer' specified but not used") - print('\033[1;32mBuild successful\033[0m') os.chdir(pwd) diff --git a/examples/cpp/infer_multiple_networks.cc b/examples/cpp/infer_multiple_networks.cc index 5347892b1..a37264967 100644 --- a/examples/cpp/infer_multiple_networks.cc +++ b/examples/cpp/infer_multiple_networks.cc @@ -71,10 +71,6 @@ void PrintVersion() { << std::endl; std::cout << "CXX11_ABI Used: " << tf::ngraph_bridge::cxx11_abi_flag() << std::endl; - std::cout << "Grappler Enabled? " - << (tf::ngraph_bridge::is_grappler_enabled() ? std::string("Yes") - : std::string("No")) - << std::endl; PrintAvailableBackends(); } diff --git a/examples/cpp/infer_single_network.cc b/examples/cpp/infer_single_network.cc index 3f5f1ee42..4b9618e96 100644 --- a/examples/cpp/infer_single_network.cc +++ b/examples/cpp/infer_single_network.cc @@ -68,10 +68,6 @@ void PrintVersion() { << std::endl; std::cout << "CXX11_ABI Used: " << tf::ngraph_bridge::cxx11_abi_flag() << std::endl; - std::cout << "Grappler Enabled? " - << (tf::ngraph_bridge::is_grappler_enabled() ? std::string("Yes") - : std::string("No")) - << std::endl; PrintAvailableBackends(); } diff --git a/examples/cpp/inference_engine.cc b/examples/cpp/inference_engine.cc index f54fb6e10..cfefadf01 100644 --- a/examples/cpp/inference_engine.cc +++ b/examples/cpp/inference_engine.cc @@ -106,26 +106,8 @@ Status InferenceEngine::CreateSession(const string& graph_filename, ->set_constant_folding(RewriterConfig::OFF); options.config.set_inter_op_parallelism_threads(2); - // The following is related to Grappler - which we are turning off - // Until we get a library fully running - if (tf::ngraph_bridge::is_grappler_enabled()) { - auto* custom_config = options.config.mutable_graph_options() - ->mutable_rewrite_options() - ->add_custom_optimizers(); - - custom_config->set_name("ngraph-optimizer"); - options.config.mutable_graph_options() - ->mutable_rewrite_options() - ->set_min_graph_nodes(-1); - - options.config.mutable_graph_options() - ->mutable_rewrite_options() - ->set_meta_optimizer_iterations(tensorflow::RewriterConfig::ONE); - } - // Load the network - Status load_graph_status = LoadGraph(graph_filename, &session, options); - return load_graph_status; + return LoadGraph(graph_filename, &session, options); } } // namespace benchmark diff --git a/maint/apply-code-format.sh b/maint/apply-code-format.sh index 1ccd6f33d..b9e5c4081 100755 --- a/maint/apply-code-format.sh +++ b/maint/apply-code-format.sh @@ -18,7 +18,7 @@ set -u # limitations under the License. # ****************************************************************************** -declare SRC_DIRS=${1:-ngraph_bridge examples test logging tools diagnostics python} +declare SRC_DIRS=${1:-ngraph_bridge examples test tools diagnostics python} # NOTE: The results of `clang-format` depend _both_ of the following factors: # - The `.clang-format` file, and diff --git a/maint/check-code-format.sh b/maint/check-code-format.sh index 7e437ac9a..cc1e7104a 100755 --- a/maint/check-code-format.sh +++ b/maint/check-code-format.sh @@ -18,7 +18,7 @@ set -u # limitations under the License. # ****************************************************************************** -declare SRC_DIRS=${1:-ngraph_bridge examples experimental test logging tools diagnostics python} +declare SRC_DIRS=${1:-ngraph_bridge examples experimental test tools diagnostics python} # NOTE: The results of `clang-format` depend _both_ of the following factors: # - The `.clang-format` file, and diff --git a/ngraph_bridge/CMakeLists.txt b/ngraph_bridge/CMakeLists.txt index 12c843263..75dbc3842 100644 --- a/ngraph_bridge/CMakeLists.txt +++ b/ngraph_bridge/CMakeLists.txt @@ -42,6 +42,7 @@ set(SRC mark_for_clustering.cc ngraph_builder.cc ngraph_conversions.cc + ngraph_optimizer.cc ngraph_rewrite_pass.cc ops/ngraph_encapsulate_op.cc pass/transpose_sinking.cc @@ -52,12 +53,6 @@ set(SRC version.cc ) -message(STATUS "NGRAPH_TF_USE_GRAPPLER_OPTIMIZER: ${NGRAPH_TF_USE_GRAPPLER_OPTIMIZER}") -if(NGRAPH_TF_USE_GRAPPLER_OPTIMIZER) - list(APPEND SRC ngraph_optimizer.cc) - add_definitions(-DNGRAPH_TF_USE_GRAPPLER_OPTIMIZER) -endif() - add_library(${LIB_NAME} SHARED ${SRC}) target_link_libraries( ${LIB_NAME} diff --git a/ngraph_bridge/ngraph_rewrite_pass.cc b/ngraph_bridge/ngraph_rewrite_pass.cc index b958d2868..6bc1cccfb 100644 --- a/ngraph_bridge/ngraph_rewrite_pass.cc +++ b/ngraph_bridge/ngraph_rewrite_pass.cc @@ -20,6 +20,7 @@ #include "tensorflow/core/common_runtime/optimization_registry.h" #include "tensorflow/core/framework/op.h" #include "tensorflow/core/graph/graph.h" +#include "tensorflow/core/public/session_options.h" #include "api.h" #include "assign_clusters.h" @@ -101,6 +102,16 @@ Status NGraphRewritePass::Rewrite( } Status NGraphRewritePass::Run(const GraphOptimizationPassOptions& options) { + auto& rewriter_config = + options.session_options->config.graph_options().rewrite_options(); + for (int i = 0; i < rewriter_config.custom_optimizers_size(); ++i) { + if (rewriter_config.custom_optimizers(i).name() == "ngraph-optimizer") { + NGRAPH_VLOG(3) << "ngraph-optimizer custom optimizer enabled; Disabling " + "nGraph rewrite-pass"; + return Status::OK(); + } + } + // If we don't get a main graph, log that fact and bail. if (options.graph == nullptr) { NGRAPH_VLOG(0) << "NGraphRewritePass: options.graph == nullptr"; @@ -111,8 +122,6 @@ Status NGraphRewritePass::Run(const GraphOptimizationPassOptions& options) { } // namespace ngraph_bridge -#ifndef NGRAPH_TF_USE_GRAPPLER_OPTIMIZER REGISTER_OPTIMIZATION(OptimizationPassRegistry::POST_REWRITE_FOR_EXEC, 0, ngraph_bridge::NGraphRewritePass); -#endif } // namespace tensorflow diff --git a/ngraph_bridge/version.cc b/ngraph_bridge/version.cc index bbe9789a5..73b42237f 100644 --- a/ngraph_bridge/version.cc +++ b/ngraph_bridge/version.cc @@ -61,14 +61,6 @@ int cxx11_abi_flag() { #endif } -bool is_grappler_enabled() { -#if defined(NGRAPH_TF_USE_GRAPPLER_OPTIMIZER) - return true; -#else - return false; -#endif -} - const char* tf_version() { return (TF_VERSION_STRING); } } // namespace ngraph_bridge diff --git a/ngraph_bridge/version.h b/ngraph_bridge/version.h index 31847ffd6..48a83fc75 100644 --- a/ngraph_bridge/version.h +++ b/ngraph_bridge/version.h @@ -37,10 +37,6 @@ const char* ngraph_version(); // _GLIBCXX_USE_CXX11_ABI set during the compilation time int cxx11_abi_flag(); -// Returns true when nGraph is using Grappler optimizer APIs for -// graph rewriting -bool is_grappler_enabled(); - // Returns the tensorflow version const char* tf_version(); } diff --git a/python/ngraph_bridge/__init__.in.py b/python/ngraph_bridge/__init__.in.py index 2d3f5fa26..e52449109 100644 --- a/python/ngraph_bridge/__init__.in.py +++ b/python/ngraph_bridge/__init__.in.py @@ -45,7 +45,7 @@ 'set_backend', 'get_backend', 'start_logging_placement', 'stop_logging_placement', 'is_logging_placement', '__version__', 'cxx11_abi_flag' - 'is_grappler_enabled', 'update_config', + 'update_config', 'set_disabled_ops', 'get_disabled_ops', ] @@ -123,7 +123,6 @@ def requested(): ngraph_bridge_lib.version.restype = ctypes.c_char_p ngraph_bridge_lib.ngraph_version.restype = ctypes.c_char_p ngraph_bridge_lib.cxx11_abi_flag.restype = ctypes.c_int - ngraph_bridge_lib.is_grappler_enabled.restype = ctypes.c_bool ngraph_bridge_lib.set_disabled_ops.argtypes = [ctypes.c_char_p] ngraph_bridge_lib.get_disabled_ops.restype = ctypes.c_char_p @@ -171,38 +170,33 @@ def is_logging_placement(): def cxx11_abi_flag(): return ngraph_bridge_lib.cxx11_abi_flag() - def is_grappler_enabled(): - return ngraph_bridge_lib.is_grappler_enabled() - def update_config(config, backend_name = "CPU", device_id = ""): - #updating session config if grappler is enabled - if(ngraph_bridge_lib.is_grappler_enabled()): - opt_name = 'ngraph-optimizer' - # If the config already has ngraph-optimizer, then do not update it - if config.HasField('graph_options'): - if config.graph_options.HasField('rewrite_options'): - custom_opts = config.graph_options.rewrite_options.custom_optimizers - for i in range(len(custom_opts)): - if custom_opts[i].name == opt_name: - return config - rewriter_options = rewriter_config_pb2.RewriterConfig() - rewriter_options.meta_optimizer_iterations=(rewriter_config_pb2.RewriterConfig.ONE) - rewriter_options.min_graph_nodes=-1 - ngraph_optimizer = rewriter_options.custom_optimizers.add() - ngraph_optimizer.name = opt_name - ngraph_optimizer.parameter_map["device_id"].s = device_id.encode() - config.MergeFrom(tf.compat.v1.ConfigProto(graph_options=tf.compat.v1.GraphOptions(rewrite_options=rewriter_options))) - # For reference, if we want to provide configuration support(backend parameters) - # in a python script using the ngraph-optimizer - # rewriter_options = rewriter_config_pb2.RewriterConfig() - # rewriter_options.meta_optimizer_iterations=(rewriter_config_pb2.RewriterConfig.ONE) - # rewriter_options.min_graph_nodes=-1 - # ngraph_optimizer = rewriter_options.custom_optimizers.add() - # ngraph_optimizer.name = "ngraph-optimizer" - # ngraph_optimizer.parameter_map["device_id"].s = device_id.encode() - # ngraph_optimizer.parameter_map["max_batch_size"].s = b'64' - # ngraph_optimizer.parameter_map["ice_cores"].s = b'12' - # config.MergeFrom(tf.compat.v1.ConfigProto(graph_options=tf.compat.v1.GraphOptions(rewrite_options=rewriter_options))) + opt_name = 'ngraph-optimizer' + # If the config already has ngraph-optimizer, then do not update it + if config.HasField('graph_options'): + if config.graph_options.HasField('rewrite_options'): + custom_opts = config.graph_options.rewrite_options.custom_optimizers + for i in range(len(custom_opts)): + if custom_opts[i].name == opt_name: + return config + rewriter_options = rewriter_config_pb2.RewriterConfig() + rewriter_options.meta_optimizer_iterations=(rewriter_config_pb2.RewriterConfig.ONE) + rewriter_options.min_graph_nodes=-1 + ngraph_optimizer = rewriter_options.custom_optimizers.add() + ngraph_optimizer.name = opt_name + ngraph_optimizer.parameter_map["device_id"].s = device_id.encode() + config.MergeFrom(tf.compat.v1.ConfigProto(graph_options=tf.compat.v1.GraphOptions(rewrite_options=rewriter_options))) + # For reference, if we want to provide configuration support(backend parameters) + # in a python script using the ngraph-optimizer + # rewriter_options = rewriter_config_pb2.RewriterConfig() + # rewriter_options.meta_optimizer_iterations=(rewriter_config_pb2.RewriterConfig.ONE) + # rewriter_options.min_graph_nodes=-1 + # ngraph_optimizer = rewriter_options.custom_optimizers.add() + # ngraph_optimizer.name = "ngraph-optimizer" + # ngraph_optimizer.parameter_map["device_id"].s = device_id.encode() + # ngraph_optimizer.parameter_map["max_batch_size"].s = b'64' + # ngraph_optimizer.parameter_map["ice_cores"].s = b'12' + # config.MergeFrom(tf.compat.v1.ConfigProto(graph_options=tf.compat.v1.GraphOptions(rewrite_options=rewriter_options))) return config def set_disabled_ops(unsupported_ops): @@ -215,5 +209,4 @@ def get_disabled_ops(): "nGraph bridge version: " + str(ngraph_bridge_lib.version()) + "\n" + \ "nGraph version used for this build: " + str(ngraph_bridge_lib.ngraph_version()) + "\n" + \ "TensorFlow version used for this build: " + TF_GIT_VERSION_BUILT_WITH + "\n" \ - "CXX11_ABI flag used for this build: " + str(ngraph_bridge_lib.cxx11_abi_flag()) + "\n" \ - "nGraph bridge built with Grappler: " + str(ngraph_bridge_lib.is_grappler_enabled()) + "\n" \ \ No newline at end of file + "CXX11_ABI flag used for this build: " + str(ngraph_bridge_lib.cxx11_abi_flag()) + "\n" \ No newline at end of file diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 0b4576ccf..51b7c2e7b 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -72,8 +72,9 @@ set(SRC graph_rewrites/assign_clusters.cc graph_rewrites/deadness_test.cc graph_rewrites/backend_manager_test.cc - graph_rewrites/encapsulate_clusters_test.cc + graph_rewrites/config_for_grappler_test.cc graph_rewrites/disable_ops_test.cc + graph_rewrites/encapsulate_clusters_test.cc graph_rewrites/mark_for_clustering_test.cc graph_rewrites/op_by_op_capability_test.cc test_utilities.cpp @@ -85,10 +86,6 @@ set(SRC pass/transpose_sinking_test.cpp ) -if(NGRAPH_TF_USE_GRAPPLER_OPTIMIZER) - list(APPEND SRC graph_rewrites/config_for_grappler_test.cc) -endif() - # The compile flag -DNDEBUG is required since # tensorflow::Core::RefCounted is error prone as explained here: # https://github.com/tensorflow/tensorflow/issues/17316 diff --git a/test/graph_rewrites/config_for_grappler_test.cc b/test/graph_rewrites/config_for_grappler_test.cc index 13acf8077..024d99d32 100644 --- a/test/graph_rewrites/config_for_grappler_test.cc +++ b/test/graph_rewrites/config_for_grappler_test.cc @@ -41,6 +41,7 @@ namespace testing { // etc.,etc. TEST(GrapplerConfig, RConfig1) { + ActivateNGraph(); // Create Graph Scope root = Scope::NewRootScope(); auto A = ops::Const(root.WithOpName("A"), {3.f, 2.f}); @@ -72,7 +73,6 @@ TEST(GrapplerConfig, RConfig1) { // Run grappler tensorflow::grappler::MetaOptimizer optimizer(nullptr, config_proto); GraphDef output; - // const Status status = optimizer.Optimize(nullptr, item, &output); ASSERT_OK(optimizer.Optimize(nullptr, item, &output)); // GraphDef to Graph @@ -91,9 +91,11 @@ TEST(GrapplerConfig, RConfig1) { ng_encap = node; } ASSERT_NE(ng_encap, nullptr); + DeactivateNGraph(); } TEST(GrapplerConfig, RConfig4) { + ActivateNGraph(); // Create Graph Scope root = Scope::NewRootScope(); auto A = ops::Const(root.WithOpName("A"), {3.f, 2.f}); @@ -152,11 +154,13 @@ TEST(GrapplerConfig, RConfig4) { string ng_test_echo; ASSERT_OK(GetNodeAttr(ng_encap->attrs(), "_ngraph_test_echo", &ng_test_echo)); ASSERT_EQ(ng_test_echo, "hi"); + DeactivateNGraph(); } // Test the failure case where the compulsory attribute device_id // is not provided using the rewriter config TEST(GrapplerConfig, RConfig5) { + ActivateNGraph(); // Create Graph Scope root = Scope::NewRootScope(); auto A = ops::Const(root.WithOpName("A"), {3.f, 2.f}); @@ -192,6 +196,7 @@ TEST(GrapplerConfig, RConfig5) { GraphDef output; ASSERT_OK(optimizer.Optimize(nullptr, item, &output)); + DeactivateNGraph(); } } // namespace testing diff --git a/test/python/common.py b/test/python/common.py index 09c245a57..756763ebc 100644 --- a/test/python/common.py +++ b/test/python/common.py @@ -53,9 +53,8 @@ def with_ngraph(self, l, config=None): # because mutable objects should not be used as defaults in python if config is None: config = tf.compat.v1.ConfigProto() - # TODO: Stop grappler on failure (Add fail_on_optimizer_errors=True) - config = ngraph_bridge.update_config(config) + # config = ngraph_bridge.update_config(config) ngraph_tf_disable_deassign_clusters = os.environ.pop( 'NGRAPH_TF_DISABLE_DEASSIGN_CLUSTERS', None) diff --git a/test/python/test_op_disable.py b/test/python/test_op_disable.py index 9e4d69fa1..82eaeaca4 100644 --- a/test/python/test_op_disable.py +++ b/test/python/test_op_disable.py @@ -51,30 +51,25 @@ def test_disable_op_1(self, op_list): @pytest.mark.parametrize(("invalid_op_list",), (('Add,_InvalidOp',), ('_nGraphEncapsulate',))) def test_disable_op_2(self, invalid_op_list): - # This test is disabled for grappler because grappler fails silently and - # TF continues to run with the unoptimized graph - # Note, tried setting fail_on_optimizer_errors, but grappler still failed silently - # TODO: enable this test for grappler as well. - if (not ngraph_bridge.is_grappler_enabled()): - ngraph_bridge.set_disabled_ops(invalid_op_list) - a = tf.compat.v1.placeholder(tf.int32, shape=(5,)) - b = tf.constant(np.ones((5,)), dtype=tf.int32) - c = a + b + ngraph_bridge.set_disabled_ops(invalid_op_list) + a = tf.compat.v1.placeholder(tf.int32, shape=(5,)) + b = tf.constant(np.ones((5,)), dtype=tf.int32) + c = a + b - def run_test(sess): - return sess.run(c, feed_dict={a: np.ones((5,))}) + def run_test(sess): + return sess.run(c, feed_dict={a: np.ones((5,))}) - assert (self.without_ngraph(run_test) == np.ones(5,) * 2).all() - #import pdb; pdb.set_trace() - try: - # This test is expected to fail, - # since all the strings passed to set_disabled_ops have invalid ops in them - res = self.with_ngraph(run_test) - except: - # Clean up - ngraph_bridge.set_disabled_ops('') - return - assert False, 'Had expected test to raise error' + assert (self.without_ngraph(run_test) == np.ones(5,) * 2).all() + #import pdb; pdb.set_trace() + try: + # This test is expected to fail, + # since all the strings passed to set_disabled_ops have invalid ops in them + res = self.with_ngraph(run_test) + except: + # Clean up + ngraph_bridge.set_disabled_ops('') + return + assert False, 'Had expected test to raise error' def test_disable_op_env(self): op_list = 'Select,Where' diff --git a/test/python/test_rewriter_config_setting.py b/test/python/test_rewriter_config_setting.py index bddd33ba4..f6fdefa5c 100644 --- a/test/python/test_rewriter_config_setting.py +++ b/test/python/test_rewriter_config_setting.py @@ -34,9 +34,6 @@ class TestRewriterConfigBackendSetting(NgraphTest): - @pytest.mark.skipif( - not ngraph_bridge.is_grappler_enabled(), - reason='Rewriter config only works for grappler path') def test_config_updater_api(self): dim1 = 3 dim2 = 4 diff --git a/test/python/test_updateconfig.py b/test/python/test_updateconfig.py index 79204c2c5..cc7c9cf9a 100644 --- a/test/python/test_updateconfig.py +++ b/test/python/test_updateconfig.py @@ -33,8 +33,6 @@ class TestUpdateConfig(NgraphTest): - @pytest.mark.skipif( - not ngraph_bridge.is_grappler_enabled(), reason='Only for Grappler') def test_update_config_adds_optimizer_only_once(self): # Helper function to count the number of occurances in a config diff --git a/test/test_utilities.cpp b/test/test_utilities.cpp index 7e5cfce9c..e1ac1e28f 100644 --- a/test/test_utilities.cpp +++ b/test/test_utilities.cpp @@ -344,20 +344,18 @@ tf::SessionOptions GetSessionOptions() { ->mutable_rewrite_options() ->set_constant_folding(tf::RewriterConfig::OFF); - if (is_grappler_enabled()) { - auto* custom_config = options.config.mutable_graph_options() - ->mutable_rewrite_options() - ->add_custom_optimizers(); - - custom_config->set_name("ngraph-optimizer"); - options.config.mutable_graph_options() - ->mutable_rewrite_options() - ->set_min_graph_nodes(-1); - - options.config.mutable_graph_options() - ->mutable_rewrite_options() - ->set_meta_optimizer_iterations(tf::RewriterConfig::ONE); - } + auto* custom_config = options.config.mutable_graph_options() + ->mutable_rewrite_options() + ->add_custom_optimizers(); + + custom_config->set_name("ngraph-optimizer"); + options.config.mutable_graph_options() + ->mutable_rewrite_options() + ->set_min_graph_nodes(-1); + + options.config.mutable_graph_options() + ->mutable_rewrite_options() + ->set_meta_optimizer_iterations(tf::RewriterConfig::ONE); return options; } diff --git a/test/tf_exec.cpp b/test/tf_exec.cpp index 5da31e018..dc85a94af 100644 --- a/test/tf_exec.cpp +++ b/test/tf_exec.cpp @@ -115,21 +115,6 @@ TEST(TFExec, axpy) { ->mutable_rewrite_options() ->set_constant_folding(tf::RewriterConfig::OFF); - if (is_grappler_enabled()) { - auto* custom_config = options.config.mutable_graph_options() - ->mutable_rewrite_options() - ->add_custom_optimizers(); - - custom_config->set_name("ngraph-optimizer"); - options.config.mutable_graph_options() - ->mutable_rewrite_options() - ->set_min_graph_nodes(-1); - - options.config.mutable_graph_options() - ->mutable_rewrite_options() - ->set_meta_optimizer_iterations(tf::RewriterConfig::ONE); - } - ConfigProto& config = options.config; config.set_allow_soft_placement(true); std::unique_ptr session(NewSession(options)); diff --git a/tools/test_utils.py b/tools/test_utils.py index 544710120..ea00d558d 100755 --- a/tools/test_utils.py +++ b/tools/test_utils.py @@ -167,9 +167,7 @@ def run_tensorflow_pytests_from_artifacts(ngraph_tf_src_dir, tf_src_dir, # Check to see if we need to apply the patch for Grappler import ngraph_bridge - patch_file_name = "test/python/tensorflow/tf_unittest_ngraph" + ( - "_with_grappler" - if ngraph_bridge.is_grappler_enabled() else "") + ".patch" + patch_file_name = "test/python/tensorflow/tf_unittest_ngraph.patch" patch_file = os.path.abspath( os.path.join(ngraph_tf_src_dir, patch_file_name))