Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unclear origin of 90- and 270-degree angle torsion phases #53

Open
mattwthompson opened this issue Aug 22, 2022 · 3 comments
Open

Unclear origin of 90- and 270-degree angle torsion phases #53

mattwthompson opened this issue Aug 22, 2022 · 3 comments

Comments

@mattwthompson
Copy link
Member

From openforcefield/openff-toolkit#1370 (comment), it is not clear why some torsions use 90 or 270 degree phases. It is not intuitive that these parameters would have "handedness" and thus far we have failed to understand or justify the added complexity. We should aim to document somewhere why these parameters do not use the typical 0/180 degree phases; we currently risk losing the reasoning here to the sands of time.

The toolkit may or may not have different behavior when using label_molecules versus actually applying parameters, but that is a separate topic from why these parameters have handedness at all.

@davidlmobley
Copy link

This is odd; I will have to think about this more, but it's not obvious to me that we should ever have 90/270 degree phase angles (unless we have both at the same time?!?)... would be interested if @pavankum has any insight. I would have also asked Chris Bayly but it seems he was in the other thread and has this question also...?

@mattwthompson
Copy link
Member Author

Unfortunately the forensics look pretty tricky, at least for me as an individual only digging through these files after the fact. Starting with what @j-wags noticed happened with t25, I thought I had a brief ah-ha when I saw that the funny phases were only in the added extra terms between versions, but (if I'm reading the SMIRKS pattern accurately) the chemistry of the torsion itself changed1, so I don't think that's an apples-to-apples comparison.

$ diff smirnoff99frosst/offxml/smirnoff99Frosst-1.0.6.offxml smirnoff99frosst/offxml/smirnoff99Frosst-1.0.7.offxml | grep t25
<     <Proper smirks="[#16X2,#16X1-1,#16X3+1:1]-[#6X3:2]-[#6X4:3]-[#1:4]" id="t25" idivf1="1" k1="0.260" periodicity1="2" phase1="0.0" periodicity2="1" idivf2="1" k2="0.350" phase2="180.0"/>
>     <Proper smirks="[#16X2,#16X1-1,#16X3+1:1]-[#6X3:2]-[#6X4:3]-[#7X4,#7X3:4]" id="t25" idivf1="1" k1="0.988" periodicity1="4" phase1="0.0" periodicity2="3" phase2="0.0" idivf2="1" k2="0.258" periodicity3="2" phase3="0.0" idivf3="1" k3="0.805" periodicity4="2" phase4="270.0" idivf4="1" k4="2.059" periodicity5="1" phase5="90.0" idivf5="1" k5="1.710"/>

The changelog for 1.0.7 includes only:

Add hydroxyl hydrogen radii (as per SMIRNOFF initial paper); remove generics with pure wildcards (not even elemental types).

Footnotes

  1. The fourth tagged atom changed from a generic hydrogen to an almost-generic nitrogen

@davidlmobley
Copy link

For the record, we have ongoing conversations about this. After discussion in a meeting last week, we more or less concluded that:

  1. 90 or 270 phases can sometimes make sense, either (a) if they are both present at the same time, or (b) in torsions of multiplicity higher than 1 (e.g. 2)
  2. We mostly do not want asymmetric torsions, which some of these currently are; it's possible some were originally (in our parm@frosst ancestor) put in for very specific reasons which might or might not be good, but at present we have no evidence for them/no demonstrated need for them and they seem to be applied to situations that are far too broad for the limited chemistry where they MIGHT be useful
  3. We likely want to remove some/all of the current 90/270 torsions and replace them; Pavan is going to have a look at doing this and has already done some test fits without them

So this is in the works. These don't seem to be a "bug" per se but more of a "legacy issue we've inherited" and probably most shoud be removed until/unless we have data indicating they are needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants