Raise exception when Topology.from_openmm is given virtual sites (#…

…1667)

* Raise exception when loading virtual sites from OpenMM

* Update release history

docs/releasehistory.md

@@ -15,6 +15,8 @@ Releases follow the `major.minor.micro` scheme recommended by [PEP440](https://w
 
 ### Bugfixes
 
+- [PR #1667](https://github.com/openforcefield/openff-toolkit/pull/1667): A more helpful exception is now raised when `Topology.from_openmm` is given an OpenMM Topology with virtual sites.
+
 ### New features
 
 ### Improved documentation and warnings

openff/toolkit/_tests/test_topology.py

@@ -60,6 +60,7 @@
     MissingUniqueMoleculesError,
     MoleculeNotInTopologyError,
     UnassignedChemistryInPDBError,
+    VirtualSitesUnsupportedError,
     WrongShapeError,
 )
 
@@ -518,6 +519,23 @@ def test_from_openmm(self):
         for omm_atom, off_atom in zip(pdbfile.topology.atoms(), topology.atoms):
             assert omm_atom.name == off_atom.name
 
+    def test_from_openmm_virtual_sites(self):
+        from openff.toolkit import ForceField
+
+        water = Molecule.from_mapped_smiles("[O:1]([H:2])[H:3]")
+        opc = ForceField("opc-1.0.1.offxml")
+
+        openmm_topology = opc.create_interchange([water]).to_openmm_topology()
+
+        with pytest.raises(
+            VirtualSitesUnsupportedError,
+            match="Atom <Atom 3 \(EP\) of chain 0 residue 0 \(UNK\)>.* a virtual site",
+        ):
+            Topology.from_openmm(
+                openmm_topology,
+                unique_molecules=[water],
+            )
+
     def test_from_openmm_missing_reference(self):
         """Test creation of an OpenFF Topology object from an OpenMM Topology when missing a unique molecule"""
         pdbfile = app.PDBFile(

openff/toolkit/topology/topology.py

@@ -61,6 +61,7 @@
     MissingUniqueMoleculesError,
     MoleculeNotInTopologyError,
     NotBondedError,
+    VirtualSitesUnsupportedError,
     WrongShapeError,
 )
 from openff.toolkit.utils.serialization import Serializable
@@ -1285,6 +1286,12 @@ def _openmm_topology_to_networkx(openmm_topology):
         # Convert all openMM mols to graphs
         omm_topology_G = nx.Graph()
         for atom in openmm_topology.atoms():
+            if atom.element is None:
+                raise VirtualSitesUnsupportedError(
+                    f"Atom {atom} has element None and is probably a virtual site. Virtual sites "
+                    "cannot be stored in `Topology` objects but can be stored in `Interchange` "
+                    "objects produced by `ForceField`s with virtual site parameters."
+                )
             omm_topology_G.add_node(
                 atom.index,
                 atomic_number=atom.element.atomic_number,
@@ -1319,6 +1326,10 @@ def from_openmm(
         Hierarchy schemes are taken from the OpenMM topology, not from
         ``unique_molecules``.
 
+        If any virtual sites are detected in the OpenMM topology,
+        ``VirtualSitesUnsupportedError`` is raised because the ``Topology``
+        object model does not store virtual sites.
+
         Parameters
         ----------
         openmm_topology

openff/toolkit/utils/exceptions.py

@@ -321,6 +321,10 @@ class HierarchySchemeNotFoundException(OpenFFToolkitException):
     that doesn't have one with the given iterator name"""
 
 
+class VirtualSitesUnsupportedError(OpenFFToolkitException):
+    """Exception raised when trying to store virtual sites in a `Molecule` or `Topology` object."""
+
+
 class MissingIndexedAttributeError(
     OpenFFToolkitException, IndexError, KeyError, AttributeError
 ):