idifv ambiguity
| JW – Matt and I recently realized that we understand the meaning of idivf="auto" differently, and that the SMIRNOFF spec doesn’t resolve which of our interpretations is correct. Matt opened an Issue summarizing the confusion. We should briefly discuss this at the next meeting to select a course of action. As a suggested course of action, Matt points out that we do not in practice make meaningful use of idivf="auto", and our current implementation will raise an error if it’s set to auto for proper torsions, so we could consider removing idivf from the spec altogether. MT – Why do we have idivf at all/what’s the history? DM – It matters in a few places. AMBER can reuse the same parameter for torsions with different multiplicities so barrier heights would add up to the right amount JC – Specifically it was added to SMIRNOFF to let CBayly to port in AMBER parameters more easily. So for us, it generally is a tool to aid conversion. I think removing it from the spec is not a good idea. DM – As for whether WE can stop using it, mostly “yes”, except in the case that Brent is investigating, where the same torsion parameter can apply to bonds with 6 vs. 9 torsions passing through them. JC – Is the remedy to develop torsions that are invariant to the number of torsions are passing through the bond? Or to come up with a new convention to account for the number of torsions passing through a bond? DM – Once BW’s thing is done, we should be able to “fix this out”. JC – MAybe not the case - Currently with idivf, the convention is to add the torsions impinging on a bond. If there was going to be some division involving the number of bonds, that would need to be studied. So that’d need a new project. JC – Also, on the science side, why are we limiting the number of periodicities we’re fitting? DM – The issue I was thinking of relates to idivf=”auto”, is that currently implemented? JW – No, and auto never has been. DM – So all current FFs use idivf=1? LW – Yes. So we could do a comparison with idivf=auto if we wanted to, but we don’t have any releases that need it. DM – So, the last I was looking at this, CBy and I were wondering whether to do tests of FFs using JW – MT read the spec naively and came to a different interpretation that me. So if we want to keep it in there and maybe implement it in the future, we should clarify. JC – We should divide by the number of unique “atom quartets” MT – So we could determine the denominator just by looking at the topology, without knowing the parameters yet? JW – I think yes. DM – Maybe not…. JC – If we require each quartet to have a torsion, then you can determine this just by looking at the topology. But if some quartets may NOT havoe torsions, then it’s not that easy. So we should specify whether each quartet needs a torsion assigned in the spec. JW – MT – So, we could implement idivf=auto… JC – Could specifically say that we divide by i-j-k-l torsion - divide by (n_j-1) * (n_k-1), where n is node degree. … LW – And for reference, this is the opposite of what BW is working on, where he’s splitting terms to only fit to nodes of specific degree. DM – So it’s possible that, if we do the auto scaling, we get torsions that generalize across multiplicies? JW – LEt me know if you want to run this expt, we could have this in the next release - likely a monthish but can do sooner if needed. LW – Since BW’s still working on his existing projects, that timeline could be good.
(General) – Infrastructure team will Open a SMIRNOFF EP clarifying the wording in the spec NOT updating the spec version Implementing auto behavior in our parameterization pipeline.
MT – This looks somewhat straightforward.
|
Torsion angle convention | JC – Two things YTZ would prefer spec to have more details on how torsion angle is defined. DM – I’m in favor of putting the definition that YTZ dug up into the spec (the OpenMM definition). LW – I think we did have non-symmetric torsions in Sage 2.0, but we removed those (because we couldn’t justify non-symmetric torsions). DM – We should add the equation that’s YTZ dug up
What are the ramification for converters? Especially those involving non-symmetric FFs? We may want to audit our converters if we haven’t thought about this. JW – Symmetric SMIRKS would be a nightmare. Edge cases? JC – Chiral SMIRKS too. LW – Current Toolkit behavior may be to treat torsions as symmetric, which could be bad for asymmetric torsions. DM – In software, we should do some checks that torsions are 0 or 180. JW – I’m in favor of just raising a notimplementederror for phase != 0,180. DM – Yeah, might be LW – Would this prevent loading Sage 2.0.0? JW – … … JW – Are we sorting … DM – I’d be concerned about exporters, and needing to validate whether other simulation engines can support non symmetric torsions in ways that match what we are doing. (We don’t want him to have to dig into the details of all the other codes.) JC – Spec should be amended to formally disallow symmetric matches when the phase is not 0 or pi. Behavior is undefined then. … Options Do nothing Forbid parameters with non pi/periodicity-multiple phase Raise warning/error if an ambiguous match occurs Have the spec say that behavior is undefined if nonsymmetric parameters might apply either way across a central bond.
JC – I think that measuring the angle i-j-k-l might just give the same result as measuring l-k-j-i LW will open SMIRNOFF EP with YTZ’s definition of the torsion angle and warning that nonsymmetric torsion applied both ways over a central bond may lead to undefined behavior. General - This won’t require a spec section version update.
|