From fcd83b0ef76a9d2198edf910928d55946b8a3bcb Mon Sep 17 00:00:00 2001 From: Sayantan Sarkar Date: Mon, 1 Jul 2019 11:50:31 -0700 Subject: [PATCH 01/13] Return exit status --- test/model_level_tests/test_main.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/model_level_tests/test_main.py b/test/model_level_tests/test_main.py index 8452e0bc9..d25d25412 100644 --- a/test/model_level_tests/test_main.py +++ b/test/model_level_tests/test_main.py @@ -17,6 +17,7 @@ import pdb, time from subprocess import check_output, call, Popen, PIPE import json, os, argparse, sys +from functools import reduce import sys # expects tools to be present at this relative location. Need access to build_utils sys.path.insert(0, os.path.abspath('../../tools')) @@ -450,6 +451,9 @@ def get_disabled_tests_info(): print('Passed:\n' + '\033[92m' + print_format(passed_tests) + '\033[0m') print('Skipped:\n' + '\033[93m' + print_format(skipped_tests) + '\033[0m') print('Failed:\n' + '\033[91m' + print_format(failed_tests) + '\033[0m') + num_failed_tests = reduce(lambda x, key: x + len(failed_tests[key]), + failed_tests, 0) + exit(0 if (num_failed_tests == 0) else 1) # TODO add a test comparing with TF run? # TODO verbose or quiet? From 51d5339d04737b3d37c99b852e8c1ac9fdda96d9 Mon Sep 17 00:00:00 2001 From: Sayantan Sarkar Date: Mon, 1 Jul 2019 11:54:47 -0700 Subject: [PATCH 02/13] Remove some comments that have already been transferred to the README --- test/model_level_tests/test_main.py | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) diff --git a/test/model_level_tests/test_main.py b/test/model_level_tests/test_main.py index d25d25412..4579720f2 100644 --- a/test/model_level_tests/test_main.py +++ b/test/model_level_tests/test_main.py @@ -468,21 +468,5 @@ def get_disabled_tests_info(): # Level3: parse prints we put. These tests are run without "NGRAPH_TF_LOG_PLACEMENT=1". the framework can provide some default parsers, but users are free to add pyscripts that provide functions for custom script parsers # These tests can be long # So we can offer options to do: {1}, {1,2}, {1,2,3}, {3} (or do we allow options for any combination of tests?) -# NOTE: Level3 and Level1 test are same (mechanics wise). Merge them. Then we have only 2 types of tests +# NOTE: Level3 and Level1 test are same (mechanics wise). We have only 2 types of tests, though Level2 is unimplemented for now -# Each model dir represents 1 repo to download. A model dir can have multiple sub tests (each sub-test could represent a different model, or the same model tested under different settings) - -# Structure of "expected json" -# dictionary of expected values. key is a config, value is the expected values json. there is a "default" config, but one can add other configs (for example for other backends etc) - -# Sample run script: -# python test_main.py --run_logparse_tests --models MLP - -# feature 1: dumps shell script at the end. dumps shell script even when the framework crashes -# feature 2: prints list of tests and their descriptions (--list) -# feature 3: "expected" values can be varied by different configs -# feature 4: cleanup script -# feature 5: sub tests folders must start with 'test' (else ignored). Can have 'disabled' in their names to disable -# feature 6: default and user-specified log parsers (named custom_log_parser.py, which is expected to contain a function custom_parse_logs) -# feature 7: filename is supposed to be expected.json -# feature 8: enable_ngraph can be placed in each test dir or in the model dir for all subtests to share. test folder's patch overrides global model folder patch From 0e20a8ae303463d406fd0978d7c78dd5d30639eb Mon Sep 17 00:00:00 2001 From: Sayantan Sarkar Date: Mon, 1 Jul 2019 11:56:53 -0700 Subject: [PATCH 03/13] Style --- test/model_level_tests/test_main.py | 1 - 1 file changed, 1 deletion(-) diff --git a/test/model_level_tests/test_main.py b/test/model_level_tests/test_main.py index 4579720f2..e07e260d4 100644 --- a/test/model_level_tests/test_main.py +++ b/test/model_level_tests/test_main.py @@ -469,4 +469,3 @@ def get_disabled_tests_info(): # These tests can be long # So we can offer options to do: {1}, {1,2}, {1,2,3}, {3} (or do we allow options for any combination of tests?) # NOTE: Level3 and Level1 test are same (mechanics wise). We have only 2 types of tests, though Level2 is unimplemented for now - From b39b93766d772bd1fb2f4702742ad7d0b5e44fd9 Mon Sep 17 00:00:00 2001 From: Sayantan Sarkar Date: Mon, 1 Jul 2019 12:03:34 -0700 Subject: [PATCH 04/13] Changing fail check condition --- test/model_level_tests/test_main.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/test/model_level_tests/test_main.py b/test/model_level_tests/test_main.py index e07e260d4..770f5bfac 100644 --- a/test/model_level_tests/test_main.py +++ b/test/model_level_tests/test_main.py @@ -17,7 +17,6 @@ import pdb, time from subprocess import check_output, call, Popen, PIPE import json, os, argparse, sys -from functools import reduce import sys # expects tools to be present at this relative location. Need access to build_utils sys.path.insert(0, os.path.abspath('../../tools')) @@ -451,9 +450,8 @@ def get_disabled_tests_info(): print('Passed:\n' + '\033[92m' + print_format(passed_tests) + '\033[0m') print('Skipped:\n' + '\033[93m' + print_format(skipped_tests) + '\033[0m') print('Failed:\n' + '\033[91m' + print_format(failed_tests) + '\033[0m') - num_failed_tests = reduce(lambda x, key: x + len(failed_tests[key]), - failed_tests, 0) - exit(0 if (num_failed_tests == 0) else 1) + all_tests_passed = all([len(failed_tests[k]) == 0 for k in failed_tests]) + exit(0 if all_tests_passed else 1) # TODO add a test comparing with TF run? # TODO verbose or quiet? From c8e27e1fd85674d54ecacc006f5c7a8fcc282920 Mon Sep 17 00:00:00 2001 From: Sayantan Sarkar Date: Mon, 1 Jul 2019 12:19:41 -0700 Subject: [PATCH 05/13] Add a separator --- test/model_level_tests/test_main.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/model_level_tests/test_main.py b/test/model_level_tests/test_main.py index 770f5bfac..989f66142 100644 --- a/test/model_level_tests/test_main.py +++ b/test/model_level_tests/test_main.py @@ -434,7 +434,8 @@ def get_disabled_tests_info(): failed_tests = {} skipped_tests = {} for test_suite in requested_test_suites: - print('Testing model/test-suite: ' + test_suite) + print('\n' + '=' * 20 + 'Testing model/test-suite: ' + test_suite + + '=' * 20) if test_suite not in disabled_test_suite: if args.run_basic_tests: passed_tests_in_suite, failed_tests_in_suite, skipped_tests_in_suite = run_test_suite( From 65f879d1c7521e4dda0164f93cb9e730ed570509 Mon Sep 17 00:00:00 2001 From: Sayantan Sarkar Date: Thu, 11 Jul 2019 17:49:32 -0700 Subject: [PATCH 06/13] Update MLP numbers --- test/model_level_tests/models/MLP/test1/expected.json | 6 +++--- test/model_level_tests/models/MLP/test2/expected.json | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/test/model_level_tests/models/MLP/test1/expected.json b/test/model_level_tests/models/MLP/test1/expected.json index 53aeed6a3..9185c236b 100644 --- a/test/model_level_tests/models/MLP/test1/expected.json +++ b/test/model_level_tests/models/MLP/test1/expected.json @@ -7,9 +7,9 @@ "num_nodes_marked_for_clustering": 0 }, "1": { - "num_ng_clusters": 17, - "num_nodes_in_graph": 310, - "num_nodes_marked_for_clustering": 244 + "num_ng_clusters": 15, + "num_nodes_in_graph": 311, + "num_nodes_marked_for_clustering": 249 } }, "time": 10 diff --git a/test/model_level_tests/models/MLP/test2/expected.json b/test/model_level_tests/models/MLP/test2/expected.json index ffbf6c47b..b22e25d3e 100644 --- a/test/model_level_tests/models/MLP/test2/expected.json +++ b/test/model_level_tests/models/MLP/test2/expected.json @@ -7,9 +7,9 @@ "num_nodes_marked_for_clustering": 0 }, "1": { - "num_ng_clusters": 21, - "num_nodes_in_graph": 448, - "num_nodes_marked_for_clustering": 358 + "num_ng_clusters": 19, + "num_nodes_in_graph": 449, + "num_nodes_marked_for_clustering": 363 } }, "time": 13 From 5bf41fedc3687500f6728315a7bad9fee6c286cf Mon Sep 17 00:00:00 2001 From: Sayantan Sarkar Date: Thu, 22 Aug 2019 15:55:53 -0700 Subject: [PATCH 07/13] Using parameterization, and asserting that TF also fails --- test/python/test_conv2D_KernelChecks.py | 23 ++++++----------------- 1 file changed, 6 insertions(+), 17 deletions(-) diff --git a/test/python/test_conv2D_KernelChecks.py b/test/python/test_conv2D_KernelChecks.py index 5edd79478..153936500 100644 --- a/test/python/test_conv2D_KernelChecks.py +++ b/test/python/test_conv2D_KernelChecks.py @@ -63,7 +63,9 @@ def make_filter_and_backprop_args(self): return x1, x2 - def test_conv2d_stride_in_batch_not_supported(self): + + @pytest.mark.parametrize(("strides",), (([2, 1, 1, 1],), ([2, 1, 1, 1],),)) + def test_conv2d_stride_in_batch_not_supported(self, strides): inp_values, filt_values = self.make_filter_and_backprop_args() def run_test(sess): @@ -80,19 +82,6 @@ def run_test(sess): self.with_ngraph(run_test) assert "Strides in batch and depth dimensions is not supported: Conv2D" in excinfo.value.message - def test_conv2d_stride_in_depth_not_supported(self): - inp_values, filt_values = self.make_filter_and_backprop_args() - - def run_test(sess): - inp = array_ops.placeholder(dtypes.float32) - filt = array_ops.placeholder(dtypes.float32) - return sess.run( - nn_ops.conv2d(inp, filt, strides=[1, 1, 1, 2], padding="SAME"), - { - inp: inp_values, - filt: filt_values - }) - - with pytest.raises(Exception) as excinfo: - self.with_ngraph(run_test) - assert "Strides in batch and depth dimensions is not supported: Conv2D" in excinfo.value.message + # TF also fails + with pytest.raises(Exception) as excinfo1: + self.without_ngraph(run_test) From 7325420d99dd0881a22aa88ae7cc50e8295fb33f Mon Sep 17 00:00:00 2001 From: Sayantan Sarkar Date: Thu, 22 Aug 2019 16:32:42 -0700 Subject: [PATCH 08/13] Style --- test/python/test_conv2D_KernelChecks.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/python/test_conv2D_KernelChecks.py b/test/python/test_conv2D_KernelChecks.py index 153936500..ab13407da 100644 --- a/test/python/test_conv2D_KernelChecks.py +++ b/test/python/test_conv2D_KernelChecks.py @@ -63,8 +63,10 @@ def make_filter_and_backprop_args(self): return x1, x2 - - @pytest.mark.parametrize(("strides",), (([2, 1, 1, 1],), ([2, 1, 1, 1],),)) + @pytest.mark.parametrize(("strides",), ( + ([2, 1, 1, 1],), + ([2, 1, 1, 1],), + )) def test_conv2d_stride_in_batch_not_supported(self, strides): inp_values, filt_values = self.make_filter_and_backprop_args() From 0cb6a281f6685e031cf860c53303789394a89b53 Mon Sep 17 00:00:00 2001 From: Sayantan Sarkar Date: Thu, 22 Aug 2019 21:36:41 -0700 Subject: [PATCH 09/13] Add grappler and varopts expected results --- .../models/MLP/test1/expected.json | 19 +++++++++++++++-- .../models/MLP/test2/expected.json | 21 ++++++++++++++++--- test/python/test_modeltester.py | 11 +++++++++- 3 files changed, 45 insertions(+), 6 deletions(-) diff --git a/test/model_level_tests/models/MLP/test1/expected.json b/test/model_level_tests/models/MLP/test1/expected.json index 9185c236b..76af1d1f0 100644 --- a/test/model_level_tests/models/MLP/test1/expected.json +++ b/test/model_level_tests/models/MLP/test1/expected.json @@ -8,8 +8,8 @@ }, "1": { "num_ng_clusters": 15, - "num_nodes_in_graph": 311, - "num_nodes_marked_for_clustering": 249 + "num_nodes_in_graph": 330, + "num_nodes_marked_for_clustering": 267 } }, "time": 10 @@ -29,6 +29,21 @@ }, "time": 10 }, + "varopts": { + "logparse": { + "0": { + "num_ng_clusters": 0, + "num_nodes_in_graph": 50, + "num_nodes_marked_for_clustering": 0 + }, + "1": { + "num_ng_clusters": 15, + "num_nodes_in_graph": 312, + "num_nodes_marked_for_clustering": 250 + } + }, + "time": 10 + }, "default_tf113": { "logparse": diff --git a/test/model_level_tests/models/MLP/test2/expected.json b/test/model_level_tests/models/MLP/test2/expected.json index b22e25d3e..b80f3f53d 100644 --- a/test/model_level_tests/models/MLP/test2/expected.json +++ b/test/model_level_tests/models/MLP/test2/expected.json @@ -22,9 +22,24 @@ "num_nodes_marked_for_clustering": 0 }, "1": { - "num_ng_clusters": 21, - "num_nodes_in_graph": 478, - "num_nodes_marked_for_clustering": 387 + "num_ng_clusters": 19, + "num_nodes_in_graph": 479, + "num_nodes_marked_for_clustering": 392 + } + }, + "time": 13 + }, + "varopts": { + "logparse": { + "0": { + "num_ng_clusters": 0, + "num_nodes_in_graph": 83, + "num_nodes_marked_for_clustering": 0 + }, + "1": { + "num_ng_clusters": 19, + "num_nodes_in_graph": 450, + "num_nodes_marked_for_clustering": 364 } }, "time": 13 diff --git a/test/python/test_modeltester.py b/test/python/test_modeltester.py index aa0cde81d..416402060 100644 --- a/test/python/test_modeltester.py +++ b/test/python/test_modeltester.py @@ -35,8 +35,17 @@ class TestModelTester(NgraphTest): def test_MLP(self): cwd = os.getcwd() os.chdir('../model_level_tests/') + grappler = ngraph_bridge.is_grappler_enabled() + varopts = ngraph_bridge.are_variables_enabled() + if grappler: + if varopts: + assert False, "Varopts and grappler does not build together right now" + else: + config = "grappler" + else: + config = "varopts" if varopts else "default" try: command_executor( - "python test_main.py --run_basic_tests --models MLP") + "python test_main.py --run_basic_tests --models MLP --configuration " + config) finally: os.chdir(cwd) From 20a72676ae390ce712c3a7a472016ae08c086bcb Mon Sep 17 00:00:00 2001 From: Sayantan Sarkar Date: Thu, 22 Aug 2019 21:37:58 -0700 Subject: [PATCH 10/13] Style --- test/python/test_modeltester.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/python/test_modeltester.py b/test/python/test_modeltester.py index 416402060..cdf3c10d5 100644 --- a/test/python/test_modeltester.py +++ b/test/python/test_modeltester.py @@ -46,6 +46,7 @@ def test_MLP(self): config = "varopts" if varopts else "default" try: command_executor( - "python test_main.py --run_basic_tests --models MLP --configuration " + config) + "python test_main.py --run_basic_tests --models MLP --configuration " + + config) finally: os.chdir(cwd) From 3819b197c3274662ab20d49b3c7cb3aca5c267b1 Mon Sep 17 00:00:00 2001 From: Sayantan Sarkar Date: Thu, 22 Aug 2019 22:29:31 -0700 Subject: [PATCH 11/13] Minor --- test/python/test_modeltester.py | 1 + 1 file changed, 1 insertion(+) diff --git a/test/python/test_modeltester.py b/test/python/test_modeltester.py index cdf3c10d5..3bb3c4b0a 100644 --- a/test/python/test_modeltester.py +++ b/test/python/test_modeltester.py @@ -27,6 +27,7 @@ from common import NgraphTest from tools.build_utils import command_executor +import ngraph_bridge class TestModelTester(NgraphTest): From 1d6b49e593d51b2ad2346bc2cc168d2929875fd4 Mon Sep 17 00:00:00 2001 From: Sayantan Sarkar Date: Fri, 23 Aug 2019 00:21:08 -0700 Subject: [PATCH 12/13] Update json --- test/model_level_tests/models/MLP/test1/expected.json | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/model_level_tests/models/MLP/test1/expected.json b/test/model_level_tests/models/MLP/test1/expected.json index 76af1d1f0..b432d0335 100644 --- a/test/model_level_tests/models/MLP/test1/expected.json +++ b/test/model_level_tests/models/MLP/test1/expected.json @@ -8,8 +8,8 @@ }, "1": { "num_ng_clusters": 15, - "num_nodes_in_graph": 330, - "num_nodes_marked_for_clustering": 267 + "num_nodes_in_graph": 311, + "num_nodes_marked_for_clustering": 249 } }, "time": 10 @@ -22,9 +22,9 @@ "num_nodes_marked_for_clustering": 0 }, "1": { - "num_ng_clusters": 17, - "num_nodes_in_graph": 329, - "num_nodes_marked_for_clustering": 262 + "num_ng_clusters": 15, + "num_nodes_in_graph": 330, + "num_nodes_marked_for_clustering": 267 } }, "time": 10 From 0fd974498ca5741ba6346dda487c76c60e4e889f Mon Sep 17 00:00:00 2001 From: Sayantan Sarkar Date: Fri, 23 Aug 2019 10:00:17 -0700 Subject: [PATCH 13/13] Update test/python/test_conv2D_KernelChecks.py --- test/python/test_conv2D_KernelChecks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/python/test_conv2D_KernelChecks.py b/test/python/test_conv2D_KernelChecks.py index ab13407da..2284478e2 100644 --- a/test/python/test_conv2D_KernelChecks.py +++ b/test/python/test_conv2D_KernelChecks.py @@ -65,7 +65,7 @@ def make_filter_and_backprop_args(self): @pytest.mark.parametrize(("strides",), ( ([2, 1, 1, 1],), - ([2, 1, 1, 1],), + ([1, 1, 1, 2],), )) def test_conv2d_stride_in_batch_not_supported(self, strides): inp_values, filt_values = self.make_filter_and_backprop_args()