Modified RB and Periodic torsions #615
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| { | ||
| "name": "PeriodicTorsionPotential", | ||
| "expression": "k0 + k1 * (1 + cos(1 * phi - phi_eq1)) + k2 * (1 + cos(2 * phi - phi_eq2)) + k3 * (1 + cos(3 * phi - phi_eq3)) + k4 * (1 + cos(4 * phi - phi_eq4)) + k5 * (1 + cos(5 * phi - phi_eq5))", | ||
| "independent_variables": "phi" | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| { | ||
| "name": "RyckaertBellemansTorsionPotential", | ||
| "expression": "c0 + c1 * cos(psi)**1 + c2 * cos(psi)**2 + c3 * cos(psi)**3 + c4 * cos(psi)**4 + c5 * cos(psi)**5", | ||
| "independent_variables": "psi" | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -51,21 +51,21 @@ | |
| </Parameters> | ||
| </AngleType> | ||
| </AngleTypes> | ||
| <DihedralTypes expression="c0 + c1 * cos(psi) + c2 * cos(psi)**2 + c3 * cos(psi)**3 + c4 * cos(psi)**4 + c5 * cos(psi)**5"> | ||
| <ParametersUnitDef parameter="c0" unit="kJ/mol"/> | ||
| <ParametersUnitDef parameter="c1" unit="kJ/mol"/> | ||
| <ParametersUnitDef parameter="c2" unit="kJ/mol"/> | ||
| <ParametersUnitDef parameter="c3" unit="kJ/mol"/> | ||
| <ParametersUnitDef parameter="c4" unit="kJ/mol"/> | ||
| <ParametersUnitDef parameter="c5" unit="kJ/mol"/> | ||
| <DihedralType name="DihedralType-RyckaertBellemans-1" type1="opls_144" type2="opls_143" type3="opls_143" type4="opls_144"> | ||
| <Parameters> | ||
| <Parameter name="c0" value="58.576"/> | ||
| <Parameter name="c1" value="0.0"/> | ||
| <Parameter name="c2" value="-58.576"/> | ||
| <Parameter name="c3" value="0.0"/> | ||
| <Parameter name="c4" value="0.0"/> | ||
| <Parameter name="c5" value="0.0"/> | ||
| </Parameters> | ||
| </DihedralType> | ||
| </DihedralTypes> | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -90,7 +90,7 @@ | |
| </ImproperType> | ||
| </DihedralTypes> | ||
|
|
||
| <DihedralTypes name="RBProper" expression="c0 + c1 * cos(psi)**1 + c2 * cos(psi)**2 + c3 * cos(psi)**3 + c4 * cos(psi)**4 + c5 * cos(psi)**5"> | ||
| <ParametersUnitDef parameter="c5" unit="kJ/mol"/> | ||
| <ParametersUnitDef parameter="c4" unit="kJ/mol"/> | ||
| <ParametersUnitDef parameter="c3" unit="kJ/mol"/> | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,66 +31,66 @@ | |
| <BondTypes name="HarmonicBond" expression="0.5 * k * (r-r_eq)**2"> | ||
| <ParametersUnitDef parameter="r_eq" unit="nm"/> | ||
| <ParametersUnitDef parameter="k" unit="kJ/mol"/> | ||
| <BondType name="BondType1" type1="Ar" type2="Ar"'> | ||
| <Parameters> | ||
| <Parameter name="r_eq" value="10.0"/> | ||
| <Parameter name="'k" value="10000"/> | ||
| </Parameters> | ||
| </BondType> | ||
| <BondType name="BondType2" type1="Xe" type2="Xe"> | ||
| <Parameters> | ||
| <Parameter name="'r_eq"' value="10"/> | ||
| <Parameter name="'k'" value="20000"/> | ||
| </Parameters> | ||
| </BondType> | ||
| </BondTypes> | ||
|
|
||
| <AngleTypes name="HarmonicAngle" expression="0.5 * z * (r-r_eq)**2"> | ||
| <ParametersUnitDef parameter="r_eq" unit="nm"/> | ||
| <ParametersUnitDef parameter="z" unit="kJ/mol"/> | ||
| <AngleType name="AngleType1" type1="'Ar" type2="Ar" type3="Ar"> | ||
| <Parameters> | ||
| <Parameter name="r_eq" value="10.0"/> | ||
| <Parameter name="z'" value="100"/> | ||
| </Parameters> | ||
| </AngleType> | ||
| <AngleType name="AngleType2" type1="Xe" type2="Xe" type3="Xe"> | ||
| <Parameters> | ||
| <Parameter name="'r_eq" value="10"/> | ||
| <Parameter name="'z"' value="20"/> | ||
| </Parameters> | ||
| </AngleType> | ||
| </AngleTypes> | ||
|
|
||
| <DihedralTypes name="PeriodicProper" expression="0.5 * z * (r-r_eq)**2"> | ||
| <ParametersUnitDef parameter="r_eq" unit="nm"/> | ||
| <ParametersUnitDef parameter="z" unit="kJ/mol"/> | ||
| <DihedralType name="ProperType1" type1="Ar" type2="Ar" type3="Ar" type4="Ar"> | ||
| <Parameters> | ||
| <Parameter name="'r_eq" value="10.0"/> | ||
| <Parameter name="'z" value="100"/> | ||
| </Parameters> | ||
| </DihedralType> | ||
| <DihedralType name="ProperType2" type1="Xe" type2="Xe" type3="Xe" type4="Xe"> | ||
| <Parameters> | ||
| <Parameter name="r_eq" value="10"/> | ||
| <Parameter name="z" value="20"/> | ||
| </Parameters> | ||
| </DihedralType> | ||
| </DihedralTypes> | ||
|
|
||
| <DihedralTypes name="PeriodicImproper" expression="0.5 * z * (r-r_eq)**2"> | ||
| <ParametersUnitDef parameter="r_eq" unit="nm"/> | ||
| <ParametersUnitDef parameter="z" unit="kJ/mol"/> | ||
| <ImproperType name="ImproperType1" type1="Ar" type2="A" type3="Ar" type4="Ar"> | ||
| <Parameters> | ||
| <Parameter name="r_eq" value="10.0"/> | ||
| <Parameter name="z" value="100"/> | ||
| </Parameters> | ||
| </ImproperType> | ||
| </DihedralTypes> | ||
|
|
||
| <DihedralTypes name="RBProper" expression="c0 + c1 * cos(psi)**1 + c2 * cos(psi)**2 + c3 * cos(psi)**3 + c4 * cos(psi)**4 + c5 * cos(psi)**5"> | ||
| <ParametersUnitDef parameter="c5" unit="kJ/mol"/> | ||
| <ParametersUnitDef parameter="c4" unit="kJ/mol"/> | ||
| <ParametersUnitDef parameter="c3" unit="kJ/mol"/> | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -57,7 +57,7 @@ | |
| </Parameters> | ||
| </AngleType> | ||
| </AngleTypes> | ||
| <DihedralTypes expression="c0 + c1 * cos(psi)**1 + c2 * cos(psi)**2 + c3 * cos(psi)**3 + c4 * cos(psi)**4 + c5 * cos(psi)**5"> | ||
| <ParametersUnitDef parameter="c5" unit="kJ/mol"/> | ||
| <ParametersUnitDef parameter="c4" unit="kJ/mol"/> | ||
| <ParametersUnitDef parameter="c3" unit="kJ/mol"/> | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,7 +33,7 @@ def test_invalid_connection_type(self, templates): | |
|
|
||
| name = ryckaert_bellemans_torsion_potential.name | ||
| expression = ( | ||
| str(ryckaert_bellemans_torsion_potential.expression) + " + 3 * phi" | ||
| ) | ||
| variables = ["phi", "psi"] | ||
|
|
||
|
|
@@ -49,7 +49,7 @@ def test_invalid_connection_type(self, templates): | |
| ryckaert_connection_type | ||
| ) | ||
|
|
||
| expression = "c0+c1+c2+c3+c4+c5+psi" | ||
| variables = ryckaert_bellemans_torsion_potential.independent_variables | ||
| ryckaert_connection_type = DihedralType( | ||
| name=name, | ||
|
|
@@ -149,7 +149,7 @@ def test_ryckaert_to_opls(self, templates): | |
| (param, val) | ||
| for param, val in { | ||
| **ryckaert_connection_type.parameters, | ||
| "psi": angle - np.pi, | ||
| }.items() | ||
| ] | ||
| ) | ||
|
|
@@ -206,7 +206,7 @@ def test_opls_to_ryckaert(self, templates): | |
| (param, val) | ||
| for param, val in { | ||
| **ryckaert_connection_type.parameters, | ||
| "psi": angle - np.pi, | ||
| }.items() | ||
| ] | ||
| ) | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -39,7 +39,8 @@ def test_opls_torsion_potential(self, templates): | |
| opls_torsion_potential = templates["OPLSTorsionPotential"] | ||
| assert opls_torsion_potential.name == "OPLSTorsionPotential" | ||
| assert opls_torsion_potential.expression == sympy.sympify( | ||
| "0.5 * k0 + " | ||
| "0.5 * k1 * (1 + cos(phi)) +" | ||
| "0.5 * k2 * (1 - cos(2*phi)) +" | ||
| "0.5 * k3 * (1 + cos(3*phi)) +" | ||
| "0.5 * k4 * (1 - cos(4*phi))" | ||
|
|
@@ -52,7 +53,12 @@ def test_periodic_torsion_potential(self, templates): | |
| periodic_torsion_potential = templates["PeriodicTorsionPotential"] | ||
| assert periodic_torsion_potential.name == "PeriodicTorsionPotential" | ||
| assert periodic_torsion_potential.expression == sympy.sympify( | ||
| "k0 + " | ||
| "k1 * (1 + cos(1 * phi - phi_eq1)) + " | ||
| "k2 * (1 + cos(2 * phi - phi_eq2)) + " | ||
| "k3 * (1 + cos(3 * phi - phi_eq3)) + " | ||
| "k4 * (1 + cos(4 * phi - phi_eq4)) + " | ||
| "k5 * (1 + cos(5 * phi - phi_eq5))" | ||
| ) | ||
| assert periodic_torsion_potential.independent_variables == { | ||
| sympy.sympify("phi") | ||
|
|
@@ -67,12 +73,12 @@ def test_ryckaert_bellemans_torsion_potential(self, templates): | |
| == "RyckaertBellemansTorsionPotential" | ||
| ) | ||
| assert ryckaert_bellemans_torsion_potential.expression == sympy.sympify( | ||
| "c0 + c1 * cos(psi)**1 +" | ||
| " c2 * cos(psi)**2 + c3 * cos(psi)**3 +" | ||
| " c4 * cos(psi)**4 + c5 * cos(psi)**5" | ||
| ) | ||
| assert ryckaert_bellemans_torsion_potential.independent_variables == { | ||
| sympy.sympify("psi") | ||
| } | ||
|
|
||
| def test_harmonic_torsion_potential(self, templates): | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -79,9 +79,17 @@ def test_ethane_periodic(self, typed_ethane): | |
|
|
||
| per_torsion = PotentialTemplateLibrary()["PeriodicTorsionPotential"] | ||
| params = { | ||
| "k0": 10 * u.Unit("kJ / mol"), | ||
| "k1": 1 * u.Unit("kJ / mol"), | ||
| "phi_eq1": 180 * u.Unit("degree"), | ||
| "k2": 2 * u.Unit("kJ / mol"), | ||
| "phi_eq2": 0 * u.Unit("degree"), | ||
| "k3": 3 * u.Unit("kJ / mol"), | ||
| "phi_eq3": 180 * u.Unit("degree"), | ||
| "k4": 4 * u.Unit("kJ / mol"), | ||
| "phi_eq4": 0 * u.Unit("degree"), | ||
| "k5": 5 * u.Unit("kJ / mol"), | ||
| "phi_eq5": 180 * u.Unit("degree"), | ||
| } | ||
| periodic_dihedral_type = ParametricPotential.from_template( | ||
| potential_template=per_torsion, parameters=params | ||
|
|
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After spending some more thought on this, we might want to be careful with this eqn, especially regarding how many terms we want to support. As in, can we keep the old untouched and create a different/derivative for 5 terms, 4 terms, etc. More ideas/thoughts are welcome
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I hear what you mean. I think, If you do not supported fixed terms it may be harder to parse when you don’t know what is there for the engines. I believe We should be consistent, meaning if RB and OPLS is fixed this should be too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also don’t believe I saw anything ever past n=5 or power=5, which are both there converting to one or the other. The fixed term way in my mind is consistent for programming and the readers. I will make the general solution working in GOMC so it is the same as others.
I’m for the fixed one to the 5th power since all other ones RB and OPLS are fixed too. , but open if there is a specific reason not to do it that way.
Interested to hear what other think.
Though, many derivatives means that the FFs won’t be directly shareable. Whatever we pick we should stick to it as a general solution and not make derivative of it, or it will be hard or impossible to share FFs without manually rewriting or fixing to some extent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After additional though, if we decide to use the original periodic form, we should have additional check for the input variables. For example:
The valuables must me k1, … kx, n1, …, nx or phi1, …, phix. This will ensure the FFs are shareable.
k1 and phi1 must have n1=1 . Kx and phix must have nx =x
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this PR ready to close then, based on this comment thread?