perf!: recover the vesin neighbour-list regression, and move the potentials into rgpot - #389
Merged
Merged
Conversation
The TheochemUI#386 port called vesin_free (via free_list) before every compute and stack-allocated a new list per force call, defeating vesin's documented allocation reuse. Hold a long-lived eonc::VesinNeighbors on LJ, Morse, LJCluster, and QSC; pass the same VesinNeighborList through vesin_neighbors. QSC builds the list once per force for energy and forces.
Hold the list in thread_local storage so vesin re-uses pair buffers across force evaluations without racing when NEB evaluates images in parallel on a shared pot (MAIN_PARALLEL default). Still no vesin_free before compute. QSC marks itself non-thread-safe and needs per-image instances because it also mutates rho_/sqrtrho_ members.
Replace the rebuild-every-call vesin usage in LJ, LJCluster, Morse, and QSC with eonc::PairListCache: a thread-local pool of CachedPairList slots built at cutoff+skin (shifts only, no distance/vector buffers). Slots match by geometry proximity (max displacement < skin/2), so NEB images sharing one pot instance each keep a live list whether images are evaluated serially or in parallel. Between rebuilds forEach re-derives exact vectors from current positions with precomputed S@H offsets; the true-cutoff r2 filter keeps the pair set identical to a fresh build (Verlet guarantee). LJCluster also drops pow()/sqrt() for the inverse-r2 formulation already used by LJ. QSC's set_verlet_skin now actually sets the skin.
Update thirdparty/vesin single-TU + header from 0.4.0 to upstream v0.6.0
(generated via vesin/scripts/create-single-cpp.py, no-CUDA variant). The
0.6.0 C API adds algorithm selection (auto / brute-force / cell list),
native Verlet-skin caching, and threaded builds; eonc::VesinNeighbors
exposes skin and n_threads (defaults keep prior behavior, n_threads=1).
Vendor the matching upstream Fortran module (fortran/src/{cdef,vesin}.f90)
under thirdparty/vesin/fortran and build it as vesin_fortran when Fortran
is enabled; Fortran pots take it via vesin_f_dep.
vesin 0.6 threads its neighbor builds (std::thread; OpenMP when present), so libvesin_internal needs the threads dependency to link with -Wl,--no-undefined.
NEB's updateForces and ImprovedDimer spawn worker threads per iteration, so thread_local cache storage is destroyed exactly as often as it would be useful. PairListCache is now a process-global, mutex-guarded pool handing out shared_ptr<const CachedPairList>: slots are immutable after build, so readers never race eviction and the lock covers only the proximity match, never the force loops. Concurrent misses on one geometry cost a duplicate build, never a wrong result. Adds a unit test pinning the Verlet guarantee: forces evaluated off a cached list (built at r0, evaluated at r1 within skin/2) match a fresh build at r1 after the slot is evicted.
Replace the O(N^2) MIC double loop in SWFortran.f90's neighbours() with a sorted full-list vesin build at RSKIN through the vendored vesin Fortran module (use vesin :: NeighborList). SW's own two-largest-displacement skin test still gates rebuilds, and the pair/angle loops re-derive MIC vectors from current positions, so the Verlet semantics are unchanged. The silent out-of-bounds write past MAXNEI becomes a hard stop, and periodic self-image pairs are skipped explicitly. Drop the dead sw_cpp static_library target (SW.cpp already builds into eonclib).
Both Goedecker-scaffold pots binned atoms into cells, replicated a ghost boundary layer with a lay() ghost-to-real mapping, and rebuilt everything per force call. A single sorted vesin full-list compute now fills the same lsta/lstb/rel structures the kernels consume: lstb holds real indices (the periodic image folds into the pair vector), rel(1:3) the unit vector of r_i - r_j (negated vesin vector), rel(4:5) distance and inverse. Multiple image pairs stay as separate entries, matching the ghost scheme's physics for boxes between one and two cutoffs. The saved NeighborList reuses vesin buffers across calls. The parallel EDIP kernel zeroes its shared accumulators under an omp single with barrier before threads critical-add into them. eon_edip / eon_lenosky link vesin_f_dep; dead edip_cpp / lenosky_cpp static-library targets are gone (the wrappers build into eonclib).
Aluminum: gagafeDblexp's O(N^2) rebuild sweep iterates a vesin half list (CSR over min(i,j), ascending, image-deduplicated) from the vesin_al helper module; the separation vectors, rectangular MIC, r_i - r_j sign, and the rskin buffer test stay in the F77 exactly as the non-rebuild fast path at label 600 expects. Hard stop when vesin pairs exceed MAXPRS. CuH2: the three species-blocked O(N^2) sweeps (Cu-Cu, Cu-H, H-H) and their embedding-derivative repeats collapse into two passes over one vesin half list with index-block species dispatch (Cu occupy 1..nCuCl). The FPI image and quantum-Cu paths are hard-set off at entry (nCuQ=0, nimpo=nimrp=1) and now guarded explicitly. natoms > maxatoms stops with a message instead of overrunning. Both targets link vesin_f_dep; dead aluminum_cpp static-library target removed.
The permutation sort behind vesin's sorted=true costs several ms on the 497k-pair LJ cluster list and dominates one-shot evaluations (ASV point fixture: 6.2ms vs 2.5ms baseline). The split plain/shifted copy added a multi-MB memcpy per rebuild on top. CachedPairList now keeps the pairs in its own VesinNeighbors buffers (unsorted) and forEach reads them directly, computing the S@H offset only for pairs with a nonzero shift; slots stay immutable after build so the shared_ptr hand-out is unchanged.
vesin 0.6's global ThreadPool constructor eagerly spawns hardware_concurrency()-1 workers, and cell_list_neighbors touches the pool even when options.n_threads == 1. Every eonclient invocation paid ~3ms of thread creation on a 32-core host for a pool it never used (the ASV point fixture ran 2.2x slower than the pre-vesin baseline on startup cost alone). Workers now spawn inside run() on the first dispatch that actually goes parallel; serial runs never touch them. Worker startup with seen_generation = 0 joins an in-flight generation correctly since run() blocks until all active workers finish their chunks. Local patch to the generated single-TU; mirrors what belongs upstream.
Tersoff: per-atom neighbour lists from a sorted vesin full list at S replace the O(N^3) all-atom j and k sweeps; ascending insertion keeps the zeta/force summation in atom-index order, and both k passes (bond order and force redistribution via lid) run over atom i's list, which is exact because every k term is gated on r_ik < S. FeHe: a vesin half list (CSR under min(i,j), image-deduplicated, built at the largest species cutoff) replaces the hardwired 3x3x3 link-cell gather in both the density and force passes; the pair sweeps keep their rectangular MIC folds and symmetric accumulation, so the forward-cell half-enumeration semantics carry over. The ~800-neighbour gather ceiling and the 27-cell O(N^2/27) scaling go away. perf(vesin): CPU VesinBruteForce implementation in the vendored TU (one nearest-image pair per (i,j), caller owns MIC validity), dispatched from cpu::neighbors; the cell grid degenerates to a couple of cells when the cutoff rivals the box and re-enumerates pairs per shift at ~20x the cost. CachedPairList picks brute + per-call MIC folding for orthorhombic boxes with the true cutoff inside half the smallest periodic width, matching the historical MIC loops' image resolution exactly; vesin_internal now links statically (one fewer shared object per eonclient start).
…tion The brute-force kernel folds each dimension directly for diagonal cells (or free boundaries) instead of the fractional-coordinate round trip, cutting the 337-atom slab build from ~0.85ms to the raw pair-scan cost. CachedPairList additionally keeps vesin's distance/vector arrays for small systems (<= 512 atoms) and serves evaluations at the exact build positions straight from them — a point job now scans the pairs once, not twice. Larger systems skip the extra arrays to hold peak memory flat.
The per-field GrowableNeighborList setters each re-check capacity through a non-inlined call, dominating dense O(n^2) fills; brute emission now does one capacity check per pair and writes the raw arrays. Free-boundary systems take a fold-free loop on both build and evaluation (the LJ cluster paid three no-op floor folds per pair per call). A MIC-mode candidate set that turns out to be the complete pair graph (cluster inside the cutoff) can never lose a pair, so its slot validates without a displacement scan and cluster minimizations stop rebuilding after the first force call.
libgomp cost ~0.3ms of load-time relocation on every eonclient start for a library vesin never calls (its pool is std::thread). The fresh-arrays evaluation saved one fold pass but paid more in per-pair sqrt and vector stores during the build; the lean build (pairs only) plus fold evaluation is the faster total on one-shot runs.
-march targets without roundsd (conda's nocona baseline) lower std::floor(t + 0.5) to a libm call, ~20 cycles per fold, three folds per pair on both the brute build scan and every MIC evaluation. A truncate cast with copysign rounds in ~5 cycles; exact-half-box inputs round away from zero instead of up, a measure-zero difference.
One-shot consumers (point jobs, the first call after any rebuild) paid the O(n^2) pair scan twice: once inside the vesin build and once in the forEach fold pass. vesin_neighbors_visit (eOn extension in the vendored TU, upstream candidate) invokes a caller-supplied visitor for every pair within the true cutoff during the same brute-force scan that fills the candidate list at cutoff+skin. PairListCache::ensureVisit routes a slot hit through forEach as before and fuses the evaluation on a miss; the pots hand their pair kernel straight to it. QSC's separate energy pass folds into the fused visit; the force pass reuses the returned slot.
The C-API visitor costs an indirect call per in-cutoff pair and forces register spills inside the scan loop (perf shows per-pair stack traffic around the call). vesin_visit.hpp ships the brute-force MIC scan as a header template: the pot's pair kernel inlines into the single O(n^2) pass, and the candidate list lands in compact flat int32 pairs — half the index bandwidth of the size_t vesin buffers on every subsequent fold evaluation. MIC-mode slots now store pairs in that form; the cell-list (shift) regime keeps the vesin buffers. The vesin C API keeps the brute/visit entry points as upstream candidates. vesin_internal objects build with hidden visibility so the statically linked copies stay out of dynamic symbol tables.
44k push_backs from an empty vector walk the whole growth-realloc ladder on the very first (and for point jobs, only) neighbour build.
…iles Newsfragments cover the fused MIC visitation, the vesin 0.6.0 vendoring with its three upstream-candidate extensions, and the Fortran potential ports; the neighbor-list guide shows ensureVisit as the pot-facing API.
Capturing the candidate list costs pair stores, slot bookkeeping, and first-touch page faults that a one-shot evaluation never amortizes — the point fixture sat a consistent ~0.1ms behind the pre-list baseline. The first sighting of a geometry family now runs the fused eval-only scan (exactly the historical per-call loop) and records only a phantom reference stamp; a second sighting within skin/2 proves reuse and captures the list in its own fused scan. Trajectories pay one extra scan once per image; point jobs match the baseline's single loop. QSC keeps eager capture (its force pass re-reads the list every call), and ensureVisit skips phantom slots outright.
Batch 1 of the potential migration. The four kernels now live in librgpot (wrap pinned to the pot-hosting branch; retargets to the v2.6.0 tag at release) and eOn's factory arms construct them through the new RgpotAdapter: eOn's flat arrays pass straight into the kernel's forceImpl, and thread-sharing policy derives from the kernel's PotCaps. rgpot becomes a hard dependency; the wrap fallback builds one profile serving both the pots and, when with_rgpot/with_serve ask for it, the RPC stack — plain builds stay capnp-free. isSharedInstanceThreadSafe() is virtual now: the central Fortran blocklist is gone and the in-tree Fortran pots carry their own override until their own migration phase. In-tree LJ/LJCluster/Morse/ZBL sources and headers are deleted; RgpotAdapterTest pins the migrated arms to the same reference numbers the in-tree kernels satisfied (kernel numerics are pinned upstream in rgpot's suite).
Kernels with immovable members (ZBLPot holds a mutex for its lazily built pair tables) cannot pass through a by-value move; the adapter takes the config and builds the kernel in its own storage.
Removes the arms and trees the build could never reach: QSC (factory case was commented out), NewPot and IMD (NEW_POT / IMD_POT are defined nowhere, the libraries built and linked for nothing — IMD's data files alone were most of 92k lines), PyAMFF, the GPR/PYTHON factory stubs, the orphaned Metatomic meson fragment, and the with_newpot option. The four potential-name vocabularies move in lockstep: PotType enum, pot_from_ssot, the eon-schema pydantic Literal (ghost names bop / bopfox / zpice / new_pot dropped; ase_nwchem joins the accepted set with the historical ase_nwcem spelling kept as an alias), and the python bindings. The features string reports LAMMPS correctly. eonc::CachedPairList / PairListCache leave eOn: their consumers now live in librgpot (rgpot::nlist::PairListCache is the maintained home); eonc::VesinNeighbors stays for Metatomic. The Verlet-guarantee test remains, pinning the rgpot cache semantics through the Morse factory arm.
SW, EDIP, Lenosky, Tersoff, EAM-Al, FeHe, CuH2, and TIP4P-H are Fortran 2018 kernels inside librgpot now, reached through RgpotAdapter like the classical pots. eOn stops building, installing, and dlopening Fortran entirely: the in-tree sources, the six eon_*.so plugin targets, the per-pot Windows .def export files, and the flang runtime handling all go with them. FortranPotLoader keeps doing the one job it still has and takes a name that says so: PluginLoader finds engine plugin shared objects (the rgpot metatomic and xtb backends) by base name across a search path. EON_POTENTIALS_PATH and [Potential] potentials_path keep working for those engines.
Subproject default_options apply only to a build directory's first configure, so a directory configured under the previous profile keeps Fortran off and the potentials vanish at link time with undefined vtables. The check names the option and the remedy instead.
Its only consumers were the in-tree Fortran potentials; the kernels that replaced them use rgpot's copy. eOn compiles no Fortran at all now.
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
rgpot's public headers no longer leak a namespace-scope using rgpot::types::AtomMatrix, so the capnp-backed engine names it itself. Only the with_rgpot RPC profile compiles this file.
eOn's vendored copy and rgpot's are the same upstream release, but rgpot carries local patches its Fortran interface binds to, so linking both left the Fortran objects referencing symbols eOn's copy does not define. The vendored TU still serves builds against an installed rgpot.
eoncbase compiles VesinNeighbors.cpp, so the vesin include path has to exist before that target is declared; the subproject is now instantiated next to the vesin choice and only takes on its eOn roles further down.
Benchmark ResultsWarning 1 benchmark(s) regressed
Regressions
Improvements
4 unchanged benchmark(s)
Details
Raw asv-spyglass output |
The Fortran kernels ship inside librgpot now, so there is no eon_sw.dll to load and no bare sw_ to export. The step loads the umbrella and fails when a legacy Fortran name reaches the export table, which is what the Linux FortranSymbolAudit checks.
The second Windows job configures --default-library=static, where the kernels land in an archive that has no export table.
…es for the cutover Adds release-note fragments for the vesin sourcing change, the wheels dropping their second shared object, and the rewritten Windows export audit.
The subproject path already checks this; the pkg-config path is what distro and EasyBuild installs use, and it linked on to undefined vtables instead. rgpot.pc advertises RGPOT_HAS_FORTRAN_POTS.
Wrap pins track release tags rather than commits, and the pkg-config path now requires >=3.0.0: that is the first release carrying the potentials, and the one whose .pc stops listing -lptlrpc.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Depends on OmniPotentRPC/rgpot#56 and #57; the wrap pins the rgpot commit those provide and retargets to the v2.6.0 tag once cut.
Why
#386moved the classical pair potentials onto vesin and cost 1.3-2.6x on all four ASV fixtures. The list was freed and rebuilt on every force call, the cell list degenerated to a couple of cells at these cutoffs and re-enumerated pairs per periodic shift, and vesin 0.6's thread pool spawned a worker per core in every short-livedeonclientprocess.Fixing that led to the second half of this branch: the classical kernels and the Fortran ones now live in
librgpot, so the neighbour-list work has one home instead of being applied twice.Neighbour lists
eonc::PairListCacheis a process-global, proximity-matched pool of Verlet-skin cached pair lists. NEB spawns fresh worker threads every iteration and shares one potential instance across images, so neither a per-instance cache (a data race) northread_localstorage (destroyed with each iteration's threads) survives; slots are immutable after build and handed out asshared_ptr, so readers never race eviction and the pool lock covers only the match.Evaluation re-derives exact vectors from current positions and filters at the true cutoff, so results match a fresh build; a unit test pins that guarantee across an eviction. Single-pass potentials capture the list lazily, so one-shot jobs run a single fused scan like the pre-list code did.
Vendored vesin moves 0.4.0 -> 0.6.0 with four local patches, all upstream candidates: lazy thread-pool spawn, a CPU implementation of the declared-but-missing
VesinBruteForce, direct-write pair emission, and a fused visitation API.Results (rg.terra, hyperfine, stable across three repetitions; branch / pre-#386 baseline /
svn-Mar_18_2026):Potentials move to rgpot
LJ, LJCluster, Morse, ZBL, and the Fortran-backed set (SW, EDIP, Lenosky, Tersoff, EAM-Al, FeHe, CuH2, TIP4P-H) evaluate through a new
RgpotAdapter, which passes eOn's flat arrays straight into the kernel and derives thread-sharing policy from the kernel's capabilities.isSharedInstanceThreadSafe()becomes virtual and its hard-coded Fortran blocklist is gone.The Fortran kernels were rewritten as Fortran 2018 rather than wrapped: modules with
implicit none, derived-type parameters replacing COMMON blocks,intenteverywhere,purekernels, nogoto, status returns instead ofstop, and pair sums restated as gathers so the atom loops run underdo concurrent.eOn therefore builds, installs, and dlopens no Fortran at all: the
eon_*.soplugin modules, their Windows.defexport files, the flang runtime handling, and the vendored vesin Fortran interface are deleted.FortranPotLoaderbecomesPluginLoader, which still finds engine plugins (the rgpot metatomic and xtb backends) acrossEON_POTENTIALS_PATHand[Potential] potentials_path.Configuration is unchanged — the same
potentialnames select the same physics.Housekeeping
Dead potential surfaces the build could never reach are removed: QSC, NewPot, IMD, PyAMFF, and the GPR/PYTHON factory stubs, plus an orphaned Metatomic meson fragment (IMD's data files alone were most of 92k deleted lines). The four potential-name vocabularies move in lockstep — the
PotTypeenum,pot_from_ssot, the eon-schema pydantic literal (ghost namesbop,bopfox,zpice,new_potdropped;ase_nwchemjoins with the historicalase_nwcemspelling kept as an alias), and the python bindings.--featuresreports LAMMPS correctly.Testing
50/50 on Linux.
SiPotTest,EAMAlTest,FeHeTest, andcuh2Testkeep their reference energies and now exercise the ported kernels, which is the equivalence gate: SW -16.204955, Tersoff -17.440266, EDIP -18.838135, Lenosky -17.284558, EAM-Al -5.217864, FeHe -43.959774.RgpotAdapterTestpins the migrated arms throughmakePotential.A configure-time check fails with its remedy if the rgpot subproject is built without Fortran potentials: meson applies subproject
default_optionsonly on a build directory's first configure, so a stale directory otherwise drops them and fails at link time with undefined vtables.