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

Test against openff-interchange-0.4.0beta1 #1930

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
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
1 change: 1 addition & 0 deletions devtools/conda-envs/conda.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
name: latest-deployment
channels:
- conda-forge/label/openff-interchange_rc
- conda-forge
dependencies:
# Base depends
Expand Down
1 change: 1 addition & 0 deletions devtools/conda-envs/conda_oe.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
name: latest-deployment
channels:
- conda-forge/label/openff-interchange_rc
- conda-forge
- openeye
dependencies:
Expand Down
1 change: 1 addition & 0 deletions devtools/conda-envs/installer.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
name: constructor

channels:
- conda-forge/label/openff-interchange_rc
- defaults
- conda-forge

Expand Down
5 changes: 3 additions & 2 deletions devtools/conda-envs/openeye-examples.yaml
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
name: openeye-examples
channels:
- conda-forge/label/openff-interchange_rc
- openeye
- conda-forge
dependencies:
# Base depends
- python
- pydantic =1
- pydantic
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For both packaging and testing yamls, should we set this to pydantic >2? Or is it redundant since it will be impossible to install this with a pydantic-1-compatible Interchange?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • I think pydantic >2/pydantic >=2 would be unwise since that bets on current builds being compatible with Pydantic v3+ out of the box, which I don't trust. pydantic =2 should be fine, if a little redundant since the toolkit itself has no direct opinions on Pydantic compatibility
  • Yes, installing v1 (install-time/packaging level) will be impossible with Interchange 0.4

- packaging
- numpy
- networkx
Expand All @@ -19,7 +20,7 @@ dependencies:
- openff-amber-ff-ports >=0.0.3
- openff-units =0.2.0
- openff-utilities >=0.1.5
- openff-interchange-base >=0.3.19
- openff-interchange-base >=0.4.0beta1
- openff-nagl-base ~=0.4.0
- openff-nagl-models ==0.3.0
- typing_extensions
Expand Down
5 changes: 3 additions & 2 deletions devtools/conda-envs/openeye.yaml
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
name: openff-toolkit-test-openeye
channels:
- conda-forge/label/openff-interchange_rc
- openeye
- conda-forge
dependencies:
# Base depends
- python
- pydantic =1
- pydantic
- packaging
- numpy
- networkx
Expand All @@ -19,7 +20,7 @@ dependencies:
- openff-units =0.2.0
- openff-amber-ff-ports
- openff-utilities >=0.1.5
- openff-interchange-base >=0.3.19
- openff-interchange-base >=0.4.0beta1
- openff-nagl-base ~=0.4.0
- openff-nagl-models ==0.3.0
- typing_extensions
Expand Down
5 changes: 3 additions & 2 deletions devtools/conda-envs/rdkit-examples.yaml
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
name: rdkit-examples
channels:
- conda-forge/label/openff-interchange_rc
- conda-forge
dependencies:
# Base depends
- python
- pydantic =1
- pydantic
- packaging
- numpy
- networkx
Expand All @@ -18,7 +19,7 @@ dependencies:
- openff-amber-ff-ports >=0.0.3
- openff-units =0.2.0
- openff-utilities >=0.1.5
- openff-interchange-base >=0.3.19
- openff-interchange-base >=0.4.0beta1
- openff-nagl-base ~=0.4.0
- openff-nagl-models ==0.3.0
- typing_extensions
Expand Down
5 changes: 3 additions & 2 deletions devtools/conda-envs/rdkit.yaml
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
name: openff-toolkit-test-rdkit
channels:
- conda-forge/label/openff-interchange_rc
- conda-forge
dependencies:
# Base depends
- python
- pydantic =1
- pydantic
- packaging
- numpy
- networkx
Expand All @@ -18,7 +19,7 @@ dependencies:
- openff-units =0.2.0
- openff-amber-ff-ports
- openff-utilities >=0.1.5
- openff-interchange-base >=0.3.19
- openff-interchange-base >=0.4.0beta1
- openff-nagl-base ~=0.4.0
- openff-nagl-models ==0.3.0
- typing_extensions
Expand Down
1 change: 1 addition & 0 deletions devtools/conda-envs/release-build.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
name: release-build
channels:
- conda-forge/label/openff-interchange_rc
- conda-forge

dependencies:
Expand Down
5 changes: 3 additions & 2 deletions devtools/conda-envs/test_env.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
name: openff-toolkit-test
channels:
- conda-forge/label/openff-interchange_rc
- openeye
- conda-forge
dependencies:
Expand All @@ -19,7 +20,7 @@ dependencies:
- openff-units =0.2.0
- openff-amber-ff-ports
- openff-utilities >=0.1.5
- openff-interchange-base >=0.3.19
- openff-interchange-base >=0.4.0beta1
- openff-nagl-base ~=0.4.0
- openff-nagl-models ==0.3.0
# Toolkit-specific
Expand Down Expand Up @@ -47,7 +48,7 @@ dependencies:
- nomkl
- mypy
- typing_extensions
- pydantic =1
- pydantic
- pip:
- types-setuptools
- types-toml
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
complex.*
complex_pointenergy.mdp
receptor.*
ligand.*
protein.pdb
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,11 @@
"repo_url = (\n",
" \"https://raw.githubusercontent.com/MobleyLab/benchmarksets/master/input_files/\"\n",
")\n",
"\n",
"sources = {\n",
" \"receptor.pdb\": repo_url + \"BRD4/pdb/BRD4.pdb\",\n",
" \"ligand.pdb\": repo_url + \"BRD4/pdb/ligand-1.pdb\",\n",
" \"ligand.sdf\": repo_url + \"BRD4/sdf/ligand-1.sdf\",\n",
" \"receptor.pdb\": f\"{repo_url}BRD4/pdb/BRD4.pdb\",\n",
" \"ligand.pdb\": f\"{repo_url}BRD4/pdb/ligand-1.pdb\",\n",
" \"ligand.sdf\": f\"{repo_url}BRD4/sdf/ligand-1.sdf\",\n",
"}\n",
"for filename, url in sources.items():\n",
" r = requests.get(url)\n",
Expand All @@ -40,44 +41,20 @@
"metadata": {},
"outputs": [],
"source": [
"from openff.toolkit import ForceField, Molecule, Topology"
]
},
{
"cell_type": "markdown",
"metadata": {},
"source": [
"Later we will use `Interchange.__add__`, which is an experimental feature. It needs to be turned on by setting the environment variable below, and by doing so we accept [some stability and accuracy risks](https://docs.openforcefield.org/projects/interchange/en/stable/using/experimental.html)."
"from openff.toolkit import ForceField, Molecule, Quantity, Topology"
]
},
{
"cell_type": "code",
"execution_count": 3,
"metadata": {},
"outputs": [
{
"name": "stdout",
"output_type": "stream",
"text": [
"env: INTERCHANGE_EXPERIMENTAL=1\n"
]
}
],
"source": [
"%env INTERCHANGE_EXPERIMENTAL=1"
]
},
{
"cell_type": "code",
"execution_count": 4,
"metadata": {
"tags": []
},
"outputs": [
{
"data": {
"application/vnd.jupyter.widget-view+json": {
"model_id": "2f3b86c392c24705b86721395cf43ea4",
"model_id": "48226cb62a2849029c42d579843ef0f7",
"version_major": 2,
"version_minor": 0
},
Expand All @@ -90,8 +67,8 @@
"name": "stderr",
"output_type": "stream",
"text": [
"/Users/jeffreywagner/projects/OpenForceField/openff-interchange/openff/interchange/_experimental.py:35: UserWarning: Interchange object combination is experimental and likely to produce strange results. Any workflow using this method is not guaranteed to be suitable for production. Use with extreme caution and thoroughly validate results!\n",
" return func(*args, **kwargs)\n"
"/Users/mattthompson/micromamba/envs/openff-toolkit-test/lib/python3.10/site-packages/openff/interchange/operations/_combine.py:52: InterchangeCombinationWarning: Interchange object combination is complex and likely to produce strange results outside of a limited set of use cases it has been tested in. Any workflow using this method is not guaranteed to be suitable for production or stable between versions. Use with extreme caution and thoroughly validate results!\n",
" warnings.warn(\n"
]
}
],
Expand All @@ -107,32 +84,32 @@
"\n",
"receptor = ff14sb.create_interchange(topology=receptor_topology)\n",
"\n",
"complex_system = receptor.combine(ligand)\n",
"\n",
"# TODO\n",
"# complex.box = pdbfile box vectors ...\n",
"# complex.positions = np.vstack([receptor.positions, ligand.positions])"
"# Interchange.combine works nicely for some cases but hasn't been tested for all - use caution\n",
"# if adapting this for more novel workflows!\n",
"complex_system = receptor.combine(ligand)"
]
},
{
"cell_type": "markdown",
"metadata": {},
"source": [
"### Export to OpenMM"
"### Export to OpenMM\n",
"\n",
"See [docs](https://docs.openforcefield.org/projects/interchange/en/stable/using/output.html#openmm) for more options."
]
},
{
"cell_type": "code",
"execution_count": 5,
"execution_count": 4,
"metadata": {},
"outputs": [
{
"data": {
"text/plain": [
"<openmm.openmm.System; proxy of <Swig Object of type 'OpenMM::System *' at 0x14380adf0> >"
"<openmm.openmm.System; proxy of <Swig Object of type 'OpenMM::System *' at 0x14ba376f0> >"
]
},
"execution_count": 5,
"execution_count": 4,
"metadata": {},
"output_type": "execute_result"
}
Expand All @@ -147,31 +124,35 @@
"tags": []
},
"source": [
"### Export to Amber"
"### Export to Amber\n",
"\n",
"See [docs](https://docs.openforcefield.org/projects/interchange/en/stable/using/output.html#amber) for more options."
]
},
{
"cell_type": "code",
"execution_count": 6,
"execution_count": 5,
"metadata": {},
"outputs": [],
"source": [
"# TODO: Fix inferring residue information with mixed topology\n",
"if False:\n",
" complex_system.to_inpcrd(\"complex.inpcrd\")\n",
" complex_system.to_prmtop(\"complex.prmtop\")"
"complex_system.to_inpcrd(\"complex.inpcrd\")\n",
"complex_system.to_prmtop(\"complex.prmtop\")"
]
},
{
"cell_type": "markdown",
"metadata": {},
"source": [
"### Export to GROMACS"
"### Export to GROMACS\n",
"\n",
"See [docs](https://docs.openforcefield.org/projects/interchange/en/stable/using/output.html#gromacs) for more options.\n",
"\n",
"GROMACS does not support vacuum simulations. In this example, we didn't add solvent and the original PDB file didn't have box vectors. We'll just arbitrarily add a 4 nm cubic box around the receptor-ligand complex."
]
},
{
"cell_type": "code",
"execution_count": 7,
"execution_count": 6,
"metadata": {
"tags": []
},
Expand All @@ -180,14 +161,16 @@
"name": "stderr",
"output_type": "stream",
"text": [
"/Users/jeffreywagner/projects/OpenForceField/openff-interchange/openff/interchange/interop/gromacs/export/_export.py:48: UserWarning: WARNING: System defined with no box vectors, which GROMACS does not offically support in versions 2020 or newer (see https://gitlab.com/gromacs/gromacs/-/issues/3526). Setting box vectors to a 5 nm cube.\n",
" self._write_gro(gro, decimal)\n"
"/Users/mattthompson/micromamba/envs/openff-toolkit-test/lib/python3.10/site-packages/openff/interchange/components/mdconfig.py:471: UserWarning: Ambiguous failure while processing constraints. Constraining h-bonds as a stopgap.\n",
" warnings.warn(\n"
]
}
],
"source": [
"complex_system.to_gro(\"complex.gro\")\n",
"complex_system.to_top(\"complex.top\")"
"complex_system.box = Quantity([4, 4, 4], \"nanometer\")\n",
"\n",
"# This step might be slow, writing some large proteins is not yet optimized\n",
"complex_system.to_gromacs(\"complex\")"
]
}
],
Expand All @@ -208,7 +191,7 @@
"name": "python",
"nbconvert_exporter": "python",
"pygments_lexer": "ipython3",
"version": "3.10.10"
"version": "3.10.14"
}
},
"nbformat": 4,
Expand Down
7 changes: 2 additions & 5 deletions openff/toolkit/_tests/test_forcefield.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from openff.units.openmm import from_openmm, to_openmm
from openmm import NonbondedForce, Platform, XmlSerializer, app
from openmm import unit as openmm_unit
from pydantic import ValidationError

from openff.toolkit import unit
from openff.toolkit._tests.create_molecules import (
Expand Down Expand Up @@ -4028,15 +4029,11 @@ def test_fractional_bondorder_invalid_interpolation_method(self):
# This error will be either from v1 of the package (if v1 is installed)
# or the faked v1 API (if v2 is installed). Ensure the v1 error or a
# mimick of it is imported
try:
from pydantic.v1 import ValidationError
except ImportError:
from pydantic import ValidationError

# If important, this can be a custom exception instead of a verbose ValidationError
with pytest.raises(
ValidationError,
match="given=invalid method name",
match="Input should be 'linear'.*input_value='invalid method name'",
):
forcefield.create_openmm_system(
topology,
Expand Down
8 changes: 5 additions & 3 deletions openff/toolkit/typing/engines/smirnoff/forcefield.py
Original file line number Diff line number Diff line change
Expand Up @@ -1399,7 +1399,7 @@ def get_partial_charges(self, molecule: "Molecule", **kwargs: Any) -> Quantity:
"""
from openff.interchange import Interchange

from openff.toolkit import Molecule, unit
from openff.toolkit import Molecule

if not isinstance(molecule, Molecule):
raise ValueError(
Expand All @@ -1412,9 +1412,11 @@ def get_partial_charges(self, molecule: "Molecule", **kwargs: Any) -> Quantity:
c.m
for c in Interchange.from_smirnoff(
force_field=self, topology=[molecule], **kwargs
)["Electrostatics"].charges.values()
)["Electrostatics"]
._get_charges()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(not blocking) Is there still a public API point for getting the charges? Accessing the private API is a bit brittle so I'd prefer a public route if possible.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll look into why this change was necessary; it shouldn't have changed.

A try/except should suffice as a compatibility workaround in the interim if you're open to that

.values()
],
unit.elementary_charge,
"elementary_charge",
)

def __getitem__(self, val: Union[str, ParameterHandler]) -> ParameterHandler:
Expand Down