Error when non-bonded 1-4 scaling factors differ significantly

.github/workflows/examples.yaml

@@ -55,10 +55,10 @@ jobs:
         SECRET_OE_LICENSE: ${{ secrets.OE_LICENSE }}
 
     - name: Run docexamples
-      run: python -m pytest -c /dev/null/ --doctest-modules openff/interchange/ --ignore=openff/interchange/_tests
+      run: python -m pytest -c pyproject.toml --doctest-modules openff/interchange/ --ignore=openff/interchange/_tests
 
     - name: Run example notebooks
       run: |
-        python -m pytest -c /dev/null/ --nbval-lax --dist loadscope -n logical --durations=20 examples/ \
+        python -m pytest -c pyproject.toml --nbval-lax --dist loadscope -n logical --durations=20 examples/ \
           --ignore=examples/deprecated/ \
           --ignore=examples/experimental

devtools/conda-envs/examples_env.yaml

@@ -30,6 +30,7 @@ dependencies:
   - mdtraj
   - pytest
   - pytest-xdist
+  - pytest-cov
   - nbval
   # Examples
   - openmmforcefields

docs/using/edges.md

@@ -45,6 +45,12 @@ For example, `Interchange.topology.get_positions()` will never include positions
 
 `Molecule` and `Topology` objects can store partial charges, but these are ignored by default in methods like `Interchange.from_smirnoff`. This is because partial charges in SMIRNOFF force fields are defined by sections in the force field. To override this behavior, use the `charge_from_molecules` argument. Be aware that charges, and as a result most physics, will differ from what's perscribed by the contents of the force field.
 
+## Quirks of `Interchange.combine`
+
+### Electrostatics 1-4 scaling factors may be slightly modified
+
+Amber family force fields historically use a 1-4 scaling factor of 1 / 1.2 (or 5/6). Some older OpenFF force field releases round this to 6 digits (0.833333) but more recent releases round this to 10 digits (0.8333333333). These are not stricly equal and arguably should not be combined, but (in this case only) for easier compatibility this difference is ignored and combination proceeds as if they were both 0.833333333 from the start. If value differ significantly, i.e. 0.5 vs 0.833333, an error is raised as this difference is non-trivial.
+
 ## Quirks of `from_openmm`
 
 ### Modified masses are ignored

openff/interchange/_tests/unit_tests/operations/test_combine.py

@@ -6,11 +6,14 @@
 from openff.utilities.testing import skip_if_missing
 
 from openff.interchange import Interchange
+from openff.interchange._tests import MoleculeWithConformer
 from openff.interchange.drivers import get_openmm_energies
 from openff.interchange.exceptions import (
     CutoffMismatchError,
     SwitchingFunctionMismatchError,
+    UnsupportedCombinationError,
 )
+from openff.interchange.warnings import InterchangeCombinationWarning
 
 
 class TestCombine:
@@ -19,8 +22,7 @@ def test_basic_combination(self, monkeypatch, sage_unconstrained):
         """Test basic use of Interchange.__add__() based on the README example"""
         monkeypatch.setenv("INTERCHANGE_EXPERIMENTAL", "1")
 
-        mol = Molecule.from_smiles("C")
-        mol.generate_conformers(n_conformers=1)
+        mol = MoleculeWithConformer.from_smiles("C")
         top = Topology.from_molecules([mol])
 
         interchange = Interchange.from_smirnoff(sage_unconstrained, top)
@@ -43,14 +45,10 @@ def test_basic_combination(self, monkeypatch, sage_unconstrained):
     def test_parameters_do_not_clash(self, monkeypatch, sage_unconstrained):
         monkeypatch.setenv("INTERCHANGE_EXPERIMENTAL", "1")
 
-        thf = Molecule.from_smiles("C1CCOC1")
-        ace = Molecule.from_smiles("CC(=O)O")
+        thf = MoleculeWithConformer.from_smiles("C1CCOC1")
+        ace = MoleculeWithConformer.from_smiles("CC(=O)O")
 
-        thf.generate_conformers(n_conformers=1)
-        ace.generate_conformers(n_conformers=1)
-
-        def make_interchange(molecule: Molecule) -> Interchange:
-            molecule.generate_conformers(n_conformers=1)
+        def make_interchange(molecule: MoleculeWithConformer) -> Interchange:
             interchange = Interchange.from_smirnoff(
                 force_field=sage_unconstrained,
                 topology=[molecule],
@@ -135,3 +133,50 @@ def test_error_mismatched_switching_function(
             sage.create_interchange(basic_top).combine(
                 sage_modified.create_interchange(basic_top),
             )
+
+    @pytest.mark.parametrize(
+        argnames=["vdw", "electrostatics"],
+        argvalues=[
+            (True, False),
+            (False, True),
+            (True, True),
+            (False, False),
+        ],
+    )
+    def test_dont_combine_mixed_14(self, sage, vdw, electrostatics):
+        """
+        Until it's implemented, error out when any non-bonded collections have non-equal 1-4 scaling factors.
+
+        See https://github.com/openforcefield/openff-interchange/issues/380
+
+        """
+        interchange1 = sage.create_interchange(MoleculeWithConformer.from_smiles("C").to_topology())
+        interchange2 = sage.create_interchange(MoleculeWithConformer.from_smiles("CCO").to_topology())
+
+        if vdw:
+            interchange2["vdW"].scale_14 = 0.444
+
+        if electrostatics:
+            interchange2["Electrostatics"].scale_14 = 0.444
+
+        if vdw or electrostatics:
+            with pytest.raises(
+                UnsupportedCombinationError,
+                match="1-4.*vdW" if vdw else "1-4.*Electro",
+            ):
+                interchange2.combine(interchange1)
+        else:
+            # if neither are modified, that error shouldn't be raised
+            interchange2.combine(interchange1)
+
+    def test_mix_different_5_6_rounding(self, parsley, sage, ethanol):
+        """Test that 0.833333 is 'upconverted' to 0.8333333333 in combination."""
+        with pytest.warns(
+            InterchangeCombinationWarning,
+            match="more digits in rounding",
+        ):
+            parsley.create_interchange(
+                ethanol.to_topology(),
+            ).combine(
+                sage.create_interchange(ethanol.to_topology()),
+            )

openff/interchange/operations/_combine.py

@@ -44,6 +44,31 @@ def _check_nonbonded_compatibility(
             f"{interchange1['vdW'].switch_width} and {interchange2['vdW'].switch_width}.",
         )
 
+    if interchange1["vdW"].scale_14 != interchange2["vdW"].scale_14:
+        raise UnsupportedCombinationError(
+            "1-4 scaling factors in vdW handler(s) do not match.",
+        )
+
+    if interchange1["Electrostatics"].scale_14 != interchange2["Electrostatics"].scale_14:
+        if sorted([interchange1["Electrostatics"].scale_14, interchange2["Electrostatics"].scale_14]) == [
+            0.833333,
+            0.8333333333,
+        ]:
+            warnings.warn(
+                "Found electrostatics 1-4 scaling factors of 5/6 with slightly different rounding "
+                "(0.833333 and 0.8333333333). This likely stems from OpenFF using more digits in rounding 1/1.2. "
+                "The value of 0.8333333333 will be used, which may or may not introduce small errors. ",
+                InterchangeCombinationWarning,
+            )
+
+            interchange1["Electrostatics"].scale_14 = 0.8333333333
+            interchange2["Electrostatics"].scale_14 = 0.8333333333
+
+        else:
+            raise UnsupportedCombinationError(
+                "1-4 scaling factors in Electrostatics handler(s) do not match.",
+            )
+
 
 def _combine(
     input1: "Interchange",