Merge pull request #274 from choderalab/fix-273

Don't raise a ParmEd version exception if parmed.version reports (0,0,0) due to dirty install

.github/workflows/CI.yaml

@@ -18,6 +18,7 @@ jobs:
           - {python-version: 3.6, openmm: "latest"}
           - {python-version: 3.7, openmm: "latest"}
           - {python-version: 3.7, openmm: "beta"}
+          - {python-version: 3.7, openmm: "rc"}
           - {python-version: 3.7, openmm: "nightly"}
           - {python-version: 3.7, openmm: "conda-forge"}
 
@@ -83,6 +84,9 @@ jobs:
           beta)
             echo "Using OpenMM beta"
             conda install --quiet -c omnia/label/beta openmm;;
+          beta)
+            echo "Using OpenMM rc"
+            conda install --quiet -c omnia/label/rc openmm;;
           nightly)
             echo "Using OpenMM nightly dev build."
             conda install --quiet -c omnia-dev openmm;;
@@ -99,14 +103,14 @@ jobs:
         export OE_LICENSE="$HOME/oe_license.txt"
         export TRAVIS=true
         pushd .
-        nosetests ${PACKAGENAME}/tests/test_forcefield_generators.py --nocapture --verbosity=3 --with-timer -a '!advanced'
-        nosetests ${PACKAGENAME}/tests/test_amber.py --nocapture --verbosity=3 --with-timer -a '!advanced'
-        nosetests ${PACKAGENAME}/tests/test_utils.py --nocapture --verbosity=3 --with-timer -a '!advanced'
-        nosetests ${PACKAGENAME}/tests/test_gromacs.py --nocapture --verbosity=3 --with-timer -a '!advanced'
-        nosetests ${PACKAGENAME}/tests/test_openeye.py --nocapture --verbosity=3 --with-timer -a '!advanced'
-        nosetests ${PACKAGENAME}/tests/test_freesolv.py --nocapture --verbosity=3 --with-timer -a '!advanced'
-        nosetests ${PACKAGENAME}/tests/test_drugs.py --nocapture --verbosity=3 --with-timer -a '!advanced'
-        nosetests ${PACKAGENAME}/tests/test_common_molecules.py --nocapture --verbosity=3 --with-timer -a '!advanced'
+        nosetests ${PACKAGENAME}/tests/test_forcefield_generators.py --nocapture --verbosity=3 --with-timer -a '!slow'
+        nosetests ${PACKAGENAME}/tests/test_amber.py --nocapture --verbosity=3 --with-timer -a '!slow'
+        nosetests ${PACKAGENAME}/tests/test_utils.py --nocapture --verbosity=3 --with-timer -a '!slow'
+        nosetests ${PACKAGENAME}/tests/test_gromacs.py --nocapture --verbosity=3 --with-timer -a '!slow'
+        nosetests ${PACKAGENAME}/tests/test_openeye.py --nocapture --verbosity=3 --with-timer -a '!slow'
+        nosetests ${PACKAGENAME}/tests/test_freesolv.py --nocapture --verbosity=3 --with-timer -a '!slow'
+        nosetests ${PACKAGENAME}/tests/test_drugs.py --nocapture --verbosity=3 --with-timer -a '!slow'
+        nosetests ${PACKAGENAME}/tests/test_common_molecules.py --nocapture --verbosity=3 --with-timer -a '!slow'
         popd
 
     - name: Deploy

devtools/travis-ci/install.sh

@@ -14,8 +14,8 @@ else
     export PYTHON_VER=$TRAVIS_PYTHON_VERSION
 fi
 MINICONDA_HOME=$HOME/miniconda
-MINICONDA_MD5=$(curl -s https://repo.continuum.io/miniconda/ | grep -A3 $MINICONDA | sed -n '4p' | sed -n 's/ *<td>\(.*\)<\/td> */\1/p')
-wget -q https://repo.continuum.io/miniconda/$MINICONDA
+MINICONDA_MD5=$(wget -qO- https://repo.anaconda.com/miniconda/ | grep -A3 $MINICONDA | sed -n '4p' | sed -n 's/ *td\(.*\)\/td */\1/p')
+wget -q https://repo.anaconda.com/miniconda/$MINICONDA
 if [[ $MINICONDA_MD5 != $(md5sum $MINICONDA | cut -d ' ' -f 1) ]]; then
     echo "Miniconda MD5 mismatch"
     exit 1

openmoltools/amber.py

@@ -232,9 +232,11 @@ def check_for_errors( outputtext, other_errors = None, ignore_errors = None ):
     If error(s) are found, raise a RuntimeError and attept to print the appropriate errors from the processed text."""
     lines = outputtext.split('\n')
     error_lines = []
+    import re
     for line in lines:
         if 'ERROR' in line.upper():
-            error_lines.append( line )
+            if not bool(re.match('Exiting LEaP: Errors = \d+; Warnings = \d+; Notes = \d+.', line)):
+                error_lines.append( line )
         if not other_errors == None:
             for err in other_errors:
                 if err.upper() in line.upper():

openmoltools/gromacs.py

@@ -5,13 +5,11 @@
 from distutils.spawn import find_executable
 import parmed
 # If ParmEd is older than 2.0.4 then halt - newer ParmEd is required to have correct FudgeLJ/FudgeQQ retained in GROMACS topologies.
-try:
-    ver = parmed.version
-except:
-    oldParmEd = Exception('ERROR: ParmEd is too old, please upgrade to 2.0.4 or later')
-    raise oldParmEd
-if ver < (2,0,4):
-    raise RuntimeError("ParmEd is too old, please upgrade to 2.0.4 or later")
+
+# Ensure ParmEd is sufficiently recent
+# TODO: Instead, check if the ParmEd API is sufficient
+from .utils import check_parmed_version
+check_parmed_version()
 
 from openmoltools.utils import getoutput
 
@@ -59,7 +57,7 @@ def extract_section(lines, section):
     Returns
     -------
     status : bool
-        Whether or not section is found. True if found, False if not. 
+        Whether or not section is found. True if found, False if not.
     indices :  list (int)
         Line indices within lines belonging to section excluding the header and counting from zero
 
@@ -115,7 +113,7 @@ def extract_section(lines, section):
 
 def change_molecules_section( input_topology, output_topology, molecule_names, molecule_numbers):
     """Create a GROMACS topology file where the  molecule numbers are replaced by new molecule numbers in the gromacs [ molecules ] section.
-        
+
     Parameters
     ----------
     input_topology : str
@@ -126,21 +124,21 @@ def change_molecules_section( input_topology, output_topology, molecule_names, m
         Molecule names to be searched in the gromacs [ molecules ] section
     molecule_numbers : list (int)
         The new molecule numbers to be used in [ molecules ] section
-        
+
     Returns
     -------
     nothing is returned
-        
+
     Notes
     -----
-    This function reads in a gromacs topology file and changes the number of atoms related to the passed molecule name list. 
+    This function reads in a gromacs topology file and changes the number of atoms related to the passed molecule name list.
     If in the topology file one molecule name is not present in the passed molecule name list an exception is raised.
     Currently assumes the components are single-residue, single-molecule (i.e. the molecule names and residue names are equivalent).
     """
-    
+
     #The molecule name list and the molecule number list must have the same size otherwise an exception is raised
     assert len(molecule_names) == len(molecule_numbers), "The molecule name list and the molecule name number must have the same size"
-    
+
     #Check for non negative integer number of molecules
     check_nni = all(item >=0  and isinstance(item, int) for item in molecule_numbers)
 
@@ -159,9 +157,9 @@ def change_molecules_section( input_topology, output_topology, molecule_names, m
     current_names = []
     for c in components:
         molecules.append( c[0] )
-        numbers.append( c[1] )    
+        numbers.append( c[1] )
         current_names.append( c[0].residues[0].name )
-    
+
     #Check length
     assert len(molecules) == len(molecule_numbers), "The number of molecules in the topology file is not equal to the number of molecule numbers/molecules provided."
 
@@ -175,16 +173,16 @@ def change_molecules_section( input_topology, output_topology, molecule_names, m
     newtop = molecules[0] * molecule_numbers[0]
     for idx in range( 1, len(molecule_names) ):
         newtop += molecules[idx] * molecule_numbers[idx]
-    
+
 
     #Write topology file
-    newtop.write( output_topology ) 
+    newtop.write( output_topology )
 
 
 def do_solvate( top_filename, gro_filename, top_solv_filename, gro_solv_filename, box_dim, box_type, water_model, water_top, FF = 'amber99sb-ildn.ff' ):
 
     """ This function creates water solvated molecule coordinate files and its corresponding topology
-        
+
         PARAMETERS:
             top_filename: str
                           Topology path/filename
@@ -203,7 +201,7 @@ def do_solvate( top_filename, gro_filename, top_solv_filename, gro_solv_filename
             water_top: str
                           Water include file to ensure is present in topology file, i.e. "tip3p.itp"
             FF : str, optional, default = 'amber99sb-ildn.ff'
-                          String specifying base force field directory for include files (i.e. 'amber99sb-ildn.ff').  
+                          String specifying base force field directory for include files (i.e. 'amber99sb-ildn.ff').
 
         NOTES:
         -----
@@ -275,23 +273,23 @@ def do_solvate( top_filename, gro_filename, top_solv_filename, gro_solv_filename
 
 def ensure_forcefield( intop, outtop, FF = 'ffamber99sb-ildn.ff'):
     """Open a topology file, and check to ensure that includes the desired forcefield itp file. If not, remove any [ defaults ] section (which will be provided by the FF) and include the forcefield itp. Useful when working with files set up by acpypi -- these need to have a water model included in order to work, and most water models require a force field included in order for them to work.
-            
+
             ARGUMENTS:
             - intop: Input topology
             - outtop: Output topology
         OPTIONAL:
         - FF: String corresponding to desired force field; default ffamber99sb.-ildn.ff
-        
+
         Limitations:
         - If you use this on a topology file that already includes a DIFFERENT forcefield, the result will be a topology file including two forcefields.
     """
-    
+
     file = open(intop, 'r')
     text= file.readlines()
     file.close()
-    
+
     FFstring = FF+'/forcefield.itp'
-    
+
     #Check if force field is included somewhere
     found = False
     for line in text:
@@ -303,7 +301,7 @@ def ensure_forcefield( intop, outtop, FF = 'ffamber99sb-ildn.ff'):
         while text[idx].find(';')==0:
             idx+=1
         text[idx] = '\n#include "%s"\n\n' % FFstring + text[idx]
-    
+
     #Remove any defaults section
     found = False
     foundidx = None
@@ -319,18 +317,18 @@ def ensure_forcefield( intop, outtop, FF = 'ffamber99sb-ildn.ff'):
             if endidx == None:
                 endidx = idx
     #Now remove defaults section
-    
+
     if found:
         text = text[0:foundidx] + text[endidx:]
-    
-    
+
+
     #Write results
     file = open( outtop, 'w')
     file.writelines(text)
     file.close()
 
 
-    
+
 def check_for_errors( outputtext, other_errors = None, ignore_errors = None ):
     """Check GROMACS package output for the string 'ERROR' (upper or lowercase) and (optionally) specified other strings and raise an exception if it is found (to avoid silent failures which might be noted to log but otherwise ignored).
 
@@ -365,7 +363,7 @@ def check_for_errors( outputtext, other_errors = None, ignore_errors = None ):
                     ignore = True
             if not ignore:
                 new_error_lines.append( err )
-        error_lines = new_error_lines 
+        error_lines = new_error_lines
 
     if len(error_lines) > 0:
         print("Unexpected errors encountered running GROMACS tool. Offending output:")
@@ -423,8 +421,8 @@ def merge_topologies( input_topologies, output_topology, system_name, molecule_n
     #Check that we've been provided with the correct number of molecule_names if any
     if molecule_names != None:
         total_molecules = 0
-        for topnr in range(N_tops): 
-            total_molecules += len( tops[topnr].residues ) 
+        for topnr in range(N_tops):
+            total_molecules += len( tops[topnr].residues )
         assert total_molecules == len( molecule_names ), "Must provide a number of molecule names equal to your total number of residues, but you have %s and %s, respectively." % ( len( molecule_names), total_molecules )
 
         #Rename residues
@@ -433,18 +431,16 @@ def merge_topologies( input_topologies, output_topology, system_name, molecule_n
             for resnr in range(len(tops[topnr].residues)):
                 tops[topnr].residues[resnr].name = molecule_names[ ctr ]
                 ctr += 1
- 
+
     #Construct final topology
-    final = tops[0] * molecule_numbers[ 0 ] 
+    final = tops[0] * molecule_numbers[ 0 ]
     for topnr in range( 1, N_tops ):
-        final += tops[ topnr ] * molecule_numbers[ topnr ] 
+        final += tops[ topnr ] * molecule_numbers[ topnr ]
 
     #Set system name
     final.title = system_name
 
     #Write topology
-    parmed.gromacs.GromacsTopologyFile.write( final, output_topology ) 
+    parmed.gromacs.GromacsTopologyFile.write( final, output_topology )
 
     return True
-
-

openmoltools/openeye.py

@@ -70,17 +70,21 @@ def get_charges(molecule, max_confs=800, strictStereo=True,
         # try charge using AM1BCCELF10
         status = oequacpac.OEAssignCharges(charged_copy, oequacpac.OEAM1BCCELF10Charges())
         # or fall back to OEAM1BCC
-        if not status: 
+        if not status:
             # 2017.2.1 OEToolkits new charging function
             status = oequacpac.OEAssignCharges(charged_copy, oequacpac.OEAM1BCCCharges())
-            if not status: raise(RuntimeError("OEAssignCharges failed."))
+            if not status:
+                # Fall back
+                status = oequacpac.OEAssignCharges(charged_copy, oequacpac.OEAM1Charges())
+
+                # Give up
+                if not status:
+                    raise(RuntimeError("OEAssignCharges failed."))
     else:
         # AM1BCCSym recommended by Chris Bayly to KAB+JDC, Oct. 20 2014.
         status = oequacpac.OEAssignPartialCharges(charged_copy, oequacpac.OECharges_AM1BCCSym)
         if not status: raise(RuntimeError("OEAssignPartialCharges returned error code %d" % status))
 
-
-
     #Determine conformations to return
     if keep_confs == None:
         #If returning original conformation

openmoltools/tests/test_openeye.py

@@ -13,7 +13,8 @@
 import mdtraj as md
 from numpy.testing import assert_raises
 
-smiles_fails_with_strictStereo = "CN1CCN(CC1)CCCOc2cc3c(cc2OC)C(=[NH+]c4cc(c(cc4Cl)Cl)OC)C(=C=[N-])C=[NH+]3"
+smiles_fails_with_strictStereo = "CN1CCN(CC1)CCCOc2cc3c(cc2OC)C(=[NH+]c4cc(c(cc4Cl)Cl)OC)C(=C=[N-])C=[NH+]3" # this is insane; C=C=[N-]?
+smiles_fails_with_strictStereo = "CN1CCN(CC1)CCCOc2cc3c(cc2OC)C(=[NH+]c4cc(c(cc4Cl)Cl)OC)C(C#N)C=[NH+]3" # more sane version
 
 try:
     oechem = utils.import_("openeye.oechem")
@@ -277,7 +278,7 @@ def test_oeassigncharges_fail():
     with assert_raises(RuntimeError):
         # Fail test for OEToolkits (2017.2.1) new charging function
         m = openmoltools.openeye.smiles_to_oemol(smiles_fails_with_strictStereo)
-        m = openmoltools.openeye.get_charges(m,  strictStereo=False, legacy=False)
+        m = openmoltools.openeye.get_charges(m,  strictStereo=True, legacy=False)
 
 @skipIf(not HAVE_OE, "Cannot test openeye module without OpenEye tools.")
 def test_oeassigncharges_success():

openmoltools/utils.py

@@ -32,7 +32,7 @@ def getoutput(cmd):
     """Compatibility function to substitute deprecated commands.getoutput in Python2.7"""
     try:
         out = subprocess.getoutput(cmd)
-    except AttributeError:
+    except (AttributeError, UnicodeDecodeError):
         out = subprocess.Popen(cmd, shell=True, stderr=subprocess.STDOUT,
                                stdout=subprocess.PIPE).communicate()[0]
     try:
@@ -391,20 +391,34 @@ def get_unique_names(n_molecules):
             return names
 
 
+def check_parmed_version():
+    """Check that parmed version is (2,0,4) or more recent.
+
+    Raises
+    ------
+    RuntimeError if ParmEd is older than 2.0.4 (unless version showes (0,0,0)
+    """
+    import parmed
+    # If ParmEd is older than 2.0.4 then halt - newer ParmEd is required to have correct FudgeLJ/FudgeQQ retained in GROMACS topologies.
+    try:
+        ver = parmed.version
+    except:
+        oldParmEd = Exception('ERROR: Installed ParmEd is too old, please upgrade to 2.0.4 or later')
+        raise oldParmEd
+    if ver < (2,0,4):
+        # Check to make sure this isn't a versioneer dirty git issue
+        if not (ver[0], ver[1], ver[2]) == (0,0,0):
+            raise RuntimeError("Installed ParmEd (%s) is too old, please upgrade to 2.0.4 or later" % str(ver))
+
 def randomize_mol2_residue_names(mol2_filenames):
     """Find unique residue names for a list of MOL2 files.  Then
     re-write the MOL2 files using ParmEd with the unique identifiers.
     """
     import parmed
 
-    # We require at least ParmEd 2.5.1 because of issues with the .mol2 writer (issue #691 on ParmEd) prior to that.
-    try: #Try to get version tag
-        ver = parmed.version
-    except: #If too old for version tag, it is too old
-        oldParmEd = Exception('ERROR: ParmEd is too old, please upgrade to 2.0.4 or later')
-        raise oldParmEd
-    if ver < (2,5,1):
-        raise RuntimeError("ParmEd is too old, please upgrade to 2.0.4 or later")
+    # Ensure ParmEd is sufficiently recent
+    # TODO: Instead, check if the ParmEd API is sufficient
+    check_parmed_version()
 
     names = get_unique_names(len(mol2_filenames))
 
@@ -524,14 +538,13 @@ def amber_to_gromacs( molecule_name, in_prmtop, in_crd, out_top = None, out_gro
 
     #Import ParmEd
     import parmed
-    #Require version 2.0.4 or later of ParmEd, otherwise ParmEd corrupts [ defaults ] section in GROMACS topologies with incorrect FudgeLJ/FudgeQQ
-    try:
+    # We require at least ParmEd 2.5.1 because of issues with the .mol2 writer (issue #691 on ParmEd) prior to that.
+    try: #Try to get version tag
         ver = parmed.version
-    except:
-        oldParmEd = Exception('ERROR: ParmEd is too old, please upgrade to 2.0.4 or later')
-        raise oldParmEd
-    if ver < (2,0,4):
-        raise RuntimeError("ParmEd is too old, please upgrade to 2.0.4 or later")
+    except: #If too old for version tag, it is too old
+        # Check to make sure this isn't a versioneer dirty git issue
+        if not (ver[0], ver[1], ver[2]) == (0,0,0):
+            raise RuntimeError("Installed ParmEd (%s) is too old, please upgrade to 2.0.4 or later" % str(ver))
 
 
     #Read AMBER to ParmEd object