Add option to skip valence check when assigning vsites (#1887)

* add option to skip valence test when assigning vsites and test * update docstring and error message * fix test * update releasenotes
openforcefield · Jun 10, 2024 · 7d339e5 · 7d339e5
1 parent fdf3832
commit 7d339e5
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 7 deletions.
diff --git a/docs/releasehistory.md b/docs/releasehistory.md
@@ -27,6 +27,7 @@ Releases follow the `major.minor.micro` scheme recommended by [PEP440](https://w
 - [PR #1866](https://github.com/openforcefield/openff-toolkit/pull/1866): Implements a safer AmberTools version check.
 - [PR #1874](https://github.com/openforcefield/openff-toolkit/pull/1874): Improves some loading times by lazy-loading `networkx`.
 - [PR #1881](https://github.com/openforcefield/openff-toolkit/pull/1881): Adds an optional suffix parameter to `generate_unique_atom_names`.
+- [PR #1887](https://github.com/openforcefield/openff-toolkit/pull/1887): Allows skipping vsite valence check for parent atoms using the `OPENFF_UNSAFE_VSITES` environment variable. 
 
 ### Improved documentation and warnings
 

diff --git a/openff/toolkit/_tests/test_parameters.py b/openff/toolkit/_tests/test_parameters.py
@@ -2215,25 +2215,44 @@ def test_serialize_roundtrip(self):
         assert offxml_string == roundtripped_force_field.to_string()
 
     @pytest.mark.parametrize(
-        "smiles, matched_indices, parameter, expected_raises",
+        "smiles, matched_indices, parameter, expected_raises, unsafe_vsites",
         [
             (
                 "[Cl:1][H:2]",
                 (1, 2),
                 VirtualSiteMocking.bond_charge_parameter("[Cl:1][H:2]"),
                 does_not_raise(),
+                False,
             ),
             (
                 "[N:1]([H:2])([H:3])[H:4]",
                 (1, 2, 3),
                 VirtualSiteMocking.monovalent_parameter("[*:2][N:1][*:3]"),
-                pytest.raises(NotImplementedError, match="please describe what it is"),
+                pytest.raises(
+                    NotImplementedError, match="currently unsupported by virtual sites"
+                ),
+                False,
+            ),
+            (
+                "[N:1]([H:2])([H:3])[H:4]",
+                (1, 2, 3),
+                VirtualSiteMocking.monovalent_parameter("[*:2][N:1][*:3]"),
+                does_not_raise(),
+                True,
             ),
         ],
     )
     def test_validate_found_match(
-        self, smiles, matched_indices, parameter, expected_raises
+        self,
+        monkeypatch,
+        smiles,
+        matched_indices,
+        parameter,
+        expected_raises,
+        unsafe_vsites,
     ):
+        if unsafe_vsites:
+            monkeypatch.setenv("OPENFF_UNSAFE_VSITES", "1")
         molecule = Molecule.from_smiles(smiles, allow_undefined_stereo=True)
         topology: Topology = molecule.to_topology()
 

diff --git a/openff/toolkit/typing/engines/smirnoff/parameters.py b/openff/toolkit/typing/engines/smirnoff/parameters.py
@@ -3653,8 +3653,14 @@ def _validate_found_match(
         supported exception, they should open an issue on GitHub explaining their exact
         use case so that we can ensure that exactly what they need is both supported
         and works as expected through expansion of the unit tests.
+
+        This validation can be disabled by assigning the environment variable
+         `OPENFF_UNSAFE_VSITES=1`.
         """
+        import os
 
+        if os.environ.get("OPENFF_UNSAFE_VSITES", "0") != "0":
+            return
         supported_connectivity = {
             # We currently expect monovalent lone pairs to be applied to something
             # like a carboxyl group, where the parent of the lone pair has a
@@ -3687,10 +3693,11 @@ def _validate_found_match(
                 f"unsupported by virtual sites of type {parameter.type}. Atom with "
                 f"smirks index={smirks_index} matched topology atom {atom_index} with "
                 f"connectivity={connectivity}, but it was expected to have connectivity "
-                f"{expected_connectivity}. If this is "
-                f"a use case you would like supported, please describe what it is "
-                f"you are trying to do in an issue on the OpenFF Toolkit GitHub: "
-                f"https://github.com/openforcefield/openff-toolkit/issues"
+                f"{expected_connectivity}. If you are getting this error and aren't "
+                f"developing your own force field, it indicates that the FF you're "
+                f"using never expected to see a molecule like this. If you ARE developing "
+                f"a force field and know that you want to squelch this error, set the "
+                f"environment variable `OPENFF_UNSAFE_VSITES=1`."
             )
 
     def check_handler_compatibility(self, other_handler: "VirtualSiteHandler"):