From f8cd09494b6ad9986002032f22339143fbe7f70f Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Sat, 6 Feb 2021 21:58:06 +0100 Subject: [PATCH 1/4] Fix skipping of test cases in case of unresolved dependencies Also added unit tests to reproduce the problem. --- reframe/frontend/dependencies.py | 40 ++++++++++++++++++-------------- unittests/test_dependencies.py | 23 +++++++++++++----- 2 files changed, 39 insertions(+), 24 deletions(-) diff --git a/reframe/frontend/dependencies.py b/reframe/frontend/dependencies.py index fc7f645865..0873f09af3 100644 --- a/reframe/frontend/dependencies.py +++ b/reframe/frontend/dependencies.py @@ -65,7 +65,7 @@ def resolve_dep(src, dst): # We use an ordered dict here, because we need to keep the order of # partitions and environments graph = collections.OrderedDict() - skipped_cases = [] + unresolved_cases = [] for c in cases: psrc = c.partition.name esrc = c.environ.name @@ -78,32 +78,36 @@ def resolve_dep(src, dst): if when((psrc, esrc), (pdst, edst)): c.deps.append(d) except DependencyError as e: - getlogger().warning(f'{e}; skipping dependent test cases:') - - # FIXME: we need to unit test this properly - skip_nodes = {c} - while skip_nodes: - v = skip_nodes.pop() - skipped_cases.append(v) - getlogger().warning(f' - {v}') - pruned_nodes = [] - for u, adj in graph.items(): - if v in adj: - skip_nodes.add(u) - pruned_nodes.append(u) - - for u in pruned_nodes: - del graph[u] - + getlogger().warning(e) + unresolved_cases.append(c) continue graph[c] = util.OrderedSet(c.deps) + # Skip also all cases that depend on the unresolved ones + skipped_cases = [] + skip_nodes = set(unresolved_cases) + while skip_nodes: + v = skip_nodes.pop() + skipped_cases.append(v) + pruned_nodes = [] + for u, adj in graph.items(): + if v in adj: + skip_nodes.add(u) + pruned_nodes.append(u) + + for u in pruned_nodes: + del graph[u] + # Calculate in-degree of each node for u, adjacent in graph.items(): for v in adjacent: v.in_degree += 1 + getlogger().warning('skipping all dependent test cases') + for c in skipped_cases: + getlogger().warning(f' - {v}') + return graph, skipped_cases diff --git a/unittests/test_dependencies.py b/unittests/test_dependencies.py index bdeebaaecc..09034a6bfe 100644 --- a/unittests/test_dependencies.py +++ b/unittests/test_dependencies.py @@ -646,25 +646,36 @@ def test_validate_deps_empty(exec_ctx): def test_skip_unresolved_deps(make_test, temp_runtime): + # + # t0 t4 + # ^ ^ ^ + # / \ / + # t1 t2 + # ^ + # | + # t3 + # + rt = temp_runtime(fixtures.TEST_CONFIG_FILE, system='sys0:p0') next(rt) t0 = make_test('t0') + t0.valid_systems = ['sys0:p1'] t1 = make_test('t1') t2 = make_test('t2') t3 = make_test('t3') - t3.valid_systems = ['sys0:p1'] + t4 = make_test('t4') t1.depends_on('t0') t2.depends_on('t0') - t2.depends_on('t3') + t2.depends_on('t4') + t3.depends_on('t2') deps, skipped_cases = dependencies.build_deps( - executors.generate_testcases([t0, t1, t2, t3]) + executors.generate_testcases([t0, t1, t2, t3, t4]) ) - - assert len(skipped_cases) == 2 + assert len(skipped_cases) == 6 skipped_tests = {c.check.name for c in skipped_cases} - assert skipped_tests == {'t2'} + assert skipped_tests == {'t1', 't2', 't3'} def assert_topological_order(cases, graph): From 93948510ede3e2b5ca69f48d809e9b005c5e5326 Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Sat, 6 Feb 2021 22:28:19 +0100 Subject: [PATCH 2/4] Fine tune implementation and fix output --- reframe/frontend/dependencies.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/reframe/frontend/dependencies.py b/reframe/frontend/dependencies.py index 0873f09af3..d665323fae 100644 --- a/reframe/frontend/dependencies.py +++ b/reframe/frontend/dependencies.py @@ -90,24 +90,28 @@ def resolve_dep(src, dst): while skip_nodes: v = skip_nodes.pop() skipped_cases.append(v) - pruned_nodes = [] for u, adj in graph.items(): if v in adj: skip_nodes.add(u) - pruned_nodes.append(u) - for u in pruned_nodes: - del graph[u] + # Prune graph + for c in skipped_cases: + # Cases originally discovered (unresolved_cases) are not in the graph, + # but we loop over them here; therefore we use pop() + graph.pop(c, None) # Calculate in-degree of each node for u, adjacent in graph.items(): for v in adjacent: v.in_degree += 1 - getlogger().warning('skipping all dependent test cases') + msg = 'skipping all dependent test cases\n' for c in skipped_cases: - getlogger().warning(f' - {v}') + msg += f' - {c}\n' + if skipped_cases: + getlogger().warning(msg) + return graph, skipped_cases From 571d6de9e03b6472894b1de8747e0f6a0bd13f0d Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Sat, 6 Feb 2021 23:00:46 +0100 Subject: [PATCH 3/4] Update documentation --- docs/tutorial_deps.rst | 74 ++++++++++++++++---------------- reframe/frontend/dependencies.py | 2 +- 2 files changed, 39 insertions(+), 37 deletions(-) diff --git a/docs/tutorial_deps.rst b/docs/tutorial_deps.rst index 771427fdbe..b25995d2ec 100644 --- a/docs/tutorial_deps.rst +++ b/docs/tutorial_deps.rst @@ -104,7 +104,7 @@ Here is the output when running the OSU tests with the asynchronous execution po [ReFrame Setup] version: 3.4-dev2 (rev: 56c6c237) command: './bin/reframe --system=daint -C tutorials/config/settings.py -c tutorials/deps/osu_benchmarks.py -r' - launched by: karakasv@dom101 + launched by: karakasv@daint101 working directory: '/users/karakasv/Devel/reframe' settings file: 'tutorials/config/settings.py' check search path: '/users/karakasv/Devel/reframe/tutorials/deps/osu_benchmarks.py' @@ -234,7 +234,7 @@ For example, if we select only the :class:`OSULatencyTest` for running, ReFrame [ReFrame Setup] version: 3.3-dev2 (rev: 8ded20cd) command: './bin/reframe -C tutorials/config/settings.py -c tutorials/deps/osu_benchmarks.py -n OSULatencyTest -l' - launched by: user@dom101 + launched by: user@daint101 working directory: '/users/user/Devel/reframe' settings file: 'tutorials/config/settings.py' check search path: '/users/user/Devel/reframe/tutorials/deps/osu_benchmarks.py' @@ -256,48 +256,50 @@ As a result, its immediate dependency :class:`OSUBuildTest` will be skipped, whi .. code-block:: console - ./bin/reframe --system=daint:gpu -c tutorials/deps/osu_benchmarks.py -n OSULatencyTest -l + ./bin/reframe -c tutorials/deps/osu_benchmarks.py -n OSULatencyTest -l .. code-block:: none [ReFrame Setup] - version: 3.4-dev2 (rev: c1a364f3) - command: './bin/reframe --system=daint:gpu -C tutorials/config/settings.py -c tutorials/deps/osu_benchmarks.py -n OSULatencyTest -l' - launched by: karakasv@dom101 - working directory: '/users/karakasv/Devel/reframe' + version: 3.5-dev0 (rev: 93948510) + command: './bin/reframe -C tutorials/config/settings.py --system=daint:gpu -c tutorials/deps/osu_benchmarks.py -l' + launched by: user@daint101 + working directory: '/users/user/Devel/reframe' settings file: 'tutorials/config/settings.py' - check search path: '/users/karakasv/Devel/reframe/tutorials/deps/osu_benchmarks.py' - stage directory: '/users/karakasv/Devel/reframe/stage' - output directory: '/users/karakasv/Devel/reframe/output' + check search path: '/users/user/Devel/reframe/tutorials/deps/osu_benchmarks.py' + stage directory: '/users/user/Devel/reframe/stage' + output directory: '/users/user/Devel/reframe/output' + + ./bin/reframe: could not resolve dependency: ('OSUBuildTest', 'daint:gpu', 'gnu') -> 'OSUDownloadTest' + ./bin/reframe: could not resolve dependency: ('OSUBuildTest', 'daint:gpu', 'intel') -> 'OSUDownloadTest' + ./bin/reframe: could not resolve dependency: ('OSUBuildTest', 'daint:gpu', 'pgi') -> 'OSUDownloadTest' + ./bin/reframe: skipping all dependent test cases + - ('OSUBuildTest', 'daint:gpu', 'intel') + - ('OSUBuildTest', 'daint:gpu', 'pgi') + - ('OSUAllreduceTest_4', 'daint:gpu', 'intel') + - ('OSUAllreduceTest_2', 'daint:gpu', 'intel') + - ('OSULatencyTest', 'daint:gpu', 'pgi') + - ('OSUBandwidthTest', 'daint:gpu', 'pgi') + - ('OSUAllreduceTest_8', 'daint:gpu', 'intel') + - ('OSUAllreduceTest_16', 'daint:gpu', 'pgi') + - ('OSUBuildTest', 'daint:gpu', 'gnu') + - ('OSUBandwidthTest', 'daint:gpu', 'intel') + - ('OSULatencyTest', 'daint:gpu', 'intel') + - ('OSUAllreduceTest_16', 'daint:gpu', 'intel') + - ('OSUAllreduceTest_8', 'daint:gpu', 'pgi') + - ('OSULatencyTest', 'daint:gpu', 'gnu') + - ('OSUBandwidthTest', 'daint:gpu', 'gnu') + - ('OSUAllreduceTest_2', 'daint:gpu', 'pgi') + - ('OSUAllreduceTest_4', 'daint:gpu', 'pgi') + - ('OSUAllreduceTest_8', 'daint:gpu', 'gnu') + - ('OSUAllreduceTest_4', 'daint:gpu', 'gnu') + - ('OSUAllreduceTest_2', 'daint:gpu', 'gnu') + - ('OSUAllreduceTest_16', 'daint:gpu', 'gnu') - ./bin/reframe: could not resolve dependency: ('OSUBuildTest', 'daint:gpu', 'gnu') -> 'OSUDownloadTest'; skipping dependent test cases: - ./bin/reframe: - ('OSUBuildTest', 'daint:gpu', 'gnu') - ./bin/reframe: - ('OSUBandwidthTest', 'daint:gpu', 'gnu') - ./bin/reframe: - ('OSUAllreduceTest_16', 'daint:gpu', 'gnu') - ./bin/reframe: - ('OSULatencyTest', 'daint:gpu', 'gnu') - ./bin/reframe: - ('OSUAllreduceTest_4', 'daint:gpu', 'gnu') - ./bin/reframe: - ('OSUAllreduceTest_8', 'daint:gpu', 'gnu') - ./bin/reframe: - ('OSUAllreduceTest_2', 'daint:gpu', 'gnu') - ./bin/reframe: could not resolve dependency: ('OSUBuildTest', 'daint:gpu', 'intel') -> 'OSUDownloadTest'; skipping dependent test cases: - ./bin/reframe: - ('OSUBuildTest', 'daint:gpu', 'intel') - ./bin/reframe: - ('OSULatencyTest', 'daint:gpu', 'intel') - ./bin/reframe: - ('OSUAllreduceTest_4', 'daint:gpu', 'intel') - ./bin/reframe: - ('OSUAllreduceTest_16', 'daint:gpu', 'intel') - ./bin/reframe: - ('OSUBandwidthTest', 'daint:gpu', 'intel') - ./bin/reframe: - ('OSUAllreduceTest_2', 'daint:gpu', 'intel') - ./bin/reframe: - ('OSUAllreduceTest_8', 'daint:gpu', 'intel') - ./bin/reframe: could not resolve dependency: ('OSUBuildTest', 'daint:gpu', 'pgi') -> 'OSUDownloadTest'; skipping dependent test cases: - ./bin/reframe: - ('OSUBuildTest', 'daint:gpu', 'pgi') - ./bin/reframe: - ('OSUAllreduceTest_8', 'daint:gpu', 'pgi') - ./bin/reframe: - ('OSUAllreduceTest_2', 'daint:gpu', 'pgi') - ./bin/reframe: - ('OSUBandwidthTest', 'daint:gpu', 'pgi') - ./bin/reframe: - ('OSUAllreduceTest_16', 'daint:gpu', 'pgi') - ./bin/reframe: - ('OSUAllreduceTest_4', 'daint:gpu', 'pgi') - ./bin/reframe: - ('OSULatencyTest', 'daint:gpu', 'pgi') [List of matched checks] Found 0 check(s) - Log file(s) saved in: '/tmp/rfm-eiay984f.log' + Log file(s) saved in: '/tmp/rfm-hjit66h2.log' Listing Dependencies @@ -381,7 +383,7 @@ The following listing shows how the actual test cases dependencies are formed wh Location: - /users/karakasv/Devel/reframe/tutorials/deps/osu_benchmarks.py + /users/user/Devel/reframe/tutorials/deps/osu_benchmarks.py Maintainers: diff --git a/reframe/frontend/dependencies.py b/reframe/frontend/dependencies.py index d665323fae..fe17dd288e 100644 --- a/reframe/frontend/dependencies.py +++ b/reframe/frontend/dependencies.py @@ -111,7 +111,7 @@ def resolve_dep(src, dst): if skipped_cases: getlogger().warning(msg) - + return graph, skipped_cases From ec6995f594d8bc01e7740465e39de59d972ed074 Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Mon, 8 Feb 2021 16:20:04 +0100 Subject: [PATCH 4/4] Fix user name in tutorial of dependencies --- docs/tutorial_deps.rst | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/tutorial_deps.rst b/docs/tutorial_deps.rst index b25995d2ec..96a1e1447e 100644 --- a/docs/tutorial_deps.rst +++ b/docs/tutorial_deps.rst @@ -104,12 +104,12 @@ Here is the output when running the OSU tests with the asynchronous execution po [ReFrame Setup] version: 3.4-dev2 (rev: 56c6c237) command: './bin/reframe --system=daint -C tutorials/config/settings.py -c tutorials/deps/osu_benchmarks.py -r' - launched by: karakasv@daint101 - working directory: '/users/karakasv/Devel/reframe' + launched by: user@daint101 + working directory: '/users/user/Devel/reframe' settings file: 'tutorials/config/settings.py' - check search path: '/users/karakasv/Devel/reframe/tutorials/deps/osu_benchmarks.py' - stage directory: '/users/karakasv/Devel/reframe/stage' - output directory: '/users/karakasv/Devel/reframe/output' + check search path: '/users/user/Devel/reframe/tutorials/deps/osu_benchmarks.py' + stage directory: '/users/user/Devel/reframe/stage' + output directory: '/users/user/Devel/reframe/output' [==========] Running 8 check(s) [==========] Started on Mon Jan 25 19:34:09 2021