Merge pull request #4182 from akohlmey/group-bitmap-accessor

Refactor code to use group bitmap accessor

src/atom.cpp

@@ -2301,7 +2301,7 @@ void Atom::first_reorder()
   // nfirst = index of first atom not in firstgroup
   // when find firstgroup atom out of place, swap it with atom nfirst
 
-  int bitmask = group->bitmask[firstgroup];
+  const int bitmask = group->get_bitmask_by_id(FLERR, firstgroupname, "Atom::first_reorder");
   nfirst = 0;
   while (nfirst < nlocal && mask[nfirst] & bitmask) nfirst++;
 

src/change_box.cpp

@@ -52,9 +52,7 @@ void ChangeBox::command(int narg, char **arg)
 
   // group
 
-  int igroup = group->find(arg[0]);
+  int groupbit = group->get_bitmask_by_id(FLERR, arg[0], "change_box");
-  if (igroup == -1) error->all(FLERR,"Could not find change_box group ID {}", arg[0]);
-  int groupbit = group->bitmask[igroup];
 
   // parse operation arguments
   // allocate ops to max possible length

src/compute.cpp

@@ -48,11 +48,11 @@ Compute::Compute(LAMMPS *lmp, int narg, char **arg) :
 
   id = utils::strdup(arg[0]);
   if (!utils::is_id(id))
-    error->all(FLERR,"Compute ID must be alphanumeric or underscore characters");
+    error->all(FLERR,"Compute ID {} must only have alphanumeric or underscore characters", id);
 
+  groupbit = group->get_bitmask_by_id(FLERR, arg[1], fmt::format("compute {}", arg[2]));
   igroup = group->find(arg[1]);
-  if (igroup == -1) error->all(FLERR,"Could not find compute group ID");
+  if (igroup == -1) error->all(FLERR,"Could not find compute group ID {}", arg[1]);
-  groupbit = group->bitmask[igroup];
 
   style = utils::strdup(arg[2]);
 

src/compute_coord_atom.cpp

@@ -39,8 +39,7 @@ ComputeCoordAtom::ComputeCoordAtom(LAMMPS *lmp, int narg, char **arg) :
 {
   if (narg < 5) error->all(FLERR, "Illegal compute coord/atom command");
 
-  jgroup = group->find("all");
+  jgroupbit = group->get_bitmask_by_id(FLERR, "all", "compute coord/atom");
-  jgroupbit = group->bitmask[jgroup];
   cstyle = NONE;
 
   if (strcmp(arg[3], "cutoff") == 0) {
@@ -50,11 +49,10 @@ ComputeCoordAtom::ComputeCoordAtom(LAMMPS *lmp, int narg, char **arg) :
 
     int iarg = 5;
     if ((narg > 6) && (strcmp(arg[5], "group") == 0)) {
+      delete[] group2;
       group2 = utils::strdup(arg[6]);
       iarg += 2;
-      jgroup = group->find(group2);
+      jgroupbit = group->get_bitmask_by_id(FLERR, group2, "compute coord/atom");
-      if (jgroup == -1) error->all(FLERR, "Compute coord/atom group2 ID does not exist");
-      jgroupbit = group->bitmask[jgroup];
     }
 
     ncol = narg - iarg + 1;

src/compute_coord_atom.h

@@ -46,7 +46,7 @@ class ComputeCoordAtom : public Compute {
   double **carray;
 
   char *group2;
-  int jgroup, jgroupbit;
+  int jgroupbit;
 
   class ComputeOrientOrderAtom *c_orientorder;
   char *id_orientorder;

src/compute_group_group.cpp

@@ -54,9 +54,7 @@ ComputeGroupGroup::ComputeGroupGroup(LAMMPS *lmp, int narg, char **arg) :
   extvector = 1;
 
   group2 = utils::strdup(arg[3]);
-  jgroup = group->find(group2);
+  jgroupbit = group->get_bitmask_by_id(FLERR, group2, "compute group/group");
-  if (jgroup == -1) error->all(FLERR, "Compute group/group group ID does not exist");
-  jgroupbit = group->bitmask[jgroup];
 
   pairflag = 1;
   kspaceflag = 0;
@@ -147,9 +145,7 @@ void ComputeGroupGroup::init()
 
   // recheck that group 2 has not been deleted
 
-  jgroup = group->find(group2);
+  jgroupbit = group->get_bitmask_by_id(FLERR, group2, "compute group/group");
-  if (jgroup == -1) error->all(FLERR, "Compute group/group group ID does not exist");
-  jgroupbit = group->bitmask[jgroup];
 
   // need an occasional half neighbor list
 

src/compute_group_group.h

@@ -35,7 +35,7 @@ class ComputeGroupGroup : public Compute {
 
  private:
   char *group2;
-  int jgroup, jgroupbit, othergroupbit;
+  int jgroupbit;
   double **cutsq;
   double e_self, e_correction;
   int pairflag, kspaceflag, boundaryflag, molflag;

src/create_bonds.cpp

@@ -59,12 +59,8 @@ void CreateBonds::command(int narg, char **arg)
   if (strcmp(arg[0], "many") == 0) {
     style = MANY;
     if (narg != 6) error->all(FLERR, "No optional keywords allowed with create_bonds many");
-    igroup = group->find(arg[1]);
+    group1bit = group->get_bitmask_by_id(FLERR, arg[1], "create_bonds");
-    if (igroup == -1) error->all(FLERR, "Cannot find create_bonds first group ID {}", arg[1]);
+    group2bit = group->get_bitmask_by_id(FLERR, arg[2], "create_bonds");
-    group1bit = group->bitmask[igroup];
-    igroup = group->find(arg[2]);
-    if (igroup == -1) error->all(FLERR, "Cannot find create_bonds second group ID {}", arg[2]);
-    group2bit = group->bitmask[igroup];
     btype = utils::inumeric(FLERR, arg[3], false, lmp);
     rmin = utils::numeric(FLERR, arg[4], false, lmp);
     rmax = utils::numeric(FLERR, arg[5], false, lmp);

src/create_bonds.h

@@ -30,7 +30,7 @@ class CreateBonds : public Command {
   void command(int, char **) override;
 
  private:
-  int igroup, group1bit, group2bit;
+  int group1bit, group2bit;
   int btype, atype, dtype;
   tagint batom1, batom2, aatom1, aatom2, aatom3, datom1, datom2, datom3, datom4;
   double rmin, rmax;

src/delete_atoms.cpp

@@ -87,8 +87,8 @@ void DeleteAtoms::command(int narg, char **arg)
     error->all(FLERR, "Unknown delete_atoms sub-command: {}", arg[0]);
 
   if (allflag) {
-    int igroup = group->find("all");
+    int igroupbit = group->get_bitmask_by_id(FLERR, "all", "delete_atoms");
-    if ((igroup >= 0) && modify->check_rigid_group_overlap(group->bitmask[igroup]))
+    if (modify->check_rigid_group_overlap(igroupbit))
       error->warning(FLERR, "Attempting to delete atoms in rigid bodies");
   } else {
     if (modify->check_rigid_list_overlap(dlist))
@@ -216,8 +216,7 @@ void DeleteAtoms::delete_group(int narg, char **arg)
 {
   if (narg < 2) utils::missing_cmd_args(FLERR, "delete_atoms group", error);
 
-  int igroup = group->find(arg[1]);
+  int groupbit = group->get_bitmask_by_id(FLERR, arg[1], "delete_atoms");
-  if (igroup == -1) error->all(FLERR, "Could not find delete_atoms group ID {}", arg[1]);
   options(narg - 2, &arg[2]);
 
   // check for special case of group = all
@@ -234,8 +233,6 @@ void DeleteAtoms::delete_group(int narg, char **arg)
   for (int i = 0; i < nlocal; i++) dlist[i] = 0;
 
   int *mask = atom->mask;
-  int groupbit = group->bitmask[igroup];
-
   for (int i = 0; i < nlocal; i++)
     if (mask[i] & groupbit) dlist[i] = 1;
 }
@@ -281,18 +278,11 @@ void DeleteAtoms::delete_overlap(int narg, char **arg)
 
   const double cut = utils::numeric(FLERR, arg[1], false, lmp);
   const double cutsq = cut * cut;
+  const int group1bit = group->get_bitmask_by_id(FLERR, arg[2], "delete_atoms");
+  const int group2bit = group->get_bitmask_by_id(FLERR, arg[3], "delete_atoms");
 
-  int igroup1 = group->find(arg[2]);
-  if (igroup1 < 0)
-    error->all(FLERR, "Could not find delete_atoms overlap first group ID {}", arg[2]);
-  int igroup2 = group->find(arg[3]);
-  if (igroup2 < 0)
-    error->all(FLERR, "Could not find delete_atoms overlap second group ID {}", arg[3]);
   options(narg - 4, &arg[4]);
 
-  const int group1bit = group->bitmask[igroup1];
-  const int group2bit = group->bitmask[igroup2];
-
   if (comm->me == 0) utils::logmesg(lmp, "System init for delete_atoms ...\n");
 
   // request a full neighbor list for use by this command
@@ -454,9 +444,7 @@ void DeleteAtoms::delete_random(int narg, char **arg)
     error->all(FLERR, "Unknown delete_atoms random style: {}", arg[1]);
   }
 
-  int igroup = group->find(arg[4]);
+  int groupbit = group->get_bitmask_by_id(FLERR, arg[4], "delete_atoms");
-  if (igroup == -1) error->all(FLERR, "Could not find delete_atoms random group ID {}", arg[4]);
-
   auto region = domain->get_region_by_id(arg[5]);
   if (!region && (strcmp(arg[5], "NULL") != 0))
     error->all(FLERR, "Could not find delete_atoms random region ID {}", arg[5]);
@@ -477,7 +465,6 @@ void DeleteAtoms::delete_random(int narg, char **arg)
   double **x = atom->x;
   int *mask = atom->mask;
 
-  int groupbit = group->bitmask[igroup];
   if (region) region->prematch();
 
   // delete approximate fraction of atoms in both group and region

src/delete_bonds.cpp

@@ -56,11 +56,9 @@ void DeleteBonds::command(int narg, char **arg)
 
   if (comm->me == 0) utils::logmesg(lmp,"Deleting bonds ...\n");
 
-  // identify group
+  // get group bitmask
 
-  int igroup = group->find(arg[0]);
+  int groupbit = group->get_bitmask_by_id(FLERR, arg[0], "delete_bonds");
-  if (igroup == -1) error->all(FLERR,"Cannot find delete_bonds group ID");
-  int groupbit = group->bitmask[igroup];
 
   // set style and which = type value
 

src/displace_atoms.cpp

@@ -74,8 +74,7 @@ void DisplaceAtoms::command(int narg, char **arg)
   // group and style
 
   igroup = group->find(arg[0]);
-  if (igroup == -1) error->all(FLERR,"Could not find displace_atoms group ID");
+  groupbit = group->get_bitmask_by_id(FLERR, arg[0], "displace_atoms");
-  groupbit = group->bitmask[igroup];
 
   if (modify->check_rigid_group_overlap(groupbit))
     error->warning(FLERR,"Attempting to displace atoms in rigid bodies");

src/dump.cpp

@@ -58,7 +58,7 @@ Dump::Dump(LAMMPS *lmp, int /*narg*/, char **arg) :
   id = utils::strdup(arg[0]);
 
   igroup = group->find(arg[1]);
-  groupbit = group->bitmask[igroup];
+  groupbit = group->get_bitmask_by_id(FLERR, arg[1], fmt::format("dump {}", arg[2]));
 
   style = utils::strdup(arg[2]);
 
@@ -311,6 +311,7 @@ void Dump::init()
 
 int Dump::count()
 {
+  // group all
   if (igroup == 0) return atom->nlocal;
 
   int *mask = atom->mask;

src/fix.cpp

@@ -48,8 +48,8 @@ Fix::Fix(LAMMPS *lmp, int /*narg*/, char **arg) :
     error->all(FLERR,"Fix ID must be alphanumeric or underscore characters");
 
   igroup = group->find(arg[1]);
-  if (igroup == -1) error->all(FLERR,"Could not find fix group ID");
+  if (igroup == -1) error->all(FLERR,"Could not find fix {} group ID {}", arg[2], arg[1]);
-  groupbit = group->bitmask[igroup];
+  groupbit = group->get_bitmask_by_id(FLERR, arg[1], fmt::format("fix {}",arg[2]));
 
   style = utils::strdup(arg[2]);
 

src/fix_nh.cpp

@@ -271,7 +271,7 @@ FixNH::FixNH(LAMMPS *lmp, int narg, char **arg) :
         delete[] id_dilate;
         id_dilate = utils::strdup(arg[iarg+1]);
         int idilate = group->find(id_dilate);
-        if (idilate == -1)
+        if (idilate < 0)
           error->all(FLERR,"Fix {} dilate group ID {} does not exist", style, id_dilate);
       }
       iarg += 2;
@@ -629,12 +629,8 @@ void FixNH::init()
 {
   // recheck that dilate group has not been deleted
 
-  if (allremap == 0) {
+  if (allremap == 0)
-    int idilate = group->find(id_dilate);
+    dilate_group_bit = group->get_bitmask_by_id(FLERR, id_dilate, fmt::format("fix {}", style));
-    if (idilate == -1)
-      error->all(FLERR,"Fix {} dilate group ID {} does not exist", style, id_dilate);
-    dilate_group_bit = group->bitmask[idilate];
-  }
 
   // ensure no conflict with fix deform
 

src/fix_recenter.cpp

@@ -41,7 +41,7 @@ enum{BOX,LATTICE,FRACTION};
 FixRecenter::FixRecenter(LAMMPS *lmp, int narg, char **arg) :
   Fix(lmp, narg, arg)
 {
-  if (narg < 6) error->all(FLERR,"Illegal fix recenter command");
+  if (narg < 6) utils::missing_cmd_args(FLERR,"fix recenter", error);
 
   xcom = ycom = zcom = 0.0;
   xflag = yflag = zflag = 1;
@@ -74,17 +74,15 @@ FixRecenter::FixRecenter(LAMMPS *lmp, int narg, char **arg) :
   int iarg = 6;
   while (iarg < narg) {
     if (strcmp(arg[iarg],"shift") == 0) {
-      int igroup2 = group->find(arg[iarg+1]);
+      group2bit = group->get_bitmask_by_id(FLERR, arg[iarg+1], "fix recenter");
-      if (igroup2 < 0) error->all(FLERR,"Could not find fix recenter group ID");
-      group2bit = group->bitmask[igroup2];
       iarg += 2;
     } else if (strcmp(arg[iarg],"units") == 0) {
       if (strcmp(arg[iarg+1],"box") == 0) scaleflag = BOX;
       else if (strcmp(arg[iarg+1],"lattice") == 0) scaleflag = LATTICE;
       else if (strcmp(arg[iarg+1],"fraction") == 0) scaleflag = FRACTION;
-      else error->all(FLERR,"Illegal fix recenter command");
+      else error->all(FLERR,"Unknown fix recenter units argument {}", arg[iarg+1]);
       iarg += 2;
-    } else error->all(FLERR,"Illegal fix recenter command");
+    } else error->all(FLERR,"Unknown fix recenter keyword {}", arg[iarg]);
   }
 
   // scale xcom,ycom,zcom
@@ -103,8 +101,7 @@ FixRecenter::FixRecenter(LAMMPS *lmp, int narg, char **arg) :
 
   // cannot have 0 atoms in group
 
-  if (group->count(igroup) == 0)
+  if (group->count(igroup) == 0) error->all(FLERR,"Fix recenter group {} has no atoms", arg[1]);
-    error->all(FLERR,"Fix recenter group has no atoms");
 }
 
 /* ---------------------------------------------------------------------- */

src/fix_spring.cpp

@@ -73,11 +73,9 @@ FixSpring::FixSpring(LAMMPS *lmp, int narg, char **arg) :
 
     group2 = utils::strdup(arg[4]);
     igroup2 = group->find(arg[4]);
-    if (igroup2 == -1)
-      error->all(FLERR,"Fix spring couple group ID does not exist");
     if (igroup2 == igroup)
-      error->all(FLERR,"Two groups cannot be the same in fix spring couple");
+      error->all(FLERR,"The two groups cannot be the same in fix spring couple");
-    group2bit = group->bitmask[igroup2];
+    group2bit = group->get_bitmask_by_id(FLERR, arg[4], "fix spring");
 
     k_spring = utils::numeric(FLERR,arg[5],false,lmp);
     xflag = yflag = zflag = 1;
@@ -121,9 +119,7 @@ void FixSpring::init()
 
   if (group2) {
     igroup2 = group->find(group2);
-    if (igroup2 == -1)
+    group2bit = group->get_bitmask_by_id(FLERR, group2, "fix spring");
-      error->all(FLERR,"Fix spring couple group ID does not exist");
-    group2bit = group->bitmask[igroup2];
   }
 
   masstotal = group->mass(igroup);

src/group.cpp

@@ -649,6 +649,19 @@ int Group::find_unused()
   return -1;
 }
 
+/* ----------------------------------------------------------------------
+   return group bitmask for given group id. Error out if group is not found.
+------------------------------------------------------------------------- */
+
+int Group::get_bitmask_by_id(const std::string &file, int line, const std::string &name,
+                             const std::string &caller)
+{
+  int igroup = find(name);
+  if (igroup < 0)
+    error->all(file, line, "Group ID {} requested by {} does not exist", name, caller);
+  return bitmask[igroup];
+}
+
 /* ----------------------------------------------------------------------
    add atoms to group that are in same molecules as atoms already in group
    do not include molID = 0

src/group.h

@@ -31,11 +31,13 @@ class Group : protected Pointers {
 
   Group(class LAMMPS *);
   ~Group() override;
+
   void assign(int, char **);                  // assign atoms to a group
   void assign(const std::string &);           // convenience function
   void create(const std::string &, int *);    // add flagged atoms to a group
   int find(const std::string &);              // lookup name in list of groups
   int find_or_create(const char *);           // lookup name or create new group
+  int get_bitmask_by_id(const std::string &, int, const std::string &, const std::string &);
   void write_restart(FILE *);
   void read_restart(FILE *);
 

unittest/commands/test_groups.cpp

@@ -256,6 +256,41 @@ TEST_F(GroupTest, Molecular)
                  command("group three include xxx"););
 }
 
+TEST_F(GroupTest, Bitmap)
+{
+    atomic_system();
+
+    BEGIN_HIDE_OUTPUT();
+    command("group one region left");
+    command("group two region right");
+    command("group three empty");
+    command("group four region left");
+    command("group four region right");
+    command("group six subtract four one");
+    END_HIDE_OUTPUT();
+
+    int bm_one   = group->get_bitmask_by_id(FLERR, "one", "unittest 1");
+    int bm_two   = group->get_bitmask_by_id(FLERR, "two", "unittest 2");
+    int bm_three = group->get_bitmask_by_id(FLERR, "three", "unittest 3");
+    int bm_four  = group->get_bitmask_by_id(FLERR, "four", "unittest 4");
+    int bm_six   = group->get_bitmask_by_id(FLERR, "six", "unittest 6");
+    int nlocal   = lmp->atom->natoms;
+    auto mask    = lmp->atom->mask;
+
+    for (int i = 0; i < nlocal; ++i) {
+        if ((mask[i] & bm_one) && (mask[i] & bm_two)) {
+            EXPECT_NE((mask[i] & bm_four), 0);
+        }
+        if (mask[i] & bm_two) {
+            EXPECT_NE((mask[i] & bm_six), 0);
+        }
+        EXPECT_EQ((mask[i] & bm_three), 0);
+    }
+
+    TEST_FAILURE(".*ERROR: Group ID five requested by unittest 5 does not exist.*",
+                 group->get_bitmask_by_id(FLERR, "five", "unittest 5"););
+}
+
 TEST_F(GroupTest, Dynamic)
 {
     atomic_system();