Fix using a neigh list issue

Hi,

I have noticed an issue with neighbour list recreation maybe it is a bug.

I have created a simple fix that adds itself into neighbour requestor to get a neighbour list through init_list() where it saves the pointer privately. Then during post_force it uses this pointer to print out pointer address. Now when I create the fix first time and do run 1 init_list is called and everything works. After that in the same lammps script I unfix the created fix and create a new one with the same arguments. After that I do again run 1, but this time init_list is not called and the pointer that should be point to neighbour list is NULL. Thus when I would do anything with the neighbour list I would get a segfault.

I think that the problem is located in the neighbor.cpp:454 (1Feb-2014 version) with the requests[i]->identical(old_requests[i]). Namely, the fix class pointers are checked to identify if a new fix different from the old fix, but there is a probability that the new fix that was created has the same memory address and thus not being registered as a new one and thus the init_list will not be called.

So in my opinion checking only the memory address is not unique enough to identify the creation of a new fix and this might result with a segfault with any fix that uses neighbour lists and that is destroyed and then created again.

Sorry for the vague description of the issue, but if needed I could provide a test case with the custom example fix.

Best wishes,
Artur Tamm

Hi,

I have noticed an issue with neighbour list recreation maybe it is a bug.

I have created a simple fix that adds itself into neighbour requestor to
get a neighbour list through init_list() where it saves the pointer

This makes no sense.

privately. Then during post_force it uses this pointer to print out
pointer address. Now when I create the fix first time and do run 1
init_list is called and everything works. After that in the same lammps
script I unfix the created fix and create a new one with the same
arguments. After that I do again run 1, but this time init_list is not
called and the pointer that should be point to neighbour list is NULL.
Thus when I would do anything with the neighbour list I would get a
segfault.

What do you want to do with it?

I think that the problem is located in the neighbor.cpp:454 (1Feb-2014
version) with the requests[i]->identical(old_requests[i]). Namely, the
fix class pointers are checked to identify if a new fix different from
the old fix, but there is a probability that the new fix that was
created has the same memory address and thus not being registered as a
new one and thus the init_list will not be called.

So in my opinion checking only the memory address is not unique enough
to identify the creation of a new fix and this might result with a
segfault with any fix that uses neighbour lists and that is destroyed
and then created again.

Sorry for the vague description of the issue, but if needed I could
provide a test case with the custom example fix.

So why don’t you do it right away?

Hi,

So here are the files. lammps_fix_test contains the input script and fixexample the fix itself. When you run this script the fix prints into logfile two pointers one containing the class adress and another containing the neighList address wchich should not become NULL. This simplified fix illustrates an issue I had with another fix that did something useful with the neighbour list and received a segfault.

Best wishes,

lammps_fix_test.tar.gz (231 KB)

fixexample.tar.gz (1.18 KB)

Hi,

So here are the files. lammps_fix_test contains the input script and fixexample the fix itself. When you run this script the fix prints into logfile two pointers one containing the class adress and another containing the neighList address wchich should not become NULL. This simplified fix illustrates an issue I had with another fix that did something useful with the neighbour list and received a segfault.

​ok. i see the problem now. there is a way to address this cleanly, but that requires a change that i first need to discuss with other developers.

​as a workaround, you could add the following two lines to your destructor.​

int irequest = neighbor->request(this);

neighbor->requests[irequest]->fix = 0;

more later.

axel.

Hi,

Another simple solution that seems to work is to do a run 0 after the
unfix because then the nrequests will be changed and the fix would be
recognized as a new one.

Artur

I think I fixed this - will be in today’s patch.
Please give it a try.

thanks,
Steve