Skip to content

Commit

Permalink
MAINT: Make OpenMM optional again
Browse files Browse the repository at this point in the history
  • Loading branch information
mattwthompson committed Nov 13, 2024
1 parent bac6e23 commit bfff200
Show file tree
Hide file tree
Showing 18 changed files with 95 additions and 52 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ jobs:
micromamba install openmm -c conda-forge
- name: Uninstall OpenMM
if: ${{ matrix.openmm == false && matrix.openeye == true }}
if: ${{ matrix.openmm == false }}
run: |
micromamba remove openmm mdtraj
# Removing mBuild also removes some leaves, need to re-install them
Expand Down
68 changes: 41 additions & 27 deletions openff/interchange/_tests/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
RDKitToolkitWrapper,
)
from openff.utilities import get_data_file_path
from openff.utilities.exceptions import MissingOptionalDependencyError
from openff.utilities.utilities import has_executable, has_package

from openff.interchange import Interchange
Expand All @@ -38,6 +39,37 @@
reason="Test requires OE toolkit",
)

HAS_OPENMM = has_package("openmm")
HAS_GROMACS = _find_gromacs_executable() is not None
HAS_LAMMPS = has_package("lammps")
HAS_SANDER = has_executable("sander")

try:
import foyer

assert "openff/interchange" not in foyer.__file__
HAS_FOYER = True
except (ModuleNotFoundError, AssertionError):
HAS_FOYER = False

needs_openmm = pytest.mark.skipif(not HAS_OPENMM, reason="Needs OpenMM")
needs_gmx = pytest.mark.skipif(not HAS_GROMACS, reason="Needs GROMACS")
needs_not_gmx = pytest.mark.skipif(
HAS_GROMACS,
reason="Needs GROMACS to NOT be installed",
)
needs_lmp = pytest.mark.skipif(not HAS_LAMMPS, reason="Needs LAMMPS")
needs_not_lmp = pytest.mark.skipif(
HAS_LAMMPS,
reason="Needs LAMMPS to NOT be installed",
)
needs_sander = pytest.mark.skipif(not HAS_SANDER, reason="Needs sander")
needs_not_sander = pytest.mark.skipif(
HAS_SANDER,
reason="sander needs to NOT be installed",
)
needs_foyer = pytest.mark.skipif(not HAS_FOYER, reason="Needs foyer")


_rng = numpy.random.default_rng(12345)

Expand Down Expand Up @@ -96,33 +128,15 @@ def from_mapped_smiles(self, smiles, name="", **kwargs):

def get_protein(name: str) -> Molecule:
"""Get a protein from openff/toolkit/data/proteins based on PDB name."""
return Topology.from_pdb(
get_data_file_path(
relative_path=f"proteins/{name}.pdb",
package_name="openff.toolkit",
),
).molecule(0)


HAS_GROMACS = _find_gromacs_executable() is not None
HAS_LAMMPS = has_package("lammps")
HAS_SANDER = has_executable("sander")

needs_gmx = pytest.mark.skipif(not HAS_GROMACS, reason="Needs GROMACS")
needs_not_gmx = pytest.mark.skipif(
HAS_GROMACS,
reason="Needs GROMACS to NOT be installed",
)
needs_lmp = pytest.mark.skipif(not HAS_LAMMPS, reason="Needs LAMMPS")
needs_not_lmp = pytest.mark.skipif(
HAS_LAMMPS,
reason="Needs LAMMPS to NOT be installed",
)
needs_sander = pytest.mark.skipif(not HAS_SANDER, reason="Needs sander")
needs_not_sander = pytest.mark.skipif(
HAS_SANDER,
reason="sander needs to NOT be installed",
)
try:
return Topology.from_pdb(
get_data_file_path(
relative_path=f"proteins/{name}.pdb",
package_name="openff.toolkit",
),
).molecule(0)
except (ModuleNotFoundError, MissingOptionalDependencyError):
return pytest.skip("Test requires OpenMM for protein loading")


def shuffle_topology(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
from openff.units import Quantity, unit

from openff.interchange._tests import MoleculeWithConformer, needs_gmx, needs_lmp
from openff.interchange._tests import MoleculeWithConformer, needs_gmx, needs_openmm
from openff.interchange.drivers import get_gromacs_energies, get_openmm_energies


class TestLJPME:
@needs_gmx
@needs_lmp
@needs_openmm
def test_ljpme(self, sage):
sage["vdW"].periodic_method = "Ewald3D"

Expand Down
6 changes: 5 additions & 1 deletion openff/interchange/_tests/energy_tests/test_energies.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@
HAS_LAMMPS,
MoleculeWithConformer,
get_test_file_path,
needs_foyer,
needs_gmx,
needs_lmp,
needs_openmm,
)
from openff.interchange.constants import kj_mol
from openff.interchange.drivers import get_openmm_energies
Expand Down Expand Up @@ -99,7 +101,7 @@ def test_energies_single_mol(self, constrained, sage, sage_unconstrained, mol_sm
lmp_energies.compare(other_energies, tolerances)

@needs_gmx
@skip_if_missing("foyer")
@needs_foyer
@skip_if_missing("mbuild")
def test_process_rb_torsions(self, oplsaa):
"""Test that the GROMACS driver reports Ryckaert-Bellemans torsions"""
Expand Down Expand Up @@ -194,6 +196,7 @@ def test_cutoff_electrostatics(self):
],
)
@needs_gmx
@needs_openmm
@pytest.mark.slow
def test_interpolated_parameters(self, smi):
xml_ff_bo_all_heavy_bonds = """<?xml version='1.0' encoding='ASCII'?>
Expand Down Expand Up @@ -240,6 +243,7 @@ def test_interpolated_parameters(self, smi):
)

@needs_gmx
@needs_openmm
def test_rb_torsion_energies(self, monkeypatch):
monkeypatch.setenv("INTERCHANGE_EXPERIMENTAL", "1")

Expand Down
3 changes: 2 additions & 1 deletion openff/interchange/_tests/test_issues.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from openff.utilities import get_data_file_path, skip_if_missing

from openff.interchange import Interchange
from openff.interchange._tests import MoleculeWithConformer, shuffle_topology
from openff.interchange._tests import MoleculeWithConformer, needs_openmm, shuffle_topology
from openff.interchange.components._packmol import pack_box
from openff.interchange.drivers import get_openmm_energies

Expand All @@ -26,6 +26,7 @@ def test_issue_723():


@pytest.mark.parametrize("pack", [True, False])
@needs_openmm
def test_issue_1022(pack):
topology = Topology.from_molecules(
[
Expand Down
4 changes: 2 additions & 2 deletions openff/interchange/_tests/unit_tests/components/test_foyer.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from openff.utilities.testing import has_package, skip_if_missing

from openff.interchange import Interchange
from openff.interchange._tests import HAS_GROMACS, get_test_files_dir_path, needs_gmx
from openff.interchange._tests import HAS_GROMACS, get_test_files_dir_path, needs_foyer, needs_gmx
from openff.interchange.components.potentials import Potential
from openff.interchange.constants import kj_mol
from openff.interchange.drivers import get_openmm_energies
Expand All @@ -27,7 +27,7 @@
)


@skip_if_missing("foyer")
@needs_foyer
class TestFoyer:
@pytest.fixture(scope="session")
def oplsaa(self):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,8 @@ def test_to_openmm_simulation(
)

def test_add_barostat(self, sage, default_integrator, default_barostat):
pytest.importorskip("openmm")

import openmm
import openmm.unit

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@
)
def test_atom_names_with_padding(molecule):
# pytest processes fixtures before the decorator can be applied
pytest.importorskip("openmm")

if molecule.endswith(".pdb"):
molecule = Topology.from_pdb(
file_path=get_test_file_path(molecule).as_posix(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ def test_to_lammps_single_mols(
TODO: Tighten tolerances
TODO: Test periodic and non-periodic
"""
pytest.importorskip("openmm")

mol = MoleculeWithConformer.from_smiles(mol)
mol.conformers[0] -= numpy.min(mol.conformers[0], axis=0)
top = Topology.from_molecules(n_mols * [mol])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,16 @@
import pytest
from openff.toolkit import Molecule, Quantity

from openff.interchange._tests import needs_openmm
from openff.interchange.exceptions import UnsupportedImportError
from openff.interchange.interop.openmm._import import from_openmm
from openff.interchange.warnings import MissingPositionsWarning


class TestUnsupportedCases:
@pytest.mark.filterwarnings("ignore:.*are you sure you don't want to pass positions")
def test_error_topology_mismatch(self, monkeypatch, sage_unconstrained, ethanol):
monkeypatch.setenv("INTERCHANGE_EXPERIMENTAL", "1")
def test_error_topology_mismatch(self, sage_unconstrained, ethanol):
pytest.importorskip("openmm")

topology = ethanol.to_topology()
topology.box_vectors = Quantity([4, 4, 4], "nanometer")
Expand All @@ -38,9 +39,8 @@ def test_error_topology_mismatch(self, monkeypatch, sage_unconstrained, ethanol)
topology=other_topology.to_openmm(),
)

def test_found_virtual_sites(self, monkeypatch, tip4p, water):
monkeypatch.setenv("INTERCHANGE_EXPERIMENTAL", "1")

@needs_openmm
def test_found_virtual_sites(self, tip4p, water):
topology = water.to_topology()
topology.box_vectors = Quantity([4, 4, 4], "nanometer")

Expand All @@ -55,9 +55,8 @@ def test_found_virtual_sites(self, monkeypatch, tip4p, water):
topology=topology.to_openmm(),
)

def test_missing_positions_warning(self, monkeypatch, sage, water):
monkeypatch.setenv("INTERCHANGE_EXPERIMENTAL", "1")

@needs_openmm
def test_missing_positions_warning(self, sage, water):
topology = water.to_topology()
topology.box_vectors = Quantity([4, 4, 4], "nanometer")

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import pytest
from openff.toolkit import Molecule, unit

from openff.interchange._tests import needs_openmm
from openff.interchange.exceptions import NegativeMassError, UnsupportedExportError


Expand Down Expand Up @@ -80,6 +81,7 @@ def test_mass_is_positive(sage):
sage.create_interchange(Molecule.from_smiles("C").to_topology()).to_openmm(hydrogen_mass=5.0)


@needs_openmm
def test_virtual_sites_unsupported(tip4p, water):
with pytest.raises(UnsupportedExportError, match="with virtual sites"):
tip4p.create_interchange(water.to_topology()).to_openmm(hydrogen_mass=2.0)
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ def test_each_molecule_with_virtual_sites_has_its_own_funnky_virtual_site_residu
tip5p,
water,
):
pytest.importorskip("openmm")

topology = Topology.from_molecules([water, water])

for index, molecule in enumerate(topology.molecules):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import pytest
from openff.toolkit import ForceField, Quantity, unit

from openff.interchange._tests import MoleculeWithConformer
from openff.interchange._tests import MoleculeWithConformer, needs_openmm
from openff.interchange.exceptions import MissingVirtualSitesError
from openff.interchange.interop._virtual_sites import get_positions_with_virtual_sites

Expand Down Expand Up @@ -37,6 +37,7 @@ def test_nonzero_positions(tip4p_interchange):
)


@needs_openmm
def test_collate_virtual_site_positions(tip4p, water_dimer):
out = tip4p.create_interchange(water_dimer)

Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import pytest
from openff.utilities import has_package, skip_if_missing

if has_package("openmm"):
import numpy
import pytest
from openff.toolkit import Molecule

from openff.interchange._tests import MoleculeWithConformer
Expand Down
3 changes: 2 additions & 1 deletion openff/interchange/_tests/unit_tests/smirnoff/test_create.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from openff.utilities.testing import skip_if_missing

from openff.interchange import Interchange
from openff.interchange._tests import get_test_file_path
from openff.interchange._tests import get_test_file_path, needs_openmm
from openff.interchange.constants import kcal_mol, kcal_mol_a2
from openff.interchange.exceptions import (
PresetChargesError,
Expand Down Expand Up @@ -65,6 +65,7 @@ def test_infer_positions(self, sage, ethanol):
3,
)

@needs_openmm
def test_cosmetic_attributes(self):
from openff.toolkit._tests.test_forcefield import xml_ff_w_cosmetic_elements

Expand Down
6 changes: 5 additions & 1 deletion openff/interchange/components/interchange.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,11 @@
from openff.interchange.warnings import InterchangeDeprecationWarning

if has_package("foyer"):
from foyer.forcefield import Forcefield as FoyerForcefield
try:
from foyer.forcefield import Forcefield as FoyerForcefield
except ModuleNotFoundError:
# case of openff/interchange/foyer/ being detected as the real package
pass
if has_package("nglview"):
import nglview

Expand Down
8 changes: 6 additions & 2 deletions openff/interchange/foyer/_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,12 @@
from openff.interchange.models import PotentialKey, TopologyKey

if has_package("foyer"):
from foyer.exceptions import MissingForceError, MissingParametersError
from foyer.forcefield import Forcefield
try:
from foyer.exceptions import MissingForceError, MissingParametersError
from foyer.forcefield import Forcefield
except ModuleNotFoundError:
# case of openff/interchange/foyer/ being detected as the real package
pass


# Is this the safest way to achieve PotentialKey id separation?
Expand Down
13 changes: 9 additions & 4 deletions openff/interchange/interop/openmm/_import/compat.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,18 @@
import openmm
import openmm.app
from typing import TYPE_CHECKING, Union

from openff.toolkit import Topology
from openff.utilities.utilities import has_package

from openff.interchange.exceptions import UnsupportedImportError

if has_package("openmm") or TYPE_CHECKING:
import openmm
import openmm.app


def _check_compatible_inputs(
system: openmm.System,
topology: openmm.app.Topology | Topology | None,
system: "openmm.System",
topology: Union["openmm.app.Topology", Topology, None],
):
"""Check that inputs are compatible and supported."""
for index in range(system.getNumParticles()):
Expand Down

0 comments on commit bfff200

Please sign in to comment.