openforcefield / openff-evaluator

A physical property evaluation toolkit from the Open Forcefield Consortium.
https://docs.openforcefield.org/projects/evaluator
MIT License
55 stars 18 forks source link

Add support for Foyer forcefields #517

Closed aehogan closed 1 year ago

aehogan commented 1 year ago

Description

Add support for Foyer forcefields

Todos

Questions

Status

mattwthompson commented 1 year ago

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[bot] commented 1 year ago

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 commented 1 year ago

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

mattwthompson commented 1 year ago

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 commented 1 year ago

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

mattwthompson commented 1 year ago

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 (https://github.com/openforcefield/standards/issues/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 commented 1 year ago

@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 commented 1 year ago

@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 commented 1 year ago

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

aehogan commented 1 year ago

@mattwthompson should be good now

mattwthompson commented 1 year ago

Awesome work!

aehogan commented 1 year ago

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