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

Add GAFF test (and GAFF skips) to CI #847

Merged
merged 18 commits into from
May 13, 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
11 changes: 9 additions & 2 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,14 @@ jobs:
os: ["ubuntu-latest"]
pydantic-version: [">1"]
python-version:
- "3.9"
Copy link
Member

Choose a reason for hiding this comment

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

Are we definitely dropping 3.9 now? What's stopping us from going 3.12? (I thought it was AmberTools but that might not be the case with these changes?)

Copy link
Contributor Author

@mikemhenry mikemhenry May 10, 2024

Choose a reason for hiding this comment

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

If we are are following https://scientific-python.org/specs/spec-0000/ then we are good to drop it.

That's a good question about 3.12, I'll open a PR and test if we can solve a 3.12 env.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

testing here #850

Copy link
Contributor Author

Choose a reason for hiding this comment

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

looks like pymbar 3.x needs a py 3.12 build https://github.com/OpenFreeEnergy/openfe/actions/runs/9033924083/job/24825299591?pr=850

I can do this, but given #833 and choderalab/pymbar#419 (comment) I think we can move to pymbar 4 soon, thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

Let's aim to get pymbar support done before the next release and temporarily shorten our support range.

Copy link
Member

Choose a reason for hiding this comment

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

Ah wait so it did work out in the end?

- "3.10"
- "3.11"
- "3.12"
openeye: ["no"]
ommff-version: ["0.13"]
include:
- os: "macos-12"
python-version: "3.11"
python-version: "3.12"
pydantic-version: ">1"
- os: "ubuntu-latest"
python-version: "3.11"
Expand All @@ -52,6 +53,11 @@ jobs:
python-version: "3.11"
pydantic-version: ">1"
openeye: "yes"
- os: "ubuntu-latest"
python-version: "3.10"
pydantic-version: ">1"
openeye: "no"
ommff-version: "<0.13"

env:
OE_LICENSE: ${{ github.workspace }}/oe_license.txt
Expand All @@ -77,6 +83,7 @@ jobs:
create-args: >-
python=${{ matrix.python-version }}
pydantic=${{ matrix.pydantic-version }}
openmmforcefields=${{ matrix.ommff-version }}
init-shell: bash

- name: "Install OpenEye"
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/mypy.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ jobs:
run: |
python -m pip install --no-deps git+https://github.com/OpenFreeEnergy/gufe@main
python -m pip install mypy
python -m pip install types-pkg_resources
IAlibay marked this conversation as resolved.
Show resolved Hide resolved
python -m pip install --no-deps -e .

- name: "Environment Information"
Expand Down
6 changes: 6 additions & 0 deletions docs/installation.rst
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,12 @@ If you already have a Mamba installation, you can install ``openfe`` with:

Note that you must run the latter line in each shell session where you want to use ``openfe``. OpenFE recommends the Mamba package manager for most users as it is orders of magnitude faster than the default Conda package manager. Mamba is a drop in replacement for Conda.

.. note::
If you plan on using GAFF to parametrize your system, you **must** install ``openmmforcefields`` version ``0.12.0``.
This will create an environment with python 3.10 and the correct version of Ambertools.

``mamba create -c conda-forge -n openfe_env openfe=1 openmmforcefields=0.12.0``

Installation with ``miniforge`` (recommended)
----------------------------------------------

Expand Down
4 changes: 2 additions & 2 deletions environment.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ dependencies:
- duecredit
- kartograf>=1.0.0
- lomap2>=3.0.0
- numpy<1.24
- numpy
- networkx
- rdkit
- pip
Expand All @@ -28,7 +28,7 @@ dependencies:
- typing-extensions
- lomap2>=2.3.0
- openmmtools
- openmmforcefields>=0.11.2
- openmmforcefields
- perses
- pooch
- py3dmol
Expand Down
27 changes: 25 additions & 2 deletions openfe/tests/protocols/test_openmm_equil_rfe_protocols.py
Original file line number Diff line number Diff line change
Expand Up @@ -245,8 +245,31 @@ def test_dry_run_gaff_vacuum(benzene_vacuum_system, toluene_vacuum_system,
)
unit = list(dag.protocol_units)[0]

with tmpdir.as_cwd():
sampler = unit.run(dry=True)['debug']['sampler']
# If we do a lot of GAFF testing, this should be refactored so we don't
IAlibay marked this conversation as resolved.
Show resolved Hide resolved
# have to copy it all over the place.
# https://github.com/OpenFreeEnergy/openfe/pull/847#issuecomment-2096810453


import openmmforcefields
from pkg_resources import packaging

ommff_version = openmmforcefields.__version__

gaff_should_fail = packaging.version.parse(
ommff_version
) >= packaging.version.parse("0.13.0")

if gaff_should_fail:
from openmmforcefields.generators.template_generators import (
GAFFNotSupportedError,
)

with pytest.raises(GAFFNotSupportedError):
with tmpdir.as_cwd():
sampler = unit.run(dry=True)["debug"]["sampler"]
else:
with tmpdir.as_cwd():
sampler = unit.run(dry=True)["debug"]["sampler"]


@pytest.mark.slow
Expand Down
26 changes: 24 additions & 2 deletions openfe/tests/protocols/test_openmm_plain_md_protocols.py
Original file line number Diff line number Diff line change
Expand Up @@ -177,8 +177,30 @@ def test_dry_run_gaff_vacuum(benzene_vacuum_system, tmpdir):
)
unit = list(dag.protocol_units)[0]

with tmpdir.as_cwd():
system = unit.run(dry=True)['debug']['system']
# If we do a lot of GAFF testing, this should be refactored so we don't
IAlibay marked this conversation as resolved.
Show resolved Hide resolved
# have to copy it all over the place.
# https://github.com/OpenFreeEnergy/openfe/pull/847#issuecomment-2096810453

import openmmforcefields
from pkg_resources import packaging

ommff_version = openmmforcefields.__version__

gaff_should_fail = packaging.version.parse(
ommff_version
) >= packaging.version.parse("0.13.0")

if gaff_should_fail:
from openmmforcefields.generators.template_generators import (
GAFFNotSupportedError,
)

with pytest.raises(GAFFNotSupportedError):
with tmpdir.as_cwd():
system = unit.run(dry=True)["debug"]["system"]
else:
with tmpdir.as_cwd():
system = unit.run(dry=True)["debug"]["system"]


@pytest.mark.parametrize('method, backend, ref_key', [
Expand Down
Loading