Open umesh-timalsina opened 4 years ago
There's a lot here so I'm just going to throw out a couple assorted thoughts
On "joining" dihedrals, this may be a nice feature to generally support with dihedrals and other core types. We'd need to add the sympy expressions, merge parameters, and do some sanity checking on the resulting object. This is less relevant for AtomType
than a few other core types but should in principle be feasible on all if we can do it for one?
On the trig functions, in practice, the sum may go to some ungodly number like n=5 but the lower terms in the series are usually zeroed out. It's rare that we'd have a periodic torsion with all 15 or whatever parameters, so the trick is more in dealing with which value of n is used. Maybe there could be a general form but some instances subclassed out of it? Like PeriodicDihedralType
for n = 1, 2, ..., 5 but PeriodicDihedralN2Type` or something for n = 2 and no other terms.
There are couple of ways to get around this. One way was adopeted in #186, with a consensus that further modification of the DataStructures
in core is not necessary.
If you have a numbered parameter set(phase1, phase2, phase3), there is a way to consilidate the parameters by regex split.
The parameters dict would now be a Map<parameter, listofparameterValues>
.
Lets say there are two parameters k
and n
(in #186) k1, k2, k3, k4, k5
and n1, n2, n3, n4, n5
:
parameters = {
k : [k1_val, k2_val, k3_val, k4_val, k5_val]
n : [n1_val, n2_val, n3_val, n4_val, n5_val]
}
However, I would be in favor subclassing if that is the route we would want to take, but it might be an overkill.
This will require changes to the _Expression
class and Potentially some changes to our writers. I will be adding this support in two parts:
Its already supported we just need a way to incorporate this when creating the
ForceField
.