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

Manually overwriting bond objects of different type is undefined behavior #4377

Closed
jngrad opened this issue Oct 22, 2021 · 1 comment · Fixed by #4350
Closed

Manually overwriting bond objects of different type is undefined behavior #4377

jngrad opened this issue Oct 22, 2021 · 1 comment · Fixed by #4350

Comments

@jngrad
Copy link
Member

jngrad commented Oct 22, 2021

The python interface currently allows overwriting bond objects. This is undefined behavior when a bond is overwritten by a bond of a different type. For example, when replacing a 2-center bond by a 3- or 4-center bond, the list of partners in the BondView is not updated and the force calculation will access span elements out of bounds. This bug can be fixed without impacting the collision detection feature (see #4225).

MWE:

import espressomd.interactions
system = espressomd.System(box_l=[20.0, 20.0, 20.0])
p1, p2, p3, p4 = system.part.add(pos=[[1, 0, 0], [0, 0, 0], [0, 1, 0], [1, 1, 0]])
system.time_step = 0.01
system.cell_system.skin = 0.4
system.bonded_inter[0] = espressomd.interactions.HarmonicBond(k=0.5, r_0=1.1)
p1.add_bond((system.bonded_inter[0], p2))
print(p1.bonds)
system.integrator.run(0, recalc_forces=True)
system.bonded_inter[0] = espressomd.interactions.Dihedral(mult=3, bend=5.2, phase=3.)
print(p1.bonds)  # issue: there is only one bonded partner, but Dihedral needs 3 partners
system.integrator.run(0, recalc_forces=True)  # segfault: access out of bounds

Output in a debug build:

((HarmonicBond(0): {'k': 0.5, 'r_0': 1.1, 'r_cut': 0.0}, 1),)
((Dihedral(0): {'mult': 3.0, 'bend': 5.2, 'phase': 3.0}, 1),)
python3: /work/jgrad/espresso/src/utils/include/utils/Span.hpp:103: constexpr T& Utils::Span<T>::operator[](Utils::Span<T>::size_type) const [with T = Particle*; Utils::Span<T>::reference = Particle*&; Utils::Span<T>::size_type = long unsigned int]: Assertion `(i < size())' failed.
[seraue:3978640] *** Process received signal ***
[seraue:3978640] Signal: Aborted (6)
[seraue:3978640] Signal code:  (-6)
[seraue:3978640] [ 0] /lib/x86_64-linux-gnu/libc.so.6(+0x46210)[0x1540b9cc1210]
[seraue:3978640] [ 1] /lib/x86_64-linux-gnu/libc.so.6(gsignal+0xcb)[0x1540b9cc118b]
[seraue:3978640] [ 2] /lib/x86_64-linux-gnu/libc.so.6(abort+0x12b)[0x1540b9ca0859]
[seraue:3978640] [ 3] /lib/x86_64-linux-gnu/libc.so.6(+0x25729)[0x1540b9ca0729]
[seraue:3978640] [ 4] /lib/x86_64-linux-gnu/libc.so.6(+0x36f36)[0x1540b9cb1f36]
[seraue:3978640] [ 5] /work/jgrad/espresso/build/src/core/EspressoCore.so(Utils::Span<Particle*>::operator[](unsigned long) const+0x45)[0x1540b84920d3]
[seraue:3978640] [ 6] /work/jgrad/espresso/build/src/core/EspressoCore.so(add_bonded_force(Particle&, int, Utils::Span<Particle*>)+0xfa)[0x1540b84c37d9]
[seraue:3978640] [ 7] /work/jgrad/espresso/build/src/core/EspressoCore.so(void CellStructure::execute_bond_handler<bool (*)(Particle&, int, Utils::Span<Particle*>)>(Particle&, bool (*)(Particle&, int, Utils::Span<Particle*>))+0x12a)[0x1540b84c91de]
[seraue:3978640] [ 8] /work/jgrad/espresso/build/src/core/EspressoCore.so(void CellStructure::bond_loop<bool (*)(Particle&, int, Utils::Span<Particle*>)>(bool (* const&)(Particle&, int, Utils::Span<Particle*>))+0xc6)[0x1540b84c7712]
...
@RudolfWeeber
Copy link
Contributor

RudolfWeeber commented Oct 23, 2021 via email

@kodiakhq kodiakhq bot closed this as completed in #4350 Oct 26, 2021
kodiakhq bot added a commit that referenced this issue Oct 26, 2021
Fixes #4225
Fixes #4377
Fixes #4169

Description of changes:
- Bonded interactions are now stored as ScriptInterface objects, are immutable and can be removed
- It is no longer possible to overwrite a bond object by a different type of bond (avoids segmentation faults)
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.

2 participants