Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warn when switching function cannot be applied #914

Merged
merged 15 commits into from
Mar 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .github/workflows/beta.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ jobs:
- "3.9"
- "3.10"
- "3.11"
pydantic-version:
- "2"
openeye:
- true
- false
Expand All @@ -37,9 +39,9 @@ jobs:
uses: mamba-org/setup-micromamba@v1
with:
environment-file: devtools/conda-envs/beta_env.yaml
channel-priority: "flexible"
create-args: >-
python=${{ matrix.python-version }}
pydantic=${{ matrix.pydantic-version }}

- name: Install package
run: |
Expand Down
2 changes: 1 addition & 1 deletion devtools/conda-envs/beta_env.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ dependencies:
- panedr
# Examples
- nglview
- pytest
- pytest =8.0
- nbval
# Typing
- mypy
Expand Down
2 changes: 1 addition & 1 deletion devtools/conda-envs/dev_env.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ dependencies:
- mdtraj
- intermol
- openeye-toolkits >=2023.2
- pytest
- pytest =8.0
- pytest-cov
- pytest-xdist
- pytest-randomly
Expand Down
2 changes: 1 addition & 1 deletion devtools/conda-envs/examples_env.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ dependencies:
- pdbfixer
- nglview
- openeye-toolkits >=2023.2
- pytest
- pytest =8.0
- pytest-xdist
- nbval
- openmmforcefields >=0.11.2
Expand Down
2 changes: 1 addition & 1 deletion devtools/conda-envs/test_env.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ dependencies:
- mdtraj
- intermol
- jax
- pytest
- pytest =8.0
- pytest-cov
- pytest-xdist
- pytest-randomly
Expand Down
2 changes: 2 additions & 0 deletions docs/releasehistory.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,11 @@ Please note that all releases prior to a version 1.0.0 are considered pre-releas

## Current development

* #912 A warning is raised when writing an input/run file (not data file) to an engine that does not implement a switching function described by SMIRNOFF.
* #916 Some internal code paths are re-organized, including removing the `openff.interchange.interop.internal` submodule.
* #916 Improves speed of `Interchange.to_lammps`, particularly for larger systems.
* #915 Deprecates `Interchange.__add__` in favor of `Interchange.combine`.
* #897 Improves energy evaluation with LAMMPS when some bonds are constrained.

## 0.3.22 - 2023-02-27

Expand Down
4 changes: 4 additions & 0 deletions docs/using/output.md
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ should be modified for the desired simulation:
mdconfig.write_lammps_input(input_file="auto_generated.in")
```

Note that LAMMPS does not seem to implement a switching function as [commonly used](https://openforcefield.github.io/standards/standards/smirnoff/#vdw) by SMIRNOFF force fields.

## OpenMM

An [`Interchange`] object can be converted to an `openmm.System` object with
Expand Down Expand Up @@ -121,6 +123,8 @@ should be modified for the desired simulation:
mdconfig.write_sander_input_file(input_file="auto_generated.in")
```

Note that Amber does not implement a switching function as [commonly used](https://openforcefield.github.io/standards/standards/smirnoff/#vdw) by SMIRNOFF force fields.

<!--
## CHARMM

Expand Down
14 changes: 7 additions & 7 deletions docs/using/status.md
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
# Capabilities

| Engine | Known bugs | HMR | 4-site water models | 5-site water models | Virtual sites on ligands | Proteins | Nucleic acids | Lipids | Carbohydrates |
|:--|:--|:--|:--|:--|:--|:--|:--|:--|:--|
| All | | | | | | <td colspan=3>No supported SMIRNOFF force fields |
| OpenMM | [Link](https://github.com/openforcefield/openff-interchange/issues?q=is%3Aissue+is%3Aopen+label%3Aopenmm+label%3Abug) | Mostly<sup>2</sup> supported | Supported | Supported | Supported | Supported |
| GROMACS | [Link](https://github.com/openforcefield/openff-interchange/issues?q=is%3Aissue+is%3Aopen+label%3Agromacs+label%3Abug) | Mostly<sup>2</sup> supported | Partially supported | Supported | Partially supported | Supported |
| Amber | [Link](https://github.com/openforcefield/openff-interchange/issues?q=is%3Aissue+is%3Aopen+label%3Aamber+label%3Abug) | Unsupported | Unsupported<sup>1</sup> | Unsupported<sup>1</sup> | Unsupported<sup>1</sup> | Supported |
| LAMMPS | [Link](https://github.com/openforcefield/openff-interchange/issues?q=is%3Aissue+is%3Aopen+label%3Alammps+label%3Abug) | Unsupported | Not supported | Not supported | Not supported | Not tested |
| Engine | Known bugs | HMR | Bond constraints | 4-site water models | 5-site water models | Virtual sites on ligands | Proteins | Nucleic acids | Lipids | Carbohydrates |
|:--|:--|:--|:--|:--|:--|:--|:--|:--|:--|:--|
| OpenMM | [Link](https://github.com/openforcefield/openff-interchange/issues?q=is%3Aissue+is%3Aopen+label%3Aopenmm+label%3Abug) | Mostly<sup>2</sup> supported | Supported | Supported | Supported | Supported | Supported |
| GROMACS | [Link](https://github.com/openforcefield/openff-interchange/issues?q=is%3Aissue+is%3Aopen+label%3Agromacs+label%3Abug) | Mostly<sup>2</sup> supported | Partially supported | Partially supported | Supported | Partially supported | Supported |
| Amber | [Link](https://github.com/openforcefield/openff-interchange/issues?q=is%3Aissue+is%3Aopen+label%3Aamber+label%3Abug) | Unsupported | Partially supported | Unsupported<sup>1</sup> | Unsupported<sup>1</sup> | Unsupported<sup>1</sup> | Supported |
| LAMMPS | [Link](https://github.com/openforcefield/openff-interchange/issues?q=is%3Aissue+is%3Aopen+label%3Alammps+label%3Abug) | Unsupported | Partially supported | Not supported | Not supported | Not supported | Not tested |
| All | | | | | | | <td colspan=3>No available SMIRNOFF force fields |

1. Unable to find upstream documentation
2. See caveats in `Interchange.to_openmm` and `Interchange.to_gromacs` docstrings.
6 changes: 6 additions & 0 deletions openff/interchange/_tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,12 @@ def sage_unconstrained():
return ForceField("openff_unconstrained-2.0.0.offxml")


@pytest.fixture()
def sage_no_switch(sage):
sage["vdW"].switch_width = Quantity(0.0, "angstrom")
return sage


@pytest.fixture()
def sage_with_bond_charge(sage):
sage["Bonds"].add_parameter(
Expand Down
10 changes: 7 additions & 3 deletions openff/interchange/_tests/data/ions.offxml
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
<?xml version="1.0" encoding='ASCII'?>
<SMIRNOFF version="0.3" aromaticity_model="OEAroModel_MDL">
<vdW version="0.3" potential="Lennard-Jones-12-6" combining_rules="Lorentz-Berthelot" scale12="0.0" scale13="0.0" scale14="0.5" scale15="1" switch_width="1.0 * angstroms" cutoff="9.0 * angstroms" method="cutoff">
<Atom smirks="[*:1]" id="n1" sigma="0.1 * nanometers" epsilon="0.0 * kilojoules_per_mole" />
<Bonds version="0.4" potential="harmonic" fractional_bondorder_method="AM1-Wiberg" fractional_bondorder_interpolation="linear">
</Bonds>
<Angles version="0.3" potential="harmonic">
</Angles>
<vdW version="0.4" potential="Lennard-Jones-12-6" combining_rules="Lorentz-Berthelot" scale12="0.0" scale13="0.0" scale14="0.5" scale15="1.0" cutoff="9.0 * angstrom ** 1" switch_width="1.0 * angstrom ** 1" periodic_method="cutoff" nonperiodic_method="no-cutoff">
<Atom smirks="[*:1]" epsilon="0.0868793154488 * mole ** -1 * kilocalorie ** 1" id="n14" rmin_half="1.953447017081 * angstrom ** 1"></Atom>
</vdW>
<Electrostatics version="0.3" scale12="0.0" scale13="0.0" scale14="0.833333" scale15="1.0" cutoff="9.0 * angstrom" switch_width="0.0 * angstrom" method="PME"></Electrostatics>
<Electrostatics version="0.4" scale12="0.0" scale13="0.0" scale14="0.8333333333" scale15="1.0" cutoff="9.0 * angstrom ** 1" switch_width="0.0 * angstrom ** 1" periodic_potential="Ewald3D-ConductingBoundary" nonperiodic_potential="Coulomb" exception_potential="Coulomb"></Electrostatics>
<LibraryCharges version="0.3">
<LibraryCharge name="foobar" smirks="[#3:1]" charge1="1.0*elementary_charge"/>
<LibraryCharge name="foobar" smirks="[#17:1]" charge1="-1.0*elementary_charge"/>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
"""Energy tests specifically relevant to Amber behaviors."""
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import pytest
from openff.toolkit import Quantity
from openff.utilities import skip_if_missing

from openff.interchange._tests import MoleculeWithConformer, requires_ambertools
from openff.interchange.constants import kcal_mol
from openff.interchange.drivers import get_amber_energies, get_openmm_energies


@requires_ambertools
@skip_if_missing("openmm")
@pytest.mark.parametrize(
"smiles",
["c1ccccc1", "C1#CC#CC#C1", "C1=CC=C2C(=C1)C=CC3=C2C=CC4=CC=CC=C43"],
)
def test_polycyclic_nonbonded(smiles, sage_no_switch):
molecule = MoleculeWithConformer.from_smiles(smiles)
topology = molecule.to_topology()
topology.box_vectors = Quantity([10, 10, 10], "nanometer")

interchange = sage_no_switch.create_interchange(topology)

openmm_vdw = get_openmm_energies(
interchange,
combine_nonbonded_forces=False,
).energies["vdW"]

amber_vdw = get_amber_energies(interchange).energies["vdW"]

assert openmm_vdw.m == pytest.approx(
amber_vdw.m,
rel=1e-5,
), f"{openmm_vdw.m_as(kcal_mol)=}, {amber_vdw.m_as(kcal_mol)=}"
82 changes: 71 additions & 11 deletions openff/interchange/_tests/unit_tests/components/test_mdconfig.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import warnings
from typing import Union

import pytest
Expand All @@ -10,6 +11,7 @@
get_smirnoff_defaults,
)
from openff.interchange.constants import _PME
from openff.interchange.warnings import SwitchingFunctionNotImplementedWarning


@pytest.fixture()
Expand Down Expand Up @@ -241,22 +243,80 @@ def test_unconstrained_ligand_rigid_water_box(
assert options["cntrl"]["ntf"] == "2"
assert options["cntrl"]["ntc"] == "1"

def test_fswitch_negative_when_no_switching_function(
def test_switching_function_warning(
self,
unconstrained_ligand_rigid_water_box,
system_no_constraints,
):
"""Reproduce issue 745."""
MDConfig.from_interchange(
unconstrained_ligand_rigid_water_box,
).write_sander_input_file("yes.in")
with pytest.warns(
SwitchingFunctionNotImplementedWarning,
):
MDConfig.from_interchange(
system_no_constraints,
).write_sander_input_file("yes.in")

# With OpenFF defaults, the switch starts at 8 A and the cutoff is at 9 A
assert parse_sander("yes.in")["cntrl"]["fswitch"] == "8.0"
# This should always be -1 (turned off) since Amber does not implement
# a switching function on the potential
assert parse_sander("yes.in")["cntrl"]["fswitch"] == "-1.0"

unconstrained_ligand_rigid_water_box["vdW"].switch_width = 0.0 * unit.nanometer
system_no_constraints["vdW"].switch_width = 0.0 * unit.nanometer

MDConfig.from_interchange(
unconstrained_ligand_rigid_water_box,
).write_sander_input_file("no.in")
# this pattern forces warnings to become errors
with warnings.catch_warnings():
warnings.simplefilter("error")

MDConfig.from_interchange(
system_no_constraints,
).write_sander_input_file("no.in")

assert parse_sander("no.in")["cntrl"]["fswitch"] == "-1.0"


class TestWriteLAMMPSInput:

def test_switching_function_warning(
self,
system_no_constraints,
tmp_path,
):
with pytest.warns(
SwitchingFunctionNotImplementedWarning,
):
MDConfig.from_interchange(
system_no_constraints,
).write_lammps_input(
interchange=system_no_constraints,
input_file=tmp_path / "yes.in",
)

def test_no_error_if_no_constraints(
self,
system_no_constraints,
tmp_path,
):
MDConfig.from_interchange(system_no_constraints).write_lammps_input(
interchange=system_no_constraints,
input_file=tmp_path / ".inp",
)

def test_error_if_unsupported_constrained(
self,
rigid_water_box,
constrained_ligand_rigid_water_box,
unconstrained_ligand_rigid_water_box,
tmp_path,
):
for system in [
rigid_water_box,
constrained_ligand_rigid_water_box,
unconstrained_ligand_rigid_water_box,
]:

with pytest.raises(
NotImplementedError,
match="unsupported constraints case",
):
MDConfig.from_interchange(system).write_lammps_input(
system,
tmp_path / ".inp",
)
14 changes: 5 additions & 9 deletions openff/interchange/_tests/unit_tests/drivers/test_all.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@

import pandas
import pytest
from openff.toolkit.topology import Molecule
from openff.toolkit import Quantity
from openff.utilities.testing import skip_if_missing

from openff.interchange import Interchange
from openff.interchange._tests import (
HAS_GROMACS,
HAS_LAMMPS,
HAS_SANDER,
MoleculeWithConformer,
needs_gmx,
needs_lmp,
needs_not_gmx,
Expand All @@ -29,16 +29,12 @@
class TestDriversAll:
@pytest.fixture()
def basic_interchange(self, sage_unconstrained):
molecule = Molecule.from_smiles("CCO")
molecule.generate_conformers(n_conformers=1)
molecule = MoleculeWithConformer.from_smiles("CCO")
molecule.name = "MOL"
topology = molecule.to_topology()
topology.box_vectors = Quantity([4, 4, 4], "nanometer")

out = Interchange.from_smirnoff(sage_unconstrained, topology)
out.positions = molecule.conformers[0]
out.box = [4, 4, 4]

return out
return sage_unconstrained.create_interchange(topology)

@needs_gmx
@needs_lmp
Expand Down
14 changes: 12 additions & 2 deletions openff/interchange/_tests/unit_tests/drivers/test_lammps.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import pytest
from openff.units import unit
from openff.toolkit import Quantity, unit

from openff.interchange._tests import MoleculeWithConformer, needs_lmp
from openff.interchange.constants import kj_mol
from openff.interchange.drivers.lammps import _process
from openff.interchange.drivers.lammps import _process, get_lammps_energies


class TestProcess:
Expand Down Expand Up @@ -46,3 +47,12 @@ def test_detailed(self, dummy_energies):
dummy_energies,
detailed=True,
)


class TestLAMMPSDriver:
@needs_lmp
def test_can_write_without_torsions(self, sage):
topology = MoleculeWithConformer.from_smiles("N").to_topology()
topology.box_vectors = Quantity([4, 4, 4], unit.nanometer)

assert get_lammps_energies(sage.create_interchange(topology))["Bond"].m == 0.0
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
"""Tests with LAMMPS."""
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
"""Tests with LAMMPS exports."""
2 changes: 1 addition & 1 deletion openff/interchange/components/interchange.py
Original file line number Diff line number Diff line change
Expand Up @@ -500,7 +500,7 @@ def to_gro(self, file_path: Union[Path, str], decimal: int = 3):
def to_lammps(self, file_path: Union[Path, str], writer="internal"):
"""Export this Interchange to a LAMMPS data file."""
if writer == "internal":
from openff.interchange.interop.lammps.export import to_lammps
from openff.interchange.interop.lammps import to_lammps

to_lammps(self, file_path)
else:
Expand Down
Loading
Loading