search

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

Should `default_idivf` in SMIRNOFF spec be "1" or "auto"? #13

Open jchodera opened 3 years ago

jchodera commented 3 years ago

Currently, the default value of default_idivf in the SMIRNOFF spec is auto, which requires some reasonable amount of complexity to compute. Should this just be 1 instead to simplify implementations? Or is the auto feature proposed---but not yet implemented---actually useful?

j-wags commented 3 years ago

I'd actually love to get rid of default_idivf altogether, and assume that it's always 1 unless a torsion parameter specifies a different value.

Even setting aside the difficulty of an auto implementation, default_idivf adds a lot of complexity, since information essential to parametrizing a torsion could EITHER be in the ProperTorsion parameter, OR in the ProperTorsions header. This was one of the structural issues that prevented me from finishing the Pydantic ForceField class soon after I joined, and it's be great to not have to deal with it again.

This would still leave the door open for us to implement idivf="auto" support on a future date.

davidlmobley commented 2 years ago

OK, so reading Jeff's point here - it seems like we ought to get rid of default_idivf to simply the format. I could see us moving to idivf="auto" in the future, but I don't see any reason one would need to be able to control the default idivf rather than simply setting idivf=1 by default as Jeff proposes.

So: 1) Yes, get rid of default_idivf because it adds lots of complexity 2) Make idivf be 1 unless otherwise specified 3) Leave door open for auto idivf in the future