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

Propagators #4820

Merged
merged 3 commits into from
Dec 20, 2023
Merged

Propagators #4820

merged 3 commits into from
Dec 20, 2023

Conversation

ThPaul
Copy link
Contributor

@ThPaul ThPaul commented Nov 21, 2023

Fixes #4603

Description of changes:

  • add a per-particle propagation flag to control which equations of motion are propagated during integration
  • all virtual sites implementations can now be used simultaneously (system.virtual_sites was removed)
  • virtual sites relative can now be coupled only to the Langevin and LB thermostats (possibly both: Langevin for rotation, LB for translation), using the new particle.vs_relate_to() keyword arguments couple_to_lb=True and couple_to_langevin=True (both are False by default)
  • finer control over the propagation mode combinations can be obtained by setting the particle.propagation bitmask using combinations of espressomd.propagation.Propagation enum values; only a small number of combinations are currently allowed, but the list will grow as new propagation schemes are developed

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@RudolfWeeber
Copy link
Contributor

RudolfWeeber commented Dec 5, 2023

Work in progress: Test status

This tries to summarize what is currently tested and which (mixed cases) we are not catching.

Single propagaiton scheme

  • LB Tracers (virtual_sites_tracers*.py) - tests force distribution from tracer to fluid and advection of tracer
  • vs_relative
    • virtual_sites_relative.py: - test placement, force distribution and runs a fluid of lj dumbells and verifies cofrces.
    • engine.py, once the python branch is merged - relies on vs orientation. Since this pr let's users control translation and rotation of vs_relative separately, this will have to be revisited.
  • Unthermalized velocity verlet (integrate.py) tests Newton's laws
  • Velociyt verlet + Langevin: langevin.py (friction, mostly) and langevin_stats.py (Maxwell distribution, velocity auto-correlation function, noise properties. No MSD)
  • langevin for particles and box size (npt): thermostat_npt.py and thermostat_npt_stats.py) - however, the latter basically checks against a magic number, as far as I understand
  • Brownian dynamics: thermostat_anisotropic.py
  • stokesian dynamics: test for two sedimenting spheres against stored data from a paper
  • viscous (langevin-like coupling to LB) lb.py::test_viscous_coupling()

Mixed cases

  • LB tracers + vs_relative + lb viscous coupling
  • LB viscous coupling for translation + langevin friction for rotation
  • vs_relative for translation + langevin for rotation
  • Velocity Verlet + vs_relative (virtual_sites_relative.py:test_lj()) (it does not test Langeivn statistics, though), thermostat_anisotropic.py (friction coefficients per Cartesian axis in particle body frame)

Further points

This is things I stumbled over while going through the tests. It's not all related to this PR.

  • thermostat act_on_virtual filtering: we have to understand, whether this becomes obsolte. Probably yes, because one can just not set the respective propagation flag on those particles
  • I don't think, we have an integration test for raspberries in a thermalized LB.
  • Are we testing an msd for a particle with Langevin eqn somewhere. I didn't find it in the testsuite, but there is a tutorial.

@RudolfWeeber
Copy link
Contributor

Tentative merge strategy

  • whitelist specific propagor settings on the particle level for which we have tessts
  • incrementally add tests and enable more propagation modes

This way, we can merge the PR and still make sure that onl tested combinations can be used.

Step 1

  • allow all propagatoins on the system wide level (as it is now in ES)

  • allow setting of (TRANS_VS_RELATIVE+ROT_VS_RELATIVE) and (TRANS_LB_TRACERS) on the particle level

  • Remove virtual flag from particle

  • make sure, only above combinatoins can be set on the particle (aside from system default)

  • adapt the act_on_virtual filter in the lb coupling to exclude vs_relative and lb_tracers propagations

  • adapt other filters for the virtual property of particles, e.g., kinetic energy

  • test non-virtual + vs_relative + inertialess tracers with lb

@RudolfWeeber
Copy link
Contributor

@jgrad the particels in testsuite/python/propagation.py are randomly placed. The forces between them are on the order of 1E6.
Are you that the test failure at 3 mpi cores is actually due to a bug or isn't just really moveing a particle more than a local box length?

src/core/integrate.cpp Show resolved Hide resolved
src/core/integrate.cpp Outdated Show resolved Hide resolved
src/core/integrate.cpp Show resolved Hide resolved
src/core/integrate.cpp Outdated Show resolved Hide resolved
src/core/integrate.cpp Outdated Show resolved Hide resolved
src/core/integrate.cpp Outdated Show resolved Hide resolved
src/script_interface/particle_data/ParticleHandle.cpp Outdated Show resolved Hide resolved
@RudolfWeeber
Copy link
Contributor

So the main open issue is coupling of virtual sites.

As I understand it, the cleanest solution would be

  • the flags TRANS_VS_RELATIVE and ROT_VS_RELATIVE do not imply any coupling to LB or Langevin noise/friction
  • if that's wanted, TRANS_LANGEVIN or TRANS_LB_MOMENTUM_EXCHANGE have to be added to the virtual site
  • There will not be a system default for this
  • for convencience, p.vs_atuo_ralte_to() gets two additional keyword arguments couple_to_lb=False and apply_langevin_noise_and_frictoin=False, which the user can override when setting up the virtual site. This will then set the respective propagatoin flags on the virtual site.

Is this (in particular the absence of teh posibility to contorl this in one place for the entire system) too anoying for users?
A system default could in principle be implemente dbut it would make the decisoi logic on virtual sites really complicated.

@jngrad jngrad requested a review from RudolfWeeber December 18, 2023 20:05
Co-authored-by: Rudolf Weeber <[email protected]>
Co-authored-by: Jean-Noël Grad <[email protected]>
RudolfWeeber
RudolfWeeber previously approved these changes Dec 20, 2023
@RudolfWeeber
Copy link
Contributor

this implements most of the infrastructure to give people more flexibility w.r.t equations of motion, which is one of the main featues we promised for the next release.
It turned out to be significantly more complex than initially expected.
Many thanks to Thilo and JN for making it happen.

@jngrad jngrad force-pushed the propagators branch 2 times, most recently from f6179ec to a7173b7 Compare December 20, 2023 11:43
@jngrad jngrad marked this pull request as ready for review December 20, 2023 12:03
@jngrad jngrad added this to the ESPResSo 4.3.0 milestone Dec 20, 2023
@kodiakhq kodiakhq bot merged commit cc3a319 into espressomd:python Dec 20, 2023
10 checks passed
@jngrad jngrad mentioned this pull request Jan 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Propagation: Add Particle property
3 participants