[pull] develop from lammps:develop#161
Merged
Merged
Conversation
change_box could leave atoms outside a non-periodic boundary without detecting them. Its move_atoms path uses Irregular::migrate_atoms(), which clamps out-of-box atoms onto the nearest proc (via comm->coord2proc) instead of removing them, unlike comm->exchange() during a run. So the existing lost-atom check never fired, atom->natoms was left inconsistent, and a subsequent write_data wrote a corrupted data file (header atom count != number of Atoms-section lines) when thermo_modify lost was "warn" or "ignore" (matsci.org/t/63775). change_box now culls atoms outside non-periodic boundaries after the box change (as a reneighbor would via comm->exchange), rebuilds the atom map, and applies the thermo_modify lost policy: "error" (default) aborts with a lost-atoms error, "warn" warns and continues, "ignore" is silent. atom->natoms is reset so the system stays self-consistent. write_data applies the same policy when the recomputed atom count differs from atom->natoms: "error" aborts (unchanged), while "warn"/"ignore" reset atom->natoms so the written data file is always self-consistent instead of having a header count that disagrees with the body. Docs for both commands updated with a versionchanged note. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
When fix qeq/point (and the other s/t-method variants qeq/shielded, qeq/slater, qeq/ctip, which share this base-class code) is applied to a subset of the atoms while the remaining atoms carry fixed charges, the equilibrated charges came out with the wrong sign and the total energy increased rather than decreased. FixQEq::init_storage() initialized both the s and the t auxiliary vectors to the current per-atom charge for *all* atoms. Initializing s to the fixed charge of atoms outside the fix group is intentional: it injects their charge as the external field felt by the group atoms through the sparse matrix-vector product. The t vector, however, only encodes the charge-neutrality (Lagrange-multiplier) response of the group; giving out-of-group atoms their fixed charge there pollutes the neutralization (u = sum_s/sum_t) and flips the sign of the result. Initialize t to 0 for atoms outside the fix group. For the common case where qeq acts on all atoms this is a no-op (verified: examples/qeq output unchanged); it only affects the subset case, where qeq/point now matches qeq/dynamic and the energy is correctly minimized. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
fix gjf was not restartable: two pieces of per-step state were lost across a write_restart/read_restart cycle. - The Marsaglia RNG state, so the stochastic trajectory could not be reproduced after a restart. - The per-atom half-step velocity array lv. In the default "vel vhalf" mode the on-site velocity needed to continue the integration is kept in lv (atom->v only stores the half-step velocity used for output), so without checkpointing lv the first post-restart step started from a zeroed on-site velocity and the trajectory diverged at full scale. Add restart_global with write_restart()/restart() for the per-proc RNG state (as in fix temp/csvr and the BROWNIAN fixes), and restart_peratom with pack_restart()/unpack_restart() for lv, setting lv_allocated on restart so the restored lv is used on the first step. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…not init() The GJF barostat coefficients gjfa/gjfb depend on the timestep, but were computed in Fix::init(). A "timestep" command issued before a run only updates update->dt during run setup, which happens after init() runs, so the coefficients could be built from a stale dt while the integrator advanced with the new one. This makes the barostat trajectory depend on the timestep history rather than the active timestep (e.g. a run after a "timestep" change, or a restart that re-derives dt, diverges). Recompute gjfa/gjfb in setup() (via reset_dt()), where update->dt is final. p_mass/p_alpha (dt-independent) stay in init(). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
fix press/langevin was not restartable: a run continued from a restart did not reproduce the original stochastic barostat trajectory. Two pieces of state were lost across write_restart/read_restart: - the Langevin piston RNG state (without it the random piston forces, and thus the box dilation, diverge at full scale); and - the current piston force f_piston, which seeds f_old_piston on the next step. Add restart_global with write_restart()/restart() saving f_piston and the per-proc RNG state (replicated, but gathered per-rank for robustness). The piston momentum p_deriv is deliberately zeroed at each run start (init()), so it is intentionally not checkpointed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Implements the bond-centric P_CC modification of AIREBO (J. Chem. Phys. 137, 054102, 2012) as a subclass of PairAIREBO, overriding only the P_CC evaluation. Adds CH.airebo-bc parameter file, documentation, example, and regression tests.
- Move versionadded directive to the description section (per review) - Add airebo/bc to doc/src/Commands_pair.rst index - Validate pCC knot count in read_file_extra() - Add pair_airebo_bc.* to src/.gitignore - Fix polyethylene spelling in example and logs
# Conflicts: # src/.gitignore
CompiledExpression::operator= overwrote each operation[] pointer with a fresh clone() without first deleting the operation the target already held, leaking those objects (the destructor deletes them, so assignment must too). valgrind flagged this in the test_lepton Lepton_Exception case. Delete the existing operations before re-cloning, with a self-assignment guard so the delete does not free the source operations. In the bundled lib/lepton copy. Found via ctest memcheck (issue #4537). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019nii2NLXtpCD2oqS4rrYAX
FFTW's planner reads past the end of internal SIMD scratch buffers in fftw_plan_awake() (reached from fftw_destroy_plan() when LAMMPS tears down a PPPM/FFT3d plan), which valgrind reports as an invalid read of size 8. This is internal to FFTW and a known false positive, not a LAMMPS defect. Add a generic FFTW.supp anchored on fftw_plan_awake. Verified the previously unsuppressed read in test_kspace_group_group_slab is now suppressed (issue #4537). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019nii2NLXtpCD2oqS4rrYAX
The KnownAnswer_Kokkos_Sine test created an FFT with create_serial_fft() but never deleted it. TearDown() intentionally only nulls the void* fft member and leaves deletion to each test method (as the roundtrip/delta tests do), so this one leaked the entire FFT plan (208 direct + 5409 indirect bytes). Add the matching delete at the end of the test. Found via ctest memcheck (issue #4537). The remaining valgrind noise in this test is the MPI_Init false positive and Kokkos-runtime possibly-lost blocks. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019nii2NLXtpCD2oqS4rrYAX
PairReaxFFKokkos::Deallocate_Lookup_Tables() skipped freeing the per-type row arrays LR[i] for unmapped types via an `if (map[i] == -1) continue;` in the outer loop. But Init_Lookup_Tables() allocates LR[i] for every type 0..ntypes (i == 0 is always unmapped), so those rows and the LR pointer array leaked when pair style reaxff/kk ran with long-range tabulation enabled. Move the map[] guard into the inner loop so it only gates the per-pair spline arrays (which exist only for mapped pairs) and free LR[i] unconditionally, matching the CPU Deallocate_Lookup_Tables(); also null api->system->LR after freeing it. Verified with test_pair_style atomic-pair-reaxff_tabulate.yaml under valgrind (kokkos_omp): 2688 bytes definitely lost -> 0 (issue #4537). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019nii2NLXtpCD2oqS4rrYAX
PairUF3Kokkos::destroy_3d()/destroy_4d() released the legacy cut_3b/min_cut_3b arrays (built by the base PairUF3::allocate() with memory->create()) using memory->sfree(), which frees only the outermost pointer and leaks the nested plane pointers and contiguous data block. Use memory->destroy() so all levels of the array are freed. The array reference is nulled afterwards, so the base ~PairUF3() memory->destroy() remains a safe no-op (no double free). Verified with test_pair_style manybody-pair-uf3.yaml under valgrind (kokkos_omp): ~2.6 KB direct + 58 KB indirect lost -> 0 (issue #4537). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019nii2NLXtpCD2oqS4rrYAX
The python-driven unit tests run the system python3 interpreter under valgrind. CPython and NumPy intentionally do not release all interpreter and module state at Py_Finalize (type objects, dtype/ufunc registrations, interned data), so valgrind reports them as leaks -- in a separate process from the LAMMPS code, hence false positives (no LAMMPS frames appear in any of these stacks). Add generic suppressions anchored on the CPython allocators (_PyMem_Raw*, _PyObject_GC_*, _PyObject_*alloc) so they survive CPython/NumPy version changes. Verified: `python3 -c "import numpy"` reports ~6.5 KB direct + ~29 KB indirect lost without these and 0 with them (issue #4537). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019nii2NLXtpCD2oqS4rrYAX
PairLdd::coeff() called allocate_species() unconditionally without freeing a previous allocation. When coeff() runs more than once (e.g. the force-style "extract" unit test re-initializes the pair), the earlier per-species arrays and the indicator/potential objects created by the first read_file() leaked -- valgrind reported ~98 bytes direct + ~2.3 KB indirect across the whole LDD family of tests (LD-constant/linear/mdpd, LDINDSET1/2, ...). Factor the per-species cleanup out of the destructor into deallocate_species() and call it at the start of coeff() (using the still-current nelements) so a repeated pair_coeff frees the prior arrays and indicator/potential objects first. The destructor now reuses the same helper. Verified with test_pair_style mol-pair-ldd-LD-constant.yaml under valgrind: ~2.4 KB lost -> 0, tests pass (issue #4537). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019nii2NLXtpCD2oqS4rrYAX
…e ldd" This reverts commit f96ad45.
PairBondVal::~PairBondVal() did not free the per-type arrays r0 and energy0, and PairBondValVec::~PairBondValVec() did not free r0 -- all allocated in the respective allocate() but missing from the destructor, so they leaked once coeff() ran. Add the missing memory->destroy() calls. Found via ctest memcheck (issue #4537). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019nii2NLXtpCD2oqS4rrYAX
…etup PairTersoffTableOMP::allocatePreLoops() allocated the per-thread pre-loop arrays without first freeing any previous allocation, so a repeated setup (e.g. the force-style "extract" test re-initializes the pair) leaked the prior arrays (~270 KB). The base PairTersoffTable::allocatePreLoops() already calls deallocatePreLoops() first; do the same in the OMP override. Found via ctest memcheck (issue #4537). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019nii2NLXtpCD2oqS4rrYAX
The python-driven unit tests run the system python3 + numpy under valgrind; NumPy's C-extension import (ufunc/dtype registration) and the CPython bytecode interpreter produce many leaks and uninitialised-value errors internal to those libraries. Add suppressions anchored on _PyImport_RunModInitFunc and _PyEval_EvalFrameDefault for the leak and uninitialised-value error kinds. Verified an `import numpy` + array workload drops from 100 unsuppressed errors (plus leaks) to 0 (issue #4537). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019nii2NLXtpCD2oqS4rrYAX
PairMorseSoft::nlambda is an int, but write_restart_settings() wrote it with sizeof(double) (emitting 4 uninitialised bytes into the restart file, flagged by valgrind) and read_restart_settings() read sizeof(double) into the int (4 bytes past it). Use sizeof(int), consistent with the following MPI_Bcast(MPI_INT). Note: this changes the (previously malformed) restart layout for this style. Found via ctest memcheck (issue #4537). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019nii2NLXtpCD2oqS4rrYAX
FixGLD::init_s_gld() only set the per-atom GLD state s_gld[] for atoms in the fix's group; for atoms outside the group it was left uninitialised and then packed into restart files (valgrind: uninitialised bytes written by WriteRestart). Zero s_gld for the non-group atoms. Found via ctest memcheck (issue #4537). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019nii2NLXtpCD2oqS4rrYAX
AtomVec::setup_fields() did `threads = new bool[ngrow]` without freeing a previous allocation; since the destructor only frees the last one, calling setup_fields() again leaked the earlier array. Delete it first. Found via ctest memcheck (issue #4537). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019nii2NLXtpCD2oqS4rrYAX
The 1d-only timing path in FFT3dKokkos used flag == -1 to select the forward transform and flag == 1 to trigger inverse scaling, the opposite of the convention used everywhere else: fft_3d_kokkos() (the real compute path), the FORWARD=1/BACKWARD=-1 enum, and the KSPACE CPU fft_1d_only() in fft3d.cpp. Align the MKL, MKL_GPU, FFTW3/ NVPL, and KISS branches, and the scaling condition, with that convention. Co-authored-by: stanmoore1 <stanmoore1@gmail.com> Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0168JQi3bP6s84dZcvoGxyQy
Under SINGLE_DEVICE the DualView edge is inert and modify_host() records legacy->device staleness on the HostKK<->Host edge (modified_legacy_hostkk), which need_sync_device() did not consult. It could therefore return false while the device (aliased to the Kokkos host mirror under SINGLE_DEVICE) was stale, so a gate such as atom_vec_*_kokkos.cpp's sync_pinned(Device, ...) would skip the required pinned-copy transform and the kernel would read stale data. OR in (SINGLE_DEVICE && modified_legacy_hostkk) so the predicate is consistent with need_sync_host() and with sync_device()'s own SINGLE_DEVICE delegation to sync_hostkk(). GPU / !SINGLE_DEVICE builds are unchanged: the added term folds to false at compile time. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Co-Authored-By: stanmoore1 <stanmoore1@gmail.com> Claude-Session: https://claude.ai/code/session_01Bqe95w49ywSEaMYT1tnP91
Add pair style airebo/bc (bond-centric AIREBO, Hur & Stuart 2012)
Add triclinic-box support to pppm/disp (and its omp/tip4p variants)
Collected bugfixes
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )