search

openmm / openmm

OpenMM is a toolkit for molecular simulation using high performance GPU code.
1.5k stars 525 forks source link

Dependence of PeriodicTorsionForce.getNumTorsions() on k=0 parameters #2504

Open leeping opened 4 years ago

leeping commented 4 years ago

I'm currently experiencing an issue in ForceBalance that derives from the current behavior of ForceField.createSystem(), in which the number of torsions being generated depends on the number of nonzero k parameters.

FB makes the assumption for two ForceField objects created from two force field .xml files that differ only in "floating-point" parameters (such as force constants, equilibrium lengths, and charges), the two System objects that result from calling ForceField.createSystem are identical in every way, except for the numerical values of parameters. This allows me to do the following for two System objects src and dest:

    for i in range(src.getNumTorsions()):
        dest.setTorsionParameters(i,*src.getTorsionParameters(i))

The issue is that when two force field XML files are exactly the same, except one torsion barrier is zero in one file and nonzero in the other file, the number of torsions is different for the two Systems, leading to an error in the above function.

I was hoping that one solution might be to add an extra keyword argument such as ForceField.createSystem(..., includeZeroTerms=False). This would result in the creation of Torsions with k=0 in the force field .xml file, and result in the desired behavior. (I think this would also require looking at whether other terms have similar behavior).

Alternatively, if there is a better way to achieve what FB is trying to do, I'm open to suggestions. Essentially, the problem is "given a Simulation object that is created from a force field .xml file and some topology, evaluate the properties of a Simulation created from another force field .xml file that is different from the original only in the numerical values of floating-point parameters." I don't want to create a separate Simulation for performance reasons; the existing approach of copying System parameters, followed by calling Force.updateParametersInContext works quite well.

peastman commented 4 years ago

It's kind of a hack, but what if you just set k to something tiny like 1e-20 instead of 0?

leeping commented 4 years ago

It's kind of a hack, but what if you just set k to something tiny like 1e-20 instead of 0?

This is also what I suggested in the original ForceBalance issue that started this: https://github.com/leeping/forcebalance/issues/168

I can also have FB throw a warning if a starting parameter value is zero because it could cause issues for OpenMM. Both of these are workarounds that will solve the issue. Do you think the feature is still worth considering?

jchodera commented 4 years ago

I think it would be (1) useful and (2) easy to add an optional argument to include these zero-barrier torsions, since it's just a change at the Python layer.