[dev question] invalid write in Dump::sort()

I have run into a nasty bug, possibly related to a fix I am developing.
Debugging with valgrind tells me that an invalid write occurs in
Dump::sort() line 550

==13864== Invalid write of size 4
==13864== at 0x81178FC: LAMMPS_NS::Dump::sort() (dump.cpp:550)
==13864== by 0x8116A74: LAMMPS_NS::Dump::write() (dump.cpp:324)
==13864== by 0x827E625: LAMMPS_NS::Output::write(long long) (output.cpp:302)
==13864== by 0x8486D66: LAMMPS_NS::Verlet::run(int) (verlet.cpp:310)
==13864== by 0x8458B36: LAMMPS_NS::Run::command(int, char**) (run.cpp:174)
==13864== by 0x8217D61: LAMMPS_NS::Input::execute_command() (run.h:16)
==13864== by 0x821590F: LAMMPS_NS::Input::file() (input.cpp:201)
==13864== by 0x8228C6B: main (main.cpp:30)
==13864== Address 0x491ac84 is not stack'd, malloc'd or (recently) free'd

I'm using the latest LAMMPS version ("7 Jun 2013")
line 550 in dump is the assignment to index[]

    if (reorderflag)
      for (i = 0; i < nme; i++)
        index[idsort[i]-idlo] = i;

To be honest, I do not quite understand what is happening in this
function, especially what "idlo" is.

My fix does a few funky things, like adding hundreds of atoms at the
beginning of the fix (in post_force) and removing most (at the end of
the fix routine).
This leaves huge gaps in the atom->tags. Could that be an issue? The
error above is the only error that is triggered (and it is completely
outside the code I modified).
Daniel

That section of code is invoked when reorderflag is set, which
indicates a “sort” can be performed w/out actually sorting. I.e.

// set reorderflag = 1 if can simply reorder local atoms rather than sort
// criteria: sorting by ID, atom IDs are consecutive from 1 to Natoms
// min/max IDs of group match size of group

From the header file:
int idlo; // lowest ID I own when reordering

For example if there were 1000 atoms and 10 procs, idlo would
be 1, 101, 201, etc on various procs, I believe.

This leaves huge gaps in the atom->tags. Could that be an issue?

yes, I think so. In that case reorderflag should be turned off, b/c
a true sort is needed. So I suggest figuring out why the dump
is not detecting that it cannot do a simple reorder.

Steve

I think I got to the bottom of it. There is a conditional block in
Dump::init() which sets reorderflag to 1 which calls

atom->tag_consecutive()

At the time I initialize the dump this returns true. But as soon as my
fix is triggered the tags are not consecutive anymore. Unfortunately
Dump never checks for consecutiveness again.
I worked around it by putting a dump_modify 1 sort off command in my input file.

I tried to trigger this bug by putting a delete_atoms command into the
every clause of the run command, but then the maxsort value is still
high enough. Just taking out ids won't for, to make Dump::sort fail a
new atom with an ID higher than maxid+1 has to be added. Maybe a
combination of delete_atoms and fix deposit could break it.

So it looks like I should have my fix scan for active dumps and
disable sorting in them to make them play nice.

So it looks like I should have my fix scan for active dumps and
disable sorting in them to make them play nice.

I think that’s backwards. Your fix should set a flag, that the
code in dump::init() checks for to force reorderflag = 0.
I.e. the presence of a fix that leaves gaps in the IDs
should force the dump to do a true sort instead of
a simpler reorder.

Steve