Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
072dce6
Adding a check to the number of requested gpus to give a meaningful m…
Dutil Sep 8, 2017
f6ab791
The package is now installed using entry_points. The old scripts used…
Dutil Sep 11, 2017
d9c4e9c
changed the call to check_output to be able to mock it.
Dutil Sep 12, 2017
4a0d63d
Adding a check for the launcher error.
Dutil Sep 12, 2017
9f63771
Adding test to test the well behaviour of the script
Dutil Sep 12, 2017
cc07bc6
Using an unexisting queue.
Dutil Sep 13, 2017
4d76f5a
more precise error message.
Dutil Sep 13, 2017
1b7ffbb
Puttig all the tests on the script in the same file.
Dutil Sep 13, 2017
90c4f4d
The tests cover more cases and are more explicit.
Dutil Sep 21, 2017
fba5bf3
removing useless lines.
Dutil Sep 21, 2017
5d99e79
Raising a new Exception instead with the stack instead of falling the…
Dutil Sep 21, 2017
7e357d8
Removing duplicate test. Putting all the tests that have to do with t…
Dutil Sep 26, 2017
c33292b
Removing duplicate test. Putting all the tests that have to do with t…
Dutil Sep 26, 2017
a83e500
Merge branch 'iss155' of https://github.com/Dutil/smartdispatch into …
Dutil Oct 3, 2017
94f58d0
refactoring the test to catch and reraise the exceptions.
Dutil Oct 9, 2017
7eb4b84
Adding specific advices depending on the cluster we are currently on.
Dutil Oct 9, 2017
cec39e6
refactoring the test utils, and testing le script when no gpus are av…
Dutil Oct 10, 2017
3b27597
Correcting some typo and use better decorator helper functions.
Dutil Oct 17, 2017
71af7a3
Adding some comments to make the tests easier to understand.
Dutil Oct 17, 2017
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
27 changes: 22 additions & 5 deletions smartdispatch/smartdispatch_script.py
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,13 @@ def main(argv=None):
# TODO: use args.memPerNode instead of args.memPerNode
queue = Queue(args.queueName, CLUSTER_NAME, args.walltime, args.coresPerNode, args.gpusPerNode, float('inf'), args.modules)

# Change the default value of the gpusPerCommand depending on the value of
if args.gpusPerCommand is None:
if queue.nb_gpus_per_node == 0:
args.gpusPerCommand = 0
else:
args.gpusPerCommand = 1

# Check that requested core number does not exceed node total
if args.coresPerCommand > queue.nb_cores_per_node:
sys.stderr.write("smart-dispatch: error: coresPerCommand exceeds nodes total: asked {req_cores} cores, nodes have {node_cores}\n"
Expand All @@ -156,8 +163,11 @@ def main(argv=None):

# Check that requested gpu number does not exceed node total
if args.gpusPerCommand > queue.nb_gpus_per_node:
sys.stderr.write("smart-dispatch: error: gpusPerCommand exceeds nodes total: asked {req_gpus} gpus, nodes have {node_gpus}\n"
.format(req_gpus=args.gpusPerCommand, node_gpus=queue.nb_gpus_per_node))

error_message = ("smart-dispatch: error: gpusPerCommand exceeds nodes total:"
"asked {req_gpus} gpus, nodes have {node_gpus}. Make sure you have specified the correct queue.\n")

sys.stderr.write(error_message.format(req_gpus=args.gpusPerCommand, node_gpus=queue.nb_gpus_per_node))
sys.exit(2)


Expand Down Expand Up @@ -194,7 +204,14 @@ def main(argv=None):

cluster_advice = utils.get_advice(CLUSTER_NAME)

sys.stderr.write("smart-dispatch: error: The launcher wasn't able the launch the job(s) properly. The following error message was returned: \n\n{}\n\nMaybe the pbs file(s) generated were invalid. {}\n\n".format(e.output, cluster_advice))
error_message = ("smart-dispatch: error: The launcher wasn't"
" able the launch the job(s) properly. The"
" following error message was returned: \n\n{}"
"\n\nMaybe the pbs file(s) generated were"
" invalid. {}\n\n")


sys.stderr.write(error_message.format(e.output, cluster_advice))
sys.exit(2)

print "\nLogs, command, and jobs id related to this batch will be in:\n {smartdispatch_folder}".format(smartdispatch_folder=path_job)
Expand All @@ -212,7 +229,7 @@ def parse_arguments(argv=None):
# parser.add_argument('-M', '--memPerNode', type=int, required=False, help='How much memory there are per node (in Gb).')

parser.add_argument('-c', '--coresPerCommand', type=int, required=False, help='How many cores a command needs.', default=1)
parser.add_argument('-g', '--gpusPerCommand', type=int, required=False, help='How many gpus a command needs.', default=1)
parser.add_argument('-g', '--gpusPerCommand', type=int, required=False, help='How many gpus a command needs. The value is 1 by default if GPUs are available on the specified queue, 0 otherwise.')
# parser.add_argument('-m', '--memPerCommand', type=float, required=False, help='How much memory a command needs (in Gb).')
parser.add_argument('-f', '--commandsFile', type=file, required=False, help='File containing commands to launch. Each command must be on a seperate line. (Replaces commandAndOptions)')

Expand Down Expand Up @@ -241,7 +258,7 @@ def parse_arguments(argv=None):
parser.error("Unknown queue, --coresPerNode/--gpusPerNode and --walltime must be set.")
if args.coresPerCommand < 1:
parser.error("coresPerNode must be at least 1")

return args


Expand Down
66 changes: 50 additions & 16 deletions smartdispatch/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,45 @@
import hashlib
import unicodedata
import json
import sys
import six

from distutils.util import strtobool
from subprocess import Popen, PIPE

HELIOS_ADVICE = ("On Helios, don't forget that the queue gpu_1, gpu_2, gpu_4"
" and gpu_8 give access to a specific amount of gpus."
"\nFor more advices, please refer to the official"
" documentation 'https://wiki.calculquebec.ca/w/Helios/en'")

MAMMOUTH_ADVICE = ("On Mammouth, please refer to the official documentation"
" for more information:"
" 'https://wiki.ccs.usherbrooke.ca/Accueil/en'")

HADES_ADVICE = ("On Hades, don't forget that the queue name '@hades' needs"
" to be used.\nFor more advices, please refer to the"
" official documentation: 'https://wiki.calculquebec.ca/w"
"/Ex%C3%A9cuter_une_t%C3%A2che/en#tab=tab5'")

GUILLIMIN_ADVICE = ("On Guillimin, please refer to the official documentation"
" for more information: 'http://www.hpc.mcgill.ca/"
"index.php/starthere'")


def get_advice(cluster_name):

if cluster_name == "helios":
return HELIOS_ADVICE
elif cluster_name == 'mammouth':
return MAMMOUTH_ADVICE
elif cluster_name == 'hades':
return HADES_ADVICE
elif cluster_name == "guillimin":
return GUILLIMIN_ADVICE

return ''


def jobname_generator(jobname, job_id):
'''Crop the jobname to a maximum of 64 characters.
Parameters
Expand Down Expand Up @@ -137,25 +172,24 @@ def get_launcher(cluster_name):
else:
return "qsub"

def get_advice(cluster_name):

helios_advice = """On Helios, don't forget that the queue gpu_1, gpu_2, gpu_4 and gpu_8 give access to a specific amount of gpus.
For more advices, please refer to the official documentation: 'https://wiki.calculquebec.ca/w/Helios/en'"""
mammouth_advice = "On Mammouth, please refer to the official documentation for more information: 'https://wiki.ccs.usherbrooke.ca/Accueil/en'"
hades_advice = """On Hades, don't forget that the queue name '@hades' needs to be use.
For more advices, please refer to the official documentation: 'https://wiki.calculquebec.ca/w/Ex%C3%A9cuter_une_t%C3%A2che/en#tab=tab5'"""
guillimin_advice = """On Guillimin, please refer to the official documentation for more information: 'http://www.hpc.mcgill.ca/index.php/starthere'"""
def rethrow_exception(exception, new_message):

if cluster_name == "helios":
return helios_advice
elif cluster_name == 'mammouth':
return mammouth_advice
elif cluster_name == 'hades':
return hades_advice
elif cluster_name == "guillimin":
return guillimin_advice
def func_wraper(func):

return ''
def test_func(*args, **kwargs):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Wouldn't be better to use @functools.wraps(func) here for the stacktrace? I mean

@functools.wraps(func)
def test_func(*args, **kwargs):

I just had a problem with stack trace because of decorators while implementing tests for Slurm clusters. I wonder if the same thing is happening here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Just tried, it didn't change the stack trace.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

OK, well you can leave it there anyway.

try:
return func(*args, **kwargs)
except exception as e:

orig_exc_type, orig_exc_value, orig_exc_traceback = sys.exc_info()
new_exc = Exception(new_message)
new_exc.reraised = True
new_exc.__cause__ = orig_exc_value

new_traceback = orig_exc_traceback
six.reraise(type(new_exc), new_exc, new_traceback)


return test_func
return func_wraper
45 changes: 19 additions & 26 deletions tests/test_smart_dispatch.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,29 +9,7 @@
from nose.tools import assert_true, assert_equal
from smartdispatch import smartdispatch_script
import six
import sys
import traceback

def rethrow_exception(exception, new_message):

def func_wraper(func):

def test_func(*args, **kwargs):
try:
return func(*args, **kwargs)
except exception as e:

orig_exc_type, orig_exc_value, orig_exc_traceback = sys.exc_info()
new_exc = Exception(new_message)
new_exc.reraised = True
new_exc.__cause__ = orig_exc_value

new_traceback = orig_exc_traceback
six.reraise(type(new_exc), new_exc, new_traceback)


return test_func
return func_wraper
from smartdispatch import utils

class TestSmartdispatcher(unittest.TestCase):

Expand Down Expand Up @@ -135,7 +113,7 @@ def test_main_launch_with_gpus_command(self):
assert_equal(exit_status_100, 2)
assert_true(os.path.isdir(self.logs_dir))

@rethrow_exception(SystemExit, "smartdispatch_script.main() raised SystemExit unexpectedly.")
@utils.rethrow_exception(SystemExit, "smartdispatch_script.main() raised SystemExit unexpectedly.")
def test_gpu_check(self):

argv = ['-x', '-g', '2', '-G', '1', '-C', '1', '-q', 'random', '-t', '00:00:10' ,'launch', 'echo', 'testing123']
Expand All @@ -150,7 +128,22 @@ def test_gpu_check(self):
argv[2] = '1'
smartdispatch_script.main(argv=argv)

@rethrow_exception(SystemExit, "smartdispatch_script.main() raised SystemExit unexpectedly.")
# Test if we don't have gpus. (and spicified in script).

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

spicified 😝

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks like you should do grep "spicified" 😝

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

In my defence, it's the same line/mistake as the other one 😜

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Coherence is important indeed

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Wait, it's the same file? hahaha! 😊

argv[2] = '0'
argv[4] = '0'
smartdispatch_script.main(argv=argv)

# Don't have gpus, but the user specofy 1 anyway.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

A variant! Specofy! 🙃

argv[2] = '1'
with self.assertRaises(SystemExit) as context:
smartdispatch_script.main(argv=argv)
self.assertTrue(context.exception.code, 2)

# Test if the user didn't specified anything.
argv = ['-x', '-C', '1', '-q', 'random', '-t', '00:00:10' ,'launch', 'echo', 'testing123']
smartdispatch_script.main(argv=argv)

@utils.rethrow_exception(SystemExit, "smartdispatch_script.main() raised SystemExit unexpectedly.")
def test_cpu_check(self):

argv = ['-x', '-c', '2', '-C', '1', '-G', '1', '-t', '00:00:10', '-q', 'random', 'launch', 'echo', 'testing123']
Expand All @@ -165,7 +158,7 @@ def test_cpu_check(self):
argv[2] = '1'
smartdispatch_script.main(argv=argv)

@rethrow_exception(subprocess.CalledProcessError, "smartdispatch_script.main() raised subprocess.CalledProcessError unexpectedly")
@utils.rethrow_exception(subprocess.CalledProcessError, "smartdispatch_script.main() raised subprocess.CalledProcessError unexpectedly")
@patch('subprocess.check_output')
def test_launch_job_check(self, mock_check_output):

Expand Down