From 8c891bea1bb5fa0c404a3856b1348c6a251e29c2 Mon Sep 17 00:00:00 2001 From: Oliver Beckstein Date: Tue, 17 Aug 2021 02:09:09 -0700 Subject: [PATCH] config.POWConfigParser fixes and enhancements - raise new NoSectionError for missing section - new warning NoOptionWarning - run.get_mdp_files() now looks for full paths for MDP files and raises ValueError if file cannot be found - getpath() of None returns None instead of TypeError - getlist() and getarray() return empty containers instead of TypeError - readfp() returns self instead of True so that one can initialize from file in one line - findfile() reliably returns None if given None as input - improve debugging with logging POWConfigParser.get() at DEBUG - add tests - update CHANGES (for 0.8.0) more testing updates (SQUASHME) --- CHANGES | 21 +++- mdpow/config.py | 116 +++++++++++++++---- mdpow/equil.py | 40 ++++--- mdpow/fep.py | 68 ++++++----- mdpow/run.py | 17 +-- mdpow/templates/runinput.yml | 6 +- mdpow/tests/test_Gsolv.py | 12 +- mdpow/tests/test_config.py | 140 +++++++++++++++++++++++ mdpow/tests/test_equilibration_script.py | 6 +- mdpow/tests/test_fep.py | 61 ++++++---- mdpow/tests/test_fep_script.py | 9 +- mdpow/tests/test_run.py | 37 ++++++ mdpow/tests/test_runinput.py | 54 ++++----- mdpow/tests/test_solvation.py | 7 +- 14 files changed, 447 insertions(+), 147 deletions(-) create mode 100644 mdpow/tests/test_config.py create mode 100644 mdpow/tests/test_run.py diff --git a/CHANGES b/CHANGES index 142c1136..81b9ed6c 100644 --- a/CHANGES +++ b/CHANGES @@ -6,12 +6,31 @@ Add summary of changes for each release. Use ISO dates. Reference GitHub issues numbers and PR numbers. 2021-??-?? 0.8.0 -ALescoulie +ALescoulie, orbeckst Changes * dropped support for Python 2 and <3.6: only Python 3.7--3.9 are supported (#177) +* removed unused POWConfigParser.getintarray() (PR #187) + +Enhancements + +* new exception config.NoSectionError to indicate missing section + ("protocol") in the YAML run input file (PR #187) +* new config.NoOptionWarning when undefined options are used (with + default None) and POWConfigParser.get() now logs when an option is used + at level DEBUG (PR #187) + + +Fixes + +* run.get_mdp_files() finds the full path for an MDP file and raises ValueError + when MDP file is not found (PR #187) +* POWConfigParser.getpath() returns None for empty options instead of raising + TypeError (PR #187) +* POWConfigParser.getlist() and getarray() return empty list or array for + empty options instead of raising TypeError (PR #187) 2021-08-04 0.7.1 diff --git a/mdpow/config.py b/mdpow/config.py index f46471dc..3d2e2878 100644 --- a/mdpow/config.py +++ b/mdpow/config.py @@ -88,6 +88,12 @@ .. autofunction:: _generate_template_dict +Exceptions +---------- + +.. autoexception:: NoSectionError +.. autoclass:: NoOptionWarning + """ import os, errno from pkg_resources import resource_filename, resource_listdir @@ -96,6 +102,7 @@ import numpy as np import gromacs.utilities +import warnings import logging logger = logging.getLogger("mdpow.config") @@ -107,8 +114,22 @@ "runinput": resource_filename(__name__, "templates/runinput.yml"), } +class NoSectionError(ValueError): + """Section entry is missing. + + .. versionadded:: 0.8.0 + """ + +# not used at the moment +# class NoOptionError(ValueError): +# """Option entry is missing from section""" + +class NoOptionWarning(UserWarning): + """Warning that an option is missing.""" + def merge_dicts(user, default): """Merge two dictionaries recursively. + Uses recursive method to accurately merge nested dictionaries """ @@ -122,7 +143,19 @@ def merge_dicts(user, default): class POWConfigParser(object): - """Parse YAML config file.""" + """Parse YAML config file. + + Raises :exc:`NoSectionError` if a protocol section is missing. + Warns with :exc:`NoOptionWarning` if option under a protocol is missing + and returns ``None``. + + .. versionchanged:: 0.8.0 + :exc:`NoSectionError` is now raised to clearly distinguish + file structure problems (missing sections) from missing + entries (options). + + The `getintarray()` method was removed. + """ def __init__(self): self.conf = None @@ -131,9 +164,14 @@ def readfp(self, fn): """Read YAML from open stream ``fn``. Overwrites everything. + + Returns self. + + .. versionchanged:: 0.8.0 + Returns self instead of ``True``. """ self.conf = yaml.safe_load(fn) - return True + return self def merge(self, fn): """Load YAML from open stream ``fn`` and merge into :attr:`conf`. @@ -155,21 +193,35 @@ def get(self, section, option): """Return option, unless its "None" --> ``None``, Conversion to basic python types str, float, int, boolean is - carried out automatically (unless it was None). + carried out automatically. .. Note:: "none" remains a string, which is essential, see `Issue 20 `_ + Conversions are performed by the pyyaml parser. .. versionchanged:: 0.6.0 Prior versions would convert case-insensitively (e.g. "NONE" and "none") """ - try: - value = self.conf[section][option] - return value if value != "None" else None - except TypeError: - return None + sec = self.conf[section] + except (KeyError, TypeError): + # TypeError when self.conf is None + raise NoSectionError(f"Config file has no section {section}") + try: + value = sec[option] + value = value if value != "None" else None # still needed?? + except KeyError: + # Returning None has been standard behavior. + #raise NoOptionError(f"Config file section {section} contains " + # f"no option {option}.") + msg = (f"Config file section {section} contains " + f"no option {option}. Using 'None'.") + warnings.warn(msg, category=NoOptionWarning) + logger.warning(msg) + value = None + logger.debug("%s: %s = %r", section, option, value) + return value # TODO: # The YAML parser does automatic conversion: the following @@ -181,40 +233,57 @@ def get(self, section, option): getboolean = get def getpath(self, section, option): - """Return option as an expanded path.""" - return os.path.expanduser(os.path.expandvars(self.get(section, option))) + """Return option as an expanded path (or ``None``). + + .. versionchanged:: 0.8.0 + If the entry is ``None`` this method returns ``None`` + instead of raising a :exc:`TypeError`. + """ + item = self.get(section, option) + return os.path.expanduser( + os.path.expandvars(item)) if item is not None else None def findfile(self, section, option): - """Return location of a file ``option``. + """Return location of a file ``option`` or ``None``. Uses :func:`mdpow.config.get_template`. """ - return get_template(self.getpath(section, option)) + pth = self.getpath(section, option) + return get_template(pth) if pth is not None else None # TODO: Change input file format to use yaml lists and make this method superfluous def getlist(self, section, option): - """Return option as a list of strings. + """Return option as a list of strings or ``[]``. *option* must be comma-separated; leading/trailing whitespace is stripped and quotes are treated verbatim. + + .. versionchanged:: 0.8.0 + If the entry is ``None``, an empty list is returned + instead of raising a :exc:`TypeError`. """ - return [x.strip() for x in str(self.get(section, option)).split(",")] + item = self.get(section, option) + return [x.strip() + for x in str(item).split(",")] if item is not None else [] def getarray(self, section, option): - """Return option as a numpy array of floats. + """Return option as a numpy array of floats or ``np.array([])``. *option* must be comma-separated; leading/trailing whitespace is stripped and quotes are treated verbatim. - """ - return np.asarray(self.getlist(section, option), dtype=np.float) - - def getintarray(self, section, option): - """Return option as a numpy array of integers. - *option* must be comma-separated; leading/trailing whitespace - is stripped and quotes are treated verbatim. + .. versionchanged:: 0.8.0 + If the entry is ``None``, an empty array is returned. """ - return np.asarray(self.getlist(section, option), dtype=np.int) + return np.asarray(self.getlist(section, option), dtype=float) + +# def getintarray(self, section, option): +# """Return option as a numpy array of integers. +# +# *option* must be comma-separated; leading/trailing whitespace +# is stripped and quotes are treated verbatim. +# """ +# return np.asarray(self.getlist(section, option), dtype=int) def get_configuration(filename=None): """Reads and parses a run input config file. @@ -420,4 +489,3 @@ def asiterable(obj): else: logger.warning("Using user-supplied environment variable GMXLIB=%r to find force fields", os.environ['GMXLIB']) logger.info("(You can use the MDPOW default by executing 'unset GMXLIB' in your shell before running MDPOW.)") - diff --git a/mdpow/equil.py b/mdpow/equil.py index 2d39a11d..a14140e4 100644 --- a/mdpow/equil.py +++ b/mdpow/equil.py @@ -316,19 +316,33 @@ def topology(self, itp='drug.itp', prm=None, **kwargs): if prm is not None: shutil.copy(prm, _prm) gromacs.cbook.edit_txt(top_template, - [('#include +"oplsaa\.ff/forcefield\.itp"', - 'oplsaa\.ff/', setting[0]), - ('#include +"compound\.itp"', 'compound\.itp', _itp), - ('#include +"oplsaa\.ff/tip4p\.itp"', - 'oplsaa\.ff/tip4p\.itp', setting[0]+self.solvent.itp), - ('#include +"oplsaa\.ff/ions_opls\.itp"', - 'oplsaa\.ff/ions_opls\.itp', setting[1]), - ('#include +"compound\.prm"', - '#include +"compound\.prm"', prm_kw), - ('#include +"water\.itp"', 'water\.itp', setting[2]), - ('Compound', 'solvent', self.solvent_type), - ('Compound', 'DRUG', self.molecule), - ('DRUG\s*1', 'DRUG', self.molecule), + [(r'#include +"oplsaa\.ff/forcefield\.itp"', + r'oplsaa\.ff/', + setting[0]), + (r'#include +"compound\.itp"', + r'compound\.itp', + _itp), + (r'#include +"oplsaa\.ff/tip4p\.itp"', + r'oplsaa\.ff/tip4p\.itp', + setting[0] + self.solvent.itp), + (r'#include +"oplsaa\.ff/ions_opls\.itp"', + r'oplsaa\.ff/ions_opls\.itp', + setting[1]), + (r'#include +"compound\.prm"', + r'#include +"compound\.prm"', + prm_kw), + (r'#include +"water\.itp"', + r'water\.itp', + setting[2]), + (r'Compound', + 'solvent', + self.solvent_type), + (r'Compound', + 'DRUG', + self.molecule), + (r'DRUG\s*1', + 'DRUG', + self.molecule), ], newname=topol) logger.info('[%(dirname)s] Created topology %(topol)r that includes %(_itp)r', vars()) diff --git a/mdpow/fep.py b/mdpow/fep.py index edc9b7c6..b0ce0dba 100644 --- a/mdpow/fep.py +++ b/mdpow/fep.py @@ -73,20 +73,20 @@ .. autoclass:: Gsolv :members: - :inherited-members: - + :inherited-members: + .. autoclass:: Ghyd :members: :inherited-members: - + .. autoclass:: Goct :members: :inherited-members: - + .. autoclass:: Gcyclo :members: :inherited-members: - + .. autofunction:: pOW .. autofunction:: pCW @@ -141,7 +141,7 @@ from glob import glob from configparser import NoOptionError -import numpy +import numpy as np import pandas as pd import scipy.integrate @@ -252,13 +252,19 @@ def load(cfg, section): cfg_get = {float: cfg.getfloat, int: cfg.getint, str: cfg.getstr, # literal strings, no conversion of None (which we need for the MDP!) + # CHECK: THIS IS LIKELY NOT GUARANTEED ANYMORE since getstr == get list: cfg.getarray # numpy float array from list } def getter(type, section, key): + value = cfg_get[type](section, key) try: - return cfg_get[type](section, key) - except NoOptionError: - return None + # for empty list [] and array array([]) + if len(value) == 0: + value = None + except TypeError: + pass + return value + # skip any None values return FEPschedule((key, getter(keytype, section, key)) for key,keytype in keys.items() if getter(keytype, section, key) is not None) @@ -327,7 +333,7 @@ class Gsolv(Journalled): label='Coul', couple_lambda0='vdw-q', couple_lambda1='vdw', sc_alpha=0, # linear scaling for coulomb - lambdas=numpy.array([0.0, 0.25, 0.5, 0.75, 1.0]), # default values + lambdas=np.array([0.0, 0.25, 0.5, 0.75, 1.0]), # default values ), 'vdw': FEPschedule(name='vdw', @@ -335,7 +341,7 @@ class Gsolv(Journalled): label='VDW', couple_lambda0='vdw', couple_lambda1='none', sc_alpha=0.5, sc_power=1, sc_sigma=0.3, # recommended values - lambdas=numpy.array([0.0, 0.05, 0.1, 0.2, 0.3, + lambdas=np.array([0.0, 0.05, 0.1, 0.2, 0.3, 0.4, 0.5, 0.6, 0.65, 0.7, 0.75, 0.8, 0.85, 0.9, 0.95, 1]), # defaults ), @@ -670,8 +676,8 @@ def _setup(self, component, lmbda, foreign_lambdas, **kwargs): logger.info('Setting dhdl file to xvg format') kwargs.setdefault('separate-dhdl-file', 'yes') - foreign_lambdas = numpy.asarray(foreign_lambdas) - lambda_index = numpy.where(foreign_lambdas == lmbda)[0][0] + foreign_lambdas = np.asarray(foreign_lambdas) + lambda_index = np.where(foreign_lambdas == lmbda)[0][0] kwargs.update(dirname=wdir, struct=self.struct, top=self.top, mdp=self.mdp, @@ -823,7 +829,7 @@ def collect(self, stride=None, autosave=True, autocompress=True): for component, lambdas in self.lambdas.items(): xvg_files = [self.dgdl_xvg(self.wdir(component, l)) for l in lambdas] - self.results.xvg[component] = (numpy.array(lambdas), + self.results.xvg[component] = (np.array(lambdas), [XVG(xvg, permissive=self.permissive, stride=self.stride) for xvg in xvg_files]) if autosave: @@ -879,9 +885,9 @@ def _lencorrupted(xvg): corrupted = {} self._corrupted = {} # debugging ... for component, (lambdas, xvgs) in self.results.xvg.items(): - corrupted[component] = numpy.any([(_lencorrupted(xvg) > 0) for xvg in xvgs]) + corrupted[component] = np.any([(_lencorrupted(xvg) > 0) for xvg in xvgs]) self._corrupted[component] = dict(((l, _lencorrupted(xvg)) for l,xvg in zip(lambdas, xvgs))) - return numpy.any([x for x in corrupted.values()]) + return np.any([x for x in corrupted.values()]) def analyze(self, force=False, stride=None, autosave=True, ncorrel=25000): r"""Extract dV/dl from output and calculate dG by TI. @@ -949,11 +955,11 @@ def analyze(self, force=False, stride=None, autosave=True, ncorrel=25000): The error on the mean of the data :math:`\epsilon_y`, taking the correlation time into account, is calculated according to [FrenkelSmit2002]_ `p526`_: - + .. math:: - + \epsilon_y = \sqrt{2 \tau_c \mathrm{acf}(0)/T} - + where :math:`\mathrm{acf}()` is the autocorrelation function of the fluctuations around the mean, :math:`y - \langle y \rangle`, :math:`\tau_c` is the correlation time, and :math:`T` @@ -990,18 +996,18 @@ def analyze(self, force=False, stride=None, autosave=True, ncorrel=25000): # for TI just get the average dv/dl value (in array column 1; col 0 is the time) # (This can take a while if the XVG is now reading the array from disk first time) # Use XVG class properties: first data in column 0! - Y = numpy.array([x.mean[0] for x in xvgs]) - stdY = numpy.array([x.std[0] for x in xvgs]) + Y = np.array([x.mean[0] for x in xvgs]) + stdY = np.array([x.std[0] for x in xvgs]) # compute auto correlation time and error estimate for independent samples # (this can take a while). x.array[0] == time, x.array[1] == dHdl # nstep is calculated to give ncorrel samples (or all samples if less than ncorrel are # available) tc_data = [numkit.timeseries.tcorrel(x.array[0], x.array[1], - nstep=int(numpy.ceil(len(x.array[0])/float(ncorrel)))) + nstep=int(np.ceil(len(x.array[0])/float(ncorrel)))) for x in xvgs] - DY = numpy.array([tc['sigma'] for tc in tc_data]) - tc = numpy.array([tc['tc'] for tc in tc_data]) + DY = np.array([tc['sigma'] for tc in tc_data]) + tc = np.array([tc['tc'] for tc in tc_data]) self.results.dvdl[component] = {'lambdas':lambdas, 'mean':Y, 'error':DY, 'stddev':stdY, 'tcorrel':tc} @@ -1050,13 +1056,13 @@ def collect_alchemlyb(self, SI=True, start=0, stop=None, stride=None, autosave=T # calculate statistical inefficiency of series statinef = statisticalInefficiency(ts, fast=False) logger.info("The statistical inefficiency value is {:.4f}.".format(statinef)) - logger.info("The data are subsampled every {:d} frames.".format(int(numpy.ceil(statinef)))) + logger.info("The data are subsampled every {:d} frames.".format(int(np.ceil(statinef)))) # use the subsampleCorrelatedData function to get the subsample index indices = subsampleCorrelatedData(ts, g=statinef, conservative=True) xvg_df = xvg_df.iloc[indices] val.append(xvg_df) - self.results.xvg[component] = (numpy.array(lambdas), pd.concat(val)) + self.results.xvg[component] = (np.array(lambdas), pd.concat(val)) if autosave: self.save() @@ -1104,8 +1110,8 @@ def analyze_alchemlyb(self, SI=True, start=0, stop=None, stride=None, force=Fals result = estimator().fit(xvgs) if self.method == 'BAR': DeltaA = QuantityWithError(0,0) - a_s= numpy.diagonal(result.delta_f_, offset=1) - da_s = numpy.diagonal(result.d_delta_f_, offset=1) + a_s= np.diagonal(result.delta_f_, offset=1) + da_s = np.diagonal(result.d_delta_f_, offset=1) for a, da in zip(a_s, da_s): DeltaA += QuantityWithError(a, da) self.results.DeltaA[component] = kBT_to_kJ(DeltaA, self.Temperature) @@ -1184,7 +1190,7 @@ def has_dVdl(self): return False except AttributeError: return False - return numpy.all(numpy.array([len(xvgs) for (lambdas,xvgs) in self.results.xvg.values()]) > 0) + return np.all(np.array([len(xvgs) for (lambdas,xvgs) in self.results.xvg.values()]) > 0) def plot(self, **kwargs): """Plot the TI data with error bars. @@ -1212,7 +1218,7 @@ def plot(self, **kwargs): dvdl = self.results.dvdl nplots = len(dvdl) fig, axs = plt.subplots(nrows=1, ncols=nplots) - for i, component in enumerate(numpy.sort(dvdl.keys())): # stable plot order + for i, component in enumerate(np.sort(dvdl.keys())): # stable plot order x,y,dy = [dvdl[component][k] for k in ('lambdas', 'mean', 'error')] iplot = i ax = axs[i] @@ -1427,7 +1433,7 @@ def p_transfer(G1, G2, **kwargs): transferFE = G2.results.DeltaA.Gibbs - G1.results.DeltaA.Gibbs # note minus sign, with our convention of the free energy difference to be # opposite to the partition coefficient. - logPow = -transferFE / (kBOLTZ * G1.Temperature) * numpy.log10(numpy.e) + logPow = -transferFE / (kBOLTZ * G1.Temperature) * np.log10(np.e) molecule = G1.molecule diff --git a/mdpow/run.py b/mdpow/run.py index 061232d9..f0b4d94c 100644 --- a/mdpow/run.py +++ b/mdpow/run.py @@ -36,7 +36,6 @@ .. autofunction:: runMD_or_exit """ -import configparser import sys import os @@ -44,7 +43,8 @@ import gromacs.run -from .config import get_configuration, set_gromacsoutput +from .config import (get_configuration, set_gromacsoutput, + NoSectionError) from . import equil from . import fep from .restart import checkpoint @@ -73,20 +73,21 @@ def get_mdp_files(cfg, protocols): mdpfiles = {} for protocol in protocols: try: - mdp = cfg.getpath(protocol, 'mdp') - except KeyError: + mdp = cfg.findfile(protocol, 'mdp') + except NoSectionError: # skip anything for which we do not define sections, such as # the dummy run protocols mdp = None - except configparser.NoOptionError: - # Should not happen... let's continue and wait for hard-coded defaults - logger.error("No 'mdp' config file entry for protocol [%s]---check input files!", protocol) - mdp = None except ValueError: # But it is a problem if we can't find a file! logger.critical("Failed to find custom MDP file %r for protocol [%s]", cfg.get(protocol, 'mdp'), protocol) raise + else: + if mdp is None: + # Should not happen... let's continue and wait for hard-coded defaults + logger.warning("No 'mdp' config file entry for protocol [%s]---check input files! " + "Using package defaults.", protocol) if mdp: mdpfiles[protocol] = mdp logger.debug("%(protocol)s: Using MDP file %(mdp)r from config file", vars()) diff --git a/mdpow/templates/runinput.yml b/mdpow/templates/runinput.yml index 9c723ddf..c3ba6ea0 100644 --- a/mdpow/templates/runinput.yml +++ b/mdpow/templates/runinput.yml @@ -48,7 +48,7 @@ MD_relaxed: runlocal: True # True: launch Gromacs mdrun and wait for it # False: produce queuing system scripts and stop - runtime: 5 + runtime: 5.0 # run time in ps mdp: 'NPT_opls.mdp' # run parameter file for Gromacs; @@ -59,7 +59,7 @@ MD_relaxed: MD_NPT: qscript: 'local.sh' # queuing system script to produce - runtime: 50000 + runtime: 50000.0 # simulation run time in ps [50 ns is generous, probably can be shorter] runlocal: False # True: launch Gromacs mdrun and wait for it @@ -75,7 +75,7 @@ FEP: # BAR (also MBAR) or TI qscript: 'local.sh' # queuing system script to produce - runtime: 5000 + runtime: 5000.0 # run time in ps for each free energy window runlocal: False # True: launch Gromacs mdrun and run all windows in a serial fashion diff --git a/mdpow/tests/test_Gsolv.py b/mdpow/tests/test_Gsolv.py index 034fe8f2..c39b352a 100644 --- a/mdpow/tests/test_Gsolv.py +++ b/mdpow/tests/test_Gsolv.py @@ -5,16 +5,15 @@ import numpy as np from gromacs.utilities import in_dir -from mdpow import fep, equil +from mdpow import fep, equil, config + +from . import RESOURCES class Test_Gsolv_manual(object): def setup(self): self.tmpdir = td.TempDir() - self.old_path = os.getcwd() - self.resources = os.path.join( - self.old_path, 'mdpow', 'tests', 'testing_resources') - self.m = pybol.Manifest(os.path.join(self.resources,'manifest.yml')) + self.m = pybol.Manifest(str(RESOURCES / 'manifest.yml')) self.m.assemble('md_npt',self.tmpdir.name) simulation_filename = os.path.join(self.tmpdir.name,'benzene', 'water.simulation') @@ -29,9 +28,10 @@ def teardown(self): self.tmpdir.dissolve() def _setup(self, **kwargs): + mdp = config.get_template("bar_opls.mdp") with in_dir(self.tmpdir.name, create=False): self.Gsolv = fep.Gsolv(simulation=self.S, molecule='BNZ', - mdp=os.path.join(self.old_path, 'mdpow', 'templates', 'bar_opls.mdp') ,**kwargs) + mdp=mdp ,**kwargs) self.Gsolv.setup(maxwarn=1) def test_default_setup(self): diff --git a/mdpow/tests/test_config.py b/mdpow/tests/test_config.py new file mode 100644 index 00000000..4a34dd25 --- /dev/null +++ b/mdpow/tests/test_config.py @@ -0,0 +1,140 @@ +import pytest +import os.path +from io import StringIO + +import numpy as np +from numpy.testing import assert_almost_equal, assert_equal + +import mdpow.config + +@pytest.fixture +def cfg(): + # default bundled config + return mdpow.config.get_configuration() + +@pytest.fixture +def minicfg(): + s = StringIO("setup:\n name: 'Alice'\n gromacsoutput: False\n") + yield s + s.close() + +class TestConfigurationParser: + def test_get_NoSectionError(self): + cfg = mdpow.config.POWConfigParser() + with pytest.raises(mdpow.config.NoSectionError, + match="Config file has no section Jabberwocky"): + cfg.get('Jabberwocky', 'elump') + + def test_get_NoOptionWarning_gives_None(self, cfg): + with pytest.warns(mdpow.config.NoOptionWarning, + match="Config file section FEP contains no " + "option Jabberwocky. Using 'None'."): + item = cfg.get("FEP", "Jabberwocky") + assert item is None + + def test_get(self, cfg): + item = cfg.get("setup","solventmodel") + assert isinstance(item, str) + assert item == "tip4p" + + def test_get_None(self, cfg): + item = cfg.get("setup", "prm") + assert item is None + + def test_getstr(self, cfg): + args = "setup","solventmodel" + item = cfg.getstr(*args) + assert isinstance(item, str) + assert item == "tip4p" + + assert item == cfg.get(*args) + + def test_getboolean(self, cfg): + args = "setup", "gromacsoutput" + item = cfg.getboolean(*args) + assert isinstance(item, bool) + assert item is True + + assert item == cfg.get(*args) + + def test_getint(self, cfg): + args = "setup", "maxwarn" + item = cfg.getboolean(*args) + assert isinstance(item, int) + assert item is 0 + + assert item == cfg.get(*args) + + def test_getfloat(self, cfg): + args = "MD_relaxed", "runtime" + item = cfg.getboolean(*args) + assert isinstance(item, float) + assert item == pytest.approx(5.0) + + assert item == cfg.get(*args) + + def test_getpath(self, cfg): + pth = "~/mirrors/jbwck.itp" + cfg.conf['setup']['itp'] = pth + + item = cfg.getpath("setup", "itp") + + assert item == os.path.expanduser(pth) + + def test_getpath_None(self, cfg): + # None gives None + assert cfg.getpath("setup", "prm") is None + + def test_findfile(self, cfg): + pth = cfg.findfile("FEP", "mdp") + + assert pth.endswith(".mdp") + assert os.path.exists(pth) + + def test_findfile_None(self, cfg): + # None gives None + assert cfg.findfile("setup", "itp") is None + + def test_getlist(self, cfg): + item = cfg.getlist("FEP_schedule_Coulomb", "lambdas") + + assert isinstance(item, list) + assert item == ['0', '0.25', '0.5', '0.75', '1.0'] + + def test_getlist_empty(self, cfg): + # get an option with None for this test + assert cfg.getlist("setup", "name") == [] + + def test_getarray(self, cfg): + item = cfg.getarray("FEP_schedule_Coulomb", "lambdas") + + assert isinstance(item, np.ndarray) + assert_almost_equal(item, [0, 0.25, 0.5, 0.75, 1.0]) + + def test_getarray_empty(self, cfg): + # get an option with None for this test + item = cfg.getarray("setup", "name") + assert isinstance(item, np.ndarray) + assert_equal(item, np.array([])) + + def test_write(self, cfg, tmp_path): + pth = tmp_path / "new.yaml" + cfg.write(pth) + + new_cfg = mdpow.config.POWConfigParser().readfp(pth.open()) + # compare dicts + assert new_cfg.conf == cfg.conf + + def test_readfp_stream(self, minicfg): + cfg = mdpow.config.POWConfigParser().readfp(minicfg) + assert cfg.get("setup", "name") == "Alice" + assert cfg.get("setup", "gromacsoutput") == False + + def test_merge(self, cfg, minicfg): + cfg.merge(minicfg) + # minicfg changes + assert cfg.get("setup", "name") == "Alice" + assert cfg.get("setup", "gromacsoutput") == False + # original + assert cfg.get("setup", "forcefield") == "OPLS-AA" + assert cfg.get("FEP", "method") == "BAR" diff --git a/mdpow/tests/test_equilibration_script.py b/mdpow/tests/test_equilibration_script.py index 23a7414f..79ddaf2a 100644 --- a/mdpow/tests/test_equilibration_script.py +++ b/mdpow/tests/test_equilibration_script.py @@ -8,14 +8,14 @@ from mdpow.run import equilibrium_simulation from mdpow.config import get_configuration +from . import RESOURCES class TestEquilibriumScript(object): def setup(self): self.tmpdir = td.TempDir() self.old_path = os.getcwd() - self.resources = os.path.join( - self.old_path, 'mdpow', 'tests', 'testing_resources') - m = pybol.Manifest(os.path.join(self.resources, 'manifest.yml')) + self.resources = RESOURCES + m = pybol.Manifest(str(self.resources / 'manifest.yml')) m.assemble('base', self.tmpdir.name) def teardown(self): diff --git a/mdpow/tests/test_fep.py b/mdpow/tests/test_fep.py index 332da868..6bda9432 100644 --- a/mdpow/tests/test_fep.py +++ b/mdpow/tests/test_fep.py @@ -52,20 +52,21 @@ class TestFEPschedule(object): 'sc_sigma': 0.3} } - def setup(self): + @pytest.fixture + def cfg(self): # load default bundled configuration - self.cfg = mdpow.config.get_configuration() + return mdpow.config.get_configuration() - def test_VDW(self): - return self._test_schedule('VDW') + def test_VDW(self, cfg): + return self._test_schedule(cfg, 'VDW') - def test_Coulomb(self): - return self._test_schedule('Coulomb') + def test_Coulomb(self, cfg): + return self._test_schedule(cfg, 'Coulomb') @pytest.mark.parametrize('component', ['VDW', 'Coulomb']) - def test_copy(self, component): + def test_copy(self, cfg, component): section = 'FEP_schedule_{0}'.format(component) - schedule = deepcopy(mdpow.fep.FEPschedule.load(self.cfg, section)) + schedule = deepcopy(mdpow.fep.FEPschedule.load(cfg, section)) reference = self.reference[component] for k in schedule: @@ -83,22 +84,24 @@ def test_copy(self, component): "mismatch between loaded FEP schedule entry {0} and reference".format(k) @pytest.mark.parametrize('component', ['VDW', 'Coulomb']) - def test_write(self, component): - self.cfg.write('cfg.yaml') - new_cfg = mdpow.config.get_configuration('cfg.yaml') - assert new_cfg.conf == self.cfg.conf - - def test_get(self): - tmp_cfg = mdpow.config.POWConfigParser() - item = tmp_cfg.get('VDW', 'label') - assert item is None - - def test_iterable(self): - assert not mdpow.config.iterable('test') - - def _test_schedule(self, component): + def test_write(self, cfg, component, tmp_path): + filename = tmp_path / "cfg.yaml" + cfg.write(filename) + new_cfg = mdpow.config.get_configuration(filename) + assert new_cfg.conf == cfg.conf + + @pytest.mark.parametrize("x,ref", [ + ("test", False), + ([1, 1, 2, 3], True), + ({1, 2, 3}, True), + (None, False), + ]) + def test_iterable(self, x, ref): + assert mdpow.config.iterable(x) == ref + + def _test_schedule(self, cfg, component): section = 'FEP_schedule_{0}'.format(component) - schedule = mdpow.fep.FEPschedule.load(self.cfg, section) + schedule = mdpow.fep.FEPschedule.load(cfg, section) reference = self.reference[component] for k in schedule: @@ -114,3 +117,15 @@ def _test_schedule(self, component): else: assert schedule[k] == reference[k], \ "mismatch between loaded FEP schedule entry {0} and reference".format(k) + + def test_skip_empty_entries(self, cfg, section="FEP_schedule_Coulomb"): + # remove some entries + del cfg.conf[section]['name'] # string + del cfg.conf[section]['lambdas'] # array + with pytest.warns(mdpow.config.NoOptionWarning): + schedule = mdpow.fep.FEPschedule.load(cfg, section) + + assert schedule['label'] == "Coul" + assert schedule['sc_power'] == 1 + assert 'name' not in schedule + assert 'lambdas' not in schedule diff --git a/mdpow/tests/test_fep_script.py b/mdpow/tests/test_fep_script.py index 3f01a3ee..2ebed629 100644 --- a/mdpow/tests/test_fep_script.py +++ b/mdpow/tests/test_fep_script.py @@ -10,14 +10,15 @@ from mdpow.run import fep_simulation from mdpow.config import get_configuration +from . import RESOURCES + class TestFEPScript(object): def setup(self): self.tmpdir = td.TempDir() self.old_path = os.getcwd() - self.resources = os.path.join( - self.old_path, 'mdpow', 'tests', 'testing_resources') - self.m = pybol.Manifest(os.path.join(self.resources,'manifest.yml')) - self.m.assemble('md_npt',self.tmpdir.name) + self.resources = RESOURCES + self.m = pybol.Manifest(str(self.resources / 'manifest.yml')) + self.m.assemble('md_npt', self.tmpdir.name) S = Simulation(filename=os.path.join( self.tmpdir.name, 'benzene', 'water.simulation')) diff --git a/mdpow/tests/test_run.py b/mdpow/tests/test_run.py new file mode 100644 index 00000000..0dd0b8b9 --- /dev/null +++ b/mdpow/tests/test_run.py @@ -0,0 +1,37 @@ +import pytest + +from numpy.testing import assert_equal + +import mdpow.run +import mdpow.config + +@pytest.fixture +def cfg(): + # default bundled config + return mdpow.config.get_configuration() + +@pytest.mark.parametrize("protocols", [["energy_minimize"], + ["MD_relaxed"], + ["MD_NPT", "FEP"], + ]) +def test_get_mdp_files(cfg, protocols): + mdpfiles = mdpow.run.get_mdp_files(cfg, protocols) + assert len(mdpfiles) == len(protocols) + assert set(mdpfiles.keys()) == set(protocols) + assert all([mdp.endswith(".mdp") for mdp in mdpfiles.values()]) + +@pytest.mark.parametrize("protocols", [["FEP"], + ["Jabberwocky", "Mad Hatter"] + ]) +def test_get_mdp_files_None(cfg, protocols): + # modify cfg + del cfg.conf['FEP']['mdp'] + with pytest.warns(mdpow.config.NoOptionWarning): + mdpfiles = mdpow.run.get_mdp_files(cfg, ["FEP"]) + assert mdpfiles == {} + +def test_get_mdp_files_ValueError(cfg): + # modify cfg with a non-existant file + cfg.conf['FEP']['mdp'] = "smoke_and_mirror.mdp" + with pytest.raises(ValueError): + mdpow.run.get_mdp_files(cfg, ["MD_NPT", "FEP"]) diff --git a/mdpow/tests/test_runinput.py b/mdpow/tests/test_runinput.py index ef7986d0..dc416279 100644 --- a/mdpow/tests/test_runinput.py +++ b/mdpow/tests/test_runinput.py @@ -1,8 +1,10 @@ -import os.path +import pytest import numpy as np from numpy.testing import assert_array_almost_equal +from . import CONFIGURATIONS + from mdpow import config class TestAlteredConfig(object): @@ -83,46 +85,44 @@ class TestAlteredConfig(object): } } - def setup(self): - # load default bundled configuration - self.cfg = config.get_configuration( - os.path.join('mdpow', 'tests', 'testing_resources', - 'test_configurations', 'altered_runinput.yml')) + @pytest.fixture + def cfg(self): + return config.get_configuration(str(CONFIGURATIONS / 'altered_runinput.yml')) - def _test_section(self,section): + def _test_section(self, cfg, section): section_dict = self.params_altered[section] for k in section_dict.keys(): if k == 'lambdas': - parsed = np.array([float(x.strip()) for x in self.cfg.get(section,k).split(",")]) + parsed = np.array([float(x.strip()) for x in cfg.get(section,k).split(",")]) assert_array_almost_equal(parsed, section_dict[k], err_msg="mismatch in lambdas") else: - assert self.cfg.get(section,k) == section_dict[k], \ + assert cfg.get(section,k) == section_dict[k], \ "mismatch in {}:{}".format(section,k) - def test_DEFAULT(self): - return self._test_section("DEFAULT") + def test_DEFAULT(self, cfg): + return self._test_section(cfg, "DEFAULT") - def test_setup(self): - return self._test_section("setup") + def test_setup(self, cfg): + return self._test_section(cfg, "setup") - def test_energy_minimize(self): - return self._test_section("energy_minimize") + def test_energy_minimize(self, cfg): + return self._test_section(cfg, "energy_minimize") - def test_MD_relaxed(self): - return self._test_section("MD_relaxed") + def test_MD_relaxed(self, cfg): + return self._test_section(cfg, "MD_relaxed") - def test_MD_NPT(self): - return self._test_section("MD_NPT") + def test_MD_NPT(self, cfg): + return self._test_section(cfg, "MD_NPT") - def test_FEP(self): - return self._test_section("FEP") + def test_FEP(self, cfg): + return self._test_section(cfg, "FEP") - def test_FEP_schedule_Coulomb(self): - return self._test_section("FEP_schedule_Coulomb") + def test_FEP_schedule_Coulomb(self, cfg): + return self._test_section(cfg, "FEP_schedule_Coulomb") - def test_FEP_schedule_VDW(self): - return self._test_section("FEP_schedule_VDW") + def test_FEP_schedule_VDW(self, cfg): + return self._test_section(cfg, "FEP_schedule_VDW") - def test_mdrun(self): - return self._test_section("mdrun") + def test_mdrun(self, cfg): + return self._test_section(cfg, "mdrun") diff --git a/mdpow/tests/test_solvation.py b/mdpow/tests/test_solvation.py index 9e605bd1..9ffadccb 100644 --- a/mdpow/tests/test_solvation.py +++ b/mdpow/tests/test_solvation.py @@ -8,6 +8,8 @@ from mdpow import equil +from . import RESOURCES + sims = {"water" : equil.WaterSimulation, "octanol" : equil.OctanolSimulation, "cyclohexane" : equil.CyclohexaneSimulation, @@ -22,13 +24,10 @@ @pytest.fixture def setup(tmpdir): newdir = tmpdir.mkdir('resources') - old_path = os.getcwd() - resources = os.path.join( - old_path, 'mdpow', 'tests', 'testing_resources') files = ['benzene.pdb', 'benzene.itp', 'benzene_charmm.itp', 'benzene_amber.itp'] for f in files: - orig = os.path.join(resources, 'molecules', 'benzene', f) + orig = RESOURCES / 'molecules' / 'benzene' / f shutil.copy(orig, newdir.dirname) return newdir.dirname