Merge pull request #930 from OpenFreeEnergy/perses_mapper

Perses mapper

news/pr_930.rst

@@ -0,0 +1,23 @@
+**Added:**
+
+* <news item>
+
+**Changed:**
+
+* The `PersesAtomMapper` now uses openff.units inline with the rest of the package.
+
+**Deprecated:**
+
+* <news item>
+
+**Removed:**
+
+* <news item>
+
+**Fixed:**
+
+* <news item>
+
+**Security:**
+
+* <news item>

openfe/setup/atom_mapping/perses_mapper.py

@@ -6,8 +6,10 @@
 
 """
 
-from openmm import unit
 from openfe.utils import requires_package
+from openff.models.types import FloatQuantity
+from openff.units import unit
+from openff.units.openmm import to_openmm
 
 from ...utils.silence_root_logging import silence_root_logging
 try:
@@ -25,12 +27,36 @@ class PersesAtomMapper(LigandAtomMapper):
     allow_ring_breaking: bool
     preserve_chirality: bool
     use_positions: bool
+    coordinate_tolerance: FloatQuantity['angstrom']
+
+    def _to_dict(self) -> dict:
+        # strip units but record values
+        return {
+            "allow_ring_breaking": self.allow_ring_breaking,
+            "preserve_chirality": self.preserve_chirality,
+            "use_positions": self.use_positions,
+            "coordinate_tolerance": self.coordinate_tolerance.m_as(
+                unit.angstrom
+            ),
+            "_tolerance_unit": "angstrom"
+        }
+
+    @classmethod
+    def _from_dict(cls, dct: dict):
+        # attach units again
+        tolerence_unit = dct.pop("_tolerance_unit")
+        dct["coordinate_tolerance"] *= getattr(unit, tolerence_unit)
+        return cls(**dct)
+
+    @classmethod
+    def _defaults(cls):
+        return {}
 
     @requires_package("perses")
     def __init__(self, allow_ring_breaking: bool = True,
                  preserve_chirality: bool = True,
                  use_positions: bool = True,
-                 coordinate_tolerance: float = 0.25 * unit.angstrom):
+                 coordinate_tolerance: unit.Quantity = 0.25 * unit.angstrom):
         """
         Suggest atom mappings with the Perses atom mapper.
 
@@ -43,7 +69,7 @@ def __init__(self, allow_ring_breaking: bool = True,
             if mappings must strictly preserve chirality, default: True
         use_positions: bool, optional
             this option defines, if the
-        coordinate_tolerance: float, optional
+        coordinate_tolerance: openff.units.unit.Quantity, optional
             tolerance on how close coordinates need to be, such they
             can be mapped, default: 0.25*unit.angstrom
 
@@ -57,7 +83,7 @@ def _mappings_generator(self, componentA, componentB):
         # Construct Perses Mapper
         _atom_mapper = AtomMapper(
             use_positions=self.use_positions,
-            coordinate_tolerance=self.coordinate_tolerance,
+            coordinate_tolerance=to_openmm(self.coordinate_tolerance),
             allow_ring_breaking=self.allow_ring_breaking)
 
         # Try generating a mapping

openfe/tests/setup/atom_mapping/test_perses_atommapper.py

@@ -2,14 +2,12 @@
 # For details, see https://github.com/OpenFreeEnergy/openfe
 import pytest
 from openfe.setup.atom_mapping import PersesAtomMapper, LigandAtomMapping
+from openff.units import unit
 
 pytest.importorskip('perses')
 pytest.importorskip('openeye')
 
-USING_NEW_OFF = True  # by default we are now
 
-
-@pytest.mark.xfail(USING_NEW_OFF, reason="Perses #1108")
 def test_simple(atom_mapping_basic_test_files):
     # basic sanity check on the LigandAtomMapper
     mol1 = atom_mapping_basic_test_files['methylcyclohexane']
@@ -25,7 +23,6 @@ def test_simple(atom_mapping_basic_test_files):
     assert len(mapping.componentA_to_componentB) == 4
 
 
-@pytest.mark.xfail(USING_NEW_OFF, reason="Perses #1108")
 def test_generator_length(atom_mapping_basic_test_files):
     # check that we get one mapping back from Lomap LigandAtomMapper then the
     # generator stops correctly
@@ -41,7 +38,6 @@ def test_generator_length(atom_mapping_basic_test_files):
         next(mapping_gen)
 
 
-@pytest.mark.xfail(USING_NEW_OFF, reason="Perses #1108")
 def test_empty_atommappings(mol_pair_to_shock_perses_mapper):
     mol1, mol2 = mol_pair_to_shock_perses_mapper
     mapper = PersesAtomMapper()
@@ -53,3 +49,14 @@ def test_empty_atommappings(mol_pair_to_shock_perses_mapper):
 
     with pytest.raises(StopIteration):
         next(mapping_gen)
+
+
+def test_dict_round_trip():
+    # use some none defaults
+    mapper1 = PersesAtomMapper(
+        allow_ring_breaking=False,
+        preserve_chirality=False,
+        coordinate_tolerance=0.01 * unit.nanometer
+    )
+    mapper2 = PersesAtomMapper.from_dict(mapper1.to_dict())
+    assert mapper2.to_dict() == mapper1.to_dict()