Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add harmonic bond, harmonic angle, periodic torsion, rb torsion to potential templates #170

Merged
merged 2 commits into from
Feb 5, 2020

Conversation

ahy3nz
Copy link
Collaborator

@ahy3nz ahy3nz commented Oct 11, 2019

No description provided.

@rmatsum836
Copy link
Collaborator

These look good to merge.

Copy link
Member

@rsdefever rsdefever left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These all look good.

A couple of questions:

(1) Why does PotentialTemplate inherit from Potential and then all other potentials inherit from PotentialTemplate? (i.e., vs. just inheriting from Potential?)
(2) The functional forms are pretty self-explanatory, but how is the documentation going to tie into all of this. Just thinking we should be careful with respect to details like the definition of phi (is phi=0 or phi=180 the cis conformation), and factors of 0.5 on the force constants.

@ahy3nz
Copy link
Collaborator Author

ahy3nz commented Jan 14, 2020

These all look good.

A couple of questions:

(1) Why does PotentialTemplate inherit from Potential and then all other potentials inherit from PotentialTemplate? (i.e., vs. just inheriting from Potential?)

See the original PR
A PotentialTemplate is a Potential that doesn't have parameters associated with it. To avoid copy+pasting redundant code to make a new PotentialTemplate class, we slightly modified the Potential class.

A LennardJonesPotential is really just an object that contains information about the canonical LJ expression, its name, and independent variables - no explicit parameters are needed. The goal of PotentialTemplates was for properly writing out topologies to simulation engines. Yes it's great that you can have a string/sympy expression for a mathematical function, but at some point you need to match it a gromacs function type integer, an openmm function object, hoomd function object, etc., and the goal of the PotentialTemplates was to be the reference comparison to ensure that YOUR Potential with an LJ expression matches a simulation engine's LJ potential, or throw an error if your custom Potential would not fit within a particular engine

(2) The functional forms are pretty self-explanatory, but how is the documentation going to tie into all of this. Just thinking we should be careful with respect to details like the definition of phi (is phi=0 or phi=180 the cis conformation), and factors of 0.5 on the force constants.

Good point, we have a running issue here to try to log all the various nuances and how this package reconciles them.

Lastly, tagging @umesh-timalsina if he has a better idea to match our topology Potential objects to respective functions supported in other engines

@mattwthompson mattwthompson merged commit 91e527b into mosdef-hub:master Feb 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants