gmso
gmso copied to clipboard
Modified RB and Periodic torsions
-
The RB torsion should just use C0 and not C0 * cos(psi)**0. Note this is changing the phi to psi.
-
The periodic torsion should just be similar to RB and go to the 5th power series (i.e., cos(5x) term)
-
We should make a decision between the CHARMM system (all constants spelled out C0-C5..) vs the Periodic term, where they are put in individually. To maximize the usability of the FFs between users, only 1 should be utilized, IMO.
Codecov Report
Merging #615 (37797b0) into master (d592215) will decrease coverage by
0.02%
. The diff coverage isn/a
.
@@ Coverage Diff @@
## master #615 +/- ##
==========================================
- Coverage 90.54% 90.51% -0.03%
==========================================
Files 56 56
Lines 4525 4525
==========================================
- Hits 4097 4096 -1
- Misses 428 429 +1
Impacted Files | Coverage Δ | |
---|---|---|
gmso/external/convert_foyer_xml.py | 92.23% <ø> (ø) |
|
gmso/external/convert_parmed.py | 90.38% <ø> (ø) |
|
gmso/formats/top.py | 92.47% <0.00%> (-1.08%) |
:arrow_down: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update d592215...37797b0. Read the comment docs.
We need to ask if we want to use the Phi variable for RB torsions, as it is used for OPLS and Periodic torsions. RB torsion Phi is not the same as OPLS and Periodic:
RB-torsion-angle = OPLS-torsion-angle - Pi = Phi - PI RB-torsion-angle = Periodic -torsion-angle - Pi = Phi - PI
Maybe we call the RB-torsion-angle Psi like the GROMACS manual (https://manual.gromacs.org/documentation/2019/reference-manual/functions/bonded-interactions.html). I changed it in this PR to Phi for the main equation.
If everyone agrees, I will change the other files accordingly. Thoughts? Comments? OK with this?
Are we naming the HarmonicTorsionPotential.json potential correctly? It seems like we should change the name or equation, or add both and change them accordingly. Thoughts?
It seems LAMMPS calls this a quadratic (https://docs.lammps.org/dihedral_quadratic.html#dihedral-style-quadratic-command)
LAMMPS calls this a harmonic (https://docs.lammps.org/dihedral_harmonic.html)
Are we naming the HarmonicTorsionPotential.json potential correctly? It seems like we should change the name or equation, or add both and change them accordingly. Thoughts?
It seems LAMMPS calls this a quadratic (https://docs.lammps.org/dihedral_quadratic.html#dihedral-style-quadratic-command)
LAMMPS calls this a harmonic (https://docs.lammps.org/dihedral_harmonic.html)
Gromacs cites this equation as a harmonic improper dihedral. So that might be where the current naming convention comes from, but the lammps docs seem to disagree with that convention. { "name": "HarmonicTorsionPotential", "expression": "0.5 * k * (phi - phi_eq)**2", "independent_variables": "phi" }
It is the form of an improper but it says dihedral/torsion, which are very different. So at a minimum, this should be renamed.
We need to ask if we want to use the Phi variable for RB torsions, as it is used for OPLS and Periodic torsions. RB torsion Phi is not the same as OPLS and Periodic:
RB-torsion-angle = OPLS-torsion-angle - Pi = Phi - PI RB-torsion-angle = Periodic -torsion-angle - Pi = Phi - PI
Maybe we call the RB-torsion-angle Psi like the GROMACS manual (https://manual.gromacs.org/documentation/2019/reference-manual/functions/bonded-interactions.html). I changed it in this PR to Phi for the main equation.
If everyone agrees, I will change the other files accordingly. Thoughts? Comments? OK with this?
I agree with this point, being clear and giving these parameters different names hopefully will reduce errors from users missing this distinction. Along the same lines, if the above harmonic torsion potential is changed to an a harmonic improper, the independent variable should be ξ.
I also think we need the proper dihedral form of the function, so I'm in favor of changing the name of this to a quadratic torsion and keeping the current form.
Side note, should we be citing in these .json
files some indication of where the functional form was pulled from?