Add GAFF test (and GAFF skips) to CI

[mikemhenry]

Checklist

• Added a news entry

Developers certificate of origin

• I certify that this contribution is covered by the MIT License here and the Developer Certificate of Origin at https://developercertificate.org/.

[mikemhenry]

=========================== short test summary info ============================
FAILED openfe/tests/protocols/test_openmm_plain_md_protocols.py::test_dry_run_gaff_vacuum - ValueError: No registered small molecule template generators could load force field 'gaff-2.11'
Available installed force fields are:
  GAFFTemplateGenerator: ['gaff-1.4', 'gaff-1.8', 'gaff-1.81', 'gaff-2.1', 'gaff-2.11']
  SMIRNOFFTemplateGenerator: ['openff-2.1.0', 'opc3-1.0.1', 'openff-1.1.0', 'openff-2.0.0-rc.1', 'tip3p_fb-1.1.1', 'openff-2.1.1', 'openff-1.3.0', 'openff-1.0.0-RC2', 'tip5p', 'tip5p-1.0.0', 'spce', 'tip3p_fb-1.0.0', 'tip4p_ew', 'tip4p_fb-1.0.0', 'opc-1.0.0', 'opc-1.0.1', 'opc-1.0.2', 'tip3p-1.0.0', 'openff-2.0.0', 'openff-1.2.1', 'openff-1.0.0-RC1', 'openff-1.3.1-alpha.1', 'tip3p-1.0.1', 'tip4p_fb', 'tip3p_fb', 'openff-2.2.0', 'openff-1.0.1', 'openff-1.3.1', 'openff-1.1.1', 'openff-2.0.0-rc.2', 'opc3', 'opc', 'tip4p_ew-1.0.0', 'spce-1.0.0', 'tip3p', 'tip3p_fb-1.1.0', 'opc3-1.0.0', 'openff-1.0.0', 'openff-2.1.0-rc.1', 'openff-2.2.0-rc1', 'openff-1.2.0', 'tip4p_fb-1.0.1', 'ff14sb_off_impropers_0.0.1', 'ff14sb_off_impropers_0.0.3', 'ff14sb_off_impropers_0.0.4', 'ff14sb_off_impropers_0.0.2', 'smirnoff99Frosst-1.0.2', 'smirnoff99Frosst-1.0.9', 'smirnoff99Frosst-1.1.0', 'smirnoff99Frosst-1.0.5', 'smirnoff99Frosst-1.0.8', 'smirnoff99Frosst-1.0.0', 'smirnoff99Frosst-1.0.6', 'smirnoff99Frosst-1.0.3', 'smirnoff99Frosst-1.0.4', 'smirnoff99Frosst-1.0.1', 'smirnoff99Frosst-1.0.7']
  EspalomaTemplateGenerator: ['espaloma-0.3.2']
FAILED openfe/tests/protocols/test_openmm_equil_rfe_protocols.py::test_dry_run_gaff_vacuum - ValueError: No registered small molecule template generators could load force field 'gaff-2.11'
Available installed force fields are:
  GAFFTemplateGenerator: ['gaff-1.4', 'gaff-1.8', 'gaff-1.81', 'gaff-2.1', 'gaff-2.11']
  SMIRNOFFTemplateGenerator: ['openff-2.1.0', 'opc3-1.0.1', 'openff-1.1.0', 'openff-2.0.0-rc.1', 'tip3p_fb-1.1.1', 'openff-2.1.1', 'openff-1.3.0', 'openff-1.0.0-RC2', 'tip5p', 'tip5p-1.0.0', 'spce', 'tip3p_fb-1.0.0', 'tip4p_ew', 'tip4p_fb-1.0.0', 'opc-1.0.0', 'opc-1.0.1', 'opc-1.0.2', 'tip3p-1.0.0', 'openff-2.0.0', 'openff-1.2.1', 'openff-1.0.0-RC1', 'openff-1.3.1-alpha.1', 'tip3p-1.0.1', 'tip4p_fb', 'tip3p_fb', 'openff-2.2.0', 'openff-1.0.1', 'openff-1.3.1', 'openff-1.1.1', 'openff-2.0.0-rc.2', 'opc3', 'opc', 'tip4p_ew-1.0.0', 'spce-1.0.0', 'tip3p', 'tip3p_fb-1.1.0', 'opc3-1.0.0', 'openff-1.0.0', 'openff-2.1.0-rc.1', 'openff-2.2.0-rc1', 'openff-1.2.0', 'tip4p_fb-1.0.1', 'ff14sb_off_impropers_0.0.1', 'ff14sb_off_impropers_0.0.3', 'ff14sb_off_impropers_0.0.4', 'ff14sb_off_impropers_0.0.2', 'smirnoff99Frosst-1.0.2', 'smirnoff99Frosst-1.0.9', 'smirnoff99Frosst-1.1.0', 'smirnoff99Frosst-1.0.5', 'smirnoff99Frosst-1.0.8', 'smirnoff99Frosst-1.0.0', 'smirnoff99Frosst-1.0.6', 'smirnoff99Frosst-1.0.3', 'smirnoff99Frosst-1.0.4', 'smirnoff99Frosst-1.0.1', 'smirnoff99Frosst-1.0.7']
  EspalomaTemplateGenerator: ['espaloma-0.3.2']
= 2 failed, 805 passed, 61 skipped, 1 xfailed, 2 xpassed, 1557 warnings in 338.71s (0:05:38) =

[mikemhenry]

Okay so it works, as in this test is asking for gaff, so we expect it to fail, but we should be getting this error message:

        raise NotImplementedError(
            "This release (0.13.x) of openmmforcefields temporarily drops GAFF support and "
            "thereby the GAFFTemplateGenerator class. Support will be re-introduced in "
            "future releases (0.14.x). To use this class, install version 0.12.0 or older."
        )

[mikemhenry]

so need to do some work in openmmff, once we get the error we expect I will mark test tests as xfail

[mikemhenry]

FAILED openfe/tests/protocols/test_openmm_plain_md_protocols.py::test_dry_run_gaff_vacuum - openmmforcefields.generators.template_generators.GAFFNotSupportedError: This release (0.13.x) of openmmforcefields temporarily drops GAFF support and thereby the GAFFTemplateGenerator class. Support will be re-introduced in future releases (0.14.x). To use this class, install version 0.12.0 or older.
FAILED openfe/tests/protocols/test_openmm_equil_rfe_protocols.py::test_dry_run_gaff_vacuum - openmmforcefields.generators.template_generators.GAFFNotSupportedError: This release (0.13.x) of openmmforcefields temporarily drops GAFF support and thereby the GAFFTemplateGenerator class. Support will be re-introduced in future releases (0.14.x). To use this class, install version 0.12.0 or older.

wooo it works!

[mikemhenry]

I will use this PR to fix the CI so we do test gaff, but skip it on envs where we can't run the gaff tests

[mikemhenry]

Okay as part of the big openmmforcefeilds (OMMFF) dust up of 2024, we only can use GAFF with ambertools 22 on python 3.10 for reasons so what we need to do is skip the GAFF tests if we are dealing with non-ambertools 22, since only with ambertools 22 do we support GAFF, because of how our packaging works, OMMFF=0.12.0 is the version we need. So the plan is for python 3.10 tests, we test OMMFF=0.12.0, which should let us test GAFF, and on the other python versions we pull in OMFF=0.13.0

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.28%. Comparing base (e8492ea) to head (2ad523f).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #847      +/-   ##
==========================================
- Coverage   93.09%   92.28%   -0.81%     
==========================================
  Files         134      134              
  Lines        9708     9804      +96     
==========================================
+ Hits         9038     9048      +10     
- Misses        670      756      +86     

Flag	Coverage Δ
fast-tests	92.28% <100.00%> (?)
slow-tests	?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[mikemhenry]

This is the note I am adding:

[mikemhenry]

Also dropping python 3.9 from test matrix

[mikemhenry]

also adding an osx arm test

[mikemhenry]

Okay so osx-arm will have to go until I can get an arm build for citeproc-py and mypy needs a mypy --install-types I think in the CI to fix missing stubs

I don't want to push any changes until the other tests get a chance to run

[mikemhenry]

Hmm, getting openfe/tests/protocols/test_openmm_equil_rfe_protocols.py::test_dry_run_gaff_vacuum - KeyError: 'system' for ommff=0.12.0 leg of the matrix

>               system = unit.run(dry=True)["debug"]["system"]
E               KeyError: 'system'

If I print out unit.run(dry=True)["debug"] for the openfe/tests/protocols/test_openmm_equil_rfe_protocols.py test I get {'sampler': <instance of HybridRepexSampler>} and if I print unit.run(dry=True)["debug"] for the openfe/tests/protocols/test_openmm_plain_md_protocols.py test I get {'debug': {'system': <openmm.openmm.System; proxy of <Swig Object of type 'OpenMM::System *' at 0x7a602b6207e0> >}}

Any ideas @IAlibay or @hannahbaumann ?

[mikemhenry]

Good idea @hannahbaumann for re-naming that variable

[mikemhenry]

Once this gets merged in we can add osx-arm to the tests conda-forge/citeproc-py-feedstock#9

[mikemhenry]

Should I name the gaff test something different? I am worried we might get confused later

[hannahbaumann]

Maybe? I'm not sure what would be better.
Would we also need the GAFF test for the afe protocol or do you think it's not necessary there?

[IAlibay]

This looks good, just a couple of questions - I'll approve early.

[IAlibay]

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?)

[mikemhenry]

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.

[mikemhenry]

testing here #850

[mikemhenry]

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?

[IAlibay]

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

[IAlibay]

Ah wait so it did work out in the end?

[mikemhenry]

Maybe? I'm not sure what would be better.

Would we also need the GAFF test for the afe protocol or do you think it's not necessary there?

I didn't get an error so the test might not be doing what we think it does, can you link it?

[pep8speaks]

Hello @mikemhenry! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

• In the file openfe/tests/protocols/test_openmm_equil_rfe_protocols.py:

Line 253:5: E303 too many blank lines (2)

Comment last updated at 2024-05-10 22:03:51 UTC

[mikemhenry]

I can't easily add a custom name without adding something to all the CI strings, I vote until it is an issue, we just leave the 2 python 3.10 named tests and we can fix it when it becomes a problem

[IAlibay]

Thanks @mikemhenry !