-
Notifications
You must be signed in to change notification settings - Fork 200
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
Customize shebang 739 #940
Merged
Merged
Changes from all commits
Commits
Show all changes
24 commits
Select commit
Hold shift + click to select a range
004c3b2
First work on #739, in which I make it possible to store shebang in t…
lekah 8178cdb
Updated tests that were failing for #739
lekah 3799315
for #739 the JobTemplate instance now gets the shebang line from the …
lekah ccdc450
Updated the travis-test for the daemon for #739
lekah d9a5bf5
Updated scheduler test (#739)
lekah ed612c1
Merge branch 'develop' into customize_shebang_739
lekah 4a27bce
Minor fix for Configuration Error message in sqlalchemy's DbComputer
lekah b224ee7
Merge branch 'develop' into customize_shebang_739
lekah 5050ccc
Merge branch 'develop' into customize_shebang_739
lekah b2001da
Merge branch 'develop' into customize_shebang_739
lekah e308ad0
Merge branch 'develop' into customize_shebang_739
lekah 7206e89
Merge branch 'customize_shebang_739' of github.com:lekah/aiida_core i…
lekah 6fc7daa
For #739 moved the set_shebang into the abstract class
lekah 0390233
For #739 I check the jobTemplate's shebang attribute, in order to pro…
lekah f112369
For #739 I test the default shebang line
lekah 8c7c9b9
Added documentation for #739
lekah 8ca5c64
Changed the order of the computer setup so that shebang line is asked…
lekah ff96a5c
Added new check if user specified an empty string for the shebang lin…
lekah af7e3b6
Improved exception message when setting shebabgn
lekah 68758fc
Corrected small bag in check for shebang line
lekah de8e5ef
Still related to changing the order of shebang and workdir
lekah 6b08696
Fixed docstring
lekah 49633da
Merge branch 'develop' into customize_shebang_739
lekah 1278e4d
Merge branch 'develop' into customize_shebang_739
giovannipizzi File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,7 @@ localhost | |
True | ||
ssh | ||
torque | ||
#!/bin/bash | ||
/scratch/{username}/aiida_run | ||
mpirun -np {tot_num_mpiprocs} | ||
1 | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -908,6 +908,7 @@ def test_submit_script(self): | |
s = PbsproScheduler() | ||
|
||
job_tmpl = JobTemplate() | ||
job_tmpl.shebang = '#!/bin/bash -l' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You put a |
||
job_tmpl.job_resource = s.create_job_resource(num_machines=1, num_mpiprocs_per_machine=1) | ||
job_tmpl.uuid = str(uuid.uuid4()) | ||
job_tmpl.max_wallclock_seconds = 24 * 3600 | ||
|
@@ -920,11 +921,39 @@ def test_submit_script(self): | |
submit_script_text = s.get_submit_script(job_tmpl) | ||
|
||
self.assertTrue('#PBS -r n' in submit_script_text) | ||
self.assertTrue(submit_script_text.startswith('#!/bin/bash')) | ||
self.assertTrue(submit_script_text.startswith('#!/bin/bash -l')) | ||
self.assertTrue('#PBS -l walltime=24:00:00' in submit_script_text) | ||
self.assertTrue('#PBS -l select=1' in submit_script_text) | ||
self.assertTrue("'mpirun' '-np' '23' 'pw.x' '-npool' '1'" + \ | ||
" < 'aiida.in'" in submit_script_text) | ||
def test_submit_script_bad_shebang(self): | ||
""" | ||
Test to verify if scripts works fine with default options | ||
""" | ||
from aiida.scheduler.datastructures import JobTemplate | ||
from aiida.common.datastructures import CodeInfo, code_run_modes | ||
|
||
s = PbsproScheduler() | ||
code_info = CodeInfo() | ||
code_info.cmdline_params = ["mpirun", "-np", "23", "pw.x", "-npool", "1"] | ||
code_info.stdin_name = 'aiida.in' | ||
|
||
for (shebang, expected_first_line) in ((None, '#!/bin/bash'), ("",""), ("NOSET", '#!/bin/bash')): | ||
job_tmpl = JobTemplate() | ||
if shebang == "NOSET": | ||
pass | ||
else: | ||
job_tmpl.shebang = shebang | ||
job_tmpl.job_resource = s.create_job_resource(num_machines=1, num_mpiprocs_per_machine=1) | ||
job_tmpl.codes_info = [code_info] | ||
job_tmpl.codes_run_mode = code_run_modes.SERIAL | ||
|
||
submit_script_text = s.get_submit_script(job_tmpl) | ||
|
||
# This tests if the implementation correctly chooses the default: | ||
self.assertEquals(submit_script_text.split('\n')[0], expected_first_line) | ||
|
||
|
||
|
||
def test_submit_script_with_num_cores_per_machine(self): | ||
""" | ||
|
@@ -937,6 +966,7 @@ def test_submit_script_with_num_cores_per_machine(self): | |
s = PbsproScheduler() | ||
|
||
job_tmpl = JobTemplate() | ||
job_tmpl.shebang = '#!/bin/bash' | ||
job_tmpl.job_resource = s.create_job_resource( | ||
num_machines=1, | ||
num_mpiprocs_per_machine=2, | ||
|
@@ -973,6 +1003,7 @@ def test_submit_script_with_num_cores_per_mpiproc(self): | |
s = PbsproScheduler() | ||
|
||
job_tmpl = JobTemplate() | ||
job_tmpl.shebang = '#!/bin/bash' | ||
job_tmpl.job_resource = s.create_job_resource( | ||
num_machines=1, | ||
num_mpiprocs_per_machine=1, | ||
|
@@ -1013,6 +1044,7 @@ def test_submit_script_with_num_cores_per_machine_and_mpiproc1(self): | |
s = PbsproScheduler() | ||
|
||
job_tmpl = JobTemplate() | ||
job_tmpl.shebang = '#!/bin/bash' | ||
job_tmpl.job_resource = s.create_job_resource( | ||
num_machines=1, | ||
num_mpiprocs_per_machine=1, | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't this logic be put in the abstract class? It's a pity that it's copy-pasted, and also even more dangerously the default is duplicated. I think the function can just stay in the base class instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't think it should. The fact that there is a member called dbcomputer is a choice of the implementation. Some other backend could call it _dbcomputer or whatever, it's not enforced. As well as the function get_shebang, also not enforced. I would leave it where it is.