mosdef-hub / foyer

A package for atom-typing as well as applying and disseminating forcefields
https://foyer.mosdef.org
MIT License
118 stars 77 forks source link

Add combination rule as a force field property #432

Closed mattwthompson closed 3 years ago

mattwthompson commented 3 years ago

Describe the behavior you would like added to Foyer Mixing rules are important properties of force fields. Foyer should encode them as such in the Forcefield object model, much like 1-4 scaling factors (https://github.com/mosdef-hub/foyer/issues/397). Not doing so adds ambiguity around the intended scope of data in a force field and also leads to to issues like https://github.com/mosdef-hub/foyer/issues/431 and issues grad students (me) using the wrong mixing rules for years.

Part of the implementation (telling ParmEd which rule to use) and a TODO are in the codebase already. Just setting structure.combining_rule is not sufficient, however, as evidenced by #431, because of how 1-4 scaling factors are passed from OpenMM to ParmEd.

Describe the solution you'd like Given the complexity with storing a non-Lorentz-Berthelot mixing rule in an openmm.System, the simplest approach might be to store the mixing rule as a high-level property. Then, if the mixing rule is set to geometric, apply at patch onto the ParmEd object before it's ultimately returned by Forcefield.apply().

Describe alternatives you've considered Downstream developers and users could apply a fix that tidies things up, but IMHO this should really be done by foyer and it does not address the original issue of a force field not knowing its combing rule.

Additional context I use "combination rule," "mixing rule", "combining rule," and possibly other terms interchangeably. This is a bad habit of mine but I think it comes from different people and fields not agreeing on a term, so it's probably not going to be fixed anytime soon.

justinGilmer commented 3 years ago

Currently satisfied by #433 . Closing for now.