Support version 0.4 of SMIRNOFF Electrostatics section

docs/releasehistory.md

@@ -58,6 +58,8 @@ print(value_roundtrip)
 
 ## Current Development
 
+- [PR #1277](https://github.com/openforcefield/openff-toolkit/pull/1277): Adds support for version
+  0.4 of the `<Electrostatics>` section of the SMIRNOFF specification.
 - [PR #1279](https://github.com/openforcefield/openforcefield/pull/1279):
   [`ParameterHandler.version`](openff.toolkit.typing.engines.smirnoff.parameters.ParameterHandler.version)
   and the ``.version`` attribute of its subclasses is now a
@@ -100,6 +102,16 @@ print(value_roundtrip)
 
 ### Behaviors changed and bugfixes
 
+- [PR #1277](https://github.com/openforcefield/openff-toolkit/pull/1277): Version 0.3 `<Electrostatics>`
+  sections of OFFXML files will automatically be up-converted (in memory) to version 0.4 according
+  to the recomendations provided in
+  [OFF-EP 0005](https://openforcefield.github.io/standards/enhancement-proposals/off-ep-0005/). Note
+  this means the `method` attribute is replaced by `periodic_potential`, `nonperiodic_potential`,
+  and `exception_potential`.
+- [PR #1277](https://github.com/openforcefield/openff-toolkit/pull/1277): Fixes a bug in which
+  attempting to convert
+  [`ElectrostaticsHandler.switch_width`](openff.toolkit.typing.engines.smirnoff.parameters.ElectrostaticsHandler)
+  did nothing.
 - [PR #1130](https://github.com/openforcefield/openforcefield/pull/1130): Running unit tests will
   no longer generate force field files in the local directory.
 - [PR #1182](https://github.com/openforcefield/openforcefield/pull/1182): Removes `Atom.element`,

openff/toolkit/data/test_forcefields/test_forcefield.offxml

@@ -341,6 +341,6 @@
 		<Atom smirks="[#35X0-1:1]" epsilon="0.0586554 * mole**-1 * kilocalorie" id="n34" rmin_half="2.608 * angstrom"></Atom>
 		<Atom smirks="[#53X0-1:1]" epsilon="0.0536816 * mole**-1 * kilocalorie" id="n35" rmin_half="2.86 * angstrom"></Atom>
 	</vdW>
-	<Electrostatics version="0.3" method="PME" scale12="0.0" scale13="0.0" scale14="0.833333" scale15="1.0" switch_width="0.0 * angstrom" cutoff="9.0 * angstrom"></Electrostatics>
+	<Electrostatics version="0.4" periodic_potential="Ewald3D-ConductingBoundary" nonperiodic_potential="Coulomb" exception_potential="Coulomb" scale12="0.0" scale13="0.0" scale14="0.833333" scale15="1.0" switch_width="0.0 * angstrom" cutoff="9.0 * angstrom"></Electrostatics>
 	<ToolkitAM1BCC version="0.3"></ToolkitAM1BCC>
 </SMIRNOFF>

openff/toolkit/tests/test_forcefield.py

@@ -741,99 +741,99 @@ def generate_monatomic_ions():
 nonbonded_resolution_matrix = [
     {
         "vdw_method": "cutoff",
-        "electrostatics_method": "Coulomb",
+        "electrostatics_periodic_potential": "Coulomb",
         "has_periodic_box": True,
         "omm_force": None,
-        "exception": IncompatibleParameterError,
+        "exception": SMIRNOFFSpecUnimplementedError,
         "exception_match": "",
     },
     {
         "vdw_method": "cutoff",
-        "electrostatics_method": "Coulomb",
+        "electrostatics_periodic_potential": "Coulomb",
         "has_periodic_box": False,
         "omm_force": openmm.NonbondedForce.NoCutoff,
         "exception": None,
         "exception_match": "",
     },
     {
         "vdw_method": "cutoff",
-        "electrostatics_method": "reaction-field",
+        "electrostatics_periodic_potential": "reaction-field",
         "has_periodic_box": True,
         "omm_force": None,
         "exception": SMIRNOFFSpecUnimplementedError,
         "exception_match": "reaction-field",
     },
     {
         "vdw_method": "cutoff",
-        "electrostatics_method": "reaction-field",
+        "electrostatics_periodic_potential": "reaction-field",
         "has_periodic_box": False,
-        "omm_force": None,
-        "exception": SMIRNOFFSpecError,
+        "omm_force": openmm.NonbondedForce.NoCutoff,
+        "exception": None,
         "exception_match": "reaction-field",
     },
     {
         "vdw_method": "cutoff",
-        "electrostatics_method": "PME",
+        "electrostatics_periodic_potential": "PME",
         "has_periodic_box": True,
         "omm_force": openmm.NonbondedForce.PME,
         "exception": None,
         "exception_match": "",
     },
     {
         "vdw_method": "cutoff",
-        "electrostatics_method": "PME",
+        "electrostatics_periodic_potential": "PME",
         "has_periodic_box": False,
         "omm_force": openmm.NonbondedForce.NoCutoff,
         "exception": None,
         "exception_match": "",
     },
     {
         "vdw_method": "PME",
-        "electrostatics_method": "Coulomb",
+        "electrostatics_periodic_potential": "Coulomb",
         "has_periodic_box": True,
         "omm_force": None,
         "exception": IncompatibleParameterError,
         "exception_match": "",
     },
     {
         "vdw_method": "PME",
-        "electrostatics_method": "Coulomb",
+        "electrostatics_periodic_potential": "Coulomb",
         "has_periodic_box": False,
-        "omm_force": openmm.NonbondedForce.NoCutoff,
-        "exception": None,
-        "exception_match": "",
+        "omm_force": None,
+        "exception": SMIRNOFFSpecError,
+        "exception_match": "vdw method PME.* is only valid for periodic systems",
     },
     {
         "vdw_method": "PME",
-        "electrostatics_method": "reaction-field",
+        "electrostatics_periodic_potential": "reaction-field",
         "has_periodic_box": True,
         "omm_force": None,
         "exception": IncompatibleParameterError,
-        "exception_match": "reaction-field",
+        "exception_match": "must also be PME",
     },
     {
         "vdw_method": "PME",
-        "electrostatics_method": "reaction-field",
+        "electrostatics_periodic_potential": "reaction-field",
         "has_periodic_box": False,
         "omm_force": None,
         "exception": SMIRNOFFSpecError,
-        "exception_match": "reaction-field",
+        "exception_match": "vdw method PME.* is only valid for periodic systems",
     },
     {
         "vdw_method": "PME",
-        "electrostatics_method": "PME",
+        "electrostatics_periodic_potential": "PME",
         "has_periodic_box": True,
         "omm_force": openmm.NonbondedForce.LJPME,
         "exception": None,
         "exception_match": "",
     },
     {
         "vdw_method": "PME",
-        "electrostatics_method": "PME",
+        "electrostatics_periodic_potential": "PME",
         "has_periodic_box": False,
         "omm_force": openmm.NonbondedForce.NoCutoff,
-        "exception": None,
-        "exception_match": "",
+        "exception": SMIRNOFFSpecError,
+        "exception_match": "vdw method PME.* is only valid for periodic systems",
     },
 ]
 
@@ -1691,6 +1691,14 @@ def test_pass_invalid_kwarg_to_create_openmm_system(
                 use_interchange=False,
             )
 
+    def test_electrostatics_switch_width_unsupported(self):
+        handler = ElectrostaticsHandler(version=0.4)
+        with pytest.raises(
+            SMIRNOFFSpecUnimplementedError,
+            match="not support an electrostatic switch width",
+        ):
+            handler.switch_width = 1.234 * unit.nanometer
+
     @pytest.mark.parametrize("mod_cuoff", [True, False])
     def test_nonbonded_cutoff_no_box_vectors(self, mod_cuoff):
         """Ensure that the NonbondedForce objects use the cutoff specified in the
@@ -1737,7 +1745,7 @@ def test_vdw_cutoff_overrides_electrostatics(self):
 
         electrostatics_handler = ElectrostaticsHandler(version=0.3)
         electrostatics_handler.cutoff = 7.0 * unit.angstrom
-        electrostatics_handler.method = "PME"
+        electrostatics_handler.periodic_potential = "PME"
         force_field.register_parameter_handler(electrostatics_handler)
 
         library_charges = LibraryChargeHandler(version=0.3)
@@ -1762,7 +1770,7 @@ def test_vdw_cutoff_overrides_electrostatics(self):
 
         # TODO: Don't change the box vectors once this case is supported
         topology.box_vectors = None
-        electrostatics_handler.method = "Coulomb"
+        electrostatics_handler.periodic_potential = "Coulomb"
         force_field.register_parameter_handler(electrostatics_handler)
 
         with pytest.raises(IncompatibleParameterError, match="cutoff must equal"):
@@ -1792,7 +1800,7 @@ def test_nondefault_nonbonded_cutoff(self):
 
         electrostatics_handler = ElectrostaticsHandler(version=0.3)
         electrostatics_handler.cutoff = 7.89 * unit.angstrom
-        electrostatics_handler.method = "PME"
+        electrostatics_handler.periodic_potential = "PME"
         force_field.register_parameter_handler(electrostatics_handler)
 
         library_charges = LibraryChargeHandler(version=0.3)
@@ -1816,7 +1824,6 @@ def test_nondefault_nonbonded_cutoff(self):
     def test_nonbonded_method_resolution(self, inputs):
         """Test predefined permutations of input options to ensure nonbonded handling is correctly resolved"""
         vdw_method = inputs["vdw_method"]
-        electrostatics_method = inputs["electrostatics_method"]
         has_periodic_box = inputs["has_periodic_box"]
         omm_force = inputs["omm_force"]
         exception = inputs["exception"]
@@ -1836,7 +1843,7 @@ def test_nonbonded_method_resolution(self, inputs):
             forcefield.get_parameter_handler("vdW", {}).method = vdw_method
             forcefield.get_parameter_handler(
                 "Electrostatics", {}
-            ).method = electrostatics_method
+            ).periodic_potential = inputs["electrostatics_periodic_potential"]
             omm_system = forcefield.create_openmm_system(
                 topology, use_interchange=False
             )
@@ -1853,7 +1860,7 @@ def test_nonbonded_method_resolution(self, inputs):
                 forcefield.get_parameter_handler("vdW", {}).method = vdw_method
                 forcefield.get_parameter_handler(
                     "Electrostatics", {}
-                ).method = electrostatics_method
+                ).periodic_potential = inputs["electrostatics_periodic_potential"]
                 omm_system = forcefield.create_openmm_system(
                     topology, use_interchange=False
                 )
@@ -4802,6 +4809,9 @@ def test_fractional_bondorder_invalid_interpolation_method(self):
 
 class TestInterchangeReturnTopology:
     # TODO: Remove these tests when `return_topology` is deprecated in version 0.12.0
+    # TODO: Turn this test back on once Interchange is tagged with a pre-release supporting
+    #       only Electrostatics version 0.4
+    @pytest.mark.skip(reason="Interchange needs to be updated")
     def test_deprecation_warning_raised(self):
         """
         Ensure that the deprecation warning is raised when `return_topology` is
@@ -4937,7 +4947,7 @@ def test_electrostatics_options(self):
     )
     for method in ["PME", "reaction-field", "Coulomb"]:
         # Change electrostatics method
-        forcefield.forces["Electrostatics"].method = method
+        forcefield.forces["Electrostatics"].periodic_potential = method
         f = partial(check_system_creation_from_molecule, forcefield, molecule)
         f.description = "Testing {} parameter assignment using molecule {}".format(
             offxml_file_path, molecule.name

openff/toolkit/tests/test_parameters.py

@@ -21,6 +21,7 @@
 from openff.toolkit.typing.engines.smirnoff.parameters import (
     BondHandler,
     ChargeIncrementModelHandler,
+    ElectrostaticsHandler,
     GBSAHandler,
     ImproperTorsionHandler,
     IndexedParameterAttribute,
@@ -45,6 +46,7 @@
     NotEnoughPointsForInterpolationError,
     ParameterLookupError,
     SMIRNOFFSpecError,
+    SMIRNOFFSpecUnimplementedError,
     SMIRNOFFVersionError,
 )
 
@@ -1850,6 +1852,72 @@ def test_sigma_rmin_half(self):
         assert "rmin_half" in param.to_dict()
 
 
+class TestElectrostaticsHandler:
+    def test_solvent_dielectric(self):
+        with pytest.raises(
+            SMIRNOFFSpecUnimplementedError,
+            match="make use of `solvent_d",
+        ):
+            ElectrostaticsHandler(version=0.3, method="PME", solvent_dielectric=4)
+
+        handler = ElectrostaticsHandler(version=0.4)
+        assert handler.solvent_dielectric is None
+
+        handler.solvent_dielectric = None
+
+        with pytest.raises(
+            SMIRNOFFSpecUnimplementedError,
+            match="make use of `solvent_d",
+        ):
+            handler.solvent_dielectric = 78.3
+
+    def test_unknown_periodic_potential(self):
+        handler = ElectrostaticsHandler(version=0.4)
+
+        with pytest.raises(
+            NotImplementedError,
+            match="unexpected periodic potential",
+        ):
+            handler.periodic_potential = "PPPM"
+
+
+class TestElectrostaticsHandlerUpconversion:
+    """
+    Test the implementation of OFF-EP-0005:
+
+    https://openforcefield.github.io/standards/enhancement-proposals/off-ep-0005/
+    """
+
+    default_reaction_field_expression = (
+        "charge1*charge2/(4*pi*epsilon0)*(1/r + k_rf*r^2 - c_rf);"
+        "k_rf=(cutoff^(-3))*(solvent_dielectric-1)/(2*solvent_dielectric+1);"
+        "c_rf=cutoff^(-1)*(3*solvent_dielectric)/(2*solvent_dielectric+1)"
+    )
+
+    @pytest.mark.parametrize(
+        ("old_method", "new_method"),
+        [
+            ("PME", "Ewald3D-ConductingBoundary"),
+            ("Coulomb", "Coulomb"),
+            ("reaction-field", default_reaction_field_expression),
+        ],
+    )
+    def test_upconversion(self, old_method, new_method):
+        handler = ElectrostaticsHandler(version=0.3, method=old_method)
+        assert handler.version == Version("0.4")
+
+        # Only `periodic_potential` is a function of the values in a version 0.3 handler ...
+        assert handler.periodic_potential == new_method
+
+        # ... for everything else, it's the same
+        assert handler.nonperiodic_potential == "Coulomb"
+        assert handler.exception_potential == "Coulomb"
+
+    def test_invalid_0_4_kwargs(self):
+        with pytest.raises(SMIRNOFFSpecError, match="removed in version 0.4 of the E"):
+            ElectrostaticsHandler(version=0.4, method="PME")
+
+
 class TestVirtualSiteHandler:
     """
     Test the creation of a VirtualSiteHandler and the implemented VirtualSiteTypes

openff/toolkit/typing/engines/smirnoff/forcefield.py

@@ -35,6 +35,8 @@
 from collections import OrderedDict
 from typing import TYPE_CHECKING, List, Tuple, Union
 
+from packaging.version import Version
+
 from openff.toolkit.topology.molecule import DEFAULT_AROMATICITY_MODEL
 from openff.toolkit.typing.engines.smirnoff.io import ParameterIOHandler
 from openff.toolkit.typing.engines.smirnoff.parameters import (
@@ -189,7 +191,7 @@ class ForceField:
 
     Modify the long-range electrostatics method:
 
-    >>> forcefield.get_parameter_handler('Electrostatics').method = 'PME'
+    >>> forcefield.get_parameter_handler('Electrostatics').periodic_potential = 'PME'
 
     Inspect the first few vdW parameters:
 
@@ -323,8 +325,8 @@ def _initialize(self):
         """
         Initialize all object fields.
         """
-        self._MIN_SUPPORTED_SMIRNOFF_VERSION = 0.1
-        self._MAX_SUPPORTED_SMIRNOFF_VERSION = 0.3
+        self._MIN_SUPPORTED_SMIRNOFF_VERSION = Version("0.1")
+        self._MAX_SUPPORTED_SMIRNOFF_VERSION = Version("0.3")
         self._disable_version_check = (
             False  # if True, will disable checking compatibility version
         )
@@ -1380,16 +1382,17 @@ def _old_create_openmm_system(self, topology, **kwargs):
         if hasattr(self.get_parameter_handler("Electrostatics"), "cutoff"):
             vdw_cutoff = self.get_parameter_handler("vdW").cutoff
             coul_cutoff = self.get_parameter_handler("Electrostatics").cutoff
-            coul_method = self.get_parameter_handler("Electrostatics").method
+            coul_periodic_potential = self.get_parameter_handler(
+                "Electrostatics"
+            ).periodic_potential
             if vdw_cutoff != coul_cutoff:
-                if coul_method == "PME":
+                if coul_periodic_potential == "Ewald3D-ConductingBoundary":
                     nonbonded_force.setCutoffDistance(to_openmm(vdw_cutoff))
                 else:
                     raise IncompatibleParameterError(
-                        "In its current implementation of the OpenFF Toolkit, with "
-                        f"With electrostatics method {coul_method}, the electrostatics "
-                        f"cutoff must equal the vdW cutoff. Found vdw cutoff {vdw_cutoff} "
-                        f"and {coul_cutoff}."
+                        "In its current implementation of the OpenFF Toolkit, with With electrostatics periodic "
+                        f"potential {coul_periodic_potential}, the electrostatics cutoff must equal the vdW cutoff. "
+                        f"Found vdw cutoff {vdw_cutoff} and {coul_cutoff}."
                     )
 
         if return_topology: