From 9c5345a6c966818ce897dcfccd4abcde4aa26be2 Mon Sep 17 00:00:00 2001 From: Eirini Koutsaniti Date: Thu, 19 Aug 2021 09:16:57 +0200 Subject: [PATCH 1/7] Add partition modules in remote partitions' autodetecting --- reframe/frontend/autodetect.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/reframe/frontend/autodetect.py b/reframe/frontend/autodetect.py index 4a1d6d55b8..1ffa178b0e 100644 --- a/reframe/frontend/autodetect.py +++ b/reframe/frontend/autodetect.py @@ -111,18 +111,17 @@ def _is_part_local(part): def _remote_detect(part): - def _emit_script(job): + def _emit_script(job, env): launcher_cmd = job.launcher.run_command(job) commands = [ f'./bootstrap.sh', f'{launcher_cmd} ./bin/reframe --detect-host-topology=topo.json' ] - job.prepare(commands, trap_errors=True) + job.prepare(commands, env, trap_errors=True) getlogger().info( f'Detecting topology of remote partition {part.fullname!r}: ' f'this may take some time...' - ) topo_info = {} try: @@ -133,7 +132,7 @@ def _emit_script(job): part.launcher_type(), name='rfm-detect-job', sched_access=part.access) - _emit_script(job) + _emit_script(job, [part.local_env]) getlogger().debug('submitting detection script') _log_contents(job.script_filename) job.submit() From 46707e8eadbad2bc5376d9048ac70a5ad0d4dfab Mon Sep 17 00:00:00 2001 From: Eirini Koutsaniti Date: Mon, 23 Aug 2021 10:57:14 +0200 Subject: [PATCH 2/7] Take into account system modules in autodetect --- reframe/frontend/autodetect.py | 18 ++++++++++++------ reframe/utility/__init__.py | 5 ++++- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/reframe/frontend/autodetect.py b/reframe/frontend/autodetect.py index 1ffa178b0e..3cad6b47ce 100644 --- a/reframe/frontend/autodetect.py +++ b/reframe/frontend/autodetect.py @@ -10,10 +10,10 @@ import tempfile import reframe as rfm +import reframe.core.runtime as runtime import reframe.utility.osext as osext from reframe.core.exceptions import ConfigError from reframe.core.logging import getlogger -from reframe.core.runtime import runtime from reframe.core.schedulers import Job from reframe.utility.cpuinfo import cpuinfo @@ -60,7 +60,7 @@ def __exit__(self, exc_type, exc_val, exc_tb): def _subschema(fragment): '''Create a configuration subschema.''' - full_schema = runtime().site_config.schema + full_schema = runtime.runtime().site_config.schema return { '$schema': full_schema['$schema'], 'defs': full_schema['defs'], @@ -125,7 +125,7 @@ def _emit_script(job, env): ) topo_info = {} try: - prefix = runtime().get_option('general/0/remote_workdir') + prefix = runtime.runtime().get_option('general/0/remote_workdir') with _copy_reframe(prefix) as dirname: with osext.change_dir(dirname): job = Job.create(part.scheduler, @@ -148,7 +148,7 @@ def _emit_script(job, env): def detect_topology(): - rt = runtime() + rt = runtime.runtime() detect_remote_systems = rt.get_option('general/0/remote_detect') topo_prefix = os.path.join(os.getenv('HOME'), '.reframe/topology') for part in rt.system.partitions: @@ -200,12 +200,18 @@ def detect_topology(): if not found_procinfo: # No topology found, try to auto-detect it getlogger().debug(f'> no topology file found; auto-detecting...') + temp_modules = rt.system.preload_environ.modules if _is_part_local(part): + temp_modules += part.local_env.modules # Unconditionally detect the system for fully local partitions - part.processor._info = cpuinfo() + with runtime.temp_environment(modules=temp_modules): + part.processor._info = cpuinfo() + _save_info(topo_file, part.processor.info) elif detect_remote_systems: - part.processor._info = _remote_detect(part) + with runtime.temp_environment(modules=temp_modules): + part.processor._info = _remote_detect(part) + if part.processor.info: _save_info(topo_file, part.processor.info) diff --git a/reframe/utility/__init__.py b/reframe/utility/__init__.py index 96f0140ddb..8dd9e85fea 100644 --- a/reframe/utility/__init__.py +++ b/reframe/utility/__init__.py @@ -1391,7 +1391,10 @@ def __add__(self, other): if not isinstance(other, collections.abc.Sequence): return NotImplemented - return SequenceView(self.__container + other) + if isinstance(other, SequenceView): + return SequenceView(self.__container + other.__container) + else: + return SequenceView(self.__container + other) def __iadd__(self, other): return NotImplemented From 5aeebbe656b7f0799e616d6ed0fea168d9243c90 Mon Sep 17 00:00:00 2001 From: Eirini Koutsaniti Date: Mon, 23 Aug 2021 11:03:03 +0200 Subject: [PATCH 3/7] Add unittest for SequenceView sum --- unittests/test_utility.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/unittests/test_utility.py b/unittests/test_utility.py index bfc7737122..bcbacc535d 100644 --- a/unittests/test_utility.py +++ b/unittests/test_utility.py @@ -1081,6 +1081,10 @@ def test_sequence_view(): assert [1, 2, 2, 3, 4] == l assert isinstance(l, util.SequenceView) + n = m + l + assert [1, 2, 2, 1, 2, 2, 3, 4] == n + assert isinstance(n, util.SequenceView) + with pytest.raises(TypeError): l[1] = 3 From cf24650fc54990a78a5281da040fb3832d2c0350 Mon Sep 17 00:00:00 2001 From: Eirini Koutsaniti Date: Tue, 24 Aug 2021 14:07:26 +0200 Subject: [PATCH 4/7] Support addition of MappingView --- reframe/utility/__init__.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/reframe/utility/__init__.py b/reframe/utility/__init__.py index 8dd9e85fea..21c445bfec 100644 --- a/reframe/utility/__init__.py +++ b/reframe/utility/__init__.py @@ -1473,3 +1473,11 @@ def __repr__(self): def __str__(self): return str(self.__mapping) + + def __add__(self, other): + if not isinstance(other, type(self)): + return NotImplemented + + new_mapping = self.__mapping.copy() + new_mapping.update(other.__mapping) + return MappingView(new_mapping) From fa85ef72d8ac62acc055bf684ef225578fd46358 Mon Sep 17 00:00:00 2001 From: Eirini Koutsaniti Date: Tue, 24 Aug 2021 14:08:23 +0200 Subject: [PATCH 5/7] Take into account system and partition variables in autodetect --- reframe/frontend/autodetect.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/reframe/frontend/autodetect.py b/reframe/frontend/autodetect.py index 3cad6b47ce..b233405c47 100644 --- a/reframe/frontend/autodetect.py +++ b/reframe/frontend/autodetect.py @@ -201,15 +201,19 @@ def detect_topology(): # No topology found, try to auto-detect it getlogger().debug(f'> no topology file found; auto-detecting...') temp_modules = rt.system.preload_environ.modules + temp_vars = rt.system.preload_environ.variables if _is_part_local(part): temp_modules += part.local_env.modules + temp_vars += part.local_env.variables # Unconditionally detect the system for fully local partitions - with runtime.temp_environment(modules=temp_modules): + with runtime.temp_environment(modules=temp_modules, + variables=temp_vars): part.processor._info = cpuinfo() _save_info(topo_file, part.processor.info) elif detect_remote_systems: - with runtime.temp_environment(modules=temp_modules): + with runtime.temp_environment(modules=temp_modules, + variables=temp_vars): part.processor._info = _remote_detect(part) if part.processor.info: From e0f8e792033345355faaa6351286a3f2d5102f03 Mon Sep 17 00:00:00 2001 From: Eirini Koutsaniti Date: Tue, 24 Aug 2021 14:13:07 +0200 Subject: [PATCH 6/7] Add unittest for MappingView addition --- unittests/test_utility.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/unittests/test_utility.py b/unittests/test_utility.py index bcbacc535d..46d0b567e2 100644 --- a/unittests/test_utility.py +++ b/unittests/test_utility.py @@ -1137,6 +1137,8 @@ def test_mapping_view(): assert d == util.MappingView({'b': 2, 'a': 1}) assert str(d) == str({'a': 1, 'b': 2}) assert {'a': 1, 'b': 2, 'c': 3} != d + e = util.MappingView({'c': 3}) + assert {'a': 1, 'b': 2, 'c': 3} == d + e # Assert immutability with pytest.raises(TypeError): From fdcf262219761d2858bb33dada8cbcb81570d002 Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Wed, 25 Aug 2021 00:30:42 +0200 Subject: [PATCH 7/7] Address PR comments --- reframe/core/schedulers/slurm.py | 2 +- reframe/frontend/autodetect.py | 12 ++++++------ reframe/utility/__init__.py | 28 +++++++++++++--------------- unittests/test_utility.py | 21 ++++++++++----------- 4 files changed, 30 insertions(+), 33 deletions(-) diff --git a/reframe/core/schedulers/slurm.py b/reframe/core/schedulers/slurm.py index bd019c9df3..6adacd47f6 100644 --- a/reframe/core/schedulers/slurm.py +++ b/reframe/core/schedulers/slurm.py @@ -292,7 +292,7 @@ def filternodes(self, job, nodes): # Collect options that restrict node selection, but we need to first # create a mutable list out of the immutable SequenceView that # sched_access is - options = list(job.sched_access + job.options + job.cli_options) + options = job.sched_access + job.options + job.cli_options option_parser = ArgumentParser() option_parser.add_argument('--reservation') option_parser.add_argument('-p', '--partition') diff --git a/reframe/frontend/autodetect.py b/reframe/frontend/autodetect.py index b233405c47..8b56882f21 100644 --- a/reframe/frontend/autodetect.py +++ b/reframe/frontend/autodetect.py @@ -200,14 +200,14 @@ def detect_topology(): if not found_procinfo: # No topology found, try to auto-detect it getlogger().debug(f'> no topology file found; auto-detecting...') - temp_modules = rt.system.preload_environ.modules - temp_vars = rt.system.preload_environ.variables + modules = list(rt.system.preload_environ.modules) + vars = dict(rt.system.preload_environ.variables.items()) if _is_part_local(part): - temp_modules += part.local_env.modules - temp_vars += part.local_env.variables + modules += part.local_env.modules + vars.update(part.local_env.variables) + # Unconditionally detect the system for fully local partitions - with runtime.temp_environment(modules=temp_modules, - variables=temp_vars): + with runtime.temp_environment(modules=modules, variables=vars): part.processor._info = cpuinfo() _save_info(topo_file, part.processor.info) diff --git a/reframe/utility/__init__.py b/reframe/utility/__init__.py index 21c445bfec..07bf095d61 100644 --- a/reframe/utility/__init__.py +++ b/reframe/utility/__init__.py @@ -1338,6 +1338,12 @@ class SequenceView(collections.abc.Sequence): :raises TypeError: If the container does not fulfill the :py:class:`collections.abc.Sequence` interface. + .. note:: + + You can concatenate a :class:`SequenceView` with a container of the + same type as the underlying container of the view, in which case a new + container with the concatenated elements will be returned. + ''' def __init__(self, container): @@ -1388,16 +1394,16 @@ def __reversed__(self): return self.__container.__reversed__() def __add__(self, other): - if not isinstance(other, collections.abc.Sequence): + if not isinstance(other, type(self.__container)): return NotImplemented - if isinstance(other, SequenceView): - return SequenceView(self.__container + other.__container) - else: - return SequenceView(self.__container + other) + return self.__container + other - def __iadd__(self, other): - return NotImplemented + def __radd__(self, other): + if not isinstance(other, type(self.__container)): + return NotImplemented + + return other + self.__container def __eq__(self, other): if isinstance(other, SequenceView): @@ -1473,11 +1479,3 @@ def __repr__(self): def __str__(self): return str(self.__mapping) - - def __add__(self, other): - if not isinstance(other, type(self)): - return NotImplemented - - new_mapping = self.__mapping.copy() - new_mapping.update(other.__mapping) - return MappingView(new_mapping) diff --git a/unittests/test_utility.py b/unittests/test_utility.py index 46d0b567e2..17f8570549 100644 --- a/unittests/test_utility.py +++ b/unittests/test_utility.py @@ -1072,18 +1072,18 @@ def test_sequence_view(): # Assert immutability m = l + [3, 4] assert [1, 2, 2, 3, 4] == m - assert isinstance(m, util.SequenceView) + assert isinstance(m, list) - m = l - l += [3, 4] - assert m is not l - assert [1, 2, 2] == m - assert [1, 2, 2, 3, 4] == l - assert isinstance(l, util.SequenceView) + m_orig = m = util.SequenceView([1]) + m += [3, 4] + assert m is not m_orig + assert [1] == m_orig + assert [1, 3, 4] == m + assert isinstance(m, list) n = m + l - assert [1, 2, 2, 1, 2, 2, 3, 4] == n - assert isinstance(n, util.SequenceView) + assert [1, 3, 4, 1, 2, 2] == n + assert isinstance(n, list) with pytest.raises(TypeError): l[1] = 3 @@ -1137,8 +1137,6 @@ def test_mapping_view(): assert d == util.MappingView({'b': 2, 'a': 1}) assert str(d) == str({'a': 1, 'b': 2}) assert {'a': 1, 'b': 2, 'c': 3} != d - e = util.MappingView({'c': 3}) - assert {'a': 1, 'b': 2, 'c': 3} == d + e # Assert immutability with pytest.raises(TypeError): @@ -1548,6 +1546,7 @@ def test_jsonext_dumps(): ) assert '{"(1, 2, 3)": 1}' == jsonext.dumps({(1, 2, 3): 1}) + # Classes to test JSON deserialization class _D(jsonext.JSONSerializable):