From 62638f50e293a6d56176681f23e43d6f6e57afaa Mon Sep 17 00:00:00 2001 From: "Matthew W. Thompson" Date: Thu, 8 Feb 2024 16:23:15 -0600 Subject: [PATCH 1/2] FIX: Improve processing boxes from OpenMM --- openff/interchange/_tests/data/README.md | 21 +++++++ openff/interchange/_tests/data/system.xml | 63 +++++++++++++++++++ .../interop/openmm/_import/test_import.py | 54 +++++++++++++++- openff/interchange/components/interchange.py | 19 +++--- .../interop/openmm/_import/_import.py | 10 ++- 5 files changed, 154 insertions(+), 13 deletions(-) create mode 100644 openff/interchange/_tests/data/system.xml diff --git a/openff/interchange/_tests/data/README.md b/openff/interchange/_tests/data/README.md index 3b02a1fa5..836402a8a 100644 --- a/openff/interchange/_tests/data/README.md +++ b/openff/interchange/_tests/data/README.md @@ -93,3 +93,24 @@ for arg in ["cube", "dodecahedron", "octahedron"]: `de-force-1.0.1.offxml` - [Source](https://github.com/jthorton/de-forcefields/blob/a6f666fc8a3f48d597bfb4db5c46826b9d5d7ed4/deforcefields/offxml/de-force-1.0.1.offxml) with vdW section removed + +`system.xml` + +Simple OpenMM System XML, written from toolkit objects. + +```python +from openff.toolkit import ForceField, Molecule, unit, Quantity +import openmm + +molecule = Molecule.from_smiles("C") + +topology = molecule.to_topology() +topology.box_vectors = Quantity([2.5, 2.5, 2.5], unit.nanometer) + + +open("system.xml", "w").write( + openmm.XmlSerializer.serialize( + ForceField("openff-2.1.0.offxml").create_openmm_system(topology) + ) +) +``` diff --git a/openff/interchange/_tests/data/system.xml b/openff/interchange/_tests/data/system.xml new file mode 100644 index 000000000..3016a8669 --- /dev/null +++ b/openff/interchange/_tests/data/system.xml @@ -0,0 +1,63 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/openff/interchange/_tests/unit_tests/interop/openmm/_import/test_import.py b/openff/interchange/_tests/unit_tests/interop/openmm/_import/test_import.py index e5833e313..2f6655670 100644 --- a/openff/interchange/_tests/unit_tests/interop/openmm/_import/test_import.py +++ b/openff/interchange/_tests/unit_tests/interop/openmm/_import/test_import.py @@ -3,7 +3,7 @@ import numpy import pytest -from openff.toolkit import Molecule, Topology, unit +from openff.toolkit import Molecule, Quantity, Topology, unit from openff.utilities import get_data_file_path, has_package, skip_if_missing from openff.interchange import Interchange @@ -29,7 +29,7 @@ def test_simple_roundtrip(self, monkeypatch, sage_unconstrained, ethanol): interchange = Interchange.from_smirnoff( sage_unconstrained, [ethanol], - box=[4, 4, 4] * unit.nanometer, + box=Quantity([4, 4, 4], unit.nanometer), ) system = interchange.to_openmm(combine_nonbonded_forces=True) @@ -56,6 +56,56 @@ def test_simple_roundtrip(self, monkeypatch, sage_unconstrained, ethanol): # OpenMM seems to avoid using the built-in type assert converted.box.m.dtype in (float, numpy.float32, numpy.float64) + @pytest.fixture() + def simple_system(self): + return openmm.XmlSerializer.deserialize( + open( + get_data_file_path( + "system.xml", + "openff.interchange._tests.data", + ), + ).read(), + ) + + @pytest.mark.parametrize("as_argument", [False, True]) + def test_different_ways_to_process_box_vectors( + self, + monkeypatch, + as_argument, + simple_system, + ): + monkeypatch.setenv("INTERCHANGE_EXPERIMENTAL", "1") + + if as_argument: + box = Interchange.from_openmm( + system=simple_system, + box_vectors=simple_system.getDefaultPeriodicBoxVectors(), + ).box + else: + box = Interchange.from_openmm(system=simple_system).box + + assert box.shape == (3, 3) + assert type(box.m[2][2]) in (float, numpy.float64, numpy.float32) + assert type(box.m[1][1]) is not Quantity + + def test_topology_and_system_box_vectors_differ( + self, + monkeypatch, + simple_system, + ): + """Ensure that, if box vectors specified in the topology and system differ, those in the topology are used.""" + monkeypatch.setenv("INTERCHANGE_EXPERIMENTAL", "1") + + topology = Molecule.from_smiles("C").to_topology() + topology.box_vectors = Quantity([4, 5, 6], unit.nanometer) + + box = Interchange.from_openmm( + system=simple_system, + topology=topology.to_openmm(), + ).box + + assert numpy.diag(box.m_as(unit.nanometer)) == pytest.approx([4, 5, 6]) + def test_openmm_roundtrip_metadata(self, monkeypatch): monkeypatch.setenv("INTERCHANGE_EXPERIMENTAL", "1") diff --git a/openff/interchange/components/interchange.py b/openff/interchange/components/interchange.py index 43521ebe6..d26cf592a 100644 --- a/openff/interchange/components/interchange.py +++ b/openff/interchange/components/interchange.py @@ -156,23 +156,24 @@ class Config: arbitrary_types_allowed = True @validator("box", allow_reuse=True) - def validate_box(cls, value): + def validate_box(cls, value) -> Optional[Quantity]: if value is None: return value - first_pass = ArrayQuantity.validate_type(value) - as_2d = np.atleast_2d(first_pass) - if as_2d.shape == (3, 3): - box = as_2d - elif as_2d.shape == (1, 3): - box = as_2d * np.eye(3) + + validated = ArrayQuantity.validate_type(value) + + dimensions = np.atleast_2d(validated).shape + + if dimensions == (3, 3): + return validated + elif dimensions == (1, 3): + return validated * np.eye(3) else: raise InvalidBoxError( f"Failed to convert value {value} to 3x3 box vectors. Please file an issue if you think this " "input should be supported and the failure is an error.", ) - return box - @validator("topology", pre=True) def validate_topology(cls, value): if value is None: diff --git a/openff/interchange/interop/openmm/_import/_import.py b/openff/interchange/interop/openmm/_import/_import.py index ccfa81ce0..932434004 100644 --- a/openff/interchange/interop/openmm/_import/_import.py +++ b/openff/interchange/interop/openmm/_import/_import.py @@ -77,9 +77,15 @@ def from_openmm( interchange.positions = positions if box_vectors is not None: - interchange.box = box_vectors + interchange.box = Interchange.validate_box(box_vectors) + elif topology is not None: + interchange.box = Interchange.validate_box( + topology.getPeriodicBoxVectors(), + ) elif system is not None: - interchange.box = system.getDefaultPeriodicBoxVectors() + interchange.box = Interchange.validate_box( + system.getDefaultPeriodicBoxVectors(), + ) return interchange From 6acb40a7dc6052ed05d72915d0bc96f4211d81d5 Mon Sep 17 00:00:00 2001 From: "Matthew W. Thompson" Date: Fri, 9 Feb 2024 09:43:02 -0600 Subject: [PATCH 2/2] FIX: Update GAFF example --- .../experimental/openmmforcefields/gaff.ipynb | 29 +++++++++++++++---- 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/examples/experimental/openmmforcefields/gaff.ipynb b/examples/experimental/openmmforcefields/gaff.ipynb index f2227c483..3c2d7a6cf 100644 --- a/examples/experimental/openmmforcefields/gaff.ipynb +++ b/examples/experimental/openmmforcefields/gaff.ipynb @@ -76,13 +76,30 @@ "from openff.interchange.interop.openmm import from_openmm\n", "\n", "imported = from_openmm(\n", - " molecule.to_topology().to_openmm(),\n", - " system_gaff,\n", - " box_vectors=None,\n", - " positions=ensure_quantity(molecule.conformers[0], \"openmm\"),\n", + " topology=topology.to_openmm(),\n", + " system=system_gaff,\n", + " positions=molecule.conformers[0].to_openmm(),\n", ")" ] }, + { + "cell_type": "markdown", + "id": "3787ec72", + "metadata": {}, + "source": [ + "This `Interchange` contains an internal representation of approximately everything contained in the `openmm.System`. Topological information (atoms, their elements, bonds, the graph they compose, etc.), forces (including non-bonded settings), and box vectors." + ] + }, + { + "cell_type": "code", + "execution_count": null, + "id": "c08d00fc", + "metadata": {}, + "outputs": [], + "source": [ + "imported.visualize()" + ] + }, { "cell_type": "markdown", "id": "579c891a", @@ -113,7 +130,7 @@ " platform=\"Reference\",\n", " ),\n", " system=system_gaff,\n", - " combine_nonbonded_forces=False,\n", + " combine_nonbonded_forces=True,\n", " detailed=False,\n", ")\n", "\n", @@ -173,7 +190,7 @@ "name": "python", "nbconvert_exporter": "python", "pygments_lexer": "ipython3", - "version": "3.11.6" + "version": "3.11.7" }, "vscode": { "interpreter": {