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

Box length change invalidates particle positions #4834

Closed
jngrad opened this issue Dec 6, 2023 · 0 comments · Fixed by #4901
Closed

Box length change invalidates particle positions #4834

jngrad opened this issue Dec 6, 2023 · 0 comments · Fixed by #4901

Comments

@jngrad
Copy link
Member

jngrad commented Dec 6, 2023

When manually modifying the box length, the new particle positions are recalculated with the wrong box size.

MWE:

import espressomd
import numpy as np
np.set_printoptions(precision=2)
system = espressomd.System(box_l=[15., 15., 15.])
system.time_step = 0.01
system.cell_system.skin = 0.4
p = system.part.add(pos=[-0.1, 15.1, 30.])
expected = p.pos_folded * 10./15.
print(f"before:   {p.pos_folded}")
system.box_l = [10., 10., 10.]
print(f"after:    {p.pos_folded}")
print(f"expected: {expected}")

Output:

before:   [14.90 0.10 0. ]
after:    [ 4.90 0.10 0. ]
expected: [ 9.93 0.07 0. ]

This bug is reproducible on the python branch and with ESPResSo releases 4.2.1 and 4.1.4.

It is unclear to me, how to gracefully handle a box resize. One could fold the particle position onto the unit cell, apply the box resize via rescaling, and re-use the older image box counter. But then, a jump may become visible in trajectories in unfolded coordinates. One would also need to maintain the old and new value in the BoxGeometry object to apply the folding and then unfolding with the two box sizes. It is also unclear what should happen with Lees-Edwards boundary conditions, where particle positions gain an offset when going through one of the three periodic boundaries.

The easiest course of action is to prevent manually changing the box length if the box contains particles, and rely instead on the system.change_volume_and_rescale_particles(), which implements the correct behavior. Alternatively, one could make the box length setter call the rescale method under-the-hood, but this introduces a new problem: the on_boxl_change event would be called three times, causing long-range solvers to re-tune all solver parameters three times with a different box size. Since the box might no longer be cubic for the first two re-tunes, the tuning code may become extremely slow, have to sample more mesh sizes, and throw an exception if the accuracy cannot be reached.

Note: this bug doesn't affect the NpT integrator, which uses the rescale algorithm.

@jngrad jngrad added this to the ESPResSo 4.2.2 milestone Dec 6, 2023
@kodiakhq kodiakhq bot closed this as completed in #4901 Apr 9, 2024
kodiakhq bot added a commit that referenced this issue Apr 9, 2024
Fixes #4834

Description of changes:
- Resizing the box via `system.box_l = new_box_l` now throws a runtime error if there are particles present, because particle position folding cannot be guaranteed to be correct; use `system.change_volume_and_rescale_particles()` instead, which properly rescales particle positions.
jngrad pushed a commit to jngrad/espresso that referenced this issue Apr 25, 2024
Fixes espressomd#4834

Description of changes:
- Resizing the box via `system.box_l = new_box_l` now throws a runtime error if there are particles present, because particle position folding cannot be guaranteed to be correct; use `system.change_volume_and_rescale_particles()` instead, which properly rescales particle positions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant