openforcefield / standards

A repository of the standards employed across the Open Force Field Consortium.
https://openforcefield.github.io/standards
MIT License
1 stars 3 forks source link

Define or remove `idivf` #59

Closed mattwthompson closed 3 months ago

mattwthompson commented 6 months ago

This exists only for historical reasons and adds significant cruft to implementations. (default_idivf="auto" is poorly defined and not evenly implemented in the toolkit, partially because it's confusing and partially because it doesn't add any useful behavior.) There's no obvious reason why implementations outside of Amber need to use this behavior - one can just use the force constant they mean to use.

With proper torsions, these are always explicitly set to 1 in our existing force fields, calling into question why idivf even exists.

With improper torsions, the use of trefoils nearly forces the value of 3, but this might as well be encoded as an implementation detail (how could it meaningfully be another value?)

In either case, if somebody wants to use an arbitrary float (imagine default_idivf="7.5") they could just modify the actual k values throughout. It is difficult to imagine, without historical influence of other software, what purpose this serves.

j-wags commented 6 months ago

Upon some thought, I think the only value in keeping idivf around is for the possibility of using auto for ProperTorsions (assuming the interpretation of ProperTorsion's auto that means different OpenMM parameters could be applied for the same SMIRNOFF FF parameter depending on the number of unique 4-atom paths through the central bond)

So knowing that this might be a useful research direction in the future and that spec changes are a pain, I'm kinda in favor of leaving auto in the spec for ProperTorsions but clarifying its meaning, and leaving it unimplemented in the Toolkit until we actually see organizational value in pursuing that research.

I'm unable to think of a useful meaning of auto for ImproperTorsions. In our current definition, it seems like auto would always evaluate to 3.