Fix circular neighbor list copy dependencies and re-enable trimming (issue #4529)

doc/src/Developer_notes.rst

@@ -192,13 +192,33 @@ internally by :doc:`Peridynamics pair styles <pair_peri>`:
 
 It is also possible to request a neighbor list that uses a different cutoff
 than what is usually inferred from the pair style settings (largest cutoff of
-all pair styles plus neighbor list skin).  The following is used in the
-:doc:`compute rdf <compute_rdf>` command implementation:
+all pair styles plus neighbor list skin).  Since the default neighbor list is
+built with a cutoff *per pair of atom types*, such a request must also state
+how its cutoff is to be interpreted, by calling exactly one of:
+
+.. list-table::
+   :header-rows: 1
+   :widths: 25 75
+
+   * - Function
+     - Interpretation of the requested cutoff
+   * - ``set_cutoff_max(cutoff)``
+     - the maximum across atom types, individual type pairs may use a shorter
+       cutoff.  This is the typical case for pair styles.
+   * - ``set_cutoff_fixed(cutoff)``
+     - applies uniformly to every pair of atom types.  This is the typical
+       case for a fix or compute analyzing a fixed range, e.g. an RDF.
+
+Using ``set_cutoff_max()`` where a uniform cutoff is meant will silently
+truncate the list for those type pairs that use a shorter pair style cutoff.
+
+The following is used in the :doc:`compute rdf <compute_rdf>` command
+implementation, where the requested cutoff applies to all atom types:
 
 .. code-block:: c++
 
   if (cutflag)
-    neighbor->add_request(this, NeighConst::REQ_OCCASIONAL)->set_cutoff(mycutneigh);
+    neighbor->add_request(this, NeighConst::REQ_OCCASIONAL)->set_cutoff_fixed(mycutneigh);
   else
     neighbor->add_request(this, NeighConst::REQ_OCCASIONAL);
 

doc/src/Developer_updating.rst

@@ -31,6 +31,7 @@ Available topics in mostly chronological order are:
 - `Refactored grid communication using Grid3d/Grid2d classes instead of GridComm`_
 - `FLERR as first argument to minimum image functions in Domain class`_
 - `Use utils::logmesg() instead of error->warning()`_
+- `Explicit interpretation for custom neighbor list cutoffs`_
 
 ----
 
@@ -681,3 +682,37 @@ New:
    if (comm->me == 0) utils::logmesg(lmp, "INFO: About to read data file: {}\n", filename);
 
 This change is **required** or else the code will not compile.
+
+Explicit interpretation for custom neighbor list cutoffs
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+The default neighbor list is built with a cutoff *per pair of atom types*, so
+a request for a custom cutoff is ambiguous on its own: the same number can
+mean "the largest cutoff used by any type pair" or "this cutoff for every type
+pair".  Assuming the former where the latter was meant silently truncates the
+list for the type pairs that use a shorter pair style cutoff.
+
+``NeighRequest::set_cutoff()`` has therefore been replaced by two functions
+that each state the intended interpretation, and must be called instead:
+
+- ``set_cutoff_max(cutoff)`` -- the maximum across atom types, individual type
+  pairs may use a shorter cutoff.  This is the typical case for pair styles.
+- ``set_cutoff_fixed(cutoff)`` -- applies uniformly to every pair of atom
+  types.  This is the typical case for a fix or compute analyzing a fixed
+  range, e.g. an RDF.
+
+Old:
+
+.. code-block:: c++
+
+   auto req = neighbor->add_request(this, NeighConst::REQ_OCCASIONAL);
+   if (cutflag) req->set_cutoff(mycutneigh);
+
+New:
+
+.. code-block:: c++
+
+   auto req = neighbor->add_request(this, NeighConst::REQ_OCCASIONAL);
+   if (cutflag) req->set_cutoff_fixed(mycutneigh);
+
+This change is **required** or else the code will not compile.

src/APIP/fix_lambda_la_csp_apip.cpp

@@ -251,7 +251,7 @@ void FixLambdaLACSPAPIP::init()
 {
   // full neighbour list for thermostating
   auto *req = neighbor->add_request(this, NeighConst::REQ_FULL);
-  req->set_cutoff(sqrt(cutsq_combined));
+  req->set_cutoff_fixed(sqrt(cutsq_combined));
 
   if (atom->tag_enable == 0) error->all(FLERR, "fix lambda/la/csp/apip requires atom IDs");
 

src/EXTRA-COMPUTE/compute_adf.cpp

@@ -332,7 +332,7 @@ void ComputeADF::init()
     if (neighbor->style == Neighbor::MULTI)
       error->all(FLERR, "Compute adf with custom cutoffs requires neighbor style 'bin' or 'nsq'");
 
-    req->set_cutoff(mycutneigh);
+    req->set_cutoff_fixed(mycutneigh);
   }
 }
 

src/EXTRA-COMPUTE/compute_ave_sphere_atom.cpp

@@ -116,7 +116,8 @@ void ComputeAveSphereAtom::init()
   // need an occasional full neighbor list
 
   auto *req = neighbor->add_request(this, NeighConst::REQ_FULL | NeighConst::REQ_OCCASIONAL);
-  if (cutflag) req->set_cutoff(cutoff);
+  // the analysis cutoff applies to all atom types
+  if (cutflag) req->set_cutoff_fixed(cutoff);
 }
 
 /* ---------------------------------------------------------------------- */

src/EXTRA-COMPUTE/compute_composition_atom.cpp

@@ -108,7 +108,8 @@ void ComputeCompositionAtom::init()
   // need an occasional full neighbor list
 
   auto *req = neighbor->add_request(this, NeighConst::REQ_FULL | NeighConst::REQ_OCCASIONAL);
-  if (cutflag) req->set_cutoff(cutoff);
+
+  if (cutflag) req->set_cutoff_fixed(cutoff);
 }
 
 /* ---------------------------------------------------------------------- */

src/EXTRA-COMPUTE/compute_efield_wolf_atom.cpp

@@ -98,7 +98,9 @@ void ComputeEfieldWolfAtom::init()
   // request an occasional full neighbor list
 
   auto *req = neighbor->add_request(this, NeighConst::REQ_FULL | NeighConst::REQ_OCCASIONAL);
-  if (cutoff_flag) req->set_cutoff(cutoff);
+  if (cutoff_flag) {
+    req->set_cutoff_fixed(cutoff);
+  }
 
   jgroup = group->find(group2);
   if (jgroup < 0) error->all(FLERR, "Compute efield/atom/wolf group {} does not exist", group2);

src/EXTRA-COMPUTE/compute_hbond_local.cpp

@@ -149,7 +149,8 @@ void ComputeHBondLocal::init()
     error->all(FLERR, Error::NOLASTLINE,
                "Compute hbond/local requires neighbor style 'bin' or 'nsq'");
   auto *req = neighbor->add_request(this, NeighConst::REQ_FULL | NeighConst::REQ_OCCASIONAL);
-  req->set_cutoff(distcutoff);
+
+  req->set_cutoff_fixed(distcutoff);
 
   // do initial memory allocation assuming all donors have two hydrogen bonds
 

src/EXTRA-FIX/fix_nonaffine_displacement.cpp

@@ -242,7 +242,8 @@ void FixNonaffineDisplacement::init()
         if (mycutneigh > cutghost)
           error->all(FLERR,"Fix nonaffine/displacement D2Min option cutoff exceeds ghost atom range - use comm_modify cutoff command");
 
-        req->set_cutoff(mycutneigh);
+        // the cutoff is required for all types
+        req->set_cutoff_fixed(mycutneigh);
       }
     }
   }

src/GPU/pair_lj_cut_tip4p_long_gpu.cpp

@@ -201,7 +201,7 @@ void PairLJCutTIP4PLongGPU::init_style()
   if (gpu_mode == GPU_FORCE) {
     auto *req = neighbor->add_request(this, NeighConst::REQ_FULL);
     // NOTE: we must not add the neighbor list skin here. It is already added automatically.
-    req->set_cutoff(cut_coulplus);
+    req->set_cutoff_max(cut_coulplus);
   }
 }
 

src/KIM/pair_kim.cpp

@@ -594,7 +594,7 @@ void PairKIM::init_style()
     if (kim_cutoff_values[i] <= neighbor->skin)
       error->all(FLERR,"Illegal neighbor request (force cutoff {:.3} <= skin {:.3})",
                  kim_cutoff_values[i], neighbor->skin);
-    req->set_cutoff(kim_cutoff_values[i]);
+    req->set_cutoff_max(kim_cutoff_values[i]);
   }
   // increment instance_me in case of need to change the neighbor list
   // request settings

src/REPLICA/fix_hyper_local.cpp

@@ -313,7 +313,8 @@ void FixHyperLocal::init()
 
   auto *req = neighbor->add_request(this, NeighConst::REQ_FULL | NeighConst::REQ_OCCASIONAL);
   req->set_id(1);
-  req->set_cutoff(dcut);
+
+  req->set_cutoff_fixed(dcut);
 
   // also need occasional half neighbor list derived from pair style
   // used for building local bond list

src/RHEO/fix_rheo_oxidation.cpp

@@ -109,7 +109,8 @@ void FixRHEOOxidation::init()
 
   // need a half neighbor list
   auto *req = neighbor->add_request(this, NeighConst::REQ_FULL);
-  req->set_cutoff(cut);
+
+  req->set_cutoff_fixed(cut);
 }
 
 /* ---------------------------------------------------------------------- */

src/RHEO/fix_rheo_thermal.cpp

@@ -294,7 +294,8 @@ void FixRHEOThermal::init()
 
     // need a half neighbor list, built only when particles freeze
     auto *req = neighbor->add_request(this, NeighConst::REQ_OCCASIONAL);
-    req->set_cutoff(cut_kernel);
+
+    req->set_cutoff_fixed(cut_kernel);
 
     // find instances of bond history to delete/shift data
     histories = modify->get_fix_by_style("BOND_HISTORY");

src/compute_rdf.cpp

@@ -219,7 +219,8 @@ void ComputeRDF::init()
     if (neighbor->style == Neighbor::MULTI)
       error->all(FLERR, Error::NOLASTLINE,
                  "Compute rdf with custom cutoff requires neighbor style 'bin' or 'nsq'");
-    req->set_cutoff(mycutneigh);
+
+    req->set_cutoff_fixed(mycutneigh);
   }
 }
 

src/fix_group.cpp

@@ -210,7 +210,8 @@ void FixGroup::init()
     } else {
       req = neighbor->add_request(this, NeighConst::REQ_FULL | NeighConst::REQ_OCCASIONAL);
     }
-    req->set_cutoff(cutoff);
+
+    req->set_cutoff_fixed(cutoff);
   }
 
   if (excludeflag) excludebit = group->get_bitmask_by_id(FLERR, idexclude, "group dynamic exclude");

src/library.cpp

@@ -6485,7 +6485,10 @@ void NeighProxy::command(int narg, char **arg)
   int flags = utils::inumeric(FLERR, arg[1], false, lmp);
   double cutoff = utils::numeric(FLERR, arg[2], false, lmp);
   req->apply_flags(flags);
-  if (cutoff > 0.0) req->set_cutoff(cutoff);
+  if (cutoff > 0.0) {
+    // library-requested cutoff applies to all types
+    req->set_cutoff_fixed(cutoff);
+  }
   lmp->init();
 
   // setup domain, communication and neighboring

src/neigh_request.cpp

@@ -54,6 +54,7 @@ NeighRequest::NeighRequest(LAMMPS *_lmp) : Pointers(_lmp), requestor(nullptr)
   // default is no Kokkos neighbor list build
   // default is no Shardlow Splitting Algorithm (SSA) neighbor list build
   // default is no list-specific cutoff
+  // default is no fixed cutoff for all atom types
   // default is no storage of auxiliary floating point values
 
   occasional = 0;
@@ -69,6 +70,7 @@ NeighRequest::NeighRequest(LAMMPS *_lmp) : Pointers(_lmp), requestor(nullptr)
   kokkos_host = kokkos_device = 0;
   ssa = 0;
   cut = 0;
+  cut_fixed = 0;
   cutoff = 0.0;
 
   // skip info, default is no skipping
@@ -170,6 +172,7 @@ int NeighRequest::identical(NeighRequest *other)
   if (ssa != other->ssa) same = 0;
   if (copy != other->copy) same = 0;
   if (cutoff != other->cutoff) same = 0;
+  if (cut_fixed != other->cut_fixed) same = 0;
 
   if (skip != other->skip) same = 0;
   if (same && skip && other->skip) same = same_skip(other);
@@ -235,6 +238,7 @@ void NeighRequest::copy_request(NeighRequest *other, int skipflag)
   kokkos_device = other->kokkos_device;
   ssa = other->ssa;
   cut = other->cut;
+  cut_fixed = other->cut_fixed;
   cutoff = other->cutoff;
 
   iskip = nullptr;
@@ -283,10 +287,28 @@ void NeighRequest::apply_flags(int flags)
 
 /* ---------------------------------------------------------------------- */
 
-void NeighRequest::set_cutoff(double _cutoff)
+// a requestor with a non-standard cutoff must state how to interpret it,
+//   by calling exactly one of the two methods below
+//   there is deliberately no plain set_cutoff(): the interpretation cannot be
+//   guessed, and getting it wrong silently truncates the list for some type pairs
+
+// _cutoff is the MAXIMUM cutoff across atom types, individual type pairs may
+//   use a shorter cutoff.  This is the usual case for pair styles
+
+void NeighRequest::set_cutoff_max(double _cutoff)
+{
+  cut = 1;
+  cutoff = _cutoff;
+}
+
+// _cutoff applies UNIFORMLY to every atom type pair.  This is the usual case
+//   for fixes/computes analyzing a fixed range (e.g. an RDF)
+
+void NeighRequest::set_cutoff_fixed(double _cutoff)
 {
   cut = 1;
   cutoff = _cutoff;
+  cut_fixed = 1;
 }
 
 void NeighRequest::set_id(int _id)

src/neigh_request.h

@@ -82,9 +82,17 @@ class NeighRequest : protected Pointers {
   int intel;           // set by INTEL package
   int kokkos_host;     // set by KOKKOS package
   int kokkos_device;
-  int ssa;          // set by DPD-REACT package, for Shardlow lists
-  int cut;          // 1 if use a non-standard cutoff length
-  double cutoff;    // special cutoff distance for this list
+  int ssa;              // set by DPD-REACT package, for Shardlow lists
+
+  // non-standard cutoffs
+  //   By default, the cutoff corresponds to the maximum cutoff across all types,
+  //     does not imply all types have the same cutoff.
+  //     This is typical of requests for pair styles
+  //   If set, cut_fixed  implies the cutoff is uniform across all atom types.
+  //     This is typical of fixes/computes with a fixed range of analysis (e.g. an RDF)
+  int cut_fixed;        // toggles cutoff interpretation, whether fixed across types
+  int cut;              // 1 if use a non-standard cutoff length
+  double cutoff;        // special cutoff distance for this list
 
   // flags set by pair hybrid
 
@@ -136,7 +144,11 @@ class NeighRequest : protected Pointers {
   void copy_request(NeighRequest *, int);
 
   void apply_flags(int);
-  void set_cutoff(double);
+  // a non-standard cutoff requires stating its interpretation, exactly one of:
+  //   max   - cutoff is the maximum across types (typical of pair styles)
+  //   fixed - cutoff applies uniformly to all types (typical of analysis)
+  void set_cutoff_max(double);
+  void set_cutoff_fixed(double);
   void set_id(int);
   void set_kokkos_device(int);
   void set_kokkos_host(int);