Merge pull request #929 from openforcefield/from-openmm-positions-war…

…ning

Warn if positions not given

docs/releasehistory.md

@@ -14,6 +14,7 @@ Please note that all releases prior to a version 1.0.0 are considered pre-releas
 ## Current development
 
 * #933 Fixes #934 in which atom order was sometimes mangled in `Interchange.from_openmm`.
+* #929 A warning is raised when positions are not passed to `Interchange.from_openmm`.
 * #930 Adds `additional_forces` argument to `create_openmm_simulation`.
 
 ## 0.3.23 - 2024-03-07

examples/experimental/openmm-import/protein-ligand.ipynb

@@ -26,13 +26,14 @@
     "import pathlib\n",
     "import sys\n",
     "import urllib.request\n",
-    "from typing import Dict, Tuple\n",
+    "from typing import Tuple\n",
     "\n",
     "import openmm\n",
     "import openmm.app\n",
     "import openmm.unit\n",
     "from openff.units.openmm import ensure_quantity\n",
     "\n",
+    "from openff.interchange import Interchange\n",
     "from openff.interchange.drivers import get_openmm_energies\n",
     "from openff.interchange.drivers.openmm import _get_openmm_energies, _process\n",
     "from openff.interchange.interop.openmm import from_openmm\n",
@@ -116,7 +117,7 @@
    "metadata": {},
    "outputs": [],
    "source": [
-    "converted_interchange = from_openmm(\n",
+    "converted_interchange = Interchange.from_openmm(\n",
     "    topology=protein.topology,\n",
     "    system=openmm_system,\n",
     "    positions=ensure_quantity(protein.getPositions(), \"openff\"),\n",

examples/experimental/openmmforcefields/gaff.ipynb

@@ -73,9 +73,9 @@
    "metadata": {},
    "outputs": [],
    "source": [
-    "from openff.interchange.interop.openmm import from_openmm\n",
+    "from openff.interchange import Interchange\n",
     "\n",
-    "imported = from_openmm(\n",
+    "imported = Interchange.from_openmm(\n",
     "    topology=topology.to_openmm(),\n",
     "    system=system_gaff,\n",
     "    positions=molecule.conformers[0].to_openmm(),\n",

openff/interchange/_tests/unit_tests/interop/openmm/_import/test_compat.py

@@ -7,6 +7,7 @@
 
 from openff.interchange.exceptions import UnsupportedImportError
 from openff.interchange.interop.openmm._import import from_openmm
+from openff.interchange.warnings import MissingPositionsWarning
 
 
 class TestUnsupportedCases:
@@ -52,3 +53,18 @@ def test_found_virtual_sites(self, monkeypatch, tip4p, water):
                 system=system,
                 topology=topology.to_openmm(),
             )
+
+    def test_missing_positions_warning(self, monkeypatch, sage, water):
+        monkeypatch.setenv("INTERCHANGE_EXPERIMENTAL", "1")
+
+        topology = water.to_topology()
+        topology.box_vectors = Quantity([4, 4, 4], "nanometer")
+
+        with pytest.warns(
+            MissingPositionsWarning,
+            match="are you sure",
+        ):
+            from_openmm(
+                system=sage.create_openmm_system(topology),
+                topology=topology.to_openmm(),
+            )

openff/interchange/interop/openmm/_import/_import.py

@@ -1,7 +1,8 @@
-from typing import TYPE_CHECKING, Union
+import warnings
+from typing import TYPE_CHECKING, Optional, Union
 
 from openff.models.types import ArrayQuantity
-from openff.toolkit import Topology
+from openff.toolkit import Quantity, Topology
 from openff.utilities.utilities import has_package, requires_package
 
 from openff.interchange._experimental import experimental
@@ -17,6 +18,7 @@
     BasicElectrostaticsCollection,
 )
 from openff.interchange.interop.openmm._import.compat import _check_compatible_inputs
+from openff.interchange.warnings import MissingPositionsWarning
 
 if has_package("openmm"):
     import openmm
@@ -34,10 +36,11 @@
 @requires_package("openmm")
 @experimental
 def from_openmm(
+    *,
     system: "openmm.System",
+    positions: Optional[Quantity] = None,
     topology: Union["openmm.app.Topology", Topology, None] = None,
-    positions=None,
-    box_vectors=None,
+    box_vectors: Optional[Quantity] = None,
 ) -> "Interchange":
     """Create an Interchange object from OpenMM data."""
     from openff.interchange import Interchange
@@ -88,7 +91,16 @@ def from_openmm(
 
         interchange.topology = topology
 
-    if positions is not None:
+    if positions is None:
+
+        warnings.warn(
+            "Nothing was passed to the `positions` argument, are you sure "
+            "you don't want to pass positions?",
+            MissingPositionsWarning,
+            stacklevel=2,
+        )
+
+    else:
 
         interchange.positions = positions
 

openff/interchange/warnings.py

@@ -7,3 +7,7 @@ class InterchangeDeprecationWarning(UserWarning):
 
 class SwitchingFunctionNotImplementedWarning(UserWarning):
     """Exporting to an engine that does not implement a switching function."""
+
+
+class MissingPositionsWarning(UserWarning):
+    """Warning for when positions are likely needed but missing."""