From: M. Eric Irrgang Date: Wed, 30 Sep 2020 20:32:20 +0000 (+0300) Subject: Handle gmxapi tmpi parameters better. X-Git-Url: http://biod.pnpi.spb.ru/gitweb/?a=commitdiff_plain;h=fff5715e08cf6cfcada830806c792bf5495a0253;p=alexxy%2Fgromacs.git Handle gmxapi tmpi parameters better. Use gmxconfig.json to support some additional testing support. Configure thread allocation for gmxapi CI jobs more carefully. Fixes #3710 --- diff --git a/admin/ci-scripts/build-and-test-py-gmxapi-0.2.sh b/admin/ci-scripts/build-and-test-py-gmxapi-0.2.sh index e7512e13e8..828538828b 100644 --- a/admin/ci-scripts/build-and-test-py-gmxapi-0.2.sh +++ b/admin/ci-scripts/build-and-test-py-gmxapi-0.2.sh @@ -39,7 +39,7 @@ pushd python_packaging/src popd # Run Python unit tests. -python -m pytest python_packaging/src/test --junitxml=$PY_UNIT_TEST_XML +python -m pytest python_packaging/src/test --junitxml=$PY_UNIT_TEST_XML --threads=2 # Note: Multiple pytest processes getting --junitxml output file argument # may cause problems, so we set the option on only one of the launched processes. @@ -47,7 +47,8 @@ python -m pytest python_packaging/src/test --junitxml=$PY_UNIT_TEST_XML # https://www.open-mpi.org/doc/v3.0/man1/mpiexec.1.php PROGRAM=(`which python` -m mpi4py -m pytest \ -p no:cacheprovider \ - $PWD/python_packaging/src/test) + $PWD/python_packaging/src/test \ + --threads=1) # shellcheck disable=SC2068 if [ -x `which mpiexec` ]; then PYTHONDONTWRITEBYTECODE=1 \ @@ -68,7 +69,8 @@ python -m pytest python_packaging/test --junitxml=$PY_ACCEPTANCE_TEST_XML # https://www.open-mpi.org/doc/v3.0/man1/mpiexec.1.php PROGRAM=(`which python` -m mpi4py -m pytest \ -p no:cacheprovider \ - $PWD/python_packaging/test) + $PWD/python_packaging/test \ + --threads=1) # shellcheck disable=SC2068 if [ -x `which mpiexec` ]; then PYTHONDONTWRITEBYTECODE=1 \ diff --git a/admin/ci-scripts/build-and-test-sample_restraint-2021.sh b/admin/ci-scripts/build-and-test-sample_restraint-2021.sh index 8de02b5270..d876257784 100644 --- a/admin/ci-scripts/build-and-test-sample_restraint-2021.sh +++ b/admin/ci-scripts/build-and-test-sample_restraint-2021.sh @@ -65,7 +65,7 @@ pushd python_packaging/sample_restraint make install popd - python -m pytest $PWD/tests --junitxml=$PLUGIN_TEST_XML + python -m pytest $PWD/tests --junitxml=$PLUGIN_TEST_XML --threads=2 # Note: Multiple pytest processes getting --junitxml output file argument # may cause problems, so we set the option on only one of the launched processes. @@ -73,7 +73,8 @@ pushd python_packaging/sample_restraint # https://www.open-mpi.org/doc/v3.0/man1/mpiexec.1.php PROGRAM=(`which python` -m mpi4py -m pytest \ -p no:cacheprovider \ - $PWD/tests) + $PWD/tests \ + --threads=1) # shellcheck disable=SC2068 if [ -x `which mpiexec` ]; then PYTHONDONTWRITEBYTECODE=1 \ diff --git a/python_packaging/sample_restraint/tests/test_binding.py b/python_packaging/sample_restraint/tests/test_binding.py index 7dff38ca63..a483fd7996 100644 --- a/python_packaging/sample_restraint/tests/test_binding.py +++ b/python_packaging/sample_restraint/tests/test_binding.py @@ -35,16 +35,14 @@ def test_import(): @pytest.mark.usefixtures("cleandir") -def test_ensemble_potential_nompi(spc_water_box): +def test_ensemble_potential_nompi(spc_water_box, mdrun_kwargs): """Test ensemble potential without an ensemble. """ tpr_filename = spc_water_box print("Testing plugin potential with input file {}".format(os.path.abspath(tpr_filename))) assert gmx.version.api_is_at_least(0, 0, 5) - # Note that *threads* argument causes errors for MPI-enabled GROMACS. - # Ref #3563 and #3573 - md = from_tpr([tpr_filename], append_output=False, threads=2) + md = from_tpr([tpr_filename], append_output=False, **mdrun_kwargs) # Create a WorkElement for the potential params = {'sites': [1, 4], @@ -75,15 +73,13 @@ def test_ensemble_potential_nompi(spc_water_box): @pytest.mark.withmpi_only @pytest.mark.usefixtures("cleandir") -def test_ensemble_potential_withmpi(spc_water_box): +def test_ensemble_potential_withmpi(spc_water_box, mdrun_kwargs): tpr_filename = spc_water_box logger.info("Testing plugin potential with input file {}".format(os.path.abspath(tpr_filename))) assert gmx_version.api_is_at_least(0, 0, 5) - # Note that *threads* argument causes errors for MPI-enabled GROMACS. - # Ref #3563 and #3573 - md = from_tpr([tpr_filename, tpr_filename], append_output=False, threads=2) + md = from_tpr([tpr_filename, tpr_filename], append_output=False, **mdrun_kwargs) # Create a WorkElement for the potential params = {'sites': [1, 4], diff --git a/python_packaging/src/gmxapi/simulation/mdrun.py b/python_packaging/src/gmxapi/simulation/mdrun.py index 92f904a0b8..ed4d1229d6 100644 --- a/python_packaging/src/gmxapi/simulation/mdrun.py +++ b/python_packaging/src/gmxapi/simulation/mdrun.py @@ -261,8 +261,7 @@ class LegacyImplementationSubscription(object): ensemble_rank, self.workdir )) - # TODO: Normalize the way we pass run time parameters to mdrun. - # See also #3573 + # TODO: (#3718) Normalize the way we pass run time parameters to mdrun. kwargs = getattr(resource_manager, 'mdrun_kwargs', {}) for key, value in kwargs.items(): logger.debug('Adding mdrun run time argument: {}'.format(key + '=' + str(value))) diff --git a/python_packaging/src/gmxapi/testsupport.py b/python_packaging/src/gmxapi/testsupport.py index 64672fab50..0ffdf02435 100644 --- a/python_packaging/src/gmxapi/testsupport.py +++ b/python_packaging/src/gmxapi/testsupport.py @@ -90,7 +90,7 @@ def pytest_runtest_setup(item): def pytest_addoption(parser): - """Add a command-line user option for the pytest invocation.""" + """Add command-line user options for the pytest invocation.""" parser.addoption( '--rm', action='store', @@ -98,6 +98,11 @@ def pytest_addoption(parser): choices=['always', 'never', 'success'], help='Remove temporary directories "always", "never", or on "success".' ) + parser.addoption( + '--threads', + type=int, + help='Maximum number of threads per process per gmxapi session.' + ) class RmOption(Enum): @@ -113,6 +118,46 @@ def remove_tempdir(request) -> RmOption: arg = request.config.getoption('--rm') return RmOption(arg) +@pytest.fixture(scope='session') +def gmxconfig(): + try: + from importlib.resources import open_text + with open_text('gmxapi', 'gmxconfig.json') as textfile: + config = json.load(textfile) + except ImportError: + # TODO: Remove this when we require Python 3.7 + try: + # A backport of importlib.resources is available as importlib_resources + # with a somewhat different interface. + from importlib_resources import files, as_file + + source = files('gmxapi').joinpath('gmxconfig.json') + with as_file(source) as gmxconfig: + with open(gmxconfig, 'r') as fp: + config = json.load(fp) + except ImportError: + config = None + yield config + +@pytest.fixture(scope='session') +def mdrun_kwargs(request, gmxconfig): + """pytest fixture to provide a mdrun_kwargs dictionary for the mdrun ResourceManager. + """ + from gmxapi.simulation.mdrun import ResourceManager as _ResourceManager + if gmxconfig is None: + raise RuntimeError('--threads argument requires a usable gmxconfig.json') + arg = request.config.getoption('--threads') + if arg is None: + return {} + mpi_type = gmxconfig['gmx_mpi_type'] + if mpi_type is not None and mpi_type == "tmpi": + kwargs = {'threads': int(arg)} + else: + kwargs = {} + # TODO: (#3718) Normalize the handling of run-time arguments. + _ResourceManager.mdrun_kwargs = dict(**kwargs) + return kwargs + @contextmanager def scoped_chdir(dir): @@ -212,29 +257,14 @@ def cleandir(remove_tempdir: RmOption): class GmxBin: """Represent the detected GROMACS installation.""" - def __init__(self): + def __init__(self, gmxconfig): # Try to use package resources to locate the "gmx" binary wrapper. - try: - from importlib.resources import open_text - with open_text('gmxapi', 'gmxconfig.json') as textfile: - config = json.load(textfile) - gmxbindir = config.get('gmx_bindir', None) - command = config.get('gmx_executable', None) - except ImportError: - try: - # A backport of importlib.resources is available as importlib_resources - # with a somewhat different interface. - from importlib_resources import files, as_file - - source = files('gmxapi').joinpath('gmxconfig.json') - with as_file(source) as gmxconfig: - with open(gmxconfig, 'r') as fp: - config = json.load(fp) - gmxbindir = config.get('gmx_bindir', None) - command = config.get('gmx_executable', None) - except ImportError: - gmxbindir = None - command = None + if gmxconfig is not None: + gmxbindir = gmxconfig.get('gmx_bindir', None) + command = gmxconfig.get('gmx_executable', None) + else: + gmxbindir = None + command = None # TODO: Remove fall-back when we can rely on gmxconfig.json via importlib.resources in Py 3.7+. allowed_command_names = ['gmx', 'gmx_mpi'] @@ -266,12 +296,9 @@ class GmxBin: return self._bindir -_gmx = GmxBin() - - @pytest.fixture(scope='session') -def gmxcli(): - command = _gmx.command() +def gmxcli(gmxconfig): + command = GmxBin(gmxconfig).command() if command is None: message = "Tests need 'gmx' command line tool, but could not find it on the path." raise RuntimeError(message) diff --git a/python_packaging/src/test/test_mdrun.py b/python_packaging/src/test/test_mdrun.py index 132267291c..48d0808f8c 100644 --- a/python_packaging/src/test/test_mdrun.py +++ b/python_packaging/src/test/test_mdrun.py @@ -48,11 +48,6 @@ import pytest import gmxapi as gmx -# TODO: (#3573) Normalize the handling of run-time arguments. -from gmxapi.simulation.mdrun import ResourceManager as _ResourceManager -# Note that *threads* argument causes errors for MPI-enabled GROMACS. -# Ref #3563 and #3573 -_ResourceManager.mdrun_kwargs = {'threads': 2} # Configure the `logging` module before proceeding any further. gmx.logger.setLevel(logging.WARNING) @@ -78,7 +73,7 @@ formatter = logging.Formatter(rank_tag + '%(name)s:%(levelname)s: %(message)s') @pytest.mark.usefixtures('cleandir') -def test_run_from_tpr(spc_water_box): +def test_run_from_tpr(spc_water_box, mdrun_kwargs): assert os.path.exists(spc_water_box) md = gmx.mdrun(spc_water_box) @@ -88,7 +83,7 @@ def test_run_from_tpr(spc_water_box): @pytest.mark.withmpi_only @pytest.mark.usefixtures('cleandir') -def test_run_trivial_ensemble(spc_water_box, caplog): +def test_run_trivial_ensemble(spc_water_box, caplog, mdrun_kwargs): from mpi4py import MPI current_rank = MPI.COMM_WORLD.Get_rank() with caplog.at_level(logging.DEBUG): @@ -124,7 +119,7 @@ def test_run_trivial_ensemble(spc_water_box, caplog): @pytest.mark.usefixtures('cleandir') -def test_run_from_read_tpr_op(spc_water_box, caplog): +def test_run_from_read_tpr_op(spc_water_box, caplog, mdrun_kwargs): with caplog.at_level(logging.DEBUG): caplog.handler.setFormatter(formatter) with caplog.at_level(logging.DEBUG, 'gmxapi'): @@ -137,7 +132,7 @@ def test_run_from_read_tpr_op(spc_water_box, caplog): @pytest.mark.usefixtures('cleandir') -def test_run_from_modify_input_op(spc_water_box, caplog): +def test_run_from_modify_input_op(spc_water_box, caplog, mdrun_kwargs): with caplog.at_level(logging.DEBUG): simulation_input = gmx.read_tpr(spc_water_box) diff --git a/python_packaging/test/pytesthelpers.py b/python_packaging/test/pytesthelpers.py index 480dd45f86..4c171d4943 100644 --- a/python_packaging/test/pytesthelpers.py +++ b/python_packaging/test/pytesthelpers.py @@ -39,10 +39,8 @@ Define the ``withmpi_only`` test decorator. import pytest -# TODO: (#3573) Normalize the handling of run-time arguments. +# TODO: (#3718) Normalize the handling of run-time arguments. from gmxapi.simulation.mdrun import ResourceManager as _ResourceManager -# Note that *threads* argument causes errors for MPI-enabled GROMACS. -# Ref #3563 and #3573 _ResourceManager.mdrun_kwargs = {'threads': 2} withmpi_only = None