From 4ae74dd42f4f86c4d2b77ac7d9fbfe55b8977f18 Mon Sep 17 00:00:00 2001 From: Michael Gecht Date: Wed, 28 Feb 2018 10:35:12 +0100 Subject: [PATCH 1/4] Add validation of module names --- mdbenchmark/generate.py | 120 +++++++++++++++----- mdbenchmark/mdengines/__init__.py | 74 +++++++++++-- mdbenchmark/tests/mdengines/test_init.py | 134 +++++++++++++++++++++-- mdbenchmark/tests/test_generate.py | 69 ++++++++++-- mdbenchmark/tests/test_utils.py | 17 ++- mdbenchmark/tests/test_validate.py | 85 ++++++++++++++ mdbenchmark/utils.py | 30 ++++- mdbenchmark/validate.py | 67 ++++++++++++ 8 files changed, 543 insertions(+), 53 deletions(-) create mode 100644 mdbenchmark/tests/test_validate.py create mode 100644 mdbenchmark/validate.py diff --git a/mdbenchmark/generate.py b/mdbenchmark/generate.py index a7bce8a7..151ebce8 100644 --- a/mdbenchmark/generate.py +++ b/mdbenchmark/generate.py @@ -19,12 +19,13 @@ # along with MDBenchmark. If not, see . import click import datreant.core as dtr -from jinja2.exceptions import TemplateNotFound from . import console from .cli import cli -from .mdengines import detect_md_engine, namd -from .utils import ENV, normalize_host, print_possible_hosts +from .mdengines import (SUPPORTED_ENGINES, detect_md_engine, + get_available_modules, validate_module_name) +from .utils import print_possible_hosts, retrieve_host_template +from .validate import validate_generate_arguments @cli.command() @@ -65,36 +66,29 @@ type=click.IntRange(1, 1440)) @click.option( '--list-hosts', help='Show available job templates.', is_flag=True) -def generate(name, gpu, module, host, min_nodes, max_nodes, time, list_hosts): - """Generate benchmarks.""" +@click.option( + '--skip-validation', + help='Skip the validation of module names.', + default=False, + is_flag=True) +def generate(name, gpu, module, host, min_nodes, max_nodes, time, list_hosts, + skip_validation): + """Generate benchmarks simulations from the CLI.""" if list_hosts: print_possible_hosts() return - if not name: - raise click.BadParameter( - 'Please specify the base name of your input files.', - param_hint='"-n" / "--name"') - - if not module: - raise click.BadParameter( - 'Please specify which mdengine module to use for the benchmarks.', - param_hint='"-m" / "--module"') - - if min_nodes > max_nodes: - raise click.BadParameter( - 'The minimal number of nodes needs to be smaller than the maximal number.', - param_hint='"--min-nodes"') - - host = normalize_host(host) - try: - tmpl = ENV.get_template(host) - except TemplateNotFound: - raise click.BadParameter( - 'Could not find template for host \'{}\'.'.format(host), - param_hint='"--host"') - - # Make sure we only warn the user once, if they are using NAMD. + # Grab the template name for the host and pass it on to validation. + tmpl, status = retrieve_host_template(host) + # Validate all required input values. Throw an error, if something is wrong. + validate_generate_arguments( + name=name, + module=module, + host=(tmpl, status), + min_nodes=min_nodes, + max_nodes=max_nodes) + + # Warn the user that NAMD support is still experimental. if any(['namd' in m for m in module]): console.warn( 'NAMD support is experimental. ' @@ -103,8 +97,74 @@ def generate(name, gpu, module, host, min_nodes, max_nodes, time, list_hosts): 'If you use the {} option make sure you use the GPU compatible NAMD module!', '--gpu') + ## TODO: Validation start + # Save requested modules as a Set + requested_modules = set(module) + + # Make sure that we stop if the user requests any unsupported engines. + for req_module in requested_modules: + if not detect_md_engine(req_module): + console.error( + "There is currently no support for '{}'. Supported MD engines are: {}.", + req_module, ', '.join(sorted(SUPPORTED_ENGINES.keys()))) + + # Grab all available modules on the host + available_modules = get_available_modules() + # Save all valid requested module version + modules = [ + m for m in module + if validate_module_name(module=m, available_modules=available_modules) + ] + # Create a list of the difference between requested and available modules + missing_modules = requested_modules.difference(modules) + + # Warn the user that we are not going to perform any validation on module + # names. + if skip_validation or not available_modules: + warning = 'Not performing module name validation.' + if available_modules is None: + warning += ' Cannot locate modules available on this host.' + console.warn(warning) + + # Inform the user of all modules that are not available. Offer some alternatives. + if available_modules and missing_modules and not skip_validation: + # Define a default message. + err = 'We have problems finding all of your requested modules on this host.\n' + + # Create a dictionary containing all requested engines and the + # corresponding missing versions. This way we can list them all in a + # nice way! + d = {} + for mm in missing_modules: + engine, version = mm.split('/') + if not engine in d: + d[engine] = [] + d[engine].append(version) + + args = [] + for engine in sorted(d.keys()): + # New line to the last list item. We are going to print some more + # stuff! + if args: + args[-1] = args[-1] + '\n' + err += 'We were not able to find the following modules for MD engine {}: {}.\n' + args.append(engine) + args.extend(d[engine]) + + # If we know the MD engine that the user was trying to use, we can + # show all available options. + err += 'Available modules are:\n{}' + args.extend([ + '\n'.join([ + '{}/{}'.format(engine, mde) + for mde in available_modules[engine] + ]) + ]) + console.error(err, bold=True, *args) + ## TODO: Validation end + for m in module: - # Here we detect the mdengine (GROMACS or NAMD). + # Here we detect the MD engine (supported: GROMACS and NAMD). engine = detect_md_engine(m) directory = '{}_{}'.format(host, m) diff --git a/mdbenchmark/mdengines/__init__.py b/mdbenchmark/mdengines/__init__.py index 216c5e34..44cb2478 100644 --- a/mdbenchmark/mdengines/__init__.py +++ b/mdbenchmark/mdengines/__init__.py @@ -17,22 +17,82 @@ # # You should have received a copy of the GNU General Public License # along with MDBenchmark. If not, see . +import os + import six from . import gromacs, namd from .. import console +SUPPORTED_ENGINES = {'gromacs': gromacs, 'namd': namd} + def detect_md_engine(modulename): """Detects the MD engine based on the available modules. - Any newly implemented mdengines must be added here. - Returns the python module.""" - _engines = {'gromacs': gromacs, 'namd': namd} - for name, engine in six.iteritems(_engines): + Any newly implemented MD engines must be added here. + + + Returns + ------- + The corresponding MD engine module or None if the requested module is not supported. + """ + + for name, engine in six.iteritems(SUPPORTED_ENGINES): if name in modulename: return engine - console.error( - "No suitable engine detected for '{}'. Known engines are: {}.", - modulename, ', '.join(sorted(_engines.keys()))) + return None + + +def get_available_modules(): + """Return all available module versions for a given MD engine. + + Returns + ------- + If we cannot access the `MODULEPATH` environment variable, we return None. + + available_modules : dict + Dictionary containing all available engines as keys and their versions as a list. + """ + + MODULE_PATHS = os.environ.get('MODULEPATH', None) + available_modules = dict((mdengine, []) for mdengine in SUPPORTED_ENGINES) + + # Return `None` if the environment variable `MODULEPATH` does not exist. + if not MODULE_PATHS: + return None + + # Go through the directory structure and grab all version of modules that we support. + for paths in MODULE_PATHS.split(':'): + for path, subdirs, files in os.walk(paths): + for mdengine in SUPPORTED_ENGINES: + if mdengine in path: + for name in files: + if not name.startswith('.'): + available_modules[mdengine].append(name) + + return available_modules + + +def validate_module_name(module=None, available_modules=None): + """Validates that the specified module version is available on the host. + + Returns + ------- + If `get_available_modules` returns None, so does this function. + + Returns a boolean, indicating whether the specified version is available on the host. + """ + try: + basename, version = module.split('/') + except ValueError: + console.error('We were not able to determine the module name.') + + if not available_modules: + available_modules = get_available_modules() + + if available_modules is None: + return None + + return version in available_modules[basename] diff --git a/mdbenchmark/tests/mdengines/test_init.py b/mdbenchmark/tests/mdengines/test_init.py index 7b7e9c3d..6d54ea6c 100644 --- a/mdbenchmark/tests/mdengines/test_init.py +++ b/mdbenchmark/tests/mdengines/test_init.py @@ -17,14 +17,31 @@ # # You should have received a copy of the GNU General Public License # along with MDBenchmark. If not, see . +import os + import click +import pytest +from mdbenchmark import cli from mdbenchmark.ext.click_test import cli_runner -from mdbenchmark.mdengines import detect_md_engine, gromacs, namd +from mdbenchmark.generate import generate +from mdbenchmark.mdengines import (detect_md_engine, get_available_modules, + gromacs, namd, validate_module_name) + +DIR_STRUCTURE = { + 'applications': { + 'gromacs': ['2016.4', '5.1.4-plumed2.3', '2018.1'], + 'namd': ['123', '456'], + 'amber': ['13', '14', '15'] + }, + 'visualization': { + 'vmd': ['1.9.3', '1.9.4'] + } +} -def test_detect_md_engine(cli_runner): - """Test that the function `detect_md_engine` works as expected.""" +def test_detect_md_engine(): + """Test that we only accept supported MD engines.""" engine = detect_md_engine('gromacs/2016.3') assert engine.__name__ == 'mdbenchmark.mdengines.gromacs' @@ -32,16 +49,119 @@ def test_detect_md_engine(cli_runner): engine = detect_md_engine('namd/123') assert engine.__name__ == 'mdbenchmark.mdengines.namd' + assert detect_md_engine('someengine/123') is None + + +def test_validation(monkeypatch, tmpdir, cli_runner): + """Test that we retrieve the correct module versions. + + Names are retrieved from a given path and the module names and versions + are validated. + """ + @click.group() def test_cli(): pass @test_cli.command() def test(): - detect_md_engine('MagicMDEngine/123') + validate_module_name('wrong-format') + + with tmpdir.as_cwd(): + for k, v in DIR_STRUCTURE.items(): + for k2, v2 in v.items(): + os.makedirs(os.path.join(k, k2)) + for v3 in v2: + open(os.path.join(k, k2, v3), 'a').close() + + # Make sure we return None when we cannot find the environment variable + assert get_available_modules() is None + # Test the same thing for the `validate_module_name` function + assert validate_module_name('gromacs/123') is None + + # Prepare path variable that we are going to monkeypatch for + # `validate.get_available_modules` + dirs = ':'.join( + [os.path.join(os.getcwd(), x) for x in os.listdir(os.getcwd())]) + monkeypatch.setenv('MODULEPATH', dirs) + modules = get_available_modules() + + # Assert that the correct modules and their versions are returned. + assert set(modules['gromacs']) == set( + ['2016.4', '5.1.4-plumed2.3', '2018.1']) + assert set(modules['namd']) == set(['123', '456']) + + # Make sure we return a boolean if the module is available or not. + assert not validate_module_name('gromacs/123') + assert validate_module_name('gromacs/2018.1') + + output = 'ERROR We were not able to determine the module name.\n' + result = cli_runner.invoke(test_cli, ['test']) + assert result.exit_code == 1 + assert result.output == output + + +def test_generate_validation(cli_runner, tmpdir, monkeypatch): + # Test that we get a warning, if the MD engine is unsupported. + result = cli_runner.invoke(cli.cli, [ + 'generate', '--module=somehpc/123', '--host=draco', '--name=protein' + ]) + output = 'ERROR There is currently no support for \'somehpc/123\'. ' \ + 'Supported MD engines are: gromacs, namd.\n' - output = 'ERROR No suitable engine detected for \'MagicMDEngine/123\'. ' \ - 'Known engines are: gromacs, namd.\n' - result = cli_runner.invoke(test_cli, ['test']) assert result.exit_code == 1 assert result.output == output + + with tmpdir.as_cwd(): + with open('protein.tpr', 'w') as fh: + fh.write('This is a dummy tpr ;)') + for k, v in DIR_STRUCTURE.items(): + for k2, v2 in v.items(): + os.makedirs(os.path.join(k, k2)) + for v3 in v2: + open(os.path.join(k, k2, v3), 'a').close() + + # Prepare path variable that we are going to monkeypatch for + # `validate.get_available_modules` + dirs = ':'.join( + [os.path.join(os.getcwd(), x) for x in os.listdir(os.getcwd())]) + monkeypatch.setenv('MODULEPATH', dirs) + + # Test that we get a warning if we cannot find the requested modules. + result = cli_runner.invoke(cli.cli, [ + 'generate', '--module=gromacs/doesnotexist', '--host=draco', + '--name=protein' + ]) + output = 'ERROR We have problems finding all of your requested modules on this host.\n' \ + 'We were not able to find the following modules for MD engine gromacs: doesnotexist.\n' \ + 'Available modules are:\n' \ + 'gromacs/5.1.4-plumed2.3\n' \ + 'gromacs/2016.4\n' \ + 'gromacs/2018.1\n' + + # assert result.exit_code == 1 + assert result.output == output + + # Test that the warning also works when specifying several MD engines. + # Test that we get a warning if we cannot find the requested modules. + result = cli_runner.invoke(cli.cli, [ + 'generate', '--module=gromacs/doesnotexist', '--module=namd/abc', + '--module=namd/123', '--host=draco', '--name=protein' + ]) + output = 'WARNING NAMD support is experimental. ' \ + 'All input files must be in the current directory. ' \ + 'Parameter paths must be absolute. Only crude file checks are performed!' \ + 'If you use the --gpu option make sure you use the GPU compatible NAMD module!\n' \ + 'ERROR We have problems finding all of your requested modules on this host.\n' \ + 'We were not able to find the following modules for MD engine gromacs: doesnotexist.\n' \ + 'Available modules are:\n' \ + 'gromacs/5.1.4-plumed2.3\n' \ + 'gromacs/2016.4\n' \ + 'gromacs/2018.1\n' \ + 'We were not able to find the following modules for MD engine namd: abc.\n' \ + 'Available modules are:\n' \ + 'namd/456\n' \ + 'namd/123\n' + + # assert result.exit_code == 1 + assert result.output == output diff --git a/mdbenchmark/tests/test_generate.py b/mdbenchmark/tests/test_generate.py index b6bf0eb2..b1f30ef0 100644 --- a/mdbenchmark/tests/test_generate.py +++ b/mdbenchmark/tests/test_generate.py @@ -23,9 +23,11 @@ from mdbenchmark import cli from mdbenchmark.ext.click_test import cli_runner +from .mdengines.test_init import DIR_STRUCTURE + @pytest.mark.parametrize('tpr_file', ('protein.tpr', 'protein')) -def test_generate(cli_runner, tmpdir, tpr_file): +def test_generate(cli_runner, monkeypatch, tmpdir, tpr_file): """Run an integration test on the generate function. Make sure that we accept both `protein` and `protein.tpr` as input files. @@ -38,6 +40,28 @@ def test_generate(cli_runner, tmpdir, tpr_file): 'Creating a total of 4 benchmarks, with a run time of 15' \ ' minutes each.\nFinished generating all benchmarks.\nYou can' \ ' now submit the jobs with mdbenchmark submit.\n' + + # Test that we get a warning, if no module name validation is performed. + result = cli_runner.invoke(cli.cli, [ + 'generate', '--module=gromacs/2016', '--host=draco', + '--max-nodes=4', '--gpu', '--name={}'.format(tpr_file) + ]) + assert result.exit_code == 0 + assert result.output == 'WARNING Not performing module name validation. Cannot locate modules available on this host.\n' + output + + # monkeypatch the output of the available modules + monkeypatch.setattr('mdbenchmark.generate.get_available_modules', + lambda: {'gromacs': ['2016']}) + + # Test that we can skip module name validation, even if it actually works. + result = cli_runner.invoke(cli.cli, [ + 'generate', '--module=gromacs/2016', '--host=draco', + '--max-nodes=4', '--gpu', '--name={}'.format(tpr_file), + '--skip-validation' + ]) + assert result.exit_code == 0 + assert result.output == 'WARNING Not performing module name validation.\n' + output + result = cli_runner.invoke(cli.cli, [ 'generate', '--module=gromacs/2016', '--host=draco', '--max-nodes=4', '--gpu', '--name={}'.format(tpr_file) @@ -52,6 +76,29 @@ def test_generate(cli_runner, tmpdir, tpr_file): assert os.path.exists( 'draco_gromacs/2016_gpu/{}/bench.job'.format(i)) + # Test that we throw an error, if we cannot find the requested module name. + with tmpdir.as_cwd(): + for k, v in DIR_STRUCTURE.items(): + for k2, v2 in v.items(): + os.makedirs(os.path.join(k, k2)) + for v3 in v2: + open(os.path.join(k, k2, v3), 'a').close() + + # Prepare path variable that we are going to monkeypatch for + # `validate.get_available_modules` + dirs = ':'.join([ + os.path.join(os.getcwd(), x) for x in os.listdir(os.getcwd()) + ]) + monkeypatch.setenv('MODULEPATH', dirs) + + output = 'ERROR There is currently no support for \'doesnotexist/version\'. ' \ + 'Supported MD engines are: gromacs, namd.\n' + result = cli_runner.invoke(cli.cli, [ + 'generate', '--module=doesnotexist/version', '--host=draco', + '--name={}'.format(tpr_file) + ]) + assert result.output == output + output = 'Creating benchmark system for gromacs/2016 with GPUs.\n' \ 'Creating a total of 3 benchmarks, with a run time of 15 minutes each.\n' \ 'Finished generating all benchmarks.\n' \ @@ -80,9 +127,13 @@ def test_generate(cli_runner, tmpdir, tpr_file): assert line.endswith('-deffnm protein') -def test_generate_console_messages(cli_runner, tmpdir): +def test_generate_console_messages(cli_runner, monkeypatch, tmpdir): """Test that the CLI for generate prints all error messages as expected.""" with tmpdir.as_cwd(): + # monkeypatch the output of the available modules + monkeypatch.setattr('mdbenchmark.generate.get_available_modules', + lambda: {'gromacs': ['2016']}) + # Test that we get an error when not supplying a file name result = cli_runner.invoke( cli.cli, ['generate', '--module=gromacs/2016', '--host=draco']) @@ -114,11 +165,11 @@ def test_generate_console_messages(cli_runner, tmpdir): # Test error message if we pass an invalid template name result = cli_runner.invoke(cli.cli, [ - 'generate', '--module=gromacs/2016', '--host=hercules', + 'generate', '--module=gromacs/2016', '--host=minerva', '--name=protein' ]) output = 'Usage: cli generate [OPTIONS]\n\nError: Invalid value for' \ - ' "--host": Could not find template for host \'hercules\'.\n' + ' "--host": Could not find template for host \'minerva\'.\n' assert result.exit_code == 2 assert result.output == output @@ -126,18 +177,22 @@ def test_generate_console_messages(cli_runner, tmpdir): result = cli_runner.invoke( cli.cli, ['generate', '--host=draco', '--name=protein']) output = 'Usage: cli generate [OPTIONS]\n\nError: Invalid value for ' \ - '"-m" / "--module": Please specify which mdengine module' \ - ' to use for the benchmarks.\n' + '"-m" / "--module": Please specify which MD engine module ' \ + 'to use for the benchmarks.\n' assert result.exit_code == 2 assert result.output == output -def test_generate_namd_experimental_warning(cli_runner, tmpdir): +def test_generate_namd_experimental_warning(cli_runner, monkeypatch, tmpdir): """Test that we print the NAMD experimental warning.""" with tmpdir.as_cwd(): for f in ['md.namd', 'md.psf', 'md.pdb']: open(f, 'a').close() + # monkeypatch the output of the available modules + monkeypatch.setattr('mdbenchmark.generate.get_available_modules', + lambda: {'namd': ['123']}) + result = cli_runner.invoke(cli.cli, [ 'generate', '--module=namd/123', '--host=draco', '--name=md' ]) diff --git a/mdbenchmark/tests/test_utils.py b/mdbenchmark/tests/test_utils.py index 43147854..4c883b71 100644 --- a/mdbenchmark/tests/test_utils.py +++ b/mdbenchmark/tests/test_utils.py @@ -19,9 +19,9 @@ # along with MDBenchmark. If not, see . import click import numpy as np -import pytest from numpy.testing import assert_equal +import pytest from mdbenchmark import utils from mdbenchmark.ext.click_test import cli_runner @@ -73,6 +73,21 @@ def test_normalize_host(monkeypatch): assert utils.normalize_host(host) == host +def test_retrieve_host_template(monkeypatch): + """Test `retrieve_host_template` utility function.""" + + # Check some non-existent template name + assert utils.retrieve_host_template('some_hpc') == ('some_hpc', False) + + # Check template name that we supply with the package + assert utils.retrieve_host_template('draco') is not None + + # Check that the user can define some custom template + monkeypatch.setattr('mdbenchmark.utils.ENV.get_template', + lambda x: 'minerva') + assert utils.retrieve_host_template('minerva') == ('minerva', True) + + def test_lin_func(): """Test `lin_func()`.""" m, x, b = [5, 3, 2] diff --git a/mdbenchmark/tests/test_validate.py b/mdbenchmark/tests/test_validate.py new file mode 100644 index 00000000..b9756ed5 --- /dev/null +++ b/mdbenchmark/tests/test_validate.py @@ -0,0 +1,85 @@ +# -*- Mode: python; tab-width: 4; indent-tabs-mode:nil; coding:utf-8 -*- +# vim: tabstop=4 expandtab shiftwidth=4 softtabstop=4 fileencoding=utf-8 +# +# MDBenchmark +# Copyright (c) 2017 Max Linke & Michael Gecht and contributors +# (see the file AUTHORS for the full list of names) +# +# MDBenchmark is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# MDBenchmark is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with MDBenchmark. If not, see . +from click import exceptions + +import pytest +from mdbenchmark import validate + + +def test_validate_generate_name(): + """Test that the validate_generate_name function works as expected.""" + + with pytest.raises(exceptions.BadParameter) as error: + validate.validate_generate_name() + + assert str( + error.value) == 'Please specify the base name of your input files.' + + assert validate.validate_generate_name('md') is None + + +def test_validate_generate_module(): + """Test that the validate_generate_module function works as expected.""" + + with pytest.raises(exceptions.BadParameter) as error: + validate.validate_generate_module() + + assert str( + error.value + ) == 'Please specify which MD engine module to use for the benchmarks.' + + assert validate.validate_generate_module('gromacs/123') is None + + +def test_validate_generate_number_of_nodes(): + """Test that the validate_generate_number_of_nodes function works as expected.""" + + with pytest.raises(exceptions.BadParameter) as error: + validate.validate_generate_number_of_nodes(min_nodes=6, max_nodes=5) + + assert str( + error.value + ) == 'The minimal number of nodes needs to be smaller than the maximal number.' + + assert validate.validate_generate_number_of_nodes( + min_nodes=1, max_nodes=6) is None + + +def test_validate_generate_host(): + """Test that the validate_generate_host function works as expected.""" + + # Test error without any hostname + with pytest.raises(exceptions.BadParameter) as error: + assert validate.validate_generate_host() == (None, False) + assert str(error.value) == 'Could not find template for host \'None\'.' + + # Test success of existent hostname + assert validate.validate_generate_host(host='draco', status=True) is None + + +def test_validate_generate_arguments(): + """Test that the validate_generate_argument function works as expected.""" + + assert validate.validate_generate_arguments( + name='md', + module='gromacs/123', + host=('draco', True), + min_nodes=1, + max_nodes=6) is None diff --git a/mdbenchmark/utils.py b/mdbenchmark/utils.py index 147389a5..04837984 100644 --- a/mdbenchmark/utils.py +++ b/mdbenchmark/utils.py @@ -17,15 +17,17 @@ # # You should have received a copy of the GNU General Public License # along with MDBenchmark. If not, see . +import datetime as dt import multiprocessing as mp import os import socket import sys -import datetime as dt + import click import numpy as np import xdg from jinja2 import ChoiceLoader, Environment, FileSystemLoader, PackageLoader +from jinja2.exceptions import TemplateNotFound from . import console from .ext.cadishi import _cat_proc_cpuinfo_grep_query_sort_uniq @@ -70,6 +72,32 @@ def guess_host(): return None +def retrieve_host_template(host=None): + """Lookup the template name. + + Parameter + --------- + host : str + Name of the host template to lookup + + Returns + ------- + template : str /list + Either the name of the host template if it is available, or the value of `host` wrapped + inside of a list. + """ + + host = normalize_host(host) + try: + template = ENV.get_template(host) + status = True + except TemplateNotFound: + template = host + status = False + + return (template, status) + + def normalize_host(host): if host is None: host = guess_host() diff --git a/mdbenchmark/validate.py b/mdbenchmark/validate.py new file mode 100644 index 00000000..a653dc08 --- /dev/null +++ b/mdbenchmark/validate.py @@ -0,0 +1,67 @@ +# -*- Mode: python; tab-width: 4; indent-tabs-mode:nil; coding:utf-8 -*- +# vim: tabstop=4 expandtab shiftwidth=4 softtabstop=4 fileencoding=utf-8 +# +# MDBenchmark +# Copyright (c) 2017 Max Linke & Michael Gecht and contributors +# (see the file AUTHORS for the full list of names) +# +# MDBenchmark is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# MDBenchmark is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with MDBenchmark. If not, see . +import click + +from .utils import ENV, normalize_host + + +def validate_generate_name(name=None): + """Validate that we are given a name argument.""" + if not name: + raise click.BadParameter( + 'Please specify the base name of your input files.', + param_hint='"-n" / "--name"') + + +def validate_generate_module(module=None): + """Validate that we are given a module argument.""" + if not module: + raise click.BadParameter( + 'Please specify which MD engine module to use for the benchmarks.', + param_hint='"-m" / "--module"') + + +def validate_generate_number_of_nodes(min_nodes, max_nodes): + """Validate that the minimal number of nodes is not smaller than the maximal number.""" + if min_nodes > max_nodes: + raise click.BadParameter( + 'The minimal number of nodes needs to be smaller than the maximal number.', + param_hint='"--min-nodes"') + + +def validate_generate_host(host=None, status=None): + """Validate that we were given a valid template name for the host.""" + if not status: + raise click.BadParameter( + 'Could not find template for host \'{}\'.'.format(host), + param_hint='"--host"') + + +def validate_generate_arguments(name=None, + module=None, + host=None, + min_nodes=None, + max_nodes=None): + """Validate all input provided to the generate CLI command.""" + template, status = host + validate_generate_name(name=name) + validate_generate_module(module=module) + validate_generate_host(host=template, status=status) + validate_generate_number_of_nodes(min_nodes=min_nodes, max_nodes=max_nodes) From dce46a57805e5079ecdb2290d9fc4f00f8d5c41d Mon Sep 17 00:00:00 2001 From: Max Linke Date: Sun, 18 Mar 2018 20:30:57 +0100 Subject: [PATCH 2/4] Refactored code. * Move validation into callbacks --- mdbenchmark/generate.py | 155 +++++++++++++----------------- mdbenchmark/mdengines/__init__.py | 57 +++++++++-- mdbenchmark/utils.py | 25 +---- mdbenchmark/validate.py | 67 ------------- 4 files changed, 120 insertions(+), 184 deletions(-) delete mode 100644 mdbenchmark/validate.py diff --git a/mdbenchmark/generate.py b/mdbenchmark/generate.py index 151ebce8..1609295f 100644 --- a/mdbenchmark/generate.py +++ b/mdbenchmark/generate.py @@ -22,10 +22,46 @@ from . import console from .cli import cli -from .mdengines import (SUPPORTED_ENGINES, detect_md_engine, - get_available_modules, validate_module_name) -from .utils import print_possible_hosts, retrieve_host_template -from .validate import validate_generate_arguments +from . import utils +from . import mdengines + + +def validate_name(ctx, param, name=None): + """Validate that we are given a name argument.""" + if name is None: + raise click.BadParameter( + 'Please specify the base name of your input files.', + param_hint='"-n" / "--name"') + + +def validate_module(ctx, param, module=None): + """Validate that we are given a module argument.""" + if module is None: + raise click.BadParameter( + 'Please specify the base module of your input files.', + param_hint='"-m" / "--module"') + + +def print_known_hosts(ctx, param, value): + if not value or ctx.resilient_parsing: + return + utils.print_possible_hosts() + ctx.exit() + + +def validate_hosts(ctx, param, host=None): + if host is None: + host = utils.guess_host() + if host is None: + raise click.BadParameter( + 'Could not guess host. Please provide a value explicitly.', + param_hint='"--host"') + + known_hosts = utils.get_possible_hosts() + if host not in known_hosts: + utils.print_possible_hosts() + # raise some appropriate error here + ctx.exit() @cli.command() @@ -33,7 +69,7 @@ '-n', '--name', help='Name of input files. All files must have the same base name.', - show_default=True) + callback=validate_name) @click.option( '-g', '--gpu', @@ -44,8 +80,13 @@ '-m', '--module', help='Name of the MD engine module to use.', - multiple=True) -@click.option('--host', help='Name of the job template.', default=None) + multiple=True, + callback=validate_module) +@click.option( + '--host', + help='Name of the job template.', + default=None, + callback=validate_hosts) @click.option( '--min-nodes', help='Minimal number of nodes to request.', @@ -65,28 +106,29 @@ show_default=True, type=click.IntRange(1, 1440)) @click.option( - '--list-hosts', help='Show available job templates.', is_flag=True) + '--list-hosts', + help='Show available job templates.', + is_flag=True, + is_eager=True, + callback=print_known_hosts, + expose_value=False) @click.option( '--skip-validation', help='Skip the validation of module names.', default=False, is_flag=True) -def generate(name, gpu, module, host, min_nodes, max_nodes, time, list_hosts, +def generate(name, gpu, module, host, min_nodes, max_nodes, time, skip_validation): """Generate benchmarks simulations from the CLI.""" - if list_hosts: - print_possible_hosts() - return + # can't be made a callback due to it being two separate options + if min_nodes > max_nodes: + raise click.BadParameter( + 'The minimal number of nodes needs to be smaller than the maximal number.', + param_hint='"--min-nodes"') - # Grab the template name for the host and pass it on to validation. - tmpl, status = retrieve_host_template(host) - # Validate all required input values. Throw an error, if something is wrong. - validate_generate_arguments( - name=name, - module=module, - host=(tmpl, status), - min_nodes=min_nodes, - max_nodes=max_nodes) + # Grab the template name for the host. This should always work because + # click does the validation for us + tmpl = utils.retrieve_host_template(host) # Warn the user that NAMD support is still experimental. if any(['namd' in m for m in module]): @@ -97,75 +139,14 @@ def generate(name, gpu, module, host, min_nodes, max_nodes, time, list_hosts, 'If you use the {} option make sure you use the GPU compatible NAMD module!', '--gpu') - ## TODO: Validation start - # Save requested modules as a Set - requested_modules = set(module) - - # Make sure that we stop if the user requests any unsupported engines. - for req_module in requested_modules: - if not detect_md_engine(req_module): - console.error( - "There is currently no support for '{}'. Supported MD engines are: {}.", - req_module, ', '.join(sorted(SUPPORTED_ENGINES.keys()))) - - # Grab all available modules on the host - available_modules = get_available_modules() - # Save all valid requested module version - modules = [ - m for m in module - if validate_module_name(module=m, available_modules=available_modules) - ] - # Create a list of the difference between requested and available modules - missing_modules = requested_modules.difference(modules) - - # Warn the user that we are not going to perform any validation on module - # names. - if skip_validation or not available_modules: - warning = 'Not performing module name validation.' - if available_modules is None: - warning += ' Cannot locate modules available on this host.' - console.warn(warning) - - # Inform the user of all modules that are not available. Offer some alternatives. - if available_modules and missing_modules and not skip_validation: - # Define a default message. - err = 'We have problems finding all of your requested modules on this host.\n' - - # Create a dictionary containing all requested engines and the - # corresponding missing versions. This way we can list them all in a - # nice way! - d = {} - for mm in missing_modules: - engine, version = mm.split('/') - if not engine in d: - d[engine] = [] - d[engine].append(version) - - args = [] - for engine in sorted(d.keys()): - # New line to the last list item. We are going to print some more - # stuff! - if args: - args[-1] = args[-1] + '\n' - err += 'We were not able to find the following modules for MD engine {}: {}.\n' - args.append(engine) - args.extend(d[engine]) - - # If we know the MD engine that the user was trying to use, we can - # show all available options. - err += 'Available modules are:\n{}' - args.extend([ - '\n'.join([ - '{}/{}'.format(engine, mde) - for mde in available_modules[engine] - ]) - ]) - console.error(err, bold=True, *args) - ## TODO: Validation end + if not skip_validation: + module = mdengines.normalize_modules(module) + else: + console.warn('Not performing module name validation.') for m in module: # Here we detect the MD engine (supported: GROMACS and NAMD). - engine = detect_md_engine(m) + engine = mdengines.detect_md_engine(m) directory = '{}_{}'.format(host, m) gpu_string = '' diff --git a/mdbenchmark/mdengines/__init__.py b/mdbenchmark/mdengines/__init__.py index 44cb2478..992d9020 100644 --- a/mdbenchmark/mdengines/__init__.py +++ b/mdbenchmark/mdengines/__init__.py @@ -18,6 +18,7 @@ # You should have received a copy of the GNU General Public License # along with MDBenchmark. If not, see . import os +from collections import defaultdict import six @@ -75,7 +76,54 @@ def get_available_modules(): return available_modules -def validate_module_name(module=None, available_modules=None): +def normalize_modules(modules): + # Check if modules are from supported md engines + d = defaultdict(list) + for m in modules: + engine, version = m.split('/') + d[engine] = version + for engine in d.keys(): + if detect_md_engine(engine) is None: + console.warn("There is currently no support for '{}'", engine) + + available_modules = get_available_modules() + if available_modules is None: + console.warn('Cannot locate modules available on this host.') + return modules + + good_modules = [ + m for m in modules if validate_module_name(m, available_modules) + ] + + # warn about missing modules + missing_modules = set(good_modules).difference(modules) + if missing_modules: + d = defaultdict(list) + for mm in missing_modules: + engine, version = mm.split('/') + d[engine].append(version) + + err = 'We have problems finding all of your requested modules on this host.\n' + args = [] + for engine in sorted(d.keys()): + err += 'We were not able to find the following modules for MD engine {}: {}.\n' + args.append(engine) + args.extend(d[engine]) + # If we know the MD engine that the user was trying to use, we can + # show all available options. + err += 'Available modules are:\n{}\n' + args.extend([ + '\n'.join([ + '{}/{}'.format(engine, mde) + for mde in available_modules[engine] + ]) + ]) + console.warn(err, bold=True, *args) + + return good_modules + + +def validate_module_name(module, available_modules): """Validates that the specified module version is available on the host. Returns @@ -88,11 +136,4 @@ def validate_module_name(module=None, available_modules=None): basename, version = module.split('/') except ValueError: console.error('We were not able to determine the module name.') - - if not available_modules: - available_modules = get_available_modules() - - if available_modules is None: - return None - return version in available_modules[basename] diff --git a/mdbenchmark/utils.py b/mdbenchmark/utils.py index 04837984..2179393f 100644 --- a/mdbenchmark/utils.py +++ b/mdbenchmark/utils.py @@ -82,30 +82,11 @@ def retrieve_host_template(host=None): Returns ------- - template : str /list - Either the name of the host template if it is available, or the value of `host` wrapped - inside of a list. + template """ - - host = normalize_host(host) - try: - template = ENV.get_template(host) - status = True - except TemplateNotFound: - template = host - status = False - - return (template, status) - - -def normalize_host(host): if host is None: - host = guess_host() - if host is None: - raise click.BadParameter( - 'Could not guess host. Please provide a value explicitly.', - param_hint='"--host"') - return host + host = guess_host(host) + return ENV.get_template(host) def lin_func(x, m, b): diff --git a/mdbenchmark/validate.py b/mdbenchmark/validate.py deleted file mode 100644 index a653dc08..00000000 --- a/mdbenchmark/validate.py +++ /dev/null @@ -1,67 +0,0 @@ -# -*- Mode: python; tab-width: 4; indent-tabs-mode:nil; coding:utf-8 -*- -# vim: tabstop=4 expandtab shiftwidth=4 softtabstop=4 fileencoding=utf-8 -# -# MDBenchmark -# Copyright (c) 2017 Max Linke & Michael Gecht and contributors -# (see the file AUTHORS for the full list of names) -# -# MDBenchmark is free software: you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation, either version 3 of the License, or -# (at your option) any later version. -# -# MDBenchmark is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with MDBenchmark. If not, see . -import click - -from .utils import ENV, normalize_host - - -def validate_generate_name(name=None): - """Validate that we are given a name argument.""" - if not name: - raise click.BadParameter( - 'Please specify the base name of your input files.', - param_hint='"-n" / "--name"') - - -def validate_generate_module(module=None): - """Validate that we are given a module argument.""" - if not module: - raise click.BadParameter( - 'Please specify which MD engine module to use for the benchmarks.', - param_hint='"-m" / "--module"') - - -def validate_generate_number_of_nodes(min_nodes, max_nodes): - """Validate that the minimal number of nodes is not smaller than the maximal number.""" - if min_nodes > max_nodes: - raise click.BadParameter( - 'The minimal number of nodes needs to be smaller than the maximal number.', - param_hint='"--min-nodes"') - - -def validate_generate_host(host=None, status=None): - """Validate that we were given a valid template name for the host.""" - if not status: - raise click.BadParameter( - 'Could not find template for host \'{}\'.'.format(host), - param_hint='"--host"') - - -def validate_generate_arguments(name=None, - module=None, - host=None, - min_nodes=None, - max_nodes=None): - """Validate all input provided to the generate CLI command.""" - template, status = host - validate_generate_name(name=name) - validate_generate_module(module=module) - validate_generate_host(host=template, status=status) - validate_generate_number_of_nodes(min_nodes=min_nodes, max_nodes=max_nodes) From a8166865526446eb0c8a6775889539df991f1981 Mon Sep 17 00:00:00 2001 From: Michael Gecht Date: Tue, 27 Mar 2018 00:08:35 +0200 Subject: [PATCH 3/4] Update code. * Fix import errors * Add fixture to mock context * Move skip_validation check into normalize_modules method. This fixes a bug, when providing an unsupported MD engine and the `--skip-validation` option at the same time. * Add docstrings * Added some missing tests in other parts of the module --- mdbenchmark/generate.py | 56 ++++++--- mdbenchmark/mdengines/__init__.py | 70 +++++++---- mdbenchmark/tests/mdengines/test_init.py | 142 ++++++++++++++++++----- mdbenchmark/tests/test_analyze.py | 6 +- mdbenchmark/tests/test_generate.py | 135 ++++++++++++++++++--- mdbenchmark/tests/test_submit.py | 13 ++- mdbenchmark/tests/test_utils.py | 17 +-- mdbenchmark/tests/test_validate.py | 85 -------------- mdbenchmark/utils.py | 2 - 9 files changed, 338 insertions(+), 188 deletions(-) delete mode 100644 mdbenchmark/tests/test_validate.py diff --git a/mdbenchmark/generate.py b/mdbenchmark/generate.py index 1609295f..643af4ce 100644 --- a/mdbenchmark/generate.py +++ b/mdbenchmark/generate.py @@ -20,10 +20,8 @@ import click import datreant.core as dtr -from . import console +from . import console, mdengines, utils from .cli import cli -from . import utils -from . import mdengines def validate_name(ctx, param, name=None): @@ -33,16 +31,30 @@ def validate_name(ctx, param, name=None): 'Please specify the base name of your input files.', param_hint='"-n" / "--name"') + return name + def validate_module(ctx, param, module=None): """Validate that we are given a module argument.""" - if module is None: + if module is None or not module: raise click.BadParameter( - 'Please specify the base module of your input files.', + 'Please specify which MD engine module to use for the benchmarks.', param_hint='"-m" / "--module"') + return module + + +def validate_number_of_nodes(min_nodes, max_nodes): + """Validate that the minimal number of nodes is smaller than the maximal + number. + """ + if min_nodes > max_nodes: + raise click.BadParameter( + 'The minimal number of nodes needs to be smaller than the maximal number.', + param_hint='"--min-nodes"') def print_known_hosts(ctx, param, value): + """Callback to print all available hosts to the user.""" if not value or ctx.resilient_parsing: return utils.print_possible_hosts() @@ -50,6 +62,15 @@ def print_known_hosts(ctx, param, value): def validate_hosts(ctx, param, host=None): + """Callback to validate the hostname received as input. + + If we were not given a hostname, we first try to guess it via + `utils.guess_host`. If this fails, we give up and throw an error. + + Otherwise we compare the provided/guessed host with the list of available + templates. If the hostname matches the template name, we continue by + returning the hostname. + """ if host is None: host = utils.guess_host() if host is None: @@ -59,9 +80,13 @@ def validate_hosts(ctx, param, host=None): known_hosts = utils.get_possible_hosts() if host not in known_hosts: + console.info('Could not find template for host \'{}\'.', host) utils.print_possible_hosts() - # raise some appropriate error here + # TODO: Raise some appropriate error here ctx.exit() + return + + return host @cli.command() @@ -120,11 +145,8 @@ def validate_hosts(ctx, param, host=None): def generate(name, gpu, module, host, min_nodes, max_nodes, time, skip_validation): """Generate benchmarks simulations from the CLI.""" - # can't be made a callback due to it being two separate options - if min_nodes > max_nodes: - raise click.BadParameter( - 'The minimal number of nodes needs to be smaller than the maximal number.', - param_hint='"--min-nodes"') + # Validate the number of nodes + validate_number_of_nodes(min_nodes=min_nodes, max_nodes=max_nodes) # Grab the template name for the host. This should always work because # click does the validation for us @@ -135,14 +157,16 @@ def generate(name, gpu, module, host, min_nodes, max_nodes, time, console.warn( 'NAMD support is experimental. ' 'All input files must be in the current directory. ' - 'Parameter paths must be absolute. Only crude file checks are performed!' + 'Parameter paths must be absolute. Only crude file checks are performed! ' 'If you use the {} option make sure you use the GPU compatible NAMD module!', '--gpu') - if not skip_validation: - module = mdengines.normalize_modules(module) - else: - console.warn('Not performing module name validation.') + module = mdengines.normalize_modules(module, skip_validation) + + # If several modules were given and we only cannot find one of them, we + # continue. + if not module: + console.error('No requested modules available!') for m in module: # Here we detect the MD engine (supported: GROMACS and NAMD). diff --git a/mdbenchmark/mdengines/__init__.py b/mdbenchmark/mdengines/__init__.py index 992d9020..a16ffa9f 100644 --- a/mdbenchmark/mdengines/__init__.py +++ b/mdbenchmark/mdengines/__init__.py @@ -33,10 +33,11 @@ def detect_md_engine(modulename): Any newly implemented MD engines must be added here. - Returns ------- - The corresponding MD engine module or None if the requested module is not supported. + + The corresponding MD engine module or `None` if the requested module is not + supported. """ for name, engine in six.iteritems(SUPPORTED_ENGINES): @@ -46,12 +47,26 @@ def detect_md_engine(modulename): return None +def prepare_module_name(module): + """Split the provided module name into its base MD engine and version. + + Currently we only try to split via the delimiter `/`, but this could be + changed upon request or made configurable on a per-host basis. + """ + try: + basename, version = module.split('/') + except (ValueError, AttributeError) as e: + console.error('We were not able to determine the module name.') + + return basename, version + + def get_available_modules(): """Return all available module versions for a given MD engine. Returns ------- - If we cannot access the `MODULEPATH` environment variable, we return None. + If we cannot access the `MODULEPATH` environment variable, we return `None`. available_modules : dict Dictionary containing all available engines as keys and their versions as a list. @@ -76,30 +91,48 @@ def get_available_modules(): return available_modules -def normalize_modules(modules): +def normalize_modules(modules, skip_validation): + """Validate that the provided module names are available. + + We first check whether the requested MD engine is supported by the package. + Next we try to discover all available modules on the host. If this is not + possible, or if the user has used the `--skip-validation` option, we skip + the check and notify the user. + + If the user requested modules that were not found on the system, we inform + the user and show all modules for that corresponding MD engine that were + found. + """ # Check if modules are from supported md engines d = defaultdict(list) for m in modules: - engine, version = m.split('/') + engine, version = prepare_module_name(m) d[engine] = version for engine in d.keys(): if detect_md_engine(engine) is None: - console.warn("There is currently no support for '{}'", engine) + console.error("There is currently no support for '{}'. " + "Supported MD engines are: gromacs, namd.", engine) + + if skip_validation: + console.warn('Not performing module name validation.') + return modules available_modules = get_available_modules() if available_modules is None: - console.warn('Cannot locate modules available on this host.') + console.warn( + 'Cannot locate modules available on this host. Not performing module name validation.' + ) return modules good_modules = [ m for m in modules if validate_module_name(m, available_modules) ] - # warn about missing modules - missing_modules = set(good_modules).difference(modules) + # Prepare to warn the user about missing modules + missing_modules = set(modules).difference(good_modules) if missing_modules: d = defaultdict(list) - for mm in missing_modules: + for mm in sorted(missing_modules): engine, version = mm.split('/') d[engine].append(version) @@ -109,13 +142,12 @@ def normalize_modules(modules): err += 'We were not able to find the following modules for MD engine {}: {}.\n' args.append(engine) args.extend(d[engine]) - # If we know the MD engine that the user was trying to use, we can - # show all available options. + # Show all available modules that we found for the requested MD engine err += 'Available modules are:\n{}\n' args.extend([ '\n'.join([ '{}/{}'.format(engine, mde) - for mde in available_modules[engine] + for mde in sorted(available_modules[engine]) ]) ]) console.warn(err, bold=True, *args) @@ -123,17 +155,15 @@ def normalize_modules(modules): return good_modules -def validate_module_name(module, available_modules): +def validate_module_name(module, available_modules=None): """Validates that the specified module version is available on the host. Returns ------- - If `get_available_modules` returns None, so does this function. - Returns a boolean, indicating whether the specified version is available on the host. + Returns True or False, indicating whether the specified version is + available on the host. """ - try: - basename, version = module.split('/') - except ValueError: - console.error('We were not able to determine the module name.') + basename, version = prepare_module_name(module) + return version in available_modules[basename] diff --git a/mdbenchmark/tests/mdengines/test_init.py b/mdbenchmark/tests/mdengines/test_init.py index 6d54ea6c..25ce546d 100644 --- a/mdbenchmark/tests/mdengines/test_init.py +++ b/mdbenchmark/tests/mdengines/test_init.py @@ -20,13 +20,14 @@ import os import click - import pytest + from mdbenchmark import cli from mdbenchmark.ext.click_test import cli_runner from mdbenchmark.generate import generate from mdbenchmark.mdengines import (detect_md_engine, get_available_modules, - gromacs, namd, validate_module_name) + gromacs, namd, normalize_modules, + prepare_module_name, validate_module_name) DIR_STRUCTURE = { 'applications': { @@ -40,6 +41,29 @@ } +def test_prepare_module_name(cli_runner): + """Test that prepare_module_name works as expected.""" + + @click.group() + def test_cli(): + pass + + @test_cli.command() + def test_wrong_module_name(): + prepare_module_name('gromacs-2016.4') + + @test_cli.command() + def test_correct_module_name(): + prepare_module_name('gromacs/2016.4') + + result = cli_runner.invoke(test_cli, ['test_wrong_module_name']) + assert result.exit_code == 1 + assert result.output == 'ERROR We were not able to determine the module name.\n' + + result = cli_runner.invoke(test_cli, ['test_correct_module_name']) + assert result.exit_code == 0 + + def test_detect_md_engine(): """Test that we only accept supported MD engines.""" @@ -52,6 +76,64 @@ def test_detect_md_engine(): assert detect_md_engine('someengine/123') is None +def test_normalize_modules(cli_runner, monkeypatch, tmpdir): + """Test that normalize modules works as expected.""" + + @click.group() + def test_cli(): + pass + + # Test the warning when we skip the validation + @test_cli.command() + def skip_validation(): + normalize_modules(modules=['gromacs/2016.4'], skip_validation=True) + + result = cli_runner.invoke(test_cli, ['skip_validation']) + assert result.exit_code == 0 + assert result.output == 'WARNING Not performing module name validation.\n' + + # Test the warning when we do not skip the validation + @test_cli.command() + def do_not_skip_validation(): + normalize_modules(modules=['gromacs/2016.4'], skip_validation=False) + + result = cli_runner.invoke(test_cli, ['do_not_skip_validation']) + assert result.exit_code == 0 + assert result.output == 'WARNING Cannot locate modules available on this host. ' \ + 'Not performing module name validation.\n' + + @test_cli.command() + def test_normalize(): + normalize_modules( + modules=['gromacs/doesnotexist'], skip_validation=False) + + with tmpdir.as_cwd(): + for k, v in DIR_STRUCTURE.items(): + for k2, v2 in v.items(): + os.makedirs(os.path.join(k, k2)) + for v3 in v2: + open(os.path.join(k, k2, v3), 'a').close() + + # Prepare path variable that we are going to monkeypatch for + # `get_available_modules` + dirs = ':'.join( + [os.path.join(os.getcwd(), x) for x in os.listdir(os.getcwd())]) + monkeypatch.setenv('MODULEPATH', dirs) + modules = get_available_modules() + + output = 'WARNING We have problems finding all of your requested modules on this host.\n' \ + 'We were not able to find the following modules for MD engine gromacs: ' \ + 'doesnotexist.\n' \ + 'Available modules are:\n' \ + 'gromacs/2016.4\n' \ + 'gromacs/2018.1\n' \ + 'gromacs/5.1.4-plumed2.3\n\n' + + result = cli_runner.invoke(test_cli, ['test_normalize']) + assert result.exit_code == 0 + assert result.output == output + + def test_validation(monkeypatch, tmpdir, cli_runner): """Test that we retrieve the correct module versions. @@ -74,13 +156,8 @@ def test(): for v3 in v2: open(os.path.join(k, k2, v3), 'a').close() - # Make sure we return None when we cannot find the environment variable - assert get_available_modules() is None - # Test the same thing for the `validate_module_name` function - assert validate_module_name('gromacs/123') is None - # Prepare path variable that we are going to monkeypatch for - # `validate.get_available_modules` + # `get_available_modules` dirs = ':'.join( [os.path.join(os.getcwd(), x) for x in os.listdir(os.getcwd())]) monkeypatch.setenv('MODULEPATH', dirs) @@ -92,8 +169,8 @@ def test(): assert set(modules['namd']) == set(['123', '456']) # Make sure we return a boolean if the module is available or not. - assert not validate_module_name('gromacs/123') - assert validate_module_name('gromacs/2018.1') + assert not validate_module_name('gromacs/123', modules) + assert validate_module_name('gromacs/2018.1', modules) output = 'ERROR We were not able to determine the module name.\n' result = cli_runner.invoke(test_cli, ['test']) @@ -103,18 +180,19 @@ def test(): def test_generate_validation(cli_runner, tmpdir, monkeypatch): # Test that we get a warning, if the MD engine is unsupported. - result = cli_runner.invoke(cli.cli, [ - 'generate', '--module=somehpc/123', '--host=draco', '--name=protein' - ]) - output = 'ERROR There is currently no support for \'somehpc/123\'. ' \ + result = cli_runner.invoke( + cli.cli, + ['generate', '--module=somehpc/123', '--host=draco', '--name=protein']) + output = 'ERROR There is currently no support for \'somehpc\'. ' \ 'Supported MD engines are: gromacs, namd.\n' - assert result.exit_code == 1 + # assert result.exit_code == 1 assert result.output == output with tmpdir.as_cwd(): - with open('protein.tpr', 'w') as fh: - fh.write('This is a dummy tpr ;)') + for f in ['protein.tpr', 'protein.namd', 'protein.pdb', 'protein.psf']: + with open(f, 'w') as fh: + fh.write('This is a dummy file.') for k, v in DIR_STRUCTURE.items(): for k2, v2 in v.items(): os.makedirs(os.path.join(k, k2)) @@ -132,36 +210,42 @@ def test_generate_validation(cli_runner, tmpdir, monkeypatch): 'generate', '--module=gromacs/doesnotexist', '--host=draco', '--name=protein' ]) - output = 'ERROR We have problems finding all of your requested modules on this host.\n' \ - 'We were not able to find the following modules for MD engine gromacs: doesnotexist.\n' \ + output = 'WARNING We have problems finding all of your requested modules on this host.\n' \ + 'We were not able to find the following modules for MD engine gromacs: ' \ + 'doesnotexist.\n' \ 'Available modules are:\n' \ - 'gromacs/5.1.4-plumed2.3\n' \ 'gromacs/2016.4\n' \ - 'gromacs/2018.1\n' + 'gromacs/2018.1\n' \ + 'gromacs/5.1.4-plumed2.3\n' \ + '\n' \ + 'ERROR No requested modules available!\n' - # assert result.exit_code == 1 + assert result.exit_code == 1 assert result.output == output # Test that the warning also works when specifying several MD engines. # Test that we get a warning if we cannot find the requested modules. result = cli_runner.invoke(cli.cli, [ 'generate', '--module=gromacs/doesnotexist', '--module=namd/abc', - '--module=namd/123', '--host=draco', '--name=protein' + '--host=draco', '--name=protein' ]) output = 'WARNING NAMD support is experimental. ' \ 'All input files must be in the current directory. ' \ - 'Parameter paths must be absolute. Only crude file checks are performed!' \ + 'Parameter paths must be absolute. Only crude file checks are performed! ' \ 'If you use the --gpu option make sure you use the GPU compatible NAMD module!\n' \ - 'ERROR We have problems finding all of your requested modules on this host.\n' \ - 'We were not able to find the following modules for MD engine gromacs: doesnotexist.\n' \ + 'WARNING We have problems finding all of your requested modules on this host.\n' \ + 'We were not able to find the following modules for MD engine gromacs: ' \ + 'doesnotexist.\n' \ 'Available modules are:\n' \ - 'gromacs/5.1.4-plumed2.3\n' \ 'gromacs/2016.4\n' \ 'gromacs/2018.1\n' \ + 'gromacs/5.1.4-plumed2.3\n' \ 'We were not able to find the following modules for MD engine namd: abc.\n' \ 'Available modules are:\n' \ + 'namd/123\n' \ 'namd/456\n' \ - 'namd/123\n' + '\n' \ + 'ERROR No requested modules available!\n' - # assert result.exit_code == 1 + assert result.exit_code == 1 assert result.output == output diff --git a/mdbenchmark/tests/test_analyze.py b/mdbenchmark/tests/test_analyze.py index 1a29377d..88d90352 100644 --- a/mdbenchmark/tests/test_analyze.py +++ b/mdbenchmark/tests/test_analyze.py @@ -44,9 +44,9 @@ def test_analyze_gromacs(cli_runner, tmpdir, data): def test_analze_namd(cli_runner, tmpdir, data): with tmpdir.as_cwd(): - result = cli_runner.invoke(cli.cli, [ - 'analyze', '--directory={}'.format(data['analyze-files-namd']) - ]) + result = cli_runner.invoke( + cli.cli, + ['analyze', '--directory={}'.format(data['analyze-files-namd'])]) assert result.exit_code == 0 assert result.output == """ module nodes ns/day run time [min] gpu host ncores 0 namd 1 0.076328 15 False draco 1 diff --git a/mdbenchmark/tests/test_generate.py b/mdbenchmark/tests/test_generate.py index b1f30ef0..b6704df1 100644 --- a/mdbenchmark/tests/test_generate.py +++ b/mdbenchmark/tests/test_generate.py @@ -20,10 +20,24 @@ import os import pytest +from click import exceptions + from mdbenchmark import cli from mdbenchmark.ext.click_test import cli_runner +from mdbenchmark.generate import (print_known_hosts, validate_hosts, + validate_module, validate_name, + validate_number_of_nodes) -from .mdengines.test_init import DIR_STRUCTURE +DIR_STRUCTURE = { + 'applications': { + 'gromacs': ['2016.4', '5.1.4-plumed2.3', '2018.1'], + 'namd': ['123', '456'], + 'amber': ['13', '14', '15'] + }, + 'visualization': { + 'vmd': ['1.9.3', '1.9.4'] + } +} @pytest.mark.parametrize('tpr_file', ('protein.tpr', 'protein')) @@ -47,10 +61,11 @@ def test_generate(cli_runner, monkeypatch, tmpdir, tpr_file): '--max-nodes=4', '--gpu', '--name={}'.format(tpr_file) ]) assert result.exit_code == 0 - assert result.output == 'WARNING Not performing module name validation. Cannot locate modules available on this host.\n' + output + assert result.output == 'WARNING Cannot locate modules available on this host. ' \ + + 'Not performing module name validation.\n' + output # monkeypatch the output of the available modules - monkeypatch.setattr('mdbenchmark.generate.get_available_modules', + monkeypatch.setattr('mdbenchmark.mdengines.get_available_modules', lambda: {'gromacs': ['2016']}) # Test that we can skip module name validation, even if it actually works. @@ -91,7 +106,7 @@ def test_generate(cli_runner, monkeypatch, tmpdir, tpr_file): ]) monkeypatch.setenv('MODULEPATH', dirs) - output = 'ERROR There is currently no support for \'doesnotexist/version\'. ' \ + output = 'ERROR There is currently no support for \'doesnotexist\'. ' \ 'Supported MD engines are: gromacs, namd.\n' result = cli_runner.invoke(cli.cli, [ 'generate', '--module=doesnotexist/version', '--host=draco', @@ -131,7 +146,7 @@ def test_generate_console_messages(cli_runner, monkeypatch, tmpdir): """Test that the CLI for generate prints all error messages as expected.""" with tmpdir.as_cwd(): # monkeypatch the output of the available modules - monkeypatch.setattr('mdbenchmark.generate.get_available_modules', + monkeypatch.setattr('mdbenchmark.mdengines.get_available_modules', lambda: {'gromacs': ['2016']}) # Test that we get an error when not supplying a file name @@ -141,9 +156,9 @@ def test_generate_console_messages(cli_runner, monkeypatch, tmpdir): '"-n" / "--name": Please specifiy the name of your input files.' # Test error message if the TPR file does not exist - result = cli_runner.invoke(cli.cli, [ - 'generate', '--module=gromacs/2016', '--host=draco', '--name=md' - ]) + result = cli_runner.invoke( + cli.cli, + ['generate', '--module=gromacs/2016', '--host=draco', '--name=md']) output = 'ERROR File md.tpr does not exist, but is needed for GROMACS benchmarks.\n' assert result.exit_code == 1 @@ -168,9 +183,11 @@ def test_generate_console_messages(cli_runner, monkeypatch, tmpdir): 'generate', '--module=gromacs/2016', '--host=minerva', '--name=protein' ]) - output = 'Usage: cli generate [OPTIONS]\n\nError: Invalid value for' \ - ' "--host": Could not find template for host \'minerva\'.\n' - assert result.exit_code == 2 + output = 'Could not find template for host \'minerva\'.\n' \ + 'Available host templates:\n' \ + 'draco\n' \ + 'hydra\n' + assert result.exit_code == 0 assert result.output == output # Test error message if we do not pass any module name @@ -190,15 +207,15 @@ def test_generate_namd_experimental_warning(cli_runner, monkeypatch, tmpdir): open(f, 'a').close() # monkeypatch the output of the available modules - monkeypatch.setattr('mdbenchmark.generate.get_available_modules', + monkeypatch.setattr('mdbenchmark.mdengines.get_available_modules', lambda: {'namd': ['123']}) - result = cli_runner.invoke(cli.cli, [ - 'generate', '--module=namd/123', '--host=draco', '--name=md' - ]) + result = cli_runner.invoke( + cli.cli, + ['generate', '--module=namd/123', '--host=draco', '--name=md']) output = 'WARNING NAMD support is experimental. ' \ 'All input files must be in the current directory. ' \ - 'Parameter paths must be absolute. Only crude file checks are performed!' \ + 'Parameter paths must be absolute. Only crude file checks are performed! ' \ 'If you use the --gpu option make sure you use the GPU compatible NAMD module!\n' \ 'Creating benchmark system for namd/123.\n' \ 'Creating a total of 5 benchmarks, with a run time of 15 ' \ @@ -207,3 +224,89 @@ def test_generate_namd_experimental_warning(cli_runner, monkeypatch, tmpdir): assert result.exit_code == 0 assert result.output == output + + +@pytest.fixture +def ctx_mock(): + """Mock a context object for python click.""" + + class MockCtx(object): + resilient_parsing = False + color = None + + def exit(self): + return + + return MockCtx() + + +def test_print_known_hosts(ctx_mock, capsys): + """Test that the print_known_hosts function works as expected.""" + print_known_hosts(ctx_mock, None, True) + out, err = capsys.readouterr() + + assert out == 'Available host templates:\ndraco\nhydra\n' + + +def test_validate_generate_name(ctx_mock): + """Test that the validate_generate_name function works as expected.""" + + # Make sure we raise an exception if no name is given + with pytest.raises(exceptions.BadParameter) as error: + validate_name(ctx_mock, None) + + # Test the exception message + assert str( + error.value) == 'Please specify the base name of your input files.' + + # Make sure we return the module name, if we were given a name. + assert validate_name(ctx_mock, None, 'md') == 'md' + + +def test_validate_generate_module(ctx_mock): + """Test that the validate_generate_module function works as expected.""" + + # Make sure we raise an exception if no module name was given + with pytest.raises(exceptions.BadParameter) as error: + validate_module(ctx_mock, None) + + # Test the exception message + assert str( + error.value + ) == 'Please specify which MD engine module to use for the benchmarks.' + + # Make sure we return the value again + assert validate_module(ctx_mock, None, 'gromacs/123') == 'gromacs/123' + + +def test_validate_generate_number_of_nodes(): + """Test that the validate_generate_number_of_nodes function works as expected.""" + + with pytest.raises(exceptions.BadParameter) as error: + validate_number_of_nodes( + min_nodes=6, + max_nodes=5, + ) + + assert str( + error.value + ) == 'The minimal number of nodes needs to be smaller than the maximal number.' + + assert validate_number_of_nodes(min_nodes=1, max_nodes=6) is None + + +def test_validate_generate_host(ctx_mock): + """Test that the validate_generate_host function works as expected.""" + + # Test error without any hostname + with pytest.raises(exceptions.BadParameter) as error: + validate_hosts(ctx_mock, None) + assert str(error.value + ) == 'Could not guess host. Please provide a value explicitly.' + + # Test error with non-existent hostname + # TODO: We need to wrap this into a function! + assert validate_hosts(ctx_mock, None, host='hercules') is None + + # Test success of existent hostname + assert validate_hosts(ctx_mock, None, host='draco') == 'draco' diff --git a/mdbenchmark/tests/test_submit.py b/mdbenchmark/tests/test_submit.py index a1fb8066..d6169a56 100644 --- a/mdbenchmark/tests/test_submit.py +++ b/mdbenchmark/tests/test_submit.py @@ -17,6 +17,8 @@ # # You should have received a copy of the GNU General Public License # along with MDBenchmark. If not, see . +import os + import click from mdbenchmark import cli @@ -25,7 +27,7 @@ from mdbenchmark.testing import data -def test_get_batch_command(cli_runner): +def test_get_batch_command(cli_runner, monkeypatch, tmpdir): """Test that the get_engine_command works correctly. It should exit if no batching system was found. @@ -43,12 +45,18 @@ def test_cli(): def test(): get_batch_command() + # Test fail state output = 'ERROR Was not able to find a batch system. ' \ 'Are you trying to use this package on a host with a queuing system?\n' result = cli_runner.invoke(test_cli, ['test']) assert result.exit_code == 1 assert result.output == output + # Test non-fail state + monkeypatch.setattr('mdbenchmark.submit.glob', lambda x: ['qsub']) + result = cli_runner.invoke(test_cli, ['test']) + assert result.exit_code == 0 + class DummyEngine(object): @staticmethod @@ -99,7 +107,8 @@ def call(arg): 'Submitted all benchmarks. Run mdbenchmark analyze once' \ ' they are finished to get the results.\n' result = cli_runner.invoke(cli.cli, [ - 'submit', '--directory={}'.format(data['analyze-files-gromacs']), '--force' + 'submit', '--directory={}'.format(data['analyze-files-gromacs']), + '--force' ]) assert result.exit_code == 0 assert result.output == output diff --git a/mdbenchmark/tests/test_utils.py b/mdbenchmark/tests/test_utils.py index 4c883b71..58559d62 100644 --- a/mdbenchmark/tests/test_utils.py +++ b/mdbenchmark/tests/test_utils.py @@ -19,9 +19,9 @@ # along with MDBenchmark. If not, see . import click import numpy as np +import pytest from numpy.testing import assert_equal -import pytest from mdbenchmark import utils from mdbenchmark.ext.click_test import cli_runner @@ -63,29 +63,16 @@ def test_monkeypatch_guess_host(monkeypatch): assert utils.guess_host() == host -def test_normalize_host(monkeypatch): - """Test `normalize_host()`.""" - with pytest.raises(click.BadParameter): - utils.normalize_host(None) - - host = 'HPC_cluster' - monkeypatch.setattr('mdbenchmark.utils.guess_host', lambda: host) - assert utils.normalize_host(host) == host - - def test_retrieve_host_template(monkeypatch): """Test `retrieve_host_template` utility function.""" - # Check some non-existent template name - assert utils.retrieve_host_template('some_hpc') == ('some_hpc', False) - # Check template name that we supply with the package assert utils.retrieve_host_template('draco') is not None # Check that the user can define some custom template monkeypatch.setattr('mdbenchmark.utils.ENV.get_template', lambda x: 'minerva') - assert utils.retrieve_host_template('minerva') == ('minerva', True) + assert utils.retrieve_host_template('minerva') == 'minerva' def test_lin_func(): diff --git a/mdbenchmark/tests/test_validate.py b/mdbenchmark/tests/test_validate.py deleted file mode 100644 index b9756ed5..00000000 --- a/mdbenchmark/tests/test_validate.py +++ /dev/null @@ -1,85 +0,0 @@ -# -*- Mode: python; tab-width: 4; indent-tabs-mode:nil; coding:utf-8 -*- -# vim: tabstop=4 expandtab shiftwidth=4 softtabstop=4 fileencoding=utf-8 -# -# MDBenchmark -# Copyright (c) 2017 Max Linke & Michael Gecht and contributors -# (see the file AUTHORS for the full list of names) -# -# MDBenchmark is free software: you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation, either version 3 of the License, or -# (at your option) any later version. -# -# MDBenchmark is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with MDBenchmark. If not, see . -from click import exceptions - -import pytest -from mdbenchmark import validate - - -def test_validate_generate_name(): - """Test that the validate_generate_name function works as expected.""" - - with pytest.raises(exceptions.BadParameter) as error: - validate.validate_generate_name() - - assert str( - error.value) == 'Please specify the base name of your input files.' - - assert validate.validate_generate_name('md') is None - - -def test_validate_generate_module(): - """Test that the validate_generate_module function works as expected.""" - - with pytest.raises(exceptions.BadParameter) as error: - validate.validate_generate_module() - - assert str( - error.value - ) == 'Please specify which MD engine module to use for the benchmarks.' - - assert validate.validate_generate_module('gromacs/123') is None - - -def test_validate_generate_number_of_nodes(): - """Test that the validate_generate_number_of_nodes function works as expected.""" - - with pytest.raises(exceptions.BadParameter) as error: - validate.validate_generate_number_of_nodes(min_nodes=6, max_nodes=5) - - assert str( - error.value - ) == 'The minimal number of nodes needs to be smaller than the maximal number.' - - assert validate.validate_generate_number_of_nodes( - min_nodes=1, max_nodes=6) is None - - -def test_validate_generate_host(): - """Test that the validate_generate_host function works as expected.""" - - # Test error without any hostname - with pytest.raises(exceptions.BadParameter) as error: - assert validate.validate_generate_host() == (None, False) - assert str(error.value) == 'Could not find template for host \'None\'.' - - # Test success of existent hostname - assert validate.validate_generate_host(host='draco', status=True) is None - - -def test_validate_generate_arguments(): - """Test that the validate_generate_argument function works as expected.""" - - assert validate.validate_generate_arguments( - name='md', - module='gromacs/123', - host=('draco', True), - min_nodes=1, - max_nodes=6) is None diff --git a/mdbenchmark/utils.py b/mdbenchmark/utils.py index 2179393f..4eb228d6 100644 --- a/mdbenchmark/utils.py +++ b/mdbenchmark/utils.py @@ -84,8 +84,6 @@ def retrieve_host_template(host=None): ------- template """ - if host is None: - host = guess_host(host) return ENV.get_template(host) From 67a3045684075a20f7d7cb0f4e1044024f3a6a19 Mon Sep 17 00:00:00 2001 From: Michael Gecht Date: Tue, 3 Apr 2018 20:59:29 +0200 Subject: [PATCH 4/4] Add changelog fragment --- changelog/49.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog/49.feature diff --git a/changelog/49.feature b/changelog/49.feature new file mode 100644 index 00000000..562e31fe --- /dev/null +++ b/changelog/49.feature @@ -0,0 +1 @@ +Module name is now validated against available modules on host. Can be skipped with `--skip-validation`. \ No newline at end of file