Remove shipping gaff/ffxml and read parms from parmchk2 (#344)

* re-enable gaff * run espaloma and gaff tests * gaff has two fs michael * Use 8.1.2rc1 * Update linters * Remove Espaloma tests * Apply suggestions from code review Co-authored-by: Iván Pulido <[email protected]> * working on parmchk output * fix pytest warnings * Migrate from pkg_resources to importlib_resources * Remove shipping gaff/ffxml and read everything from parmchk2 * make sure we use older openmm version for testing * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * test on ci now * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * decided to keep shipping the xmls * cleaned things up, gave a better name to the set we track gaff atom types * Update openmmforcefields/generators/template_generators.py * updated change log * Added note about how we might remove openmmforcefields/ffxml/amber/gaff/ffxml in a future release --------- Co-authored-by: Matthew W. Thompson <[email protected]> Co-authored-by: Iván Pulido <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
openmm · Jul 24, 2024 · 051cbcb · 051cbcb
1 parent 38eb754
commit 051cbcb
Show file tree

Hide file tree

Showing 7 changed files with 58 additions and 30 deletions.
diff --git a/README.md b/README.md
@@ -410,6 +410,14 @@ See the corresponding directories for information on how to use the provided con
 
 # [Changelog](https://github.com/openmm/openmmforcefields/releases)
 
+## 0.14.1 Bring back GAFFTemplateGenerator for OpenMM >=7.6.0
+
+This release brings back GAFF force feild support for all versions of OpenMM previously supported.
+Additionally, we now use the output of parmchk2 for all GAFF parameters.
+Previously we used gaff.dat + parmchk2 output to generate forcefield parameters.
+Functionally this doesn't change the end user experience but means we do not need to create new forcefield XML files for newer GAFF versions and now support whatever GAFF versions that parmchk2 supports for the installed AmberTools version.
+The XML files in `openmmforcefields/ffxml/amber/gaff/ffxml` may be removed in a future release.
+
 ## 0.14.0 Bring back GAFFTemplateGenerator
 
 This release effectively reverts the changes in 0.13.0. This release is **only** compatible with OpenMM 8.1.2. No other changes were made.

diff --git a/amber/convert_amber.py b/amber/convert_amber.py
@@ -21,7 +21,7 @@
 import yaml
 from lxml import etree as et
 from parmed.exceptions import ParameterWarning
-from pkg_resources import resource_filename
+import importlib_resources
 
 warnings.filterwarnings("error", category=ParameterWarning)
 
@@ -500,7 +500,7 @@ def convert_yaml(yaml_name, ffxml_dir, ignore=ignore):
                 _filename = os.path.join(AMBERHOME, "dat/leap/", source_file)
             elif MODE == "GAFF":
                 _filename = os.path.join(
-                    resource_filename("openmmforcefields", "ffxml/amber/gaff/dat"),
+                    str(importlib_resources.files("openmmforcefields") / "ffxml" / "amber" / " gaff" / "dat"),
                     source_file,
                 )
             files.append(_filename)

diff --git a/devtools/conda-envs/test_env.yaml b/devtools/conda-envs/test_env.yaml
@@ -8,11 +8,11 @@ dependencies:
   - pytest-cov
   - pytest-xdist
   - pytest-randomly
-  - openmm >=8.1.2,<9
+  - openmm <8.1.1
   - openff-toolkit >=0.11
   - pyyaml
   - ambertools >=22,<24
   - lxml
   - networkx
   - tinydb
-  - validators
+  - validators
diff --git a/openmmforcefields/generators/template_generators.py b/openmmforcefields/generators/template_generators.py
@@ -537,6 +537,8 @@ def __init__(self, molecules=None, forcefield=None, cache=None, **kwargs):
 
         # Track which OpenMM ForceField objects have loaded the relevant GAFF parameters
         self._gaff_parameters_loaded = dict()
+        # Track which atom types we have told OpenMM about
+        self._gaff_atom_types_observed = set()
 
     @property
     def gaff_version(self):
@@ -556,22 +558,30 @@ def gaff_minor_version(self):
     @property
     def gaff_dat_filename(self):
         """File path to the GAFF .dat AMBER force field file"""
-        from pkg_resources import resource_filename
-
-        filename = resource_filename(
-            "openmmforcefields",
-            os.path.join("ffxml", "amber", "gaff", "dat", f"{self._forcefield}.dat"),
+        import importlib_resources
+
+        filename = str(
+            importlib_resources.files("openmmforcefields")
+            / "ffxml"
+            / "amber"
+            / "gaff"
+            / "dat"
+            / f"{self._forcefield}.dat"
         )
         return filename
 
     @property
     def gaff_xml_filename(self):
         """File path to the GAFF .ffxml OpenMM force field file"""
-        from pkg_resources import resource_filename
-
-        filename = resource_filename(
-            "openmmforcefields",
-            os.path.join("ffxml", "amber", "gaff", "ffxml", f"{self._forcefield}.xml"),
+        import importlib_resources
+
+        filename = str(
+            importlib_resources.files("openmmforcefields")
+            / "ffxml"
+            / "amber"
+            / "gaff"
+            / "ffxml"
+            / f"{self._forcefield}.xml"
         )
         return filename
 
@@ -593,12 +603,6 @@ def generator(self, forcefield, residue):
             If the generator cannot parameterize the residue, it should return `False` and not modify `forcefield`.
 
         """
-        # Load the GAFF parameters if we haven't done so already for this force field
-        if forcefield not in self._gaff_parameters_loaded:
-            # Instruct the ForceField to load the GAFF parameters
-            forcefield.loadFile(self.gaff_xml_filename)
-            # Note that we've loaded the GAFF parameters
-            self._gaff_parameters_loaded[forcefield] = True
 
         return super().generator(forcefield, residue)
 
@@ -711,7 +715,6 @@ def generate_residue_template(self, molecule, residue_atoms=None):
         _logger.debug(f"{molecule.partial_charges}")
 
         # Generate additional parameters if needed
-        # TODO: Do we have to make sure that we don't duplicate existing parameters already loaded in the forcefield?
         _logger.debug("Creating ffxml contents for additional parameters...")
         from inspect import (  # use introspection to support multiple parmed versions
             signature,
@@ -730,6 +733,22 @@ def generate_residue_template(self, molecule, residue_atoms=None):
         kwargs = {}
         if "write_unused" in signature(params.write).parameters:
             kwargs["write_unused"] = True
+
+        # We need to make sure we don't duplicate atom types we have already told
+        # OpenMM about. To do this we will keep track of atom types we have already
+        # seen and ensure we only record new atom types in the xml.
+
+        # iterate over a copy of the atom types
+        for atom_type in params.atom_types.copy().keys():
+            if atom_type not in self._gaff_atom_types_observed:
+                self._gaff_atom_types_observed.add(atom_type)
+                _logger.debug(f"added {atom_type} to set of seen types")
+            # if we have seen the atom type, delete it from the OG params,
+            # not the copy!
+            else:
+                del params.atom_types[atom_type]
+                _logger.debug(f"{atom_type} already recorded")
+
         params.write(ffxml, **kwargs)
         ffxml_contents = ffxml.getvalue()
 
@@ -878,7 +897,8 @@ def read_file_contents(filename):
 
             # Run parmchk.
             shutil.copy(self.gaff_dat_filename, "gaff.dat")
-            cmd = f"parmchk2 -i out.mol2 -f mol2 -p gaff.dat -o out.frcmod -s {self._gaff_major_version}"
+            cmd = f"parmchk2 -i out.mol2 -f mol2 -p gaff.dat -o out.frcmod -s {self._gaff_major_version} -a Y"
+
             _logger.debug(cmd)
             output = subprocess.getoutput(cmd)
             if not os.path.exists("out.frcmod"):

diff --git a/openmmforcefields/tests/conftest.py b/openmmforcefields/tests/conftest.py
@@ -21,6 +21,7 @@ def pytest_addoption(parser):
 
 def pytest_configure(config):
     config.addinivalue_line("markers", "espaloma: mark test as slow to run")
+    config.addinivalue_line("markers", "gaff: mark test as using gaff")
 
 
 def pytest_collection_modifyitems(config, items):

diff --git a/openmmforcefields/tests/test_amber_import.py b/openmmforcefields/tests/test_amber_import.py
@@ -80,7 +80,7 @@ def test_amber_parameterize_ff94():
     Test parameterizing explicit villin with ff94
 
     """
-    from pkg_resources import resource_filename
+    import importlib_resources
 
-    pdb_filename = resource_filename("openmm.app", "data/test.pdb")
+    pdb_filename = str(importlib_resources.files("openmm.app") / "data" / "test.pdb")
     check_ffxml_parameterize(pdb_filename, "amber/ff94.xml")
diff --git a/openmmforcefields/utils.py b/openmmforcefields/utils.py
@@ -2,6 +2,7 @@
 import functools
 import logging
 import time
+import importlib_resources
 
 _logger = logging.getLogger("openmmforcefields.generators.gaff")
 
@@ -15,10 +16,9 @@ def get_ffxml_path():
     path : str
         The absolute path where OpenMM ffxml forcefield files are stored in this package
     """
-    from pkg_resources import resource_filename
 
-    filename = resource_filename("openmmforcefields", "ffxml")
-    return filename
+    filename = importlib_resources.files("openmmforcefields") / "ffxml"
+    return str(filename.absolute())
 
 
 def get_data_filename(relative_path):
@@ -38,16 +38,15 @@ def get_data_filename(relative_path):
     Absolute path to file
 
     """
-    from pkg_resources import resource_filename
 
-    fn = resource_filename("openmmforcefields", "data/" + relative_path)
+    fn = importlib_resources.files("openmmforcefields") / "data" / relative_path
 
     import os
 
     if not os.path.exists(fn):
         raise ValueError(f"sorry! {fn} does not exist. if you just added it, you'll have to re-install")
 
-    return fn
+    return str(fn.absolute())
 
 
 # =============================================================================