-
Notifications
You must be signed in to change notification settings - Fork 55
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
Added the force fields necessary for Calvados2 #189
Conversation
Signed-off-by: ywitzky <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #189 +/- ##
==========================================
+ Coverage 74.20% 74.49% +0.28%
==========================================
Files 36 36
Lines 4943 5038 +95
==========================================
+ Hits 3668 3753 +85
- Misses 1275 1285 +10 ☔ View full report in Codecov by Sentry. |
Looks great. I just merged a major change to master, would you mind rebasing/merging on top of that? It will involve changes to the mixing functions and zero shortcuts, hopefully you can use the changes to |
I should have done the changes you mentioned. Let me know if i forgot something. |
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.
Thanks, just minor stuff, only thing to think about is the cutoff point.
test/interactions.jl
Outdated
@@ -76,14 +76,15 @@ | |||
0.0253410816u"kJ * mol^-1"; | |||
atol=1e-9u"kJ * mol^-1", | |||
) | |||
|
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.
Don't add these lines.
test/interactions.jl
Outdated
atol=1e-9u"kJ * mol^-1 * nm^-1", | ||
) | ||
|
||
c2 = SVector(1.28, 1.0, 1.0)u"nm" |
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.
Use c4
and dr14
here to avoid confusion if more tests are added.
src/interactions/lennard_jones.jl
Outdated
|
||
|
||
@doc raw""" | ||
AshbaughHatch(; cutoff, α, λ, p, use_neighbors, lorentz_mixing, weight_special, |
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.
Update the arguments.
src/interactions/lennard_jones.jl
Outdated
σ::S =0.0u"nm" | ||
ϵ::E =0.0u"kJ * mol^-1" | ||
λ::L=1.0 | ||
solute::Bool = false |
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.
Remove solute
.
``` | ||
|
||
```math | ||
V_{\text{AH}}(r_{ij}) = |
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.
As it stands I think we don't apply the VLJ(rc) term by default? So either remove it in the docstring or add it to the code, in which case probably remove the cutoff
option and don't use the cutoff path.
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 removed it to make the implementation more general.
src/interactions/coulomb.jl
Outdated
|
||
|
||
@doc raw""" | ||
Yukawa(; cutoff, use_neighbors, weight_special, coulomb_const, force_units, energy_units) |
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.
Update arguments.
src/interactions/coulomb.jl
Outdated
use_neighbors::Bool= false | ||
weight_special::W =1 | ||
coulomb_const::T = coulomb_const | ||
kappa::D = kappa=1.0*u"nm^-1" |
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.
Repetition.
src/interactions/coulomb.jl
Outdated
|
||
function force_divr(::Yukawa, r2, invr2, (coulomb_const, qi, qj, kappa)) | ||
r = sqrt(r2) | ||
return (coulomb_const * qi * qj) / r^3 *exp(-kappa*r) * (kappa*r+1.0) |
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.
+1
rather than +1.0
to avoid promoting Float32
to Float64
.
I added the remarks in the code and added some tests for the options "special" and "shortcut". |
Great, thanks. |
Calvados2 is a strongly coarse grained model for intrinsically disordered proteins (IDP) which requires a yukawa potential, an harmonic potential and a modified Lennard Jones potential.