From 4eb3751203b39a67ffcfe8d2c74cce8c4ca45c26 Mon Sep 17 00:00:00 2001 From: Theofilos Manitaras Date: Wed, 3 Jul 2019 16:24:18 +0200 Subject: [PATCH 1/6] Allow module self loops in module mappings --- reframe/core/modules.py | 13 ++++++++++--- unittests/test_modules.py | 41 ++++++++++++++++++++++++++++++++------- 2 files changed, 44 insertions(+), 10 deletions(-) diff --git a/reframe/core/modules.py b/reframe/core/modules.py index 12907c4112..08fcf0eaf1 100644 --- a/reframe/core/modules.py +++ b/reframe/core/modules.py @@ -12,6 +12,7 @@ import reframe.utility.typecheck as types from reframe.core.exceptions import (ConfigError, EnvironError, SpawnedProcessError) +from reframe.utility import OrderedSet class Module: @@ -75,6 +76,7 @@ class ModulesSystem: """A modules system abstraction inside ReFrame. This class interfaces between the framework internals and the actual +re modules systems implementation. """ @@ -109,13 +111,19 @@ def resolve_module(self, name): visited = set() unvisited = [(name, None)] path = [] + loop_nodes = set() while unvisited: node, parent = unvisited.pop() - # Adjust the path while path and path[-1] != parent: path.pop() + # Handle modules mappings with self loops + if node == parent and node not in loop_nodes: + loop_nodes.add(node) + ret.append(node) + continue + try: # We insert the adjacent nodes in reverse order, so as to # preserve the DFS access order @@ -126,10 +134,9 @@ def resolve_module(self, name): else: path.append(node) for m in adjacent: - if m in path: + if m in path and m !=node: raise EnvironError('module cyclic dependency: ' + '->'.join(path + [m])) - if m not in visited: unvisited.append((m, node)) diff --git a/unittests/test_modules.py b/unittests/test_modules.py index 10e487664d..0ce6498fcd 100644 --- a/unittests/test_modules.py +++ b/unittests/test_modules.py @@ -455,14 +455,16 @@ def test_mapping_cycle_simple(self): self.assertRaises(EnvironError, self.modules_system.load_module, 'm0') self.assertRaises(EnvironError, self.modules_system.load_module, 'm1') - def test_mapping_cycle_self(self): + def test_mapping_single_module_self_loop(self): # # m0 -> m0 # self.modules_system.module_map = { 'm0': ['m0'], } - self.assertRaises(EnvironError, self.modules_system.load_module, 'm0') + self.modules_system.load_module('m0') + assert self.modules_system.is_module_loaded('m0') + assert ['m0'] == self.modules_system.backend.load_seq def test_mapping_deep_cycle(self): # @@ -499,7 +501,7 @@ def test_mapping_from_file_simple(self): self.modules_system.load_mapping_from_file(self.mapping_file.name) self.assertEqual(reference_map, self.modules_system.module_map) - def test_maping_from_file_missing_key_separator(self): + def test_mapping_from_file_missing_key_separator(self): with self.mapping_file: self.mapping_file.write('m1 m2') @@ -507,7 +509,7 @@ def test_maping_from_file_missing_key_separator(self): self.modules_system.load_mapping_from_file, self.mapping_file.name) - def test_maping_from_file_empty_value(self): + def test_mapping_from_file_empty_value(self): with self.mapping_file: self.mapping_file.write('m1: # m2') @@ -515,7 +517,7 @@ def test_maping_from_file_empty_value(self): self.modules_system.load_mapping_from_file, self.mapping_file.name) - def test_maping_from_file_multiple_key_separators(self): + def test_mapping_from_file_multiple_key_separators(self): with self.mapping_file: self.mapping_file.write('m1 : m2 : m3') @@ -523,7 +525,7 @@ def test_maping_from_file_multiple_key_separators(self): self.modules_system.load_mapping_from_file, self.mapping_file.name) - def test_maping_from_file_empty_key(self): + def test_mapping_from_file_empty_key(self): with self.mapping_file: self.mapping_file.write(' : m2') @@ -531,7 +533,32 @@ def test_maping_from_file_empty_key(self): self.modules_system.load_mapping_from_file, self.mapping_file.name) - def test_maping_from_file_missing_file(self): + def test_mapping_from_file_missing_file(self): self.assertRaises(OSError, self.modules_system.load_mapping_from_file, 'foo') + + def test_mapping_with_self_loop(self): + self.modules_system.module_map = { + 'm0': ['m1', 'm0', 'm2'], + 'm1': ['m4', 'm3'] + } + self.modules_system.load_module('m0') + assert self.modules_system.is_module_loaded('m0') + assert self.modules_system.is_module_loaded('m1') + assert self.modules_system.is_module_loaded('m2') + assert self.modules_system.is_module_loaded('m3') + assert self.modules_system.is_module_loaded('m4') + assert ['m4', 'm3', 'm0', 'm2'] == self.modules_system.backend.load_seq + + def test_mapping_with_self_loop_and_duplicate_modules(self): + self.modules_system.module_map = { + 'm0': ['m0', 'm0', 'm1', 'm1'], + 'm1': ['m2', 'm3'] + } + self.modules_system.load_module('m0') + assert self.modules_system.is_module_loaded('m0') + assert self.modules_system.is_module_loaded('m1') + assert self.modules_system.is_module_loaded('m2') + assert self.modules_system.is_module_loaded('m3') + assert ['m0', 'm2', 'm3'] == self.modules_system.backend.load_seq From 06fa5ecf554f6a015e85b52cdfc8aa3d23a085c8 Mon Sep 17 00:00:00 2001 From: Theofilos Manitaras Date: Wed, 3 Jul 2019 17:35:07 +0200 Subject: [PATCH 2/6] Fixed PEP8 issues --- reframe/core/modules.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/reframe/core/modules.py b/reframe/core/modules.py index 08fcf0eaf1..6d033cdf85 100644 --- a/reframe/core/modules.py +++ b/reframe/core/modules.py @@ -120,9 +120,9 @@ def resolve_module(self, name): # Handle modules mappings with self loops if node == parent and node not in loop_nodes: - loop_nodes.add(node) - ret.append(node) - continue + loop_nodes.add(node) + ret.append(node) + continue try: # We insert the adjacent nodes in reverse order, so as to @@ -134,7 +134,7 @@ def resolve_module(self, name): else: path.append(node) for m in adjacent: - if m in path and m !=node: + if m in path and m != node: raise EnvironError('module cyclic dependency: ' + '->'.join(path + [m])) if m not in visited: From 72d48622cade06b26c51e0b63d4f2d387c1c45c6 Mon Sep 17 00:00:00 2001 From: Theofilos Manitaras Date: Wed, 3 Jul 2019 23:05:27 +0200 Subject: [PATCH 3/6] Remove mistyped line --- reframe/core/modules.py | 1 - 1 file changed, 1 deletion(-) diff --git a/reframe/core/modules.py b/reframe/core/modules.py index 6d033cdf85..2ff79adc0e 100644 --- a/reframe/core/modules.py +++ b/reframe/core/modules.py @@ -76,7 +76,6 @@ class ModulesSystem: """A modules system abstraction inside ReFrame. This class interfaces between the framework internals and the actual -re modules systems implementation. """ From 4c7ebd0ae0c987a87479e7acae01ed0de13d92c8 Mon Sep 17 00:00:00 2001 From: Theofilos Manitaras Date: Wed, 3 Jul 2019 23:06:24 +0200 Subject: [PATCH 4/6] Remove stale import --- reframe/core/modules.py | 1 - 1 file changed, 1 deletion(-) diff --git a/reframe/core/modules.py b/reframe/core/modules.py index 2ff79adc0e..ad1760cfbc 100644 --- a/reframe/core/modules.py +++ b/reframe/core/modules.py @@ -12,7 +12,6 @@ import reframe.utility.typecheck as types from reframe.core.exceptions import (ConfigError, EnvironError, SpawnedProcessError) -from reframe.utility import OrderedSet class Module: From eb7a92d27b3a1649e5f3dce76ab1bba34eaaa307 Mon Sep 17 00:00:00 2001 From: Theofilos Manitaras Date: Thu, 4 Jul 2019 14:07:46 +0200 Subject: [PATCH 5/6] Address PR comments --- docs/running.rst | 11 +++++++++++ reframe/core/modules.py | 13 ++++++------- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/docs/running.rst b/docs/running.rst index 00a5f2580a..937a52f30a 100644 --- a/docs/running.rst +++ b/docs/running.rst @@ -1137,6 +1137,10 @@ Manipulating modules .. versionadded:: 2.11 +.. note:: + .. versionchanged:: 2.19 + Module self loops are now allowed in *module mappings*. + ReFrame allows you to change the modules loaded by a regression test on-the-fly without having to edit the regression test file. This feature is extremely useful when you need to quickly test a newer version of a module, but it also allows you to completely decouple the module names used in your regression tests from the real module names in a system, thus making your test even more portable. This is achieved by defining *module mappings*. @@ -1206,6 +1210,13 @@ If you now try to run a test that loads the module `cudatoolkit`, the following * Reason: caught framework exception: module cyclic dependency: cudatoolkit->foo->bar->foobar->cudatoolkit ------------------------------------------------------------------------------ +On the other hand, module mappings containing self loops are allowed. +In the following example, ReFrame will load both ``module-1`` and ``module-2`` whenever the ``module-1`` is encountered: + +.. code-block:: none + + --map-module 'module-1: module-1 module-2' + Controlling the Flexible Task Allocation ---------------------------------------- diff --git a/reframe/core/modules.py b/reframe/core/modules.py index ad1760cfbc..648659f66a 100644 --- a/reframe/core/modules.py +++ b/reframe/core/modules.py @@ -12,6 +12,7 @@ import reframe.utility.typecheck as types from reframe.core.exceptions import (ConfigError, EnvironError, SpawnedProcessError) +from reframe.utility import OrderedSet class Module: @@ -105,11 +106,10 @@ def resolve_module(self, name): :raises: :class:`reframe.core.exceptions.ConfigError` if the mapping contains a cycle. """ - ret = [] + ret = OrderedSet() visited = set() unvisited = [(name, None)] path = [] - loop_nodes = set() while unvisited: node, parent = unvisited.pop() # Adjust the path @@ -117,9 +117,8 @@ def resolve_module(self, name): path.pop() # Handle modules mappings with self loops - if node == parent and node not in loop_nodes: - loop_nodes.add(node) - ret.append(node) + if node == parent and node not in ret: + ret.add(node) continue try: @@ -128,7 +127,7 @@ def resolve_module(self, name): adjacent = reversed(self.module_map[node]) except KeyError: # We have reached a terminal node - ret.append(node) + ret.add(node) else: path.append(node) for m in adjacent: @@ -140,7 +139,7 @@ def resolve_module(self, name): visited.add(node) - return ret + return list(ret) @property def backend(self): From b11d7da4e10a7f2fa1cee52924e764563e7e906c Mon Sep 17 00:00:00 2001 From: Theofilos Manitaras Date: Thu, 4 Jul 2019 14:34:02 +0200 Subject: [PATCH 6/6] Address PR comments (version 2) --- reframe/core/modules.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/reframe/core/modules.py b/reframe/core/modules.py index 648659f66a..dea5c9af12 100644 --- a/reframe/core/modules.py +++ b/reframe/core/modules.py @@ -117,7 +117,7 @@ def resolve_module(self, name): path.pop() # Handle modules mappings with self loops - if node == parent and node not in ret: + if node == parent: ret.add(node) continue