Add support for Foyer forcefields #517

aehogan · 2023-07-17T21:43:18Z

Description

Add support for Foyer forcefields

Todos

Add ForceFieldSource and build system protocol
Add tests
Debugging

Questions

When running a test enthalpy of vaporization calculation I get NotImplementedError() in assign_parameters_gas

Status

Ready to go

for more information, see https://pre-commit.ci

mattwthompson · 2023-07-17T21:56:48Z

Wow, this is a nice surprise!

Make sure you add foyer into the test environment.

Don't worry about the OpenEye parts of the matrix, but you can force the tests to run in those cases by adding if: always() to that step in the workflow.

codecov · 2023-07-18T15:58:37Z

Codecov Report

Merging #517 (de3e374) into main (c6fb943) will decrease coverage by 2.48%.
The diff coverage is 93.82%.

Additional details and impacted files

aehogan · 2023-07-18T16:11:01Z

@mattwthompson with the current code the tests run fine

assign_parameters = BuildFoyerSystem("assign_parameters")
assign_parameters.force_field_path = force_field_source_path
assign_parameters.coordinate_file_path = build_coordinates.coordinate_file_path
assign_parameters.substance = substance
assign_parameters.execute(directory)

but when I run a delta H vaporization simulation using my production script I get

a19a99f346ff46e3819f8fb530c67ba5|assign_parameters_gas failed to execute.

Traceback (most recent call last):
File "[...]/lib/python3.9/site-packages/openff/evaluator/workflow/protocols.py", line 1193, in _execute_protocol
protocol.execute(directory, available_resources)
File "[...]/lib/python3.9/site-packages/openff/evaluator/workflow/protocols.py", line 680, in execute
self._execute(directory, available_resources)
File "[...]/lib/python3.9/site-packages/openff/evaluator/protocols/forcefield.py", line 246, in _execute
raise NotImplementedError()
NotImplementedError

Would you know off hand why that is?

…ou want)

mattwthompson · 2023-07-18T16:32:29Z

The traceback looks like it's not coming from a development/editable install - does from openff.evaluator import __file__; print(__file__) point to your clone of this repo?

aehogan · 2023-07-18T17:28:35Z

Yes, I pip installed it to test it with the production script

openff/evaluator/forcefield/__init__.py

openff/evaluator/forcefield/forcefield.py

openff/evaluator/protocols/forcefield.py

openff/evaluator/tests/test_protocols/test_forcefield.py

for more information, see https://pre-commit.ci

Co-authored-by: Matt Thompson <matt.thompson@openforcefield.org>

for more information, see https://pre-commit.ci

… other mixing rules

mattwthompson · 2023-07-19T15:28:54Z

The error I'm seeing in tests:

openff.interchange.exceptions.UnsupportedCutoffMethodError: When using openmm.CustomNonbondedForce, vdW and electrostatics cutoff methods must agree on whether or not periodic boundary conditions should be used.OpenMM will throw an error. Found vdw method 1, and electrostatics method 0,

usually stems from the periodicity not being set. If Topology.box_vectors is set, which I assume it would be after the PACKMOL protocol finishes, then Interchange.box should also be set. Unless the error bubbles out from a system that is not periodic, in which case you're running up against a nasty confluence of corner cases in which the correct behavior is undefined (openforcefield/standards#7). We're hoping to resolve this within a month or so (we have a meeting in two weeks about it) but as a short-term stop-gap, you could just set the gas-phase calculations to have a large (say, 5 nm or greater) periodic box. If you choose this option please note it in a comment in the code so it can be reverted later.

mattwthompson · 2023-07-19T21:32:15Z

openff/evaluator/tests/test_protocols/test_forcefield.py

+ <HarmonicBondForce>
+  <Bond class1="OW_tip3p" class2="HW_tip3p" length="0.09572" k="1884.06"/>
+ </HarmonicBondForce>
+ <HarmonicAngleForce>
+  <Angle class1="HW_tip3p" class2="OW_tip3p" class3="HW_tip3p" angle="1.82421813" k="230.274"/>
+ </HarmonicAngleForce>


TIP3P is supposed to be a fixed model, but I don't think Foyer support constraints in its force field schema? We always did that at simulation runtime, I think

The test builds a rigid tip3p model HOWEVER this seems due to a short circuit in TemplateBuildSystem._execute that forces all water to be rigid tip3p with TemplateBuildSystem._build_tip3p_system. Is the intent to always override any water with tip3p in evaluator?

I don't think that's the intent

mattwthompson · 2023-07-19T21:35:33Z

openff/evaluator/protocols/forcefield.py

        interchange = Interchange.from_foyer(topology=topology, force_field=force_field)

+        openmm_pdb_file = app.PDBFile(self.coordinate_file_path)
+        if openmm_pdb_file.topology.getPeriodicBoxVectors() is not None:
+            interchange.box = openmm_pdb_file.topology.getPeriodicBoxVectors()
+


Does this mean Interchange.from_foyer isn't parsing the box vectors on the topology argument? If so, I think that's a bug

topology is not an argument to _parameterize_molecule, just the molecule itself which contains no box information, the better fix would be to pass the topology to _parameterize_molecule but I'm trying to make the smallest changes I can

the topology being passed to interchange is from molecule.to_topology() not from TemplateBuildSystem._execute.topology (which isn't passed as an argument to _parameterize_molecule)

That makes sense, I was misreading the code then

…e to _append_system

for more information, see https://pre-commit.ci

… properly

for more information, see https://pre-commit.ci

mattwthompson · 2023-07-20T16:31:22Z

@aehogan I think this is ready to go? Is there anything more you were looking to do, and have you been able to use this in your studies? If there aren't issues you've ran into, I think I'll give this another review and look to merge it into a release soon. How does that sound to you?

The changeset is smaller than I originally anticipated, but thinking through it again, it shouldn't be a surprise that it doesn't need to do much more than swap out the force field source.

aehogan · 2023-07-20T17:23:45Z

@mattwthompson There are still one or two more issues with running a full delta H vaporization simulation I need to iron out.

Besides that I was going to change out the xml test with a methane so that part of the code is actually tested.

mattwthompson · 2023-07-20T17:29:53Z

Sounds good, let me know when you think it's ready - no rush

…hane, add energy minimization to oplsaa test to ensure openmm successfully runs

…tem is built correctly

aehogan · 2023-07-24T17:30:58Z

@mattwthompson should be good now

mattwthompson

I once again trust this is working for simulations in your research; the code looks great but it's hard to say much comprehensively without battle-testing it since Evaluator relies so much on things that take a long time to test.

Could you also merge the main branch back in add a one-liner to the "Current development" section? Copying the name of the PR is plenty.

Merge openforcefield/main

mattwthompson · 2023-07-24T18:15:44Z

Awesome work!

aehogan · 2023-07-24T18:40:00Z

Thanks! Delta H vaporization, delta H mixing and densities have all run successfully now.

aehogan and others added 7 commits July 14, 2023 16:46

Initial FoyerForceField classes

c1c8ebe

Initial version of _parameterize_molecule

6eb1478

Add foyer forcefield test

92b3435

Add support for RBTorsionForce

0ab6877

Add tests for both oplsaa and a custom foyer xml, bug fixes

dc0bf5e

Add FoyerForceFieldSource to Forcefield init

7e93342

[pre-commit.ci] auto fixes from pre-commit.com hooks

5bec898

for more information, see https://pre-commit.ci

aehogan added 3 commits July 18, 2023 11:53

Add foyer to test env

3410745

Merge remote-tracking branch 'origin/main'

3a3c6c7

Merge branch 'openforcefield:main' into main

5085cdd

aehogan added 2 commits July 18, 2023 12:19

Remove explicit support for trappeua (can still use the xml file if y…

197db42

…ou want)

Merge remote-tracking branch 'origin/main'

1d6ae35

mattwthompson reviewed Jul 18, 2023

View reviewed changes

aehogan and others added 8 commits July 18, 2023 14:35

Add FoyerForceFieldSource to __all__

b41b42e

[pre-commit.ci] auto fixes from pre-commit.com hooks

0f5062b

for more information, see https://pre-commit.ci

Add default cutoff to doc string

2ab59e7

Co-authored-by: Matt Thompson <matt.thompson@openforcefield.org>

Remove forcing OPLS-AA to LB mixing rules

7b9b3fd

Merge remote-tracking branch 'origin/main'

93043b8

Rename ambiguous forcefield object from Foyer

37be6ea

[pre-commit.ci] auto fixes from pre-commit.com hooks

6991efb

for more information, see https://pre-commit.ci

Set combine_nonbonded_forces=false for Foyer forcefields to accomdate…

5570036

… other mixing rules

This was referenced Jul 24, 2023

Remove .mdtop from docs openforcefield/openff-interchange#773

Closed

Update migration guide to not muck with combination rule openforcefield/openff-interchange#772

Closed

Use tip3p for foyer xml-based forcefield test

c65f2ae

mattwthompson reviewed Jul 19, 2023

View reviewed changes

Get foyer interchange box vectors from the coordinate file

53f62b3

mattwthompson reviewed Jul 19, 2023

View reviewed changes

aehogan and others added 6 commits July 19, 2023 18:28

Add support for openmm.CustomBondForce and openmm.CustomNonbondedForc…

e5296bd

…e to _append_system

[pre-commit.ci] auto fixes from pre-commit.com hooks

bdae81c

for more information, see https://pre-commit.ci

CustomBondForce's weren't getting added to the system

7924842

Merge remote-tracking branch 'origin/main'

8033b74

CustomBondForce and CustomNonbondedForce weren't getting index mapped…

2f8e638

… properly

[pre-commit.ci] auto fixes from pre-commit.com hooks

4a59916

for more information, see https://pre-commit.ci

Add BuildFoyerSystem to EvaluatorClient._default_protocol_replacements

5cdb86a

aehogan added 3 commits July 21, 2023 18:23

Set exclusions for CustomNonbondedForce, change foyer xml test to met…

d126895

…hane, add energy minimization to oplsaa test to ensure openmm successfully runs

Add openmm energy minimization to foyer XML test to ensure openmm sys…

6024b93

…tem is built correctly

Check energy function of openmm custom forces for existence check

281e9a5

mattwthompson approved these changes Jul 24, 2023

View reviewed changes

aehogan added 2 commits July 24, 2023 14:04

Merge pull request #1 from openforcefield/main

ccb30ad

Merge openforcefield/main

Update releasehistory.rst

de3e374

mattwthompson merged commit f6bdd5b into openforcefield:main Jul 24, 2023
7 of 13 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for Foyer forcefields #517

Add support for Foyer forcefields #517

aehogan commented Jul 17, 2023 •

edited

Loading

mattwthompson commented Jul 17, 2023

codecov bot commented Jul 18, 2023 •

edited

Loading

aehogan commented Jul 18, 2023

mattwthompson commented Jul 18, 2023

aehogan commented Jul 18, 2023

mattwthompson commented Jul 19, 2023

mattwthompson Jul 19, 2023

aehogan Jul 19, 2023

mattwthompson Jul 20, 2023

mattwthompson Jul 19, 2023

aehogan Jul 19, 2023

aehogan Jul 19, 2023

mattwthompson Jul 19, 2023

mattwthompson commented Jul 20, 2023

aehogan commented Jul 20, 2023

mattwthompson commented Jul 20, 2023

aehogan commented Jul 24, 2023

mattwthompson left a comment

mattwthompson commented Jul 24, 2023

aehogan commented Jul 24, 2023

Add support for Foyer forcefields #517

Add support for Foyer forcefields #517

Conversation

aehogan commented Jul 17, 2023 • edited Loading

Description

Todos

Questions

Status

mattwthompson commented Jul 17, 2023

codecov bot commented Jul 18, 2023 • edited Loading

Codecov Report

aehogan commented Jul 18, 2023

mattwthompson commented Jul 18, 2023

aehogan commented Jul 18, 2023

mattwthompson commented Jul 19, 2023

mattwthompson Jul 19, 2023

Choose a reason for hiding this comment

aehogan Jul 19, 2023

Choose a reason for hiding this comment

mattwthompson Jul 20, 2023

Choose a reason for hiding this comment

mattwthompson Jul 19, 2023

Choose a reason for hiding this comment

aehogan Jul 19, 2023

Choose a reason for hiding this comment

aehogan Jul 19, 2023

Choose a reason for hiding this comment

mattwthompson Jul 19, 2023

Choose a reason for hiding this comment

mattwthompson commented Jul 20, 2023

aehogan commented Jul 20, 2023

mattwthompson commented Jul 20, 2023

aehogan commented Jul 24, 2023

mattwthompson left a comment

Choose a reason for hiding this comment

mattwthompson commented Jul 24, 2023

aehogan commented Jul 24, 2023

aehogan commented Jul 17, 2023 •

edited

Loading

codecov bot commented Jul 18, 2023 •

edited

Loading