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

Simplify VS_Relative, fix VS support for Lees Edwards #4564

Merged
merged 3 commits into from
Mar 28, 2023

Conversation

RudolfWeeber
Copy link
Contributor

  • Removs the less-than-transparent shifting logic which prevents posiiton folding of vs relative. this is no longer needed, since we now use minimum_image_distance everywhere.
  • Apply Lees Edwards position shift to VS on update
  • Alwasy fold shear plane normal coordinates in Lees Edwards minimum image dsitance

The trajectory reconstruciton stuff isn't working yet.

@RudolfWeeber RudolfWeeber changed the title WIP: Simplify VS_Relative, fix VS support for Lees Edwards Simplify VS_Relative, fix VS support for Lees Edwards Sep 6, 2022
@RudolfWeeber
Copy link
Contributor Author

I sorted out the trajectory reconstruciotn (p.lees_edwards_offset) as follows:
The lees_edwards_offset tracked in the particle now exactly matches the sum of all position offset aplied on Lees-Edwards boundary crossings. I also formulated the test like that. IMO, this is more consistent than what was there previously.

Should we re-instate a 2 step position update (as we have a two-step integrator and one can argue that the LE position offset has changed during the half-step), this can be done by having a pos update in the Push and UpdateOffset, each.
I'd not re-introduce a two step velocity update, as the velocity shift should match teh particle position: when the particle has crossed the boudnary, the velocity shift has to be applied completely, otherwise, there will be huge forces in a simulation with Lattice-Boltzmann coupling.
There, once, once the boundary is crossed, the flow velocity changes from +v_shear/2 to -v_shear/2, so it would lead to a big force to couple a particle with v=+v_shear/2 - v_shear/2 = 0. Note that thee is only one LB couling, even if we use a two-step integrator.

@RudolfWeeber RudolfWeeber requested a review from jngrad September 6, 2022 06:49
@jngrad
Copy link
Member

jngrad commented Oct 24, 2022

This work was merged in the walberla branch. Closing.

@jngrad jngrad added this to the Espresso 4.2.1 milestone Mar 28, 2023
@jngrad jngrad added Core BugFix automerge Merge with kodiak labels Mar 28, 2023
@kodiakhq kodiakhq bot merged commit 83573ae into espressomd:python Mar 28, 2023
jngrad pushed a commit to jngrad/espresso that referenced this pull request Mar 29, 2023
* Remove the less-than-transparent shifting logic which prevents posiiton folding of vs relative. this is no longer needed, since we now use minimum_image_distance everywhere.
* Apply Lees Edwards position shift to VS on update
* Always fold shear plane normal coordinates in Lees Edwards minimum image distance
kodiakhq bot added a commit that referenced this pull request Apr 14, 2023
Fixes #4703

Description of changes:
- bugfix: virtual sites relative are now properly folded again (the regression was introduced by #4564)
- bugfix: uninitialized virtual sites now throw a runtime error instead of implicitly tracking the particle with pid=0
- write more thorough tests for virtual sites relative: integration through periodic boundaries, checkpointing
jngrad pushed a commit to jngrad/espresso that referenced this pull request Apr 14, 2023
Fixes espressomd#4703

Description of changes:
- bugfix: virtual sites relative are now properly folded again (the regression was introduced by espressomd#4564)
- bugfix: uninitialized virtual sites now throw a runtime error instead of implicitly tracking the particle with pid=0
- write more thorough tests for virtual sites relative: integration through periodic boundaries, checkpointing
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.

2 participants