Skip to content
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.

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

support 'UNLIMITED' and 'NOT_SET' slurm times #3586

Merged
merged 4 commits into from
Nov 28, 2019

Conversation

ltalirz
Copy link
Member

@ltalirz ltalirz commented Nov 27, 2019

Fixes #3415

requested walltimes in slurm can be 'UNLIMITED' and 'NOT_SET'.
now supporting these in the slurm plugin (& added tests).

requested walltimes in slurm can be 'UNLIMITED' and 'NOT_SET'.
now supporting these in the slurm plugin (& added tests).
@ltalirz ltalirz requested a review from sphuber November 27, 2019 21:32
Copy link
Contributor

@sphuber sphuber left a comment

Choose a reason for hiding this comment

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

Thanks @ltalirz aside from the suggestion to change the get_job function to just creating a mapping once and indexing that, this is good for me. However, I would like @greschd and @giovannipizzi to weigh in on the final choice of values for UNLIMITED and NOT_SET

aiida/schedulers/plugins/test_slurm.py Outdated Show resolved Hide resolved
@@ -46,43 +54,46 @@ def test_parse_common_joblist_output(self):

job_list = scheduler._parse_joblist_output(retval, stdout, stderr)

def get_job(id):
try:
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably be a bit easier and more efficient to just turn job_list into a mapping on job_id and then index that in the tests instead of doing get_job

Copy link
Member Author

Choose a reason for hiding this comment

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

done

Copy link
Member

@giovannipizzi giovannipizzi left a comment

Choose a reason for hiding this comment

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

Thanks, looks very good to me!

@greschd
Copy link
Member

greschd commented Nov 28, 2019

Looks good to me, too. Thanks for implementing this!

@sphuber sphuber merged commit 196a074 into aiidateam:develop Nov 28, 2019
@sphuber sphuber deleted the issue_3415_slurm_parsing branch November 28, 2019 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Parse UNLIMITED and NOT_SET time in SLURM scheduler
4 participants