From 2dcc00761020cf93cee9276bf5b789bbf9666760 Mon Sep 17 00:00:00 2001 From: "Matthew W. Thompson" Date: Thu, 14 Mar 2024 16:53:35 -0500 Subject: [PATCH 1/2] API: Warn if positions not given --- .../openmm-import/protein-ligand.ipynb | 5 +++-- .../experimental/openmmforcefields/gaff.ipynb | 4 ++-- .../interop/openmm/_import/test_compat.py | 16 ++++++++++++++ .../interop/openmm/_import/_import.py | 22 ++++++++++++++----- openff/interchange/warnings.py | 4 ++++ 5 files changed, 42 insertions(+), 9 deletions(-) diff --git a/examples/experimental/openmm-import/protein-ligand.ipynb b/examples/experimental/openmm-import/protein-ligand.ipynb index 36063b709..f6f98ca7e 100644 --- a/examples/experimental/openmm-import/protein-ligand.ipynb +++ b/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", diff --git a/examples/experimental/openmmforcefields/gaff.ipynb b/examples/experimental/openmmforcefields/gaff.ipynb index 0b0bfddda..a5bdfb37b 100644 --- a/examples/experimental/openmmforcefields/gaff.ipynb +++ b/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", diff --git a/openff/interchange/_tests/unit_tests/interop/openmm/_import/test_compat.py b/openff/interchange/_tests/unit_tests/interop/openmm/_import/test_compat.py index 34798be3e..43ab075c7 100644 --- a/openff/interchange/_tests/unit_tests/interop/openmm/_import/test_compat.py +++ b/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(), + ) diff --git a/openff/interchange/interop/openmm/_import/_import.py b/openff/interchange/interop/openmm/_import/_import.py index ef0f81320..0704318c0 100644 --- a/openff/interchange/interop/openmm/_import/_import.py +++ b/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 diff --git a/openff/interchange/warnings.py b/openff/interchange/warnings.py index 9115aa284..5df8dd3bc 100644 --- a/openff/interchange/warnings.py +++ b/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.""" From 9e4cee919cd25fc294b3afd5026901146fb1241c Mon Sep 17 00:00:00 2001 From: "Matthew W. Thompson" Date: Tue, 19 Mar 2024 08:44:51 -0500 Subject: [PATCH 2/2] DOC: Update release history --- docs/releasehistory.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/releasehistory.md b/docs/releasehistory.md index 9d9ced1e7..ffff5d1b6 100644 --- a/docs/releasehistory.md +++ b/docs/releasehistory.md @@ -11,6 +11,11 @@ Dates are given in YYYY-MM-DD format. Please note that all releases prior to a version 1.0.0 are considered pre-releases and many API changes will come before a stable release. +## 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`. + ## 0.3.23 - 2024-03-07 * #923 An error is raised in `Interchange.from_openmm` when the topology and system are incompatible.