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

Update ONE_4PI_EPS0 #522

Closed
jchodera opened this issue Oct 6, 2021 · 2 comments · Fixed by #525
Closed

Update ONE_4PI_EPS0 #522

jchodera opened this issue Oct 6, 2021 · 2 comments · Fixed by #525
Assignees

Comments

@jchodera
Copy link
Member

jchodera commented Oct 6, 2021

OpenMM recently updated all physical constants to match the CODATA 2018 self-consistent constants. We should update our version of this constant.

@ijpulidos
Copy link
Contributor

The easy way is just to compute the new value and hard-code it into our current code which would be something like

ONE_4PI_EPS0 = 138.93545764438198

which is the approach that works in this moment.Would that do it? This is an easy fix, and hopefully the value change in the sixth digit after the point won't make things explode for us.

OR we want to have something a bit more elegant, maybe with fundamental quantities, such as electron charge in Coulombs, and having units, etc. and computing it from these? Which would be something like

E_CHARGE = 1.602176634e-19 * u.coulomb
EPSILON0 = 8.8541878128e-12 * u.farad/u.meter
ONE_4PI_EPS0 = 1e6*(u.AVOGADRO_CONSTANT_NA*E_CHARGE**2)/(4*pi*EPSILON0)

(Probably missing some units there). Which could possibly make things fail in the short-term if things are not meant to deal with Quantity objects.

I guess there is even a third option, which is doing this in the openmm side and importing the constants from there. What's the desired approach for this one?

@jchodera
Copy link
Member Author

jchodera commented Nov 3, 2021

@ijpulidos Please go for it!

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 a pull request may close this issue.

2 participants