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

Weeks-Chandler-Anderson Potential #2292

Merged
merged 10 commits into from
Oct 4, 2018
Merged

Weeks-Chandler-Anderson Potential #2292

merged 10 commits into from
Oct 4, 2018

Conversation

fweik
Copy link
Contributor

@fweik fweik commented Oct 2, 2018

Fixes me typing 2**(1./6.) a lot. Is also a little bit faster than LENNARD_JONES.

Description of changes:

  • Added an explicit WCA pair potential.

PR Checklist

  • Tests?
    • Interface
    • Core
  • Docs?

Copy link
Member

@hmenke hmenke left a comment

Choose a reason for hiding this comment

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

Looks good to me. Just one design question: LennardJonesInteraction is spelled out, but WCAInteraction is abbreviated. Should it be more consistent?

@codecov
Copy link

codecov bot commented Oct 2, 2018

Codecov Report

Merging #2292 into python will decrease coverage by <1%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           python   #2292    +/-   ##
=======================================
- Coverage      71%     71%    -1%     
=======================================
  Files         380     377     -3     
  Lines       18938   18798   -140     
=======================================
- Hits        13588   13477   -111     
+ Misses       5350    5321    -29
Impacted Files Coverage Δ
...bonded_interactions/nonbonded_interaction_data.hpp 88% <ø> (ø) ⬆️
src/core/energy_inline.hpp 58% <ø> (-1%) ⬇️
src/core/forces_inline.hpp 75% <ø> (ø) ⬆️
src/core/lattice.cpp 82% <0%> (-7%) ⬇️
src/core/domain_decomposition.cpp 87% <0%> (-4%) ⬇️
src/core/tuning.cpp 33% <0%> (-3%) ⬇️
src/core/utils/Factory.hpp 84% <0%> (-3%) ⬇️
src/core/grid_based_algorithms/lb.hpp 69% <0%> (-2%) ⬇️
src/core/utils.hpp 90% <0%> (-2%) ⬇️
src/core/utils/List.hpp 97% <0%> (-2%) ⬇️
... and 53 more

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 c1d850e...0566b6a. Read the comment docs.

@fweik
Copy link
Contributor Author

fweik commented Oct 2, 2018

Ah you are probably right. I'll rename it in the interface. In the the stuff is also called lj.
Nah, non_bonded_inter[0,0].weeks_chandler_anderson.set_params is too long. @KaiSzuttor what do you think?

@KaiSzuttor
Copy link
Member

In the context of interactions i think WCA is a well known abbreviation whereas I would say LJ is not too common. In papers we always talk about WCA interaction but would not write LJ interaction. Thus, I think it's fine as it is.

Copy link
Member

@KaiSzuttor KaiSzuttor left a comment

Choose a reason for hiding this comment

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

please change doc for the typo

@@ -216,6 +216,40 @@ interaction, while :math:`\delta` varies how smoothly the potential goes to zero
alchemical transformations, where a group of atoms can be slowly turned
on/off during a simulation.

.. _Weeks-Chandler-Anderson interaction:

Weeks-Chandler-Anderson interaction
Copy link
Member

Choose a reason for hiding this comment

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

it's Andersen not Anderson

\label{eq:wca}
V_\mathrm{WCA}(r) =
\begin{cases}
4 \epsilon \left[ \left(\frac{\sigma}{r}\right)^{12}
Copy link
Member

Choose a reason for hiding this comment

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

It may be handy to allow an offset similar to the offset in the Lennard-Jones interaction.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My point was to rather keep it simple, as there is already a Lennard-Jones that has all the bells and whistles.

Copy link
Member

Choose a reason for hiding this comment

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

It's open for extension, so I'm okay

@fweik fweik merged commit dfd792c into espressomd:python Oct 4, 2018
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.

3 participants