From 3f79a00f130c5eee4aec06857c6e9ba700a00656 Mon Sep 17 00:00:00 2001 From: Abhishek Kulkarni Date: Mon, 1 Feb 2021 12:35:50 -0800 Subject: [PATCH 1/7] Remove use-grappler-optimizer compile option * Unconditionally enable grappler path * Remove is_grappler_enabled() API call * Run tests using the grappler pass --- build_ngtf.py | 17 ------ examples/cpp/infer_multiple_networks.cc | 4 -- examples/cpp/infer_single_network.cc | 4 -- examples/cpp/inference_engine.cc | 20 +------ ngraph_bridge/CMakeLists.txt | 7 +-- ngraph_bridge/ngraph_rewrite_pass.cc | 13 ++++- ngraph_bridge/version.cc | 8 --- ngraph_bridge/version.h | 4 -- python/ngraph_bridge/__init__.in.py | 63 +++++++++------------ test/CMakeLists.txt | 7 +-- test/python/test_op_disable.py | 39 ++++++------- test/python/test_rewriter_config_setting.py | 3 - test/python/test_updateconfig.py | 2 - test/test_utilities.cpp | 26 ++++----- test/tf_exec.cpp | 15 ----- 15 files changed, 72 insertions(+), 160 deletions(-) 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 292987151..eaab163e6 100644 --- a/examples/cpp/infer_multiple_networks.cc +++ b/examples/cpp/infer_multiple_networks.cc @@ -81,10 +81,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 bfe59f4fa..6cb9d629b 100644 --- a/examples/cpp/infer_single_network.cc +++ b/examples/cpp/infer_single_network.cc @@ -69,10 +69,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/ngraph_bridge/CMakeLists.txt b/ngraph_bridge/CMakeLists.txt index 430006840..968d3e72f 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 @@ -51,12 +52,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 21045352f..a03305794 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 \ No newline at end of file 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/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 988249d1a..b8342fae1 100644 --- a/test/tf_exec.cpp +++ b/test/tf_exec.cpp @@ -114,21 +114,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)); From 22e1ee92c7ab2ed38f71e896f484b23f8ab4e256 Mon Sep 17 00:00:00 2001 From: Abhishek Kulkarni Date: Mon, 1 Feb 2021 18:48:53 -0800 Subject: [PATCH 2/7] Disable deassign clusters for grappler tests --- test/graph_rewrites/config_for_grappler_test.cc | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) 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 From d419286b1e5b4408f7a1fa22beb335069abf16ec Mon Sep 17 00:00:00 2001 From: Abhishek Kulkarni Date: Mon, 1 Feb 2021 22:36:43 -0800 Subject: [PATCH 3/7] Fix error in test_disable_op_2 The grappler optimizer, now used by default for tests, does not fail on optimizer error unless explicitly specified in the rewriter config. Invert the logic in test_disable_op_2 to not expect a session failure when trying to disable invalid ops. --- test/python/test_op_disable.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/python/test_op_disable.py b/test/python/test_op_disable.py index 82eaeaca4..81587f013 100644 --- a/test/python/test_op_disable.py +++ b/test/python/test_op_disable.py @@ -62,14 +62,14 @@ def run_test(sess): 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 + # This test is expected to pass even though + # 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 False, 'Had expected test to raise error' + + ngraph_bridge.set_disabled_ops('') + return def test_disable_op_env(self): op_list = 'Select,Where' From 570dd79ee461253a0e6b7520e451fdae0f8fc661 Mon Sep 17 00:00:00 2001 From: Abhishek Kulkarni Date: Mon, 1 Feb 2021 22:38:32 -0800 Subject: [PATCH 4/7] Remove "logging" subdirectory in code format scripts --- maint/apply-code-format.sh | 2 +- maint/check-code-format.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) 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 From 625ab1d31cb6b821e716ecdc021f40b37ca87b9b Mon Sep 17 00:00:00 2001 From: Abhishek Kulkarni Date: Tue, 2 Feb 2021 09:24:03 -0800 Subject: [PATCH 5/7] Do not use grappler for TF python tests --- tools/test_utils.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) 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)) From c07964832f666e803c960809cb595deb9df55e71 Mon Sep 17 00:00:00 2001 From: Abhishek Kulkarni Date: Fri, 5 Feb 2021 14:55:09 -0800 Subject: [PATCH 6/7] Do not use grappler pass for running python tests --- test/python/common.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) 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) From d5dc16c055c3a0b317207f024442094f8175161d Mon Sep 17 00:00:00 2001 From: Abhishek Kulkarni Date: Mon, 8 Feb 2021 21:55:04 -0800 Subject: [PATCH 7/7] Revert changes to test_op_disable --- test/python/test_op_disable.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/python/test_op_disable.py b/test/python/test_op_disable.py index 81587f013..82eaeaca4 100644 --- a/test/python/test_op_disable.py +++ b/test/python/test_op_disable.py @@ -62,14 +62,14 @@ def run_test(sess): assert (self.without_ngraph(run_test) == np.ones(5,) * 2).all() #import pdb; pdb.set_trace() try: - # This test is expected to pass even though - # all the strings passed to set_disabled_ops have invalid ops in them + # 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: - assert False, 'Had expected test to raise error' - - ngraph_bridge.set_disabled_ops('') - return + # 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'