Suggestion

Dear all,

I'm not sure if this suggestion is reasonable for you,
however I noticed when using lammps_scatter_atoms, and
there is no atom_map defined, then the program will segfault.

I'm not sure, if one should warn the user, that the
input he gave, in fact might not be suitable for t
he considered subsequent (parallel) computations.

Best,
Stephan

Dear all,

I'm not sure if this suggestion is reasonable for you,
however I noticed when using lammps_scatter_atoms, and
there is no atom_map defined, then the program will segfault.

I'm not sure, if one should warn the user, that the
input he gave, in fact might not be suitable for t
he considered subsequent (parallel) computations.

​the code for lammps_scatter_atoms() has this test at the beginning:

  // error if tags are not defined or not consecutive or no atom map

  int flag = 0;
  if (lmp->atom->tag_enable == 0 || lmp->atom->tag_consecutive() == 0) flag
= 1;
  if (lmp->atom->natoms > MAXSMALLINT) flag = 1;
  if (lmp->atom->map_style == 0) flag = 1;
  if (flag && lmp->comm->me == 0) {
    lmp->error->warning(FLERR,"Library error in lammps_scatter_atoms");
    return;
  }

which should already print a warning, if no map is defined or other
insufficient conditions are detected.
​are you saying, you get a segfault *before* the warning is printed?

​axel.​

Oh,

yeah I see this message, you're again right.
Okay, then the execution is not aborted,
but the behaviour might be undefined.

That makes sense to me.

Best,
Stephan

Oh,

yeah I see this message, you're again right.
Okay, then the execution is not aborted,
but the behaviour might be undefined.

​right, there is a logic error in how the warning is handled. only MPI rank
0 prints the warning and ​returns, but all ranks have to return and only
rank 0 should print the warning.
thus the library.cpp file should be changed as follows.

axel.

diff --git a/src/library.cpp b/src/library.cpp
index da81194..e518134 100644
--- a/src/library.cpp
+++ b/src/library.cpp
@@ -417,8 +417,9 @@ void lammps_gather_atoms(void *ptr, char *name,
   int flag = 0;
   if (lmp->atom->tag_enable == 0 || lmp->atom->tag_consecutive() == 0)
flag = 1;
   if (lmp->atom->natoms > MAXSMALLINT) flag = 1;
- if (flag && lmp->comm->me == 0) {
- lmp->error->warning(FLERR,"Library error in lammps_gather_atoms");
+ if (flag) {
+ if (lmp->comm->me == 0)
+ lmp->error->warning(FLERR,"Library error in lammps_gather_atoms");
     return;
   }

@@ -506,8 +507,9 @@ void lammps_scatter_atoms(void *ptr, char *name,
   if (lmp->atom->tag_enable == 0 || lmp->atom->tag_consecutive() == 0)
flag = 1;
   if (lmp->atom->natoms > MAXSMALLINT) flag = 1;
   if (lmp->atom->map_style == 0) flag = 1;
- if (flag && lmp->comm->me == 0) {
- lmp->error->warning(FLERR,"Library error in lammps_scatter_atoms");
+ if (flag) {
+ if (lmp->comm->me == 0)
+ lmp->error->warning(FLERR,"Library error in lammps_scatter_atoms");
     return;
   }

in addition, it might be beneficial, to return a success/failure status from this function, so that a calling code can detect, if the scatter/gather operation worked or not.

axel.

thanks,
I opened a pullrequest on Github.

Best,
S.