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

Brownian Dynamics within the existing VV loop #1842

Merged
merged 207 commits into from
Jan 25, 2020

Conversation

bogdan-tanygin
Copy link
Contributor

Fixes #1360, also it contains the integrated changes from PRs #1622, #1757 which were required to validate both existing Langevin- (LD) and new Brownian Dynamics (BD) viscous and fluctuation dynamics including the rotational one.

Description of changes:

  • Generally, this is a conventional simplest BD [schlick2010].
  • Corresponding Sphinx documentation section is populated with the required details to reveal the concept.
  • A velocity, even though it is optional for a simplest implementation of the BD, has been added for a consistency with the existing VV loop (c.f. Do we need a Brownian Dynamics (BD) integrator ? #1360 (comment)) and all the existing tests and new tests. It does not impact the positional mechanics though which is natural for the BD [schlick2010].
  • The velocity and position random walk has been added according to the classical diffusion equations [schlick2010,Pottier2014].
  • Rotational motion BD is implemented analogously. Existing quaternion approach has been reused where possible.
  • New test brownian_thermostat.py is based on the existing langevin_thermostat.py. It contains some commented out fragments which are inconsistent for the simplest BD cause ones correspond to more fine dynamical structure compare to the BD temporal relation time_step >> mass / gamma. The Maxwell distribution test is passing well and the Gaussian noise type is crucial for this which has been done in the thermostat.hpp.
  • The mass-and-rinertia_per_particle.py contains changes described within the PRs Anisotropic rotational diffusion test #1622, Drag terminal velocity tests #1757. Probably, it is of interest for both the LD and BD independently. This is why I've opened that PRs in advance.
  • Existing automatic tests execution timeline is minimized.
  • Additional "heavy" test has been added without an automatic run: mass-and-rinertia_per_particle_rotdiff-longrun.py. It requires ~2 hours of a run and provides the intensive validation of the existing LD and new BD dynamics in the same way.

PR Checklist

  • Tests?
    • Interface
    • Core
  • Docs?

References
schlick2010: https://link.springer.com/book/10.1007%2F978-1-4419-6351-2
Pottier2014: https://link.springer.com/article/10.1007/s10955-010-0114-6

..aka the drift in case of the electrostatics
@jngrad
Copy link
Member

jngrad commented Jan 21, 2020

@RudolfWeeber Let me know if ede6e7e is what you had in mind. Don't forget to add #define BROWNIAN_PER_PARTICLE in your myconfig.hpp.

@RudolfWeeber
Copy link
Contributor

@jngrad, yes. Could you please gather all the Brownian-dynamics related globals in a sinlge struct? Then, it's ready, I think.

@jngrad
Copy link
Member

jngrad commented Jan 22, 2020

The Brownian globals are now in a struct.

In jngrad/therm-refactor I also moved the Langevin and NPT globals in structs and moved the Langevin code to integrators/langevin_inline.hpp for consistency with integrators/brownian_inline.hpp. Feel free to do git merge --ff-only. If you consider it's out of scope, I'll open a dedicated PR once this one is merged.

@RudolfWeeber
Copy link
Contributor

@hmenke, from my side, this PR is ready. Please check your open review for things not addressed. If there aren't any, please approve the PR.

if (!(p.p.ext_flag & COORD_FIXED(j)))
#endif
{
p.r.p[j] += p.f.f[j] * dt / (local_gamma[j]);
Copy link
Member

Choose a reason for hiding this comment

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

still missing a check for virtual particles here and probably at other places too

@RudolfWeeber
Copy link
Contributor

@KaiSzuttor, the virtual sites are excluded for translational degrees in
void brownian_dynamics_propagator(const ParticleRange &particles)

For rotational degrees, VS are not excluded, which is correct. If the virtual sites have_quanterion feature is used, the virtual sites update will assign the correc orientation after the propagation step.

@espresso-ci
Copy link

espresso-ci commented Jan 23, 2020 via email

@RudolfWeeber
Copy link
Contributor

I found a different but related issue. The testcase for checking thermo_virtual with Brownian dynamics was incorrect (copy/paste error from the Langevin test).
Fixing the test revealed that thermo_virtual=True was ignored by the virtual sites guards. Pushing the fix now.

src/core/thermostat.cpp Outdated Show resolved Hide resolved
@fweik
Copy link
Contributor

fweik commented Jan 25, 2020

@jngrad I think we should merge this now, or you should push your tests onto this branch.

@jngrad jngrad added the automerge Merge with kodiak label Jan 25, 2020
@kodiakhq kodiakhq bot merged commit b06b9b5 into espressomd:python Jan 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Do we need a Brownian Dynamics (BD) integrator ?
7 participants