Skip to content

[feat] Remove sched_* internal variables #1372

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

[feat] Remove sched_* internal variables #1372

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0e99cd7
remove sched_* variables
rsarm Jun 10, 2020
d221bf6
remove commented line
rsarm Jun 12, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 0 additions & 15 deletions reframe/core/launchers/mpi.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,28 +110,13 @@ def command(self, job):
if job.num_cpus_per_task:
ret += ['--cpus-per-task=%s' % str(job.num_cpus_per_task)]

if job.sched_partition:
ret += ['--partition=%s' % str(job.sched_partition)]

if job.sched_exclusive_access:
ret += ['--exclusive']

if job.use_smt is not None:
hint = 'multithread' if job.use_smt else 'nomultithread'
ret += ['--hint=%s' % hint]

if job.sched_partition:
ret += ['--partition=%s' % str(job.sched_partition)]

if job.sched_account:
ret += ['--account=%s' % str(job.sched_account)]

if job.sched_nodelist:
ret += ['--nodelist=%s' % str(job.sched_nodelist)]

if job.sched_exclude_nodelist:
ret += ['--exclude=%s' % str(job.sched_exclude_nodelist)]

for opt in job.options:
if opt.startswith('#'):
continue
Expand Down
30 changes: 0 additions & 30 deletions reframe/core/schedulers/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,11 +200,6 @@ def __init__(self,
max_pending_time=None,
sched_flex_alloc_nodes=None,
sched_access=[],
sched_account=None,
sched_partition=None,
sched_reservation=None,
sched_nodelist=None,
sched_exclude_nodelist=None,
sched_exclusive_access=None,
sched_options=None):

Expand Down Expand Up @@ -236,11 +231,6 @@ def __init__(self,
# Backend scheduler related information
self._sched_flex_alloc_nodes = sched_flex_alloc_nodes
self._sched_access = sched_access
self._sched_nodelist = sched_nodelist
self._sched_exclude_nodelist = sched_exclude_nodelist
self._sched_partition = sched_partition
self._sched_reservation = sched_reservation
self._sched_account = sched_account
self._sched_exclusive_access = sched_exclusive_access

@classmethod
Expand Down Expand Up @@ -281,26 +271,6 @@ def sched_flex_alloc_nodes(self):
def sched_access(self):
return self._sched_access

@property
def sched_nodelist(self):
return self._sched_nodelist

@property
def sched_exclude_nodelist(self):
return self._sched_exclude_nodelist

@property
def sched_partition(self):
return self._sched_partition

@property
def sched_reservation(self):
return self._sched_reservation

@property
def sched_account(self):
return self._sched_account

@property
def sched_exclusive_access(self):
return self._sched_exclusive_access
Expand Down
4 changes: 0 additions & 4 deletions reframe/core/schedulers/pbs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,6 @@ def emit_preamble(self, job):
preamble.append(
self._format_option('-l walltime=%d:%d:%d' % (h, m, s)))

if job.sched_partition:
preamble.append(
self._format_option('-q %s' % job.sched_partition))

preamble += self._emit_lselect_option(job)

# PBS starts the job in the home directory by default
Expand Down
19 changes: 0 additions & 19 deletions reframe/core/schedulers/slurm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,11 +161,6 @@ def emit_preamble(self, job):
self._format_option(job.num_tasks_per_socket,
'--ntasks-per-socket={0}'),
self._format_option(job.num_cpus_per_task, '--cpus-per-task={0}'),
self._format_option(job.sched_partition, '--partition={0}'),
self._format_option(job.sched_account, '--account={0}'),
self._format_option(job.sched_nodelist, '--nodelist={0}'),
self._format_option(job.sched_exclude_nodelist, '--exclude={0}'),
self._format_option(job.sched_reservation, '--reservation={0}')
]

# Slurm replaces '%a' by the corresponding SLURM_ARRAY_TASK_ID
Expand Down Expand Up @@ -254,20 +249,6 @@ def filternodes(self, job, nodes):
# create a mutable list out of the immutable SequenceView that
# sched_access is
options = list(job.sched_access + job.options)
if job.sched_partition:
options.append('--partition=%s' % job.sched_partition)

if job.sched_account:
options.append('--account=%s' % job.sched_account)

if job.sched_nodelist:
options.append('--nodelist=%s' % job.sched_nodelist)

if job.sched_exclude_nodelist:
options.append('--exclude=%s' % job.sched_exclude_nodelist)

if job.sched_reservation:
options.append('--reservation=%s' % job.sched_reservation)

option_parser = ArgumentParser()
option_parser.add_argument('--reservation')
Expand Down
11 changes: 5 additions & 6 deletions reframe/frontend/cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -595,28 +595,33 @@ def print_infoline(param, value):

options.flex_alloc_nodes = options.flex_alloc_nodes or 'idle'
if options.account:
options.job_options.append(f'account={options.account}')
printer.warning(f"`--account' is deprecated and "
f"will be removed in the future; you should "
f"use `-J account={options.account}'")

if options.partition:
options.job_options.append(f'partition={options.partition}')
Copy link
Contributor

@ekouts ekouts Jun 12, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vkarak do we want to keep the option working only for slurm? Is there a way to pass this argument for both pbs and slurm through job_options?

Copy link
Contributor

@vkarak vkarak Jun 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ekout, you are right. We cannot drop the support for other schedulers now, so we should keep the sched_partition until we completely remove the scheduler-specific options.

printer.warning(f"`--partition' is deprecated and "
f"will be removed in the future; you should "
f"use `-J partition={options.partition}' "
f"or `-J q={options.partition}' depending on your "
f"scheduler")

if options.reservation:
options.job_options.append(f'reservation={options.reservation}')
printer.warning(f"`--reservation' is deprecated and "
f"will be removed in the future; you should "
f"use `-J reservation={options.reservation}'")

if options.nodelist:
options.job_options.append(f'nodelist={options.nodelist}')
printer.warning(f"`--nodelist' is deprecated and "
f"will be removed in the future; you should "
f"use `-J nodelist={options.nodelist}'")

if options.exclude_nodes:
options.job_options.append(f'exclude={options.exclude_nodes}')
printer.warning(f"`--exclude-nodes' is deprecated and "
f"will be removed in the future; you should "
f"use `-J exclude={options.exclude_nodes}'")
Expand Down Expand Up @@ -663,12 +668,6 @@ def print_infoline(param, value):
sched_flex_alloc_nodes = options.flex_alloc_nodes

exec_policy.sched_flex_alloc_nodes = sched_flex_alloc_nodes
exec_policy.flex_alloc_nodes = options.flex_alloc_nodes
exec_policy.sched_account = options.account
exec_policy.sched_partition = options.partition
exec_policy.sched_reservation = options.reservation
exec_policy.sched_nodelist = options.nodelist
exec_policy.sched_exclude_nodelist = options.exclude_nodes
parsed_job_options = []
for opt in options.job_options:
if opt.startswith('-') or opt.startswith('#'):
Expand Down
5 changes: 0 additions & 5 deletions reframe/frontend/executors/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -454,11 +454,6 @@ def __init__(self):

# Scheduler options
self.sched_flex_alloc_nodes = None
self.sched_account = None
self.sched_partition = None
self.sched_reservation = None
self.sched_nodelist = None
self.sched_exclude_nodelist = None
self.sched_options = []

# Task event listeners
Expand Down
10 changes: 0 additions & 10 deletions reframe/frontend/executors/policies.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,6 @@ def runcase(self, case):

task.setup(partition, environ,
sched_flex_alloc_nodes=self.sched_flex_alloc_nodes,
sched_account=self.sched_account,
sched_partition=self.sched_partition,
sched_reservation=self.sched_reservation,
sched_nodelist=self.sched_nodelist,
sched_exclude_nodelist=self.sched_exclude_nodelist,
sched_options=self.sched_options)

task.compile()
Expand Down Expand Up @@ -257,11 +252,6 @@ def _setup_task(self, task):
task.setup(task.testcase.partition,
task.testcase.environ,
sched_flex_alloc_nodes=self.sched_flex_alloc_nodes,
sched_account=self.sched_account,
sched_partition=self.sched_partition,
sched_reservation=self.sched_reservation,
sched_nodelist=self.sched_nodelist,
sched_exclude_nodelist=self.sched_exclude_nodelist,
sched_options=self.sched_options)
except TaskExit:
return False
Expand Down
10 changes: 0 additions & 10 deletions unittests/test_launchers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,6 @@ def setUp(self):
script_filename='fake_script',
stdout='fake_stdout',
stderr='fake_stderr',
sched_account='fake_account',
sched_partition='fake_partition',
sched_reservation='fake_reservation',
sched_nodelist="mynode",
sched_exclude_nodelist='fake_exclude_nodelist',
sched_exclusive_access='fake_exclude_access',
sched_options=['--fake'])
self.job.num_tasks = 4
Expand Down Expand Up @@ -128,13 +123,8 @@ def expected_command(self):
'--ntasks-per-core=1 '
'--ntasks-per-socket=1 '
'--cpus-per-task=2 '
'--partition=fake_partition '
'--exclusive '
'--hint=multithread '
'--partition=fake_partition '
'--account=fake_account '
'--nodelist=mynode '
'--exclude=fake_exclude_nodelist '
'--fake '
'--gres=gpu:4 '
'--foo')
Expand Down
47 changes: 3 additions & 44 deletions unittests/test_schedulers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,11 +105,6 @@ def minimal_job(make_job):
@pytest.fixture
def fake_job(make_job):
ret = make_job(
sched_nodelist='nid000[00-17]',
sched_exclude_nodelist='nid00016',
sched_partition='foo',
sched_reservation='bar',
sched_account='spam',
sched_exclusive_access=True
)
ret.time_limit = '5m'
Expand Down Expand Up @@ -160,11 +155,6 @@ def _expected_slurm_directives(job):
'#SBATCH --ntasks-per-socket=%s' % job.num_tasks_per_socket,
'#SBATCH --cpus-per-task=%s' % job.num_cpus_per_task,
'#SBATCH --hint=multithread',
'#SBATCH --nodelist=%s' % job.sched_nodelist,
'#SBATCH --exclude=%s' % job.sched_exclude_nodelist,
'#SBATCH --partition=%s' % job.sched_partition,
'#SBATCH --reservation=%s' % job.sched_reservation,
'#SBATCH --account=%s' % job.sched_account,
'#SBATCH --exclusive',
# Custom options and directives
'#SBATCH --gres=gpu:4',
Expand All @@ -188,7 +178,6 @@ def _expected_pbs_directives(job):
':mem=100GB:cpu_type=haswell' % (num_nodes,
job.num_tasks_per_node,
num_cpus_per_node),
'#PBS -q %s' % job.sched_partition,
'#PBS --gres=gpu:4',
'#DW jobdw capacity=100GB',
'#DW stage_in source=/foo'
Expand All @@ -205,7 +194,6 @@ def _expected_torque_directives(job):
'#PBS -e %s' % job.stderr,
'#PBS -l nodes=%s:ppn=%s:haswell' % (num_nodes, num_cpus_per_node),
'#PBS -l mem=100GB',
'#PBS -q %s' % job.sched_partition,
'#PBS --gres=gpu:4',
'#DW jobdw capacity=100GB',
'#DW stage_in source=/foo'
Expand Down Expand Up @@ -681,8 +669,7 @@ def test_flex_alloc_sched_access_idle_sequence_view(make_flexible_job):
from reframe.utility import SequenceView

job = make_flexible_job('idle',
sched_access=SequenceView(['--constraint=f3']),
sched_partition='p3')
sched_access=SequenceView(['--constraint=f3']))
prepare_job(job)
assert job.num_tasks == 4

Expand Down Expand Up @@ -715,7 +702,8 @@ def test_flex_alloc_constraint_idle(make_flexible_job):


def test_flex_alloc_partition_idle(make_flexible_job):
job = make_flexible_job('idle', sched_partition='p2')
job = make_flexible_job('idle')
job.options = ['--partition=p2']
with pytest.raises(JobError):
prepare_job(job)

Expand All @@ -734,12 +722,6 @@ def test_flex_alloc_valid_multiple_constraints(make_flexible_job):
assert job.num_tasks == 4


def test_flex_alloc_valid_partition_cmd(make_flexible_job):
job = make_flexible_job('all', sched_partition='p2')
prepare_job(job)
assert job.num_tasks == 8


def test_flex_alloc_valid_partition_opt(make_flexible_job):
job = make_flexible_job('all')
job.options = ['-p p2']
Expand All @@ -761,12 +743,6 @@ def test_flex_alloc_valid_constraint_partition(make_flexible_job):
assert job.num_tasks == 4


def test_flex_alloc_invalid_partition_cmd(make_flexible_job):
job = make_flexible_job('all', sched_partition='invalid')
with pytest.raises(JobError):
prepare_job(job)


def test_flex_alloc_invalid_partition_opt(make_flexible_job):
job = make_flexible_job('all')
job.options = ['--partition=invalid']
Expand All @@ -781,30 +757,13 @@ def test_flex_alloc_invalid_constraint(make_flexible_job):
prepare_job(job)


def test_flex_alloc_valid_reservation_cmd(make_flexible_job):
job = make_flexible_job('all',
sched_access=['--constraint=f2'],
sched_reservation='dummy')
Copy link
Contributor

@vkarak vkarak Jun 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test should remain except that the reservation should be passed through sched_options.


prepare_job(job)
assert job.num_tasks == 4


def test_flex_alloc_valid_reservation_option(make_flexible_job):
job = make_flexible_job('all', sched_access=['--constraint=f2'])
job.options = ['--reservation=dummy']
prepare_job(job)
assert job.num_tasks == 4


def test_flex_alloc_exclude_nodes_cmd(make_flexible_job):
job = make_flexible_job('all',
sched_access=['--constraint=f1'],
sched_exclude_nodelist='nid00001')
Copy link
Contributor

@vkarak vkarak Jun 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as above.

prepare_job(job)
assert job.num_tasks == 8


def test_flex_alloc_exclude_nodes_opt(make_flexible_job):
job = make_flexible_job('all', sched_access=['--constraint=f1'])
job.options = ['-x nid00001']
Expand Down