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

[Feature Request] Improve efficiency of fix_ipi #4155

Open
ceriottm opened this issue May 1, 2024 · 0 comments · May be fixed by #4157
Open

[Feature Request] Improve efficiency of fix_ipi #4155

ceriottm opened this issue May 1, 2024 · 0 comments · May be fixed by #4157

Comments

@ceriottm
Copy link
Collaborator

ceriottm commented May 1, 2024

Summary

In the process of preparing a new release of i-pi we realized that the client-side implementation in LAMMPS have some issues that introduce unnecessary overheads. These are not really bugs, but affect negatively the performance of code running together with i-PI (as well as code such as ASE using the same protocol). We are already working on a fix but we would like to get feedback to facilitate integration.

Detailed Description

There are two separate issues, one easy one trickier:

TCP/IP slowdown with small systems

The problem with an easy fix is that we noticed a paradoxical slowdown of TCP/IP socket communication when the simulation is small - this is due to something called Nagle's algorithm, and we need to change one flag at socket creation. This will be limited to fix_ipi and should be uncontroversial.

Unnecessary NL build

Basically, as far as I understand, LAMMPS expects trajectories to be continuous, but also atom positions not to stray too far from the (0,0,0) replica, so it folds back atoms into the unit cell when the NL is updated. Given that i-PI either never folds, or folds every time before passing the atoms, LAMMPS detects large atoms movements at almost every step, triggering many unnecessary NL updates. We found this to lead to significant overhead in some cases and we would like to address it. I see two possible fixes but both are quite invasive and so I would like to hear if developers here have preferences, or have a better idea

  • solution 1: applying PBC when checking atom drift in neighbor.cpp, lines 2384-2386; this adds a small but probably noticeable overhead over an operation that is done for every MD simulation step, and I suspect that it's not currently done for a reason. Possible mitigation would be to add a neigh_modify option that defaults to false, and have i-PI set it to true. This IMO is the cleanest, and my preferred, solution.
  • solution 2: every time we receive new positions in fix_ipi, we match positions to neighbor->xhold to ensure continuity, so effectively it's like solution 1, but done within fix_ipi, so we don't mess around with one of the core LAMMPS routines. the problem is that neighbor.xhold is declared protected, and so we need to either make it public, or make fix_ipi, that is not a core class, friend of neighbor. again, we need to touch a core class, although in a less invasive way.

Feedback welcome. If we don't hear any, we'll open a PR implementing solution 1.

@ceriottm ceriottm changed the title Improve efficiency of fix_ipi [Feature Request] Improve efficiency of fix_ipi May 1, 2024
@ceriottm ceriottm linked a pull request May 2, 2024 that will close this issue
11 tasks
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