Merge pull request #109 from openmm/add-kwargs-exception

Throw exception if nonbondedMethod provided to forcefield_kwargs in SystemGenerator; use OpenMM 7.4.2 rc

.travis.yml

@@ -47,10 +47,6 @@ install:
     # DEBUG
   - echo "OE_LICENSE path:"
   - echo $OE_LICENSE
-    # Workaround for https://github.com/openmm/openmm/pull/2511
-    # TODO: Remove this when OpenMM builds including https://github.com/openmm/openmm/pull/2511 are released
-  - echo "Overwriting OpenMM forcefield.py with fix from https://github.com/openmm/openmm/pull/2511"
-  - wget -q https://raw.githubusercontent.com/openmm/openmm/4f48402f1be3e0f049ae0e8595db638d297b0d75/wrappers/python/simtk/openmm/app/forcefield.py -O `python -c "from simtk.openmm.app import forcefield; print(forcefield.__file__)"`
     # Build and install package
     # Can't use 'develop' mode because we remove symlinks after install is complete
     #- python setup.py develop --no-deps
@@ -60,8 +56,10 @@ install:
 script:
     # Test installed package
   #- pytest -v --cov=openmmforcefields openmmforcefields/tests
-  - pytest -v -x -s --log-cli-level INFO --cov=openmmforcefields openmmforcefields/tests/test_amber_import.py
-  - pytest -v -x -s --log-cli-level INFO --cov=openmmforcefields openmmforcefields/tests/test_template_generators.py
+  - export LOGLEVEL="INFO"
+  - pytest -v -x -s --log-cli-level $LOGLEVEL --cov=openmmforcefields openmmforcefields/tests/test_amber_import.py
+  - pytest -v -x -s --log-cli-level $LOGLEVEL --cov=openmmforcefields openmmforcefields/tests/test_template_generators.py
+  - pytest -v -x -s --log-cli-level $LOGLEVEL --cov=openmmforcefields openmmforcefields/tests/test_system_generator.py
 
     # Store current working directory
   - export WORK=`pwd`

README.md

@@ -229,6 +229,7 @@ system_generator = SystemGenerator(forcefields=['amber/ff14SB.xml', 'amber/tip3p
 # Create an OpenMM System from an Open Force Field toolkit Topology object
 system = system_generator.create_system(openforcefield_topology)
 # Alternatively, create an OpenMM System from an OpenMM Topology object and a list of openforcefield Molecule objects
+molecules = Molecule.from_file('molecules.sdf', file_format='sdf')
 system = system_generator.create_system(openmm_topology, molecules=molecules)
 ```
 Parameterized molecules are cached in `db.json`.
@@ -268,6 +269,12 @@ See the corresponding directories for information on how to use the provided con
 
 # Changelog
 
+##
+
+## 0.7.2 Bugfix release: More error checking; OpenMM 7.4.2 minimum version requirement
+
+* Raise a `ValueError` if `SystemGenerator` receives a `nonbondedMethod` key in `forcefield_kwargs`; these should go into `periodic_forcefield_kwargs` or `nonperiodic_forcefield_kwargs`.
+
 ## 0.7.1 Bugfix release: Fix GAFF AM1-BCC charging bug for some molecules
 
 * When using the OpenEye toolkit, some molecules failed to charge with GAFF. See https://github.com/openforcefield/openforcefield/issues/492

devtools/conda-envs/test_env.yaml

@@ -3,6 +3,7 @@ name: test
 channels:
   - conda-forge
   - omnia
+  - omnia/label/rc
   - openeye
   - defaults
 
@@ -17,7 +18,7 @@ dependencies:
   - codecov
 
     # Requirements for converted force field installer
-  - openmm >=7.4.1
+  - openmm >=7.4.2
 
     # Requirements for conversion tools
   - pyyaml

devtools/scripts/create_conda_env.py

@@ -28,7 +28,7 @@
         except (KeyError, ImportError, IndexError):
             raise ImportError("No YAML parser could be found in this or the conda environment. "
                               "Could not find PyYAML or Ruamel YAML in the current environment, "
-                              "AND could not find Ruamel YAML in the base conda environment through CONDA_EXE path. " 
+                              "AND could not find Ruamel YAML in the base conda environment through CONDA_EXE path. "
                               "Environment not created!")
     loader = yaml.YAML(typ="safe").load  # typ="safe" avoids odd typing on output
 

devtools/travis-ci/before_install.sh

@@ -3,7 +3,6 @@ pushd .
 cd $HOME
 # Make sure some level of pip is installed
 python -m ensurepip
-
 # Install Miniconda
 if [ "$TRAVIS_OS_NAME" == "osx" ]; then
     # Make OSX md5 mimic md5sum from linux, alias does not work
@@ -15,8 +14,8 @@ else
     MINICONDA=Miniconda3-latest-Linux-x86_64.sh
 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
@@ -30,9 +29,9 @@ echo ". $MINICONDA_HOME/etc/profile.d/conda.sh" >> ~/.bashrc  # Source the profi
 echo "conda activate" >> ~/.bashrc  # Activate conda
 source ~/.bashrc  # source file to get new commands
 #export PATH=$MINICONDA_HOME/bin:$PATH  # Old way, should not be needed anymore
-    
+
 conda config --add channels conda-forge
-    
+
 conda config --set always_yes yes
 conda install conda conda-build jinja2 anaconda-client
 conda update --quiet --all

openmmforcefields/generators/system_generators.py

@@ -48,6 +48,12 @@ class SystemGenerator(object):
     forcefield_kwargs : dict
         Keyword arguments fed to ``simtk.openmm.app.ForceField.createSystem()`` during System generation.
         These keyword arguments can be modified at any time.
+    periodic_forcefield_kwargs : dict
+        Keyword arguments fed to ``simtk.openmm.app.ForceField.createSystem()`` during System generation for periodic systems.
+        These keyword arguments can be modified at any time.
+    nonperiodic_forcefield_kwargs : dict
+        Keyword arguments fed to ``simtk.openmm.app.ForceField.createSystem()`` during System generation for non-periodic systems.
+        These keyword arguments can be modified at any time.
     template_generator : openmmforcefields.generators.SmallMoleculeTemplateGenerator
         The small molecule residue template generator subclass used for small molecules.
     barostat : simtk.openmm.MonteCarloBarostat
@@ -170,6 +176,12 @@ def __init__(self, forcefields=None, small_molecule_forcefield='openff-1.0.0', f
         self.nonperiodic_forcefield_kwargs = nonperiodic_forcefield_kwargs if nonperiodic_forcefield_kwargs is not None else {'nonbondedMethod' : app.NoCutoff}
         self.periodic_forcefield_kwargs = periodic_forcefield_kwargs if periodic_forcefield_kwargs is not None else {'nonbondedMethod' : app.PME}
 
+        # Raise an exception if nonbondedForce is specified in forcefield_kwargs
+        if 'nonbondedMethod' in self.forcefield_kwargs:
+            raise ValueError("""nonbondedMethod cannot be specified in forcefield_kwargs;
+                 must be specified in either periodic_forcefield_kwargs (if it should be applied to periodic systems)
+                 or nonperiodic_forcefield_kwargs (if it should be applied to non-periodic systems)""")
+
         # Create and cache a residue template generator
         from openmmforcefields.generators.template_generators import SmallMoleculeTemplateGenerator
         self.template_generator = None

openmmforcefields/generators/template_generators.py

@@ -278,14 +278,14 @@ def generator(self, forcefield, residue):
                     # See if the template matches
                     from openforcefield.topology import Molecule
                     molecule_template = Molecule.from_smiles(entry['smiles'], allow_undefined_stereo=True)
-                    print(f"Checking against {entry['smiles']}")
+                    _logger.debug(f"Checking against {entry['smiles']}")
                     if self._match_residue(residue, molecule_template):
                         ffxml_contents = entry['ffxml']
 
                         # Write to debug file if requested
                         if self.debug_ffxml_filename is not None:
                             with open(self.debug_ffxml_filename, 'w') as outfile:
-                                _logger.info(f'writing ffxml to {self.debug_ffxml_filename}')
+                                _logger.debug(f'writing ffxml to {self.debug_ffxml_filename}')
                                 outfile.write(ffxml_contents)
 
                         # Add parameters and residue template for this residue
@@ -303,7 +303,7 @@ def generator(self, forcefield, residue):
                 # Write to debug file if requested
                 if self.debug_ffxml_filename is not None:
                     with open(self.debug_ffxml_filename, 'w') as outfile:
-                        _logger.info(f'writing ffxml to {self.debug_ffxml_filename}')
+                        _logger.debug(f'writing ffxml to {self.debug_ffxml_filename}')
                         outfile.write(ffxml_contents)
 
                 # Add the parameters and residue definition
@@ -312,7 +312,7 @@ def generator(self, forcefield, residue):
                 if self._cache is not None:
                     with self._open_db() as db:
                         table = db.table(self._database_table_name)
-                        _logger.info(f'Writing residue template for {smiles} to cache {self._cache}')
+                        _logger.debug(f'Writing residue template for {smiles} to cache {self._cache}')
                         record = {'smiles' : smiles, 'ffxml' : ffxml_contents}
                         # Add the IUPAC name for convenience if we can
                         try:
@@ -560,7 +560,7 @@ def generate_residue_template(self, molecule, residue_atoms=None):
         """
         # Use the canonical isomeric SMILES to uniquely name the template
         smiles = molecule.to_smiles()
-        _logger.info(f'Generating a residue template for {smiles}')
+        _logger.info(f'Generating a residue template for {smiles} using {self._forcefield}')
 
         # Generate unique atom names
         self._generate_unique_atom_names(molecule)
@@ -846,9 +846,9 @@ def _check_for_errors(self, outputtext, other_errors=None, ignore_errors=None):
             error_lines = new_error_lines
 
         if len(error_lines) > 0:
-            _logger.info("Unexpected errors encountered running AMBER tool. Offending output:")
+            _logger.warning("Unexpected errors encountered running AMBER tool. Offending output:")
             for line in error_lines:
-                _logger.info(line)
+                _logger.warning(line)
             raise(RuntimeError("Error encountered running AMBER tool. Exiting."))
 
         return
@@ -994,7 +994,7 @@ def __init__(self, molecules=None, cache=None, forcefield=None):
                 filename += '.offxml'
             self._smirnoff_forcefield = openforcefield.typing.engines.smirnoff.ForceField(filename)
         except Exception as e:
-            print(e)
+            _logger.error(e)
             raise ValueError(f"Can't find specified SMIRNOFF force field ({forcefield}) in install paths")
 
         # Delete constraints, if present
@@ -1096,7 +1096,7 @@ def generate_residue_template(self, molecule, residue_atoms=None):
         """
         # Use the canonical isomeric SMILES to uniquely name the template
         smiles = molecule.to_smiles()
-        _logger.info(f'Generating a residue template for {smiles}')
+        _logger.info(f'Generating a residue template for {smiles} using {self._forcefield}')
 
         # Generate unique atom names
         self._generate_unique_atom_names(molecule)