Skip to content

Commit

Permalink
Fix #839: Make jobid 1-based.
Browse files Browse the repository at this point in the history
  • Loading branch information
andrrizzi committed Dec 1, 2017
1 parent d94cbcc commit e0ae107
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 7 deletions.
2 changes: 1 addition & 1 deletion Yank/commands/script.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
Optional Arguments:
--jobid=INTEGER You can run only a subset of the experiments by specifying jobid and njobs, where
0 <= job_id <= n_jobs-1. In this case, njobs must be specified as well and YANK will
1 <= job_id <= n_jobs. In this case, njobs must be specified as well and YANK will
run only 1/n_jobs of the experiments. This can be used to run several separate YANK
executions in parallel starting from the same script.
--njobs=INTEGER Specify the total number of parallel executions. jobid has to be specified too.
Expand Down
11 changes: 7 additions & 4 deletions Yank/experiment.py
Original file line number Diff line number Diff line change
Expand Up @@ -502,7 +502,7 @@ class ExperimentBuilder(object):
can load it later by using :func:`parse` (default is None).
job_id : None or int
If you want to split the experiments among different executions,
you can set this to an integer 0 <= job_id <= n_jobs-1, and this
you can set this to an integer 1 <= job_id <= n_jobs, and this
:class:`ExperimentBuilder` will run only 1/n_jobs of the experiments.
n_jobs : None or int
If ``job_id`` is specified, this is the total number of jobs that
Expand Down Expand Up @@ -606,8 +606,8 @@ def __init__(self, script=None, job_id=None, n_jobs=None):
if job_id is not None:
if n_jobs is None:
raise ValueError('n_jobs must be specified together with job_id')
if not 0 <= job_id <= n_jobs:
raise ValueError('job_id must be between 0 and n_jobs ({})'.format(n_jobs))
if not 1 <= job_id <= n_jobs:
raise ValueError('job_id must be between 1 and n_jobs ({})'.format(n_jobs))

self._job_id = job_id
self._n_jobs = n_jobs
Expand Down Expand Up @@ -1021,7 +1021,10 @@ def _expand_experiments(self):

# Loop over all combinations
for name, combination in experiment.named_combinations(separator='_', max_name_length=50):
if self._job_id is None or experiment_id % self._n_jobs == self._job_id:
# Both self._job_id and self._job_id-1 work (self._job_id is 1-based),
# but we use the latter just because it makes it easier to identify in
# advance which job ids are associated to which experiments.
if self._job_id is None or experiment_id % self._n_jobs == self._job_id-1:
yield os.path.join(output_dir, name), combination
experiment_id += 1

Expand Down
4 changes: 2 additions & 2 deletions Yank/tests/test_experiment.py
Original file line number Diff line number Diff line change
Expand Up @@ -1875,11 +1875,11 @@ def test_expand_experiments():
experiment_systems = utils.CombinatorialLeaf(['explicit-system', 'implicit-system', 'hydration-system'])
template_script['experiments']['system'] = experiment_systems

exp_builder = ExperimentBuilder(script=template_script, job_id=0, n_jobs=2)
exp_builder = ExperimentBuilder(script=template_script, job_id=1, n_jobs=2)
experiments = list(exp_builder._expand_experiments())
assert len(experiments) == 2

exp_builder = ExperimentBuilder(script=template_script, job_id=1, n_jobs=2)
exp_builder = ExperimentBuilder(script=template_script, job_id=2, n_jobs=2)
experiments = list(exp_builder._expand_experiments())
assert len(experiments) == 1

Expand Down

0 comments on commit e0ae107

Please sign in to comment.