From 54f0099ac9e24348dbaba1af01d54e84671f4ebf Mon Sep 17 00:00:00 2001 From: jtclemm Date: Wed, 1 Jul 2026 11:16:46 -0600 Subject: [PATCH 1/3] Clarifying meaning of cutoff in nrequest --- src/EXTRA-COMPUTE/compute_adf.cpp | 2 ++ src/EXTRA-COMPUTE/compute_composition_atom.cpp | 3 +++ src/EXTRA-COMPUTE/compute_efield_wolf_atom.cpp | 6 +++++- src/EXTRA-COMPUTE/compute_hbond_local.cpp | 3 +++ src/EXTRA-FIX/fix_nonaffine_displacement.cpp | 3 ++- src/REPLICA/fix_hyper_local.cpp | 3 +++ src/RHEO/fix_rheo_oxidation.cpp | 3 +++ src/RHEO/fix_rheo_thermal.cpp | 3 +++ src/compute_rdf.cpp | 3 +++ src/fix_group.cpp | 3 +++ src/neigh_request.cpp | 9 +++++++++ src/neigh_request.h | 15 ++++++++++++--- src/neighbor.cpp | 16 ++++++++++++---- 13 files changed, 63 insertions(+), 9 deletions(-) diff --git a/src/EXTRA-COMPUTE/compute_adf.cpp b/src/EXTRA-COMPUTE/compute_adf.cpp index 1fc8c305f2c..694b1d8ea90 100644 --- a/src/EXTRA-COMPUTE/compute_adf.cpp +++ b/src/EXTRA-COMPUTE/compute_adf.cpp @@ -329,6 +329,8 @@ void ComputeADF::init() if (neighbor->style == Neighbor::MULTI) error->all(FLERR, "Compute adf with custom cutoffs requires neighbor style 'bin' or 'nsq'"); + // Cutoff applies to all types + req->set_cut_fixed(1); req->set_cutoff(mycutneigh); } } diff --git a/src/EXTRA-COMPUTE/compute_composition_atom.cpp b/src/EXTRA-COMPUTE/compute_composition_atom.cpp index 346ced33734..6208146d174 100644 --- a/src/EXTRA-COMPUTE/compute_composition_atom.cpp +++ b/src/EXTRA-COMPUTE/compute_composition_atom.cpp @@ -108,6 +108,9 @@ void ComputeCompositionAtom::init() // need an occasional full neighbor list auto *req = neighbor->add_request(this, NeighConst::REQ_FULL | NeighConst::REQ_OCCASIONAL); + + // Cutoff applies to all types + req->set_cut_fixed(1); if (cutflag) req->set_cutoff(cutoff); } diff --git a/src/EXTRA-COMPUTE/compute_efield_wolf_atom.cpp b/src/EXTRA-COMPUTE/compute_efield_wolf_atom.cpp index 50f8fef02be..5e82d91e3c3 100644 --- a/src/EXTRA-COMPUTE/compute_efield_wolf_atom.cpp +++ b/src/EXTRA-COMPUTE/compute_efield_wolf_atom.cpp @@ -98,7 +98,11 @@ 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) { + // Cutoff applies to all types + req->set_cut_fixed(1); + req->set_cutoff(cutoff); + } jgroup = group->find(group2); if (jgroup < 0) error->all(FLERR, "Compute efield/atom/wolf group {} does not exist", group2); diff --git a/src/EXTRA-COMPUTE/compute_hbond_local.cpp b/src/EXTRA-COMPUTE/compute_hbond_local.cpp index ef09ffa82d7..e3fd9205ee5 100644 --- a/src/EXTRA-COMPUTE/compute_hbond_local.cpp +++ b/src/EXTRA-COMPUTE/compute_hbond_local.cpp @@ -149,6 +149,9 @@ 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); + + // Cutoff applies to all types + req->set_cut_fixed(1); req->set_cutoff(distcutoff); // do initial memory allocation assuming all donors have two hydrogen bonds diff --git a/src/EXTRA-FIX/fix_nonaffine_displacement.cpp b/src/EXTRA-FIX/fix_nonaffine_displacement.cpp index b7862d36cc7..bb973e28975 100644 --- a/src/EXTRA-FIX/fix_nonaffine_displacement.cpp +++ b/src/EXTRA-FIX/fix_nonaffine_displacement.cpp @@ -235,7 +235,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); + // Set the minimum cutoff required for all types + req->set_cutoff_min(mycutneigh); } } } diff --git a/src/REPLICA/fix_hyper_local.cpp b/src/REPLICA/fix_hyper_local.cpp index 8332d89d087..f9fdb185f88 100644 --- a/src/REPLICA/fix_hyper_local.cpp +++ b/src/REPLICA/fix_hyper_local.cpp @@ -312,6 +312,9 @@ void FixHyperLocal::init() auto *req = neighbor->add_request(this, NeighConst::REQ_FULL | NeighConst::REQ_OCCASIONAL); req->set_id(1); + + // Cutoff applies to all types + req->set_cut_fixed(1); req->set_cutoff(dcut); // also need occasional half neighbor list derived from pair style diff --git a/src/RHEO/fix_rheo_oxidation.cpp b/src/RHEO/fix_rheo_oxidation.cpp index 9770dcc1432..c4a82b6058c 100644 --- a/src/RHEO/fix_rheo_oxidation.cpp +++ b/src/RHEO/fix_rheo_oxidation.cpp @@ -109,6 +109,9 @@ void FixRHEOOxidation::init() // need a half neighbor list auto *req = neighbor->add_request(this, NeighConst::REQ_FULL); + + // Cutoff applies to all types + req->set_cut_fixed(1); req->set_cutoff(cut); } diff --git a/src/RHEO/fix_rheo_thermal.cpp b/src/RHEO/fix_rheo_thermal.cpp index 64b93b7f73e..ab803aa4b73 100644 --- a/src/RHEO/fix_rheo_thermal.cpp +++ b/src/RHEO/fix_rheo_thermal.cpp @@ -294,6 +294,9 @@ void FixRHEOThermal::init() // need a half neighbor list, built only when particles freeze auto *req = neighbor->add_request(this, NeighConst::REQ_OCCASIONAL); + + // Cutoff applies to all types + req->set_cut_fixed(1); req->set_cutoff(cut_kernel); // find instances of bond history to delete/shift data diff --git a/src/compute_rdf.cpp b/src/compute_rdf.cpp index 28ba8b22bed..ae6baa210b0 100644 --- a/src/compute_rdf.cpp +++ b/src/compute_rdf.cpp @@ -219,6 +219,9 @@ void ComputeRDF::init() if (neighbor->style == Neighbor::MULTI) error->all(FLERR, Error::NOLASTLINE, "Compute rdf with custom cutoff requires neighbor style 'bin' or 'nsq'"); + + // Cutoff applies to all types + req->set_cut_fixed(1); req->set_cutoff(mycutneigh); } } diff --git a/src/fix_group.cpp b/src/fix_group.cpp index 8074054151e..3599878a4f7 100644 --- a/src/fix_group.cpp +++ b/src/fix_group.cpp @@ -210,6 +210,9 @@ void FixGroup::init() } else { req = neighbor->add_request(this, NeighConst::REQ_FULL | NeighConst::REQ_OCCASIONAL); } + + // Cutoff applies to all types + req->set_cut_fixed(1); req->set_cutoff(cutoff); } diff --git a/src/neigh_request.cpp b/src/neigh_request.cpp index 50c27702b0b..2545a075184 100644 --- a/src/neigh_request.cpp +++ b/src/neigh_request.cpp @@ -54,6 +54,7 @@ NeighRequest::NeighRequest(LAMMPS *_lmp) : Pointers(_lmp) // 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) 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; @@ -289,6 +293,11 @@ void NeighRequest::set_cutoff(double _cutoff) cutoff = _cutoff; } +void NeighRequest::set_cut_fixed(int flag) +{ + cut_fixed = flag; +} + void NeighRequest::set_id(int _id) { id = _id; diff --git a/src/neigh_request.h b/src/neigh_request.h index a438527d0d4..042a72ae5ac 100644 --- a/src/neigh_request.h +++ b/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 @@ -137,6 +145,7 @@ class NeighRequest : protected Pointers { void apply_flags(int); void set_cutoff(double); + void set_cut_fixed(int); void set_id(int); void set_kokkos_device(int); void set_kokkos_host(int); diff --git a/src/neighbor.cpp b/src/neighbor.cpp index d777adc7c31..36cc1e6210d 100644 --- a/src/neighbor.cpp +++ b/src/neighbor.cpp @@ -1199,8 +1199,12 @@ void Neighbor::morph_unique() for (int i = 0; i < nrequest; i++) { irq = requests[i]; - // if cut flag set by requestor and cutoff is larger than minimum for default, - // and the list is not a skip list, set unique flag; otherwise unset cut flag + // if cut flag set by requestor have two options + // (1) cut_fixed is set (all types have the same cutoff) + // check if cutoff is larger than minimum + // (2) cut_fixed is not set (cutoff is just the maximum across types) + // check if cutoff differs from current maximum + // Then if the list is not a skip list, set unique flag; otherwise unset cut flag // this forces Pair,Stencil,Bin styles to be instantiated separately // also add skin to cutoff of perpetual lists @@ -1208,7 +1212,9 @@ void Neighbor::morph_unique() if (!irq->occasional) irq->cutoff += skin; - if ((irq->cutoff > cutneighmin) && !irq->skip) { + if (irq->cut_fixed && (irq->cutoff > cutneighmin) && !irq->skip) { + irq->unique = 1; + } else if ((irq->cutoff != cutneighmax) && !irq->skip) { irq->unique = 1; } else { irq->cut = 0; @@ -1609,7 +1615,8 @@ void Neighbor::morph_copy_trim() if (jj < nrequest) { irq->copy = 1; irq->trim = trim_flag; - if (jrq->copy && irq->cutoff == requests[jrq->copylist]->cutoff) + if (jrq->copy && irq->cut_fixed == requests[jrq->copylist]->cut_fixed && + irq->cutoff == requests[jrq->copylist]->cutoff) irq->copylist = jrq->copylist; else irq->copylist = j; @@ -1897,6 +1904,7 @@ void Neighbor::print_pairwise_info() if (rq->kokkos_host) out += ", kokkos_host"; if (rq->ssa) out += ", ssa"; if (rq->cut) out += fmt::format(", cut {}", rq->cutoff); + if (rq->cut_fixed) out += fmt::format(", cut fixed {}", rq->cut_fixed); if (rq->off2on) out += ", off2on"; out += "\n"; From 9f970adec276902a590dc9f113ebb305c2e743e3 Mon Sep 17 00:00:00 2001 From: Stan Moore Date: Fri, 21 Aug 2026 04:54:22 +0000 Subject: [PATCH 2/3] Complete the cutoff interpretation for custom neighbor list requests Builds on the parent commit, which distinguishes a cutoff that is the maximum across atom types from one that applies uniformly to all of them. That distinction is what lets the circular copy dependency of issue #4529 be fixed without reintroducing the truncated analysis lists of issue #4437. Finish it so that it compiles, covers every requestor, and cannot be omitted. Define the missing setter. fix nonaffine/displacement called set_cutoff_min(), which was never defined, so EXTRA-FIX did not compile. Its cutoff applies to all types, so it now uses set_cutoff_fixed(). Make the interpretation impossible to omit. Requesting a non-standard cutoff used to be a bare set_cutoff() whose meaning defaulted to "maximum across types", with an optional second set_cut_fixed(1) call to mean "uniform across types". Forgetting that second call silently truncates the list for the short-cutoff type pairs, with no error, and it was in fact missing at several call sites. Replace both with two single-call setters that each state the intent, and remove the bare set_cutoff(): set_cutoff_max(C) C is the maximum across types (pair styles) set_cutoff_fixed(C) C applies uniformly to every type pair (analysis) All 17 call sites are converted, so omitting the interpretation is now a compile error rather than a wrong answer. This also removes a latent case in compute composition/atom, which set the uniform flag unconditionally but the cutoff only conditionally, leaving the flag set on a list with no custom cutoff. Clear cut_fixed whenever morph_unique() drops the cut flag. A list reverting to the default cutoff kept a stale interpretation, which then fed the cutoff comparison that collapses copy chains in morph_copy_trim() and could point a copy at an intermediate list instead of its root, the same shape of problem as the circular chains in issue #4529. Restructure the decision in morph_unique() into one explicit predicate. The previous else-if chain let a uniform-cutoff request that failed its own test fall through to the maximum-cutoff test. That fallthrough is deliberate, it is how a shorter uniform cutoff still gets its own list so it can be trimmed rather than silently widened, but nothing said so. Behavior is unchanged. Re-enable the one_trim_half_list_{newton,nonewton} tests, which pass again now that trimming is detected, and add a regression test for a uniform-cutoff list whose cutoff equals cutneighmax in a system with heterogeneous per-type pair cutoffs. It asserts that such a list's contents do not depend on the pair style's per-type cutoffs, by comparing neighbor counts against the same system with uniform pair cutoffs. --- doc/src/Developer_notes.rst | 26 ++++++++-- doc/src/Developer_updating.rst | 35 +++++++++++++ src/APIP/fix_lambda_la_csp_apip.cpp | 2 +- src/EXTRA-COMPUTE/compute_adf.cpp | 4 +- src/EXTRA-COMPUTE/compute_ave_sphere_atom.cpp | 3 +- .../compute_composition_atom.cpp | 4 +- .../compute_efield_wolf_atom.cpp | 4 +- src/EXTRA-COMPUTE/compute_hbond_local.cpp | 4 +- src/EXTRA-FIX/fix_nonaffine_displacement.cpp | 4 +- src/GPU/pair_lj_cut_tip4p_long_gpu.cpp | 2 +- src/KIM/pair_kim.cpp | 2 +- src/REPLICA/fix_hyper_local.cpp | 4 +- src/RHEO/fix_rheo_oxidation.cpp | 4 +- src/RHEO/fix_rheo_thermal.cpp | 4 +- src/compute_rdf.cpp | 4 +- src/fix_group.cpp | 4 +- src/library.cpp | 5 +- src/neigh_request.cpp | 19 +++++-- src/neigh_request.h | 7 ++- src/neighbor.cpp | 40 ++++++++++----- src/pair_hybrid.cpp | 2 +- src/pair_hybrid_molecular.cpp | 2 +- unittest/cplusplus/test_neighbor_class.cpp | 51 +++++++++++++++++-- 23 files changed, 176 insertions(+), 60 deletions(-) diff --git a/doc/src/Developer_notes.rst b/doc/src/Developer_notes.rst index 6da4fd7395a..bdb9cc912c7 100644 --- a/doc/src/Developer_notes.rst +++ b/doc/src/Developer_notes.rst @@ -192,13 +192,33 @@ internally by :doc:`Peridynamics pair styles `: 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 ` 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 ` 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); diff --git a/doc/src/Developer_updating.rst b/doc/src/Developer_updating.rst index 364c0e6ea0c..0f41e06390f 100644 --- a/doc/src/Developer_updating.rst +++ b/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. diff --git a/src/APIP/fix_lambda_la_csp_apip.cpp b/src/APIP/fix_lambda_la_csp_apip.cpp index 132f3b3bc97..15e95a3e6e2 100644 --- a/src/APIP/fix_lambda_la_csp_apip.cpp +++ b/src/APIP/fix_lambda_la_csp_apip.cpp @@ -257,7 +257,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"); diff --git a/src/EXTRA-COMPUTE/compute_adf.cpp b/src/EXTRA-COMPUTE/compute_adf.cpp index 694b1d8ea90..054c0eb7304 100644 --- a/src/EXTRA-COMPUTE/compute_adf.cpp +++ b/src/EXTRA-COMPUTE/compute_adf.cpp @@ -329,9 +329,7 @@ void ComputeADF::init() if (neighbor->style == Neighbor::MULTI) error->all(FLERR, "Compute adf with custom cutoffs requires neighbor style 'bin' or 'nsq'"); - // Cutoff applies to all types - req->set_cut_fixed(1); - req->set_cutoff(mycutneigh); + req->set_cutoff_fixed(mycutneigh); } } diff --git a/src/EXTRA-COMPUTE/compute_ave_sphere_atom.cpp b/src/EXTRA-COMPUTE/compute_ave_sphere_atom.cpp index 72a628c37c7..ded00ec75ba 100644 --- a/src/EXTRA-COMPUTE/compute_ave_sphere_atom.cpp +++ b/src/EXTRA-COMPUTE/compute_ave_sphere_atom.cpp @@ -114,7 +114,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); } /* ---------------------------------------------------------------------- */ diff --git a/src/EXTRA-COMPUTE/compute_composition_atom.cpp b/src/EXTRA-COMPUTE/compute_composition_atom.cpp index 6208146d174..e4e3dc3f83d 100644 --- a/src/EXTRA-COMPUTE/compute_composition_atom.cpp +++ b/src/EXTRA-COMPUTE/compute_composition_atom.cpp @@ -109,9 +109,7 @@ void ComputeCompositionAtom::init() auto *req = neighbor->add_request(this, NeighConst::REQ_FULL | NeighConst::REQ_OCCASIONAL); - // Cutoff applies to all types - req->set_cut_fixed(1); - if (cutflag) req->set_cutoff(cutoff); + if (cutflag) req->set_cutoff_fixed(cutoff); } /* ---------------------------------------------------------------------- */ diff --git a/src/EXTRA-COMPUTE/compute_efield_wolf_atom.cpp b/src/EXTRA-COMPUTE/compute_efield_wolf_atom.cpp index 5e82d91e3c3..750cacc37c1 100644 --- a/src/EXTRA-COMPUTE/compute_efield_wolf_atom.cpp +++ b/src/EXTRA-COMPUTE/compute_efield_wolf_atom.cpp @@ -99,9 +99,7 @@ void ComputeEfieldWolfAtom::init() auto *req = neighbor->add_request(this, NeighConst::REQ_FULL | NeighConst::REQ_OCCASIONAL); if (cutoff_flag) { - // Cutoff applies to all types - req->set_cut_fixed(1); - req->set_cutoff(cutoff); + req->set_cutoff_fixed(cutoff); } jgroup = group->find(group2); diff --git a/src/EXTRA-COMPUTE/compute_hbond_local.cpp b/src/EXTRA-COMPUTE/compute_hbond_local.cpp index e3fd9205ee5..8006f4b0533 100644 --- a/src/EXTRA-COMPUTE/compute_hbond_local.cpp +++ b/src/EXTRA-COMPUTE/compute_hbond_local.cpp @@ -150,9 +150,7 @@ void ComputeHBondLocal::init() "Compute hbond/local requires neighbor style 'bin' or 'nsq'"); auto *req = neighbor->add_request(this, NeighConst::REQ_FULL | NeighConst::REQ_OCCASIONAL); - // Cutoff applies to all types - req->set_cut_fixed(1); - req->set_cutoff(distcutoff); + req->set_cutoff_fixed(distcutoff); // do initial memory allocation assuming all donors have two hydrogen bonds diff --git a/src/EXTRA-FIX/fix_nonaffine_displacement.cpp b/src/EXTRA-FIX/fix_nonaffine_displacement.cpp index bb973e28975..4a26526e986 100644 --- a/src/EXTRA-FIX/fix_nonaffine_displacement.cpp +++ b/src/EXTRA-FIX/fix_nonaffine_displacement.cpp @@ -235,8 +235,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"); - // Set the minimum cutoff required for all types - req->set_cutoff_min(mycutneigh); + // the cutoff is required for all types + req->set_cutoff_fixed(mycutneigh); } } } diff --git a/src/GPU/pair_lj_cut_tip4p_long_gpu.cpp b/src/GPU/pair_lj_cut_tip4p_long_gpu.cpp index 6268d8e4945..4ccee7ebfba 100644 --- a/src/GPU/pair_lj_cut_tip4p_long_gpu.cpp +++ b/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); } } diff --git a/src/KIM/pair_kim.cpp b/src/KIM/pair_kim.cpp index 4ad89ad1482..961bfec4adb 100644 --- a/src/KIM/pair_kim.cpp +++ b/src/KIM/pair_kim.cpp @@ -596,7 +596,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 diff --git a/src/REPLICA/fix_hyper_local.cpp b/src/REPLICA/fix_hyper_local.cpp index f9fdb185f88..985e7faafe2 100644 --- a/src/REPLICA/fix_hyper_local.cpp +++ b/src/REPLICA/fix_hyper_local.cpp @@ -313,9 +313,7 @@ void FixHyperLocal::init() auto *req = neighbor->add_request(this, NeighConst::REQ_FULL | NeighConst::REQ_OCCASIONAL); req->set_id(1); - // Cutoff applies to all types - req->set_cut_fixed(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 diff --git a/src/RHEO/fix_rheo_oxidation.cpp b/src/RHEO/fix_rheo_oxidation.cpp index c4a82b6058c..8e37460eb8e 100644 --- a/src/RHEO/fix_rheo_oxidation.cpp +++ b/src/RHEO/fix_rheo_oxidation.cpp @@ -110,9 +110,7 @@ void FixRHEOOxidation::init() // need a half neighbor list auto *req = neighbor->add_request(this, NeighConst::REQ_FULL); - // Cutoff applies to all types - req->set_cut_fixed(1); - req->set_cutoff(cut); + req->set_cutoff_fixed(cut); } /* ---------------------------------------------------------------------- */ diff --git a/src/RHEO/fix_rheo_thermal.cpp b/src/RHEO/fix_rheo_thermal.cpp index ab803aa4b73..b0a26de46ea 100644 --- a/src/RHEO/fix_rheo_thermal.cpp +++ b/src/RHEO/fix_rheo_thermal.cpp @@ -295,9 +295,7 @@ void FixRHEOThermal::init() // need a half neighbor list, built only when particles freeze auto *req = neighbor->add_request(this, NeighConst::REQ_OCCASIONAL); - // Cutoff applies to all types - req->set_cut_fixed(1); - 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"); diff --git a/src/compute_rdf.cpp b/src/compute_rdf.cpp index ae6baa210b0..06c38973498 100644 --- a/src/compute_rdf.cpp +++ b/src/compute_rdf.cpp @@ -220,9 +220,7 @@ void ComputeRDF::init() error->all(FLERR, Error::NOLASTLINE, "Compute rdf with custom cutoff requires neighbor style 'bin' or 'nsq'"); - // Cutoff applies to all types - req->set_cut_fixed(1); - req->set_cutoff(mycutneigh); + req->set_cutoff_fixed(mycutneigh); } } diff --git a/src/fix_group.cpp b/src/fix_group.cpp index 3599878a4f7..cf4b853dbcb 100644 --- a/src/fix_group.cpp +++ b/src/fix_group.cpp @@ -211,9 +211,7 @@ void FixGroup::init() req = neighbor->add_request(this, NeighConst::REQ_FULL | NeighConst::REQ_OCCASIONAL); } - // Cutoff applies to all types - req->set_cut_fixed(1); - req->set_cutoff(cutoff); + req->set_cutoff_fixed(cutoff); } if (excludeflag) excludebit = group->get_bitmask_by_id(FLERR, idexclude, "group dynamic exclude"); diff --git a/src/library.cpp b/src/library.cpp index ac89c431753..b9c47c7e426 100644 --- a/src/library.cpp +++ b/src/library.cpp @@ -6485,7 +6485,10 @@ void NeighProxy::command(int narg, char **arg) int flags = atoi(arg[1]); double cutoff = atof(arg[2]); 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 diff --git a/src/neigh_request.cpp b/src/neigh_request.cpp index 2545a075184..6564d82866a 100644 --- a/src/neigh_request.cpp +++ b/src/neigh_request.cpp @@ -287,15 +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; } -void NeighRequest::set_cut_fixed(int flag) +// _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_fixed = flag; + cut = 1; + cutoff = _cutoff; + cut_fixed = 1; } void NeighRequest::set_id(int _id) diff --git a/src/neigh_request.h b/src/neigh_request.h index 042a72ae5ac..2939f948444 100644 --- a/src/neigh_request.h +++ b/src/neigh_request.h @@ -144,8 +144,11 @@ class NeighRequest : protected Pointers { void copy_request(NeighRequest *, int); void apply_flags(int); - void set_cutoff(double); - void set_cut_fixed(int); + // 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); diff --git a/src/neighbor.cpp b/src/neighbor.cpp index 36cc1e6210d..ad30a46f4a5 100644 --- a/src/neighbor.cpp +++ b/src/neighbor.cpp @@ -1199,26 +1199,40 @@ void Neighbor::morph_unique() for (int i = 0; i < nrequest; i++) { irq = requests[i]; - // if cut flag set by requestor have two options - // (1) cut_fixed is set (all types have the same cutoff) - // check if cutoff is larger than minimum - // (2) cut_fixed is not set (cutoff is just the maximum across types) - // check if cutoff differs from current maximum - // Then if the list is not a skip list, set unique flag; otherwise unset cut flag - // this forces Pair,Stencil,Bin styles to be instantiated separately + // decide whether a requested non-standard cutoff needs a list of its own + // (unique = its own Bin,Stencil,Pair styles are instantiated) or whether + // the default cutoffs already serve it, in which case the cut flag is + // unset and the request falls back on the default list // also add skin to cutoff of perpetual lists if (irq->cut) { if (!irq->occasional) irq->cutoff += skin; - if (irq->cut_fixed && (irq->cutoff > cutneighmin) && !irq->skip) { - irq->unique = 1; - } else if ((irq->cutoff != cutneighmax) && !irq->skip) { + int needs_own_cutoff; + if (irq->skip) { + // skip lists inherit the cutoff of the parent they skip from + needs_own_cutoff = 0; + } else if (irq->cut_fixed && (irq->cutoff > cutneighmin)) { + // uniform cutoff that some type pair of the default list does not + // reach, so the default list would be truncated for those pairs + needs_own_cutoff = 1; + } else { + // remaining cases, for either interpretation of the cutoff, only need + // their own list if the requested value differs from the default one + // they would otherwise inherit. A shorter cutoff still lands here so + // that morph_copy_trim() can trim it down instead of widening it + needs_own_cutoff = (irq->cutoff != cutneighmax); + } + + if (needs_own_cutoff) { irq->unique = 1; } else { + // reverts to the default cutoff, so drop its interpretation as well, + // otherwise the stale flags leak into later cutoff comparisons irq->cut = 0; irq->cutoff = 0.0; + irq->cut_fixed = 0; } } @@ -1903,8 +1917,10 @@ void Neighbor::print_pairwise_info() if (rq->kokkos_device) out += ", kokkos_device"; if (rq->kokkos_host) out += ", kokkos_host"; if (rq->ssa) out += ", ssa"; - if (rq->cut) out += fmt::format(", cut {}", rq->cutoff); - if (rq->cut_fixed) out += fmt::format(", cut fixed {}", rq->cut_fixed); + if (rq->cut) { + out += fmt::format(", cut {}", rq->cutoff); + if (rq->cut_fixed) out += fmt::format(", cut fixed {}", rq->cut_fixed); + } if (rq->off2on) out += ", off2on"; out += "\n"; diff --git a/src/pair_hybrid.cpp b/src/pair_hybrid.cpp index 95109260527..d95bcc65f49 100644 --- a/src/pair_hybrid.cpp +++ b/src/pair_hybrid.cpp @@ -762,7 +762,7 @@ double PairHybrid::init_one(int i, int j) for (const auto &request : neighbor->get_pair_requests()) { if (styles[istyle] == request->get_requestor()) { - request->set_cutoff(cutmax_style[istyle]); + request->set_cutoff_max(cutmax_style[istyle]); break; } } diff --git a/src/pair_hybrid_molecular.cpp b/src/pair_hybrid_molecular.cpp index 3635b770507..85149ee6f61 100644 --- a/src/pair_hybrid_molecular.cpp +++ b/src/pair_hybrid_molecular.cpp @@ -119,7 +119,7 @@ double PairHybridMolecular::init_one(int i, int j) for (const auto &request : neighbor->get_pair_requests()) { if (styles[istyle] == request->get_requestor()) { - request->set_cutoff(cutmax_style[istyle]); + request->set_cutoff_max(cutmax_style[istyle]); break; } } diff --git a/unittest/cplusplus/test_neighbor_class.cpp b/unittest/cplusplus/test_neighbor_class.cpp index 9facc38f048..aa670a31b41 100644 --- a/unittest/cplusplus/test_neighbor_class.cpp +++ b/unittest/cplusplus/test_neighbor_class.cpp @@ -184,6 +184,20 @@ class NeighborListsBin : public LAMMPSTest { } return 0; } + + // total number of neighbors stored in a neighbor list, -1 if there is none + int count_neighbors(int nlidx) + { + if (nlidx < 0) return -1; + int total = 0, numneigh = -1, iatom = -1; + int *neighbors = nullptr; + int inum = lammps_neighlist_num_elements(lmp, nlidx); + for (int i = 0; i < inum; ++i) { + lammps_neighlist_element_neighbors(lmp, nlidx, i, &iatom, &numneigh, &neighbors); + total += numneigh; + } + return total; + } }; class NeighborListsNsq : public NeighborListsBin { @@ -560,8 +574,6 @@ TEST_F(NeighborListsBin, one_hybrid_half_list_nonewton) } } -#if 0 -// FIXME: currently trim is not detected and this test will thus fail TEST_F(NeighborListsBin, one_trim_half_list_newton) { create_system("charge", "real", "on"); @@ -615,7 +627,6 @@ TEST_F(NeighborListsBin, one_trim_half_list_newton) } } -// FIXME: currently trim is not detected and this test will thus fail TEST_F(NeighborListsBin, one_trim_half_list_nonewton) { create_system("charge", "real", "off"); @@ -668,7 +679,39 @@ TEST_F(NeighborListsBin, one_trim_half_list_nonewton) GTEST_FAIL() << "No suitable neighbor list info found"; } } -#endif + +// a list requested with a uniform cutoff must reach that cutoff for every atom +// type, also when the pair style uses a shorter cutoff for some type pairs and +// the default list is therefore truncated for them. The contents of such a +// list may not depend on the pair style's per-type cutoffs at all +TEST_F(NeighborListsBin, perpetual_fixed_cutoff_uniform) +{ + create_system("atomic", "real", "on"); + BEGIN_HIDE_OUTPUT(); + // small skin, so that a truncated per-type cutoff really does lose pairs + command("neighbor 0.3 bin"); + command("pair_style lj/cut 3.5"); + // 1-1 pairs use a shorter cutoff, so the default list is truncated for them + command("pair_coeff 1 1 0.01 2.0 2.5"); + command("pair_coeff 1 2 0.01 2.0 3.5"); + command("pair_coeff 2 2 0.01 2.0 3.5"); + // perpetual full list with a uniform cutoff of 3.5 + skin, equal to cutneighmax + command("group dyn dynamic all within 3.5 every 1"); + command("run 0 post no"); + END_HIDE_OUTPUT(); + int nneigh_mixed = count_neighbors(lammps_find_fix_neighlist(lmp, "GROUP_dyn", 0)); + + // same uniform group cutoff, but now no type pair uses a shorter cutoff, so + // the default list cannot be truncated and yields the reference answer + BEGIN_HIDE_OUTPUT(); + command("pair_coeff 1 1 0.01 2.0 3.5"); + command("run 0 post no"); + END_HIDE_OUTPUT(); + int nneigh_uniform = count_neighbors(lammps_find_fix_neighlist(lmp, "GROUP_dyn", 0)); + + EXPECT_GT(nneigh_uniform, 0); + EXPECT_EQ(nneigh_mixed, nneigh_uniform); +} TEST_F(NeighborListsBin, one_atomic_full) { From c41eb4565f20000d0482681169e229e513aec9ee Mon Sep 17 00:00:00 2001 From: Stan Moore Date: Sun, 23 Aug 2026 06:03:58 +0000 Subject: [PATCH 3/3] Prevent copy cycles between perpetual lists with equal custom cutoffs morph_copy_trim() orders copies by list index to keep three or more perpetual lists from being turned into copies of each other, which is a circular dependency that reordering the build order cannot resolve. That ordering was skipped for any list carrying a custom cutoff: if (!irq->occasional && !irq->cut && j > i) continue; The !irq->cut term stood in for "both lists use the default cutoff", which was close enough while few lists kept a custom cutoff. Now that a request keeps its cutoff whenever it differs from cutneighmax, several sub-styles sharing one cutoff all skip the ordering and can form a cycle. With pair_style hybrid/overlay lj/cut 2.5 lj/cut 1.2 lj/cut 1.2 lj/cut 1.2 initialization failed with "Failed to reorder neighbor lists to satisfy constraints", the very error of issue #4529, on an input that develop handles. Two equal cutoffs stayed fine, since the existing jrq->copylist == i check catches a 2-cycle, so this only appeared with three or more. Order by cutoff instead, which is what actually bounds the chain: a copy or trim always points at a list whose cutoff is greater or equal, so a strictly longer cutoff makes the chain increase and it can never close. Only equal cutoffs need the index as a tie break: if (!irq->occasional && (icut == jcut) && j > i) continue; For lists on the default cutoff icut and jcut are both cutneighmax, so their behavior is unchanged, and trimming from a longer list is still allowed regardless of index. Add a regression test for three sub-styles sharing a cutoff. It checks the resulting lists through the library interface rather than the printed summary, and uses the HIDE_OUTPUT lambda so a failure is reported as a failed test instead of aborting the whole test binary. --- src/neighbor.cpp | 7 ++++-- unittest/cplusplus/test_neighbor_class.cpp | 28 ++++++++++++++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/src/neighbor.cpp b/src/neighbor.cpp index ad30a46f4a5..f34e593dbb6 100644 --- a/src/neighbor.cpp +++ b/src/neighbor.cpp @@ -1566,11 +1566,14 @@ void Neighbor::morph_copy_trim() // other list (jrq) to copy from must be perpetual // list that becomes a copy list (irq) can be perpetual or occasional - // if both lists are perpetual, require j < i + // if both lists are perpetual and have the same cutoff, require j < i // to prevent circular dependence with 3 or more copies of a list + // lists with a shorter cutoff are exempt: they are trimmed from a strictly + // longer one, so the cutoff increases along the chain and it cannot close + // into a cycle. Only equal cutoffs need the index ordering as tie break if (jrq->occasional) continue; - if (!irq->occasional && !irq->cut && j > i) continue; + if (!irq->occasional && (icut == jcut) && j > i) continue; // both lists must be half, or both full diff --git a/unittest/cplusplus/test_neighbor_class.cpp b/unittest/cplusplus/test_neighbor_class.cpp index aa670a31b41..6ba318439ac 100644 --- a/unittest/cplusplus/test_neighbor_class.cpp +++ b/unittest/cplusplus/test_neighbor_class.cpp @@ -680,6 +680,34 @@ TEST_F(NeighborListsBin, one_trim_half_list_nonewton) } } +// three or more perpetual lists that share the same custom cutoff must not be +// turned into copies of each other, which would be a circular dependency that +// the reordering of the build order cannot resolve (issue #4529). Sub-styles +// with a shorter cutoff are trimmed from the longest one instead +TEST_F(NeighborListsBin, several_sublists_equal_cutoff) +{ + create_system("atomic", "real", "on"); + // the lambda form releases the output capture if setting up the lists fails, + // so that a circular dependency shows up as a failed test and not an abort + ASSERT_NO_THROW(HIDE_OUTPUT([&] { + command("pair_style hybrid/overlay lj/cut 3.5 lj/cut 2.0 lj/cut 2.0 lj/cut 2.0"); + for (int i = 1; i <= 4; ++i) + command(fmt::format("pair_coeff * * lj/cut {} 0.01 2.0", i)); + command("run 0 post no"); + })); + + // every sub-style must end up with a list, the three that share a cutoff + // must hold the same neighbors, and the longer ranged one must hold more + int n1 = count_neighbors(lammps_find_pair_neighlist(lmp, "lj/cut", 1, 1, 0)); + int n2 = count_neighbors(lammps_find_pair_neighlist(lmp, "lj/cut", 1, 2, 0)); + int n3 = count_neighbors(lammps_find_pair_neighlist(lmp, "lj/cut", 1, 3, 0)); + int n4 = count_neighbors(lammps_find_pair_neighlist(lmp, "lj/cut", 1, 4, 0)); + EXPECT_GT(n2, 0); + EXPECT_EQ(n2, n3); + EXPECT_EQ(n2, n4); + EXPECT_GT(n1, n2); +} + // a list requested with a uniform cutoff must reach that cutoff for every atom // type, also when the pair style uses a shorter cutoff for some type pairs and // the default list is therefore truncated for them. The contents of such a