From 5fe2a08b7d549249c92ff5a0dfad12f71584b0ba Mon Sep 17 00:00:00 2001 From: Theofilos Manitaras Date: Mon, 8 Jun 2020 10:45:40 +0200 Subject: [PATCH 1/5] Combine access and job options slurm '--constraint' * The `--constraint` options passed via `access` and `job.options` are combined in the resulting job script. * Add unittests to test the expected behavior. --- reframe/core/schedulers/slurm.py | 29 ++++++++++++++++++++++++----- unittests/test_schedulers.py | 22 +++++++++++++++++++++- 2 files changed, 45 insertions(+), 6 deletions(-) diff --git a/reframe/core/schedulers/slurm.py b/reframe/core/schedulers/slurm.py index 55a910d3c0..46f659fbcb 100644 --- a/reframe/core/schedulers/slurm.py +++ b/reframe/core/schedulers/slurm.py @@ -195,15 +195,34 @@ def emit_preamble(self, job): hint = 'multithread' if job.use_smt else 'nomultithread' for opt in job.sched_access: - preamble.append('%s %s' % (self._prefix, opt)) + if not opt.strip().startswith(('-C', '--constraint')): + preamble.append('%s %s' % (self._prefix, opt)) + + constraints = [] + option_parser = ArgumentParser() + option_parser.add_argument('-C', '--constraint') + parsed_args, _ = option_parser.parse_known_args(job.sched_access) + if parsed_args.constraint: + constraints += [parsed_args.constraint] + + # NOTE: Here last of the passed --constraint job options is taken + # into account in order to respect the behavior of slurm. + parsed_args, _ = option_parser.parse_known_args(job.options) + if parsed_args.constraint: + constraints += [parsed_args.constraint] + + if constraints: + preamble.append( + self._format_option(','.join(constraints), '--constraint={0}')) preamble.append(self._format_option(hint, '--hint={0}')) prefix_patt = re.compile(r'(#\w+)') for opt in job.options: - if not prefix_patt.match(opt): - preamble.append('%s %s' % (self._prefix, opt)) - else: - preamble.append(opt) + if not opt.strip().startswith(('-C', '--constraint')): + if not prefix_patt.match(opt): + preamble.append('%s %s' % (self._prefix, opt)) + else: + preamble.append(opt) # Filter out empty statements before returning return list(filter(None, preamble)) diff --git a/unittests/test_schedulers.py b/unittests/test_schedulers.py index ecf02555f6..cab96c7589 100644 --- a/unittests/test_schedulers.py +++ b/unittests/test_schedulers.py @@ -67,7 +67,7 @@ def exec_ctx(temp_runtime, scheduler): next(rt) if scheduler.registered_name == 'squeue': # slurm backend fulfills the functionality of the squeue backend, so - # if squeue is not configured, use slurrm instead + # if squeue is not configured, use slurm instead partition = (fixtures.partition_by_scheduler('squeue') or fixtures.partition_by_scheduler('slurm')) else: @@ -370,6 +370,26 @@ def test_no_empty_lines_in_preamble(minimal_job): assert line != '' +def test_combined_access_constraint(make_job, slurm_only): + job = make_job(sched_access=['--constraint=c1']) + job.options = ['--constraint=c2,c3'] + prepare_job(job) + with open(job.script_filename) as fp: + assert re.search(r'--constraint=c1,c2,c3$', fp.read(), re.MULTILINE) + assert re.search(r'--constraint=(c1|c2,c3)$', fp.read(), + re.MULTILINE) is None + + +def test_combined_access_multiple_constraints(make_job, slurm_only): + job = make_job(sched_access=['--constraint=c1']) + job.options = ['--constraint=c2', '--constraint=c3'] + prepare_job(job) + with open(job.script_filename) as fp: + assert re.search(r'--constraint=c1,c3$', fp.read(), re.MULTILINE) + assert re.search(r'--constraint=(c1|c2|c3)$', fp.read(), + re.MULTILINE) is None + + def test_guess_num_tasks(minimal_job, scheduler): minimal_job.num_tasks = 0 if scheduler.registered_name == 'local': From 0fddd0a0837195d2ec1771aaedee35676f307375 Mon Sep 17 00:00:00 2001 From: Theofilos Manitaras Date: Wed, 10 Jun 2020 09:47:43 +0200 Subject: [PATCH 2/5] Address PR comments --- reframe/core/schedulers/slurm.py | 33 ++++++++++++++++++-------------- unittests/test_schedulers.py | 14 ++++++-------- 2 files changed, 25 insertions(+), 22 deletions(-) diff --git a/reframe/core/schedulers/slurm.py b/reframe/core/schedulers/slurm.py index 46f659fbcb..a89002c8e8 100644 --- a/reframe/core/schedulers/slurm.py +++ b/reframe/core/schedulers/slurm.py @@ -199,30 +199,35 @@ def emit_preamble(self, job): preamble.append('%s %s' % (self._prefix, opt)) constraints = [] - option_parser = ArgumentParser() - option_parser.add_argument('-C', '--constraint') - parsed_args, _ = option_parser.parse_known_args(job.sched_access) - if parsed_args.constraint: - constraints += [parsed_args.constraint] + constraint_parser = ArgumentParser() + constraint_parser.add_argument('-C', '--constraint') + parsed_options, _ = constraint_parser.parse_known_args( + job.sched_access) + if parsed_options.constraint: + constraints.append(parsed_options.constraint.strip()) # NOTE: Here last of the passed --constraint job options is taken # into account in order to respect the behavior of slurm. - parsed_args, _ = option_parser.parse_known_args(job.options) - if parsed_args.constraint: - constraints += [parsed_args.constraint] + parsed_options, _ = constraint_parser.parse_known_args(job.options) + if parsed_options.constraint: + constraints.append(parsed_options.constraint.strip()) if constraints: preamble.append( - self._format_option(','.join(constraints), '--constraint={0}')) + self._format_option(','.join(constraints), '--constraint={0}') + ) preamble.append(self._format_option(hint, '--hint={0}')) prefix_patt = re.compile(r'(#\w+)') for opt in job.options: - if not opt.strip().startswith(('-C', '--constraint')): - if not prefix_patt.match(opt): - preamble.append('%s %s' % (self._prefix, opt)) - else: - preamble.append(opt) + if opt.strip().startswith(('-C', '--constraint')): + # Constraints are already processed + continue + + if not prefix_patt.match(opt): + preamble.append('%s %s' % (self._prefix, opt)) + else: + preamble.append(opt) # Filter out empty statements before returning return list(filter(None, preamble)) diff --git a/unittests/test_schedulers.py b/unittests/test_schedulers.py index cab96c7589..aa905bc27b 100644 --- a/unittests/test_schedulers.py +++ b/unittests/test_schedulers.py @@ -372,22 +372,20 @@ def test_no_empty_lines_in_preamble(minimal_job): def test_combined_access_constraint(make_job, slurm_only): job = make_job(sched_access=['--constraint=c1']) - job.options = ['--constraint=c2,c3'] + job.options = ['-C c2,c3'] prepare_job(job) with open(job.script_filename) as fp: - assert re.search(r'--constraint=c1,c2,c3$', fp.read(), re.MULTILINE) - assert re.search(r'--constraint=(c1|c2,c3)$', fp.read(), - re.MULTILINE) is None + assert re.search(r'(?m)--constraint=c1,c2,c3$', fp.read()) + assert re.search(r'(?m)--constraint=(c1|c2,c3)$', fp.read()) is None def test_combined_access_multiple_constraints(make_job, slurm_only): job = make_job(sched_access=['--constraint=c1']) - job.options = ['--constraint=c2', '--constraint=c3'] + job.options = ['--constraint=c2', '-C c3'] prepare_job(job) with open(job.script_filename) as fp: - assert re.search(r'--constraint=c1,c3$', fp.read(), re.MULTILINE) - assert re.search(r'--constraint=(c1|c2|c3)$', fp.read(), - re.MULTILINE) is None + assert re.search(r'(?m)--constraint=c1,c3$', fp.read()) + assert re.search(r'(?m)--constraint=(c1|c2|c3)$', fp.read()) is None def test_guess_num_tasks(minimal_job, scheduler): From 4c6411325e6d37df47e34430e10dedf396b37fa2 Mon Sep 17 00:00:00 2001 From: Theofilos Manitaras Date: Fri, 12 Jun 2020 11:41:00 +0200 Subject: [PATCH 3/5] Add unittest for verbatim constraint --- unittests/test_schedulers.py | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/unittests/test_schedulers.py b/unittests/test_schedulers.py index aa905bc27b..37d8ca9e9a 100644 --- a/unittests/test_schedulers.py +++ b/unittests/test_schedulers.py @@ -375,8 +375,10 @@ def test_combined_access_constraint(make_job, slurm_only): job.options = ['-C c2,c3'] prepare_job(job) with open(job.script_filename) as fp: - assert re.search(r'(?m)--constraint=c1,c2,c3$', fp.read()) - assert re.search(r'(?m)--constraint=(c1|c2,c3)$', fp.read()) is None + script_content = fp.read() + + assert re.search(r'(?m)--constraint=c1,c2,c3$', script_content) + assert re.search(r'(?m)--constraint=(c1|c2,c3)$', script_content) is None def test_combined_access_multiple_constraints(make_job, slurm_only): @@ -384,8 +386,22 @@ def test_combined_access_multiple_constraints(make_job, slurm_only): job.options = ['--constraint=c2', '-C c3'] prepare_job(job) with open(job.script_filename) as fp: - assert re.search(r'(?m)--constraint=c1,c3$', fp.read()) - assert re.search(r'(?m)--constraint=(c1|c2|c3)$', fp.read()) is None + script_content = fp.read() + + assert re.search(r'(?m)--constraint=c1,c3$', script_content) + assert re.search(r'(?m)--constraint=(c1|c2|c3)$', script_content) is None + + +def test_combined_access_verbatim_constraint(make_job, slurm_only): + job = make_job(sched_access=['--constraint=c1']) + job.options = ['#SBATCH --constraint=c2', '#SBATCH -C c3'] + prepare_job(job) + with open(job.script_filename) as fp: + script_content = fp.read() + + assert re.search(r'(?m)--constraint=c1$', script_content) + assert re.search(r'(?m)^#SBATCH --constraint=c2$', script_content) + assert re.search(r'(?m)^#SBATCH -C c3$', script_content) def test_guess_num_tasks(minimal_job, scheduler): From 82f5984e5822a0ad3bfb557079a9080139255764 Mon Sep 17 00:00:00 2001 From: Theofilos Manitaras Date: Mon, 15 Jun 2020 10:10:13 +0200 Subject: [PATCH 4/5] Document the constraint/access combination --- docs/manpage.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/manpage.rst b/docs/manpage.rst index 9955d5ec56..b21fe9fb6c 100644 --- a/docs/manpage.rst +++ b/docs/manpage.rst @@ -341,6 +341,10 @@ Options controlling job submission If ``key`` starts with ``-`` or ``#``, the option will be passed verbatim to the job script. Otherwise, ReFrame will add ``-`` or ``--`` as well as the directive corresponding to the current scheduler. This option will be emitted after any options specified in the :js:attr:`access` system partition configuration parameter. + Especially for the Slurm scheduler, constraint options, i.e ``-J constraint=value``, ``-J C=value``, ``-J --constraint=value``, ``-J -C=value` are going to be combined with the corresponding ones specified in the :js:attr:`access` system partition configuration parameter. + If multiple constraint options are specified with separate key-value pairs, only the last one is going to be taken into account. + For multiple combined constraints use the ``-J constraint=value1,value2`` syntax. + Note that the above is not valid if ``key`` starts with ``#`` in which case the option is going to be passed verbatim to the job script. ------------------------ From ffd51c3a9f9d9913cbd333e246df5e4b66362a22 Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Tue, 16 Jun 2020 22:51:25 +0200 Subject: [PATCH 5/5] Fine tune description of `-J` option --- docs/manpage.rst | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/docs/manpage.rst b/docs/manpage.rst index b21fe9fb6c..a5c76db856 100644 --- a/docs/manpage.rst +++ b/docs/manpage.rst @@ -337,15 +337,19 @@ Options controlling job submission .. option:: -J, --job-option=OPTION Pass ``OPTION`` directly to the job scheduler backend. - The syntax for this option is ``-J key=value``. - If ``key`` starts with ``-`` or ``#``, the option will be passed verbatim to the job script. - Otherwise, ReFrame will add ``-`` or ``--`` as well as the directive corresponding to the current scheduler. - This option will be emitted after any options specified in the :js:attr:`access` system partition configuration parameter. - Especially for the Slurm scheduler, constraint options, i.e ``-J constraint=value``, ``-J C=value``, ``-J --constraint=value``, ``-J -C=value` are going to be combined with the corresponding ones specified in the :js:attr:`access` system partition configuration parameter. - If multiple constraint options are specified with separate key-value pairs, only the last one is going to be taken into account. - For multiple combined constraints use the ``-J constraint=value1,value2`` syntax. - Note that the above is not valid if ``key`` starts with ``#`` in which case the option is going to be passed verbatim to the job script. - + The syntax of ``OPTION`` is ``-J key=value``. + If ``OPTION`` starts with ``-`` it will be passed verbatim to the backend job scheduler. + If ``OPTION`` starts with ``#`` it will be emitted verbatim in the job script. + Otherwise, ReFrame will pass ``--key=value`` or ``-k value`` (if ``key`` is a single character) to the backend scheduler. + Any job options specified with this command-line option will be emitted after any job options specified in the :js:attr:`access` system partition configuration parameter. + + Especially for the Slurm backends, constraint options, such as ``-J constraint=value``, ``-J C=value``, ``-J --constraint=value`` or ``-J -C=value``, are going to be combined with any constraint options specified in the :js:attr:`access` system partition configuration parameter. + For example, if ``-C x`` is specified in the :js:attr:`access` and ``-J C=y`` is passed to the command-line, ReFrame will pass ``-C x,y`` as a constraint to the scheduler. + Notice, however, that if constraint options are specified through multiple :option:`-J` options, only the last one will be considered. + If you wish to completely overwrite any constraint options passed in :js:attr:`access`, you should consider passing explicitly the Slurm directive with ``-J '#SBATCH --constraint=new'``. + + .. versionchanged:: 3.0 + This option has become more flexible. ------------------------ Flexible node allocation