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

use correct Maxwell-Boltzmann velocity distribution in reaction ensemble #2377

Merged
merged 7 commits into from
Nov 30, 2018

Conversation

thepith
Copy link
Contributor

@thepith thepith commented Nov 15, 2018

Fixes #1770 and #2377

The velocities of the particles inserted by the reaction ensemble did not follow the Maxwell-Boltzmann velocity distribution [1]. Even worse, with increasing temperature the mean squared velocity was decreasing. The effect of this PR on the velocity distribution function are shown in the following figure:

maxwell_plot

As you can see, now the measured velocity distribution from the simulation (marked as Sim in the plots).becomes wider with increasing temperature and follows the Maxwell-Boltzmann distribution (marked as Theo in the plots).

[1]: Atkins, P. W., and J. de Paula. Atkins’ Physical Chemistry. Oxford, UK: Oxford University Press, 2006.

@codecov
Copy link

codecov bot commented Nov 15, 2018

Codecov Report

Merging #2377 into python will increase coverage by <1%.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           python   #2377    +/-   ##
=======================================
+ Coverage      72%     72%   +<1%     
=======================================
  Files         397     397            
  Lines       18773   18778     +5     
=======================================
+ Hits        13580   13585     +5     
  Misses       5193    5193
Impacted Files Coverage Δ
src/core/reaction_ensemble.cpp 72% <100%> (ø) ⬆️

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 4ba6b58...1146eca. Read the comment docs.

@fweik
Copy link
Contributor

fweik commented Nov 15, 2018

Thank you for your contribution. This looks more correct than before to me, but finally @jonaslandsgesell has to decide.

src/core/reaction_ensemble.cpp Outdated Show resolved Hide resolved
Draw a random velocity according to the Maxwell-Boltzmann distribution.

Fixes espressomd#1770
@jonaslandsgesell
Copy link
Member

This PR fixes #1770 and #2377 for particles of mass 1. For particles with different masses the Maxwell-Boltzmann distribution needs to take into account the particle mass which has to be provided to the reaction ensemble algorithm as input then. This is not yet fixed with this PR and remains an issue. I opened issue #2388 to document this.

@fweik
Copy link
Contributor

fweik commented Nov 26, 2018

I don't get it, ReactionAlgorithm::create_particle only creates particles of mass 1, no?

@jonaslandsgesell
Copy link
Member

jonaslandsgesell commented Nov 26, 2018 via email

@fweik
Copy link
Contributor

fweik commented Nov 26, 2018

I mean you could just draw the velocity from the correct MBD for the particle, and this would be correct for ever right? Why can't you just put in the mass?

@jonaslandsgesell
Copy link
Member

jonaslandsgesell commented Nov 27, 2018 via email

@thepith
Copy link
Contributor Author

thepith commented Nov 27, 2018

Or perhaps to rephrase myself: I currently do not have the capacity to do it. If @jonaslandsgesell wants to do it, we can incorporate it in this pull request.

Maintainers are allowed to edit, so I believe Jonas can push to my branch...

src/python/espressomd/reaction_ensemble.pyx Outdated Show resolved Hide resolved
src/python/espressomd/reaction_ensemble.pyx Outdated Show resolved Hide resolved
@RudolfWeeber RudolfWeeber merged commit c9410e8 into espressomd:python Nov 30, 2018
RudolfWeeber added a commit to RudolfWeeber/espresso that referenced this pull request Nov 30, 2018
use correct Maxwell-Boltzmann velocity distribution in reaction ensemble
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.

6 participants