[pull] develop from lammps:develop - #169
Merged
Merged
Conversation
A typo (jm1 = j = 1 instead of jm1 = j - 1, as used in the three parallel branches) made 2d grid averaging of a single bracketed per-atom value like c_ID[3] silently accumulate column 2 instead of the requested column, and clobbered the value index j. Found while triaging Coverity scan defects (no CID; discovered by inspection of neighboring flagged code paths). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YNvuaNpJNkbCKaAE8ngPQv
The selection-sort for angleneigh executed its swap statements on every inner iteration (outside the improvement test), unlike the correct bondneigh twin loop directly above. With a stale swap index this duplicates the running minimum and drops an element for most input orderings, so the 1-3 special neighbor list used for Tinker- convention multipole frame assignment could be corrupted, yielding wrong AMOEBA/HIPPO frames and forces for unlucky atom-ID orderings. Coverity CID 1661393 (uninitialized index read); the real defect is the misplaced braces. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YNvuaNpJNkbCKaAE8ngPQv
In pair styles lj/cut/tip4p/long/omp and lj/long/tip4p/long/omp the bookkeeping for ev_tally_list_thr() (the n/key counters and the vlist atom-index list) was only maintained when the virial was requested (VFLAG resp. a runtime vflag check), but ev_tally_list_thr() is called whenever EVFLAG is set and uses key/vlist to distribute the pair energy over the 2/4/6 listed atoms when eflag_atom is set. With per-atom energy requested but no virial (e.g. compute pe/atom without stress output) the tally indexed eatom_pair[] with uninitialized garbage indices, causing out-of-bounds writes. Restructure to match the base classes and the other five TIP4P OMP variants: maintain n/key/vlist under EVFLAG, keep only the v[] virial component computation under the virial condition. Coverity CIDs 1662776, 1661025, 1662344, 1661588, 1662877. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YNvuaNpJNkbCKaAE8ngPQv
Pair::init_tables() left the espflag branch of the table-parameter dispatch empty and had no espflag case in the cut_respa table fill, so the respa branch consumed uninitialized grij/expm2/derfc values: tabulated Coulomb forces and energies were garbage when the PS solver pair style (respa_enable=1) ran under run_style respa with table lookups enabled (the default ncoultablebits 12). Evaluate the ESP force/energy polynomial factors once in the dispatch (deduplicating the two existing inline copies) and add the espflag cases to the respa fill and the outer switching region, using the same full-minus-short-range split (fesp - 1.0) as the style's analytic compute_outer() path. Coverity CIDs 1662249, 1661639, 1662021. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YNvuaNpJNkbCKaAE8ngPQv
…rature cutoff The heat-transfer increments dQc/dQd/dQr are computed only when r < cutT, but the Newton update Q[j] -= (dQc - dQd + dQr) ran for every pair inside the force cutoff. Whenever the temperature cutoff is smaller than the force cutoff, neighbor atoms accumulated uninitialized garbage heat flux, corrupting the eDPD temperature evolution. Initialize the increments to zero so pairs beyond the temperature cutoff transfer no heat, consistent with the Q[i] update. Coverity CIDs 1661734, 1662410, 1662587. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YNvuaNpJNkbCKaAE8ngPQv
The random heat-flux term drew a fresh gaussian into randnumT but the following clamp statement clamped randnum (the force random number) instead, discarding the fresh draw. The random heat flux dQr thus reused the same random number as the dissipative force term instead of an independent one, spoiling the fluctuation-dissipation relation for the temperature evolution. Found by inspection while fixing the adjacent heat-flux bug; changes trajectories of eDPD runs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YNvuaNpJNkbCKaAE8ngPQv
…ld/tip4p The variable-field branch of post_force() tallied virial contributions at the M, H1, and H2 sites from the unwrap[] buffer without ever calling domain->unmap(), so it used stale coordinates left over from another atom (or garbage), producing a wrong global virial whenever the field is defined via variables. Add the same unmap calls the constant-field branch uses. The atom-style energy variable was also summed as efield[0][3] (the value of atom 0) at all four tally sites instead of the per-atom values; index with the site atom like fix efield does. Coverity CID 1661598. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YNvuaNpJNkbCKaAE8ngPQv
The per-(i,j) energy/virial tally after the sub-particle loops passed fpair/delx/dely/delz values belonging to the last sub-particle pair inside the sub cutoff, or stale/uninitialized data when all sub-particle pairs were skipped, producing a garbage (potentially NaN) pairwise virial contribution for per-atom stress requests. The sub-particle forces are applied at the atom centers and the global virial comes from fdotr, so zero the pairwise force/displacement for discretized interactions and tally only the accumulated energy. Coverity CID 1661075 (pair line/lj); pair tri/lj shares the pattern. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YNvuaNpJNkbCKaAE8ngPQv
When a pair was outside the Coulomb cutoff (reachable whenever the LJ cutoff is larger) or at near-zero distance, only forcecoul was zeroed while efield_i kept the value from a previously processed neighbor, so a garbage electric field contribution was accumulated. Zero both, as all sibling dielectric pair styles do. Also apply the same rsq > EPSILON guard to the Coulomb energy block that the force block uses, so coincident atoms cannot evaluate the energy from stale prefactor/egamma/table values. Coverity CIDs 1662310, 1662191, 1661019. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YNvuaNpJNkbCKaAE8ngPQv
…es in fix gemc The random insertion point for an exchange move is sampled on rank 0 (as fractional coordinates for triclinic boxes) and only the Cartesian coordinates are broadcast, but the owning-subdomain test read lamda[] on all ranks. Every rank except 0 tested uninitialized stack values, so the wrong rank (or no rank, or several) could claim the inserted atom, losing or duplicating atoms in triclinic GEMC runs. Recompute the fractional coordinates from the broadcast position on all ranks, as fix gcmc does. Coverity CID 1662351. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YNvuaNpJNkbCKaAE8ngPQv
In FixRigidSmall::resample_momenta() (used by fix hmc) the body-frame angular velocity wbody is drawn only for bodies in the fix group, but the rotation into b->omega ran for every body, overwriting the angular velocity of non-group bodies with values rotated from uninitialized or stale wbody data. Move the rotation inside the group test. In the momentum-zeroing loop the group test dereferenced b before it was assigned to the current body, so the previous iteration's (or the first loop's last) body decided whether the current body's velocity was corrected. Assign b first. Coverity CID 1662345. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YNvuaNpJNkbCKaAE8ngPQv
…uck6d For 1-3 pairs beyond the LJ cutoff (a routine case) only forcebuck6d was zeroed, and fpair = forcebuck6d * r32inv multiplied zero by an uninitialized or stale value. If the stale bit pattern happens to be Inf or NaN the product is NaN and silently poisons the forces. Zero r32inv together with forcebuck6d. Coverity CID 1662641. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YNvuaNpJNkbCKaAE8ngPQv
The hi-argument parsing block of the region cone constructor had three defects its correct lo-argument twin does not: - histyle was never initialized to CONSTANT (only set for v_ input), so shape_update() of a variable-shape cone with a constant hi bound tested an uninitialized member and could evaluate a garbage variable reference (Coverity CID 1661727). - The EDGE test checked arg[7] (the lo argument), so 'lo EDGE' with a numeric hi silently replaced the given hi value with the box edge. - The y-axis branch was missing its else, so 'hi INF' on a y-axis cone was immediately overwritten with the upper box boundary. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YNvuaNpJNkbCKaAE8ngPQv
…nders In surface_exterior() of region cylinder with an open side face (e.g. 'open 3'), a particle outside the radius closest to a cap was assigned the cap coordinate along the axis, but the two in-plane contact-point coordinates were only set for r < radius; with the side face open they remained uninitialized (or stale), so granular wall forces from fix wall/gran/region acted toward a garbage point. Project the contact point onto the cap rim in that case, in all three axis orientations. For closed cylinders this reproduces the values the curved-surface block already computed, so behavior there is unchanged. Coverity CIDs 1662234, 1662893, 1661299. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YNvuaNpJNkbCKaAE8ngPQv
The mean squared separation was always averaged over 6 entries of the distance array even when an atom has fewer than 6 neighbors inside the cutoff, reading stale data from previous atoms or past the end of the allocation when no atom in the system has 6 neighbors. Atoms with fewer than 6 neighbors (free surfaces, small clusters, gas phase) got a garbage, run-to-run varying structure classification. Average over min(6, n) neighbors instead, in both the base and the OpenMP version, which classifies undercoordinated atoms deterministically (typically as UNKNOWN). Coverity CIDs 1661953, 2108 (OpenMP); the base class shared the flaw. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YNvuaNpJNkbCKaAE8ngPQv
- compute pressure/bocs: p_basis_type, N_basis, N_mol, and vavg were only assigned when fix bocs pushes its coarse-grained correction via send_cg_info(). Used without fix bocs, compute_scalar() compared the uninitialized basis type against BASIS_ANALYTIC (== 0, a common value for uninitialized memory) and could dereference the null phi_coeff or apply a garbage pressure correction. Initialize the basis type to -1 so no correction is applied until one is provided. (Coverity CID 1662610) - fix bond/react: a rate_limit keyword with a numeric Nlimit left RateLimit::var_flag (and var_id) uninitialized, so the rate-limit check could treat garbage as an equal-style variable reference and evaluate a garbage variable index. Use default member initializers. (Coverity CID 1661322) - fix controller: err/sumerr/deltaerr/olderr were uninitialized until the first end_of_step(), but compute_vector() can be invoked before that (e.g. thermo output at setup), reporting garbage PID terms. (Coverity CID 1662773) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YNvuaNpJNkbCKaAE8ngPQv
…tarts The limit_damping flag parsed by pair styles gran/hooke, gran/hooke/history, and gran/hertz/history was not written to or read from the restart file, and settings() is not called when reading a restart, so after read_restart the flag was uninitialized: the damping limitation could silently turn itself on or off. Append it to the shared restart settings I/O of the base class. Coverity CID 1661997. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YNvuaNpJNkbCKaAE8ngPQv
Of the six pair_style dsmc arguments only max_cell_size and seed were stored in the restart file: weighting, T_ref (and the derived kT_ref), recompute_vsigmamax_stride, and vsigmamax_samples were uninitialized after read_restart, giving garbage collision statistics. Save and restore all of them and re-derive kT_ref. Coverity CID 1661563. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YNvuaNpJNkbCKaAE8ngPQv
…estarts The seqdepflag chosen with the seqav/seqdep pair_coeff argument of pair styles oxdna/hbond, oxdna/stk, and oxrna2/stk (and their oxdna2/ oxrna2 variants derived from them) was not stored in the restart file, but init_one() uses it to decide whether the sequence-dependence scaling of epsilon and shift is applied when symmetrizing the type pairs. After read_restart an uninitialized flag could silently switch between sequence-averaged and sequence-dependent interactions. Coverity CIDs 1661087, 1661247; pair oxdna/stk shared the flaw. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YNvuaNpJNkbCKaAE8ngPQv
Only the global cutoff was stored in the restart file. The limit/eradius and pressure/evirials flags, the per-type ECP element assignments, and the Pauli core parameters (including per-type values customized via pair_coeff) were all uninitialized after read_restart, and the unit-conversion constants h2e/hhmss2e (flagged by Coverity) were garbage in every force evaluation. Serialize the settings state and re-derive the unit constants the same way settings() does. Coverity CID 1662077. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YNvuaNpJNkbCKaAE8ngPQv
- next command: listing the same index/loop variable twice made the first increment exhaust and remove it, so the second lookup returned -1 and variables[-1] was incremented (out-of-bounds write). Reject duplicate variable names with a clear error. (Coverity CID 1661328) - variable formulas: a py_-prefixed function name that does not match any variable passed find() == -1 straight into variables[pyvar], an out-of-bounds read, instead of the intended error message. (Coverity CID 1661224) - fix store/state: an atom-style variable value dereferenced the per-atom storage array unconditionally, which is a null pointer on procs without atoms; fix ave/atom already guards the same call. (Coverity CID 1662474) - fix ave/grid: a bracketed atom-style variable reference (v_name[2]) was accepted but has no array to read, so accumulation dereferenced an uninitialized array pointer; reject it at parse time like other averaging styles do. Also correct three error messages that referred to fix ave/atom. (Coverity CID 1661185) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YNvuaNpJNkbCKaAE8ngPQv
…d_order
Both commands document a default output file name (dynmat.dyn and
third_order.dat), but the file was only opened when the file keyword
was given: without it third_order crashed on rank 0 calling ferror()
on a null FILE pointer after every written block, and dynamical_matrix
silently discarded the entire matrix. Open the file unconditionally
and align third_order's default name with its documentation
('Third Order.dat' -> 'third_order.dat').
Coverity CID 1661320.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YNvuaNpJNkbCKaAE8ngPQv
pair agni: parameter tags (eta, Rc, n_train, ...) appearing before the n_elements and interaction headers dereferenced a null params array with an uninitialized section index; reject such malformed files with a clean error. (Coverity CID 1662437) pair rann: none of the comment/blank-line skip loops checked the fgets return value. Since fgets leaves the buffer unchanged at EOF, an empty or comment-only potential file - or a valid one with trailing blank lines - spun forever re-reading EOF. Error out (or fall through to the existing EOF handling) when the file ends. (Coverity CID 1662231) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YNvuaNpJNkbCKaAE8ngPQv
MLIAPData: natoms was narrowed from bigint to int before the too-many-atoms check, so the check compared already-truncated 32-bit arithmetic against MAXSMALLINT and could never trigger; the actual overflow then silently corrupted the global array dimensions. Do the check on atom->natoms in bigint arithmetic first. (CIDs 1661505, 1662169) NPair::coord2bin_big: the bin index was computed with 32-bit multiplications and only widened on assignment, defeating the purpose of the bigint variant that exists precisely for bin counts beyond 2^31 (hash bins for multi-collection neighbor lists). (CIDs 1662186, 1661428) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YNvuaNpJNkbCKaAE8ngPQv
- MyPoolChunk: on invalid constructor parameters the early return left npage and the page/freelist pointers uninitialized, so destroying the partially constructed pool freed garbage pointers. Initialize all destructor-relevant members first and validate the parameters before any allocation. (Coverity CIDs 1661308, 1661903) - fix halt: the MPI_Request array allocated for universe-wide halt messaging was never freed. (Coverity CID 1661334) - fix vector: the nmax keyword accepted values beyond 2^31; the storage grow() then truncated while the bigint ring index did not, an eventual out-of-bounds write. Reject such values. (CID 1662309) - fix imd: the protocol-v3 message length is computed in 32-bit as up to 36 bytes per atom, overflowing for more than ~59M atoms despite the existing atom-count check; tighten that check so the message length always fits. (Coverity CID 1662882) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YNvuaNpJNkbCKaAE8ngPQv
- compute saed/xrd and fix reaxff/bonds/kk reported memory usage from counters that are only set during the first compute or end_of_step, printing garbage sizes at setup. (CIDs 1661547, 1661687, 1663254) - fix surface/local memory_usage overwrote instead of accumulating the connectivity terms (missing +=). (Coverity CID 1660985) - pair runner zeroed 9 doubles into the 6-element Pair::virial array, clobbering the 24 bytes of adjacent class members on every global virial evaluation, and left the directory pointer uninitialized in the constructor although the destructor deletes it. (CIDs 1661855, 1661672, 1662613) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YNvuaNpJNkbCKaAE8ngPQv
- mliap/so3/kk computed allocation sizes and the per-pair data offset with 32-bit multiplications before widening to bigint, giving wrong (wrapped) allocation sizes and device indices for large systems; the CPU version already computes these in 64-bit. (Coverity CIDs 1663226, 1663089, 1663395, 1663238) - compute temp/deform/kk declared h_ratelo as Few<double,6> but initializes it from the 3-element domain->h_ratelo array, a 24-byte overread on every bias removal. (Coverity CID 1663243) - pair exp6/rx/kk passed errorFlag by value into the vectorized mixing-weight path, so the too-few-molecules abort conditions were silently swallowed on host builds while device builds abort. (Coverity CID 1663155) - fix wall/gran/old with the DMT normal model read the cohesion energy from an uninitialized local that only the JKR branch set, giving a garbage friction limit Fncrit. (Coverity CID 1663148) - fix wall/region/kk silently did nothing (zero forces, undefined wall energy and virial) when the region style has no KOKKOS variant; now init() reports a clear error. (Coverity CID 1663218) - pair dpd/fdt/energy/kk instantiated the no-splitting kernels with STACKPARAMS=false in both branches of the ntypes dispatch, so the stack-parameter fast path was dead code; results are unaffected. (Coverity CID 1663182) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YNvuaNpJNkbCKaAE8ngPQv
…romptly pair multi/lucy/rx divided by the total molecule count of a CG particle without the 10*DBL_EPSILON sanity check that pair table/rx and pair exp6/rx perform, silently producing NaN weights; add the same check on the CPU and (via Kokkos::abort) in the KOKKOS version, and restore it in pair table/rx/kk which defined MY_EPSILON but never used it after the port. (Coverity CIDs 1661036, 1662158, 1663121, 1663023, 1663371, 1663376) The destroy_kokkos()/destroy_3d/destroy_4d helpers took the view by value, so resetting it released only the local copy and the caller's view kept the allocation alive until reassignment; pass by reference so device memory is released when requested. (Coverity CIDs 1663205, 1663073, 1663169, 1663306, 1663189) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YNvuaNpJNkbCKaAE8ngPQv
… batch Mechanical follow-up to the batch merged via PR #5058, covering newer files and members the batch engine missed: cached per-atom array pointers in atom_vec_apip; the type2rhor/type2z2r maps of pair eam/apip; the neighbor list pointer of compute ave/sphere/atom; the per-type coefficient arrays of pair lj/pirani; pair granular's extra_svector count; pair hdnnp's settings-stage scalars and directory string (whose garbage value was never freed but also never safely testable); the per-thread centroid-stress accumulators of ThrData (also nulled per step in init_force like their eatom/vatom siblings); the paux1/paux2 pointers of the vendored PTM Voronoi cell; the matvecs_s/matvecs_t counters of fix qeq/reaxff and qtpie/reaxff; the cached box/domain pointers and box_drop/box_other/box_touch/point_drop function pointers of comm_tiled (via init_pointers, covering the Comm-copy constructor); the write_choice (and dump_atom convert_choice) function pointers of the atom/cfg/custom/xyz dumps; the overlap-list counters of Grid2d/Grid3d in both constructors; the linemin function pointer of min_linesearch; and the multi stencil pointers of NPair. Also value-initialize Neighbor::init_pair()'s neigh_bin/neigh_stencil arrays so the existing defensive null checks compare real null pointers instead of uninitialized memory. Coverity CIDs 1662103, 1662812, 1661101, 1661000, 1662461, 1661489, 1661005, 1662718, 1661900, 1661288, 1661621, 1662871, 1661301, 1662252, 1661566, 1661753, 1662305, 1662554, 1662232, 1661906, 1662444, 1661856, 1661718, 1662007. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YNvuaNpJNkbCKaAE8ngPQv
Counts and string lengths read from binary restart files were used directly for allocations and loop bounds; a corrupted or truncated file produced allocation failures, heap overflows (the LEPTON styles read per-entry expression lengths into a buffer sized by an unchecked maxlen), out-of-bounds writes (pair granular nmodels vs maxmodels), or stalled unpack loops (read_restart per-atom chunks). Add range checks with clear 'restart file' error messages right after each read: the hybrid styles' nstyles and keyword lengths, LEPTON expression buffer and entry lengths, the gaussian/fourier/nharmonic/spherical term counts, granular model names, pair tracker fix ID and value counts, group names, Modify's global/per-atom restart counts (and the buffer size accumulation, now done in 64-bit), the native dump reader's label-line length and chunk atom count, and the per-atom section of read_restart itself. Guards on broadcast data are placed after the broadcast so all ranks take the error path collectively. fix bond/react map files: template atom IDs were checked against the upper bound only, so zero or negative IDs in a hand-edited map file wrote out of bounds; add the lower bounds, a missing sscanf return check, and a range check on the constraint count. Coverity CIDs 1661508, 1661577, 1661546, 1662928, 1662591, 1661650, 1663159, 1661659, 1661596, 1662805, 1662556, 1660975, 1661125, 1661707, 1661116, 1662541, 1663011, 1663348, 1663285, 1663253, 1663022, 1661793, 1662734, 1661685, 1661527, 1661593, 1661079, 1662143, 1661625, 1661169, 1662250, 1662573, 1662683, 1661056, 1661054, 1661570, 1662274. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YNvuaNpJNkbCKaAE8ngPQv
26 scenes: acolor with wildcard color cycling, custom color definitions (RGB triplet and hex), adiam type ranges, color maps in all four styles (ca/cf/da/sf) on the atom charge plus a custom bond value map (bmap), per-type bond colors and diameters, atom transparency as a water shell around the peptide with and without FSAA, mixed-transparency bonds (regression test for the half-bond transparency fix), bond transparency (btrans), background gradient (backcolor2), light settings, up vector, FSAA-only, box/axes/subbox line transparency, ellipsoid rendering as triangle mesh and wireframe, body particle rendering (faces, wireframe, both), and region plane/sphere/open-face-block drawing. Notes added to the README: btrans only applies when bonds are not colored by atom (with 'bond atom' the halves follow atrans); continuous color maps require literal min/max first/last entries and discrete maps a final catch-all bin.
When bonds are colored by atom, each bond is drawn as two half cylinders so that each half can follow the color of its atom. The second half was drawn with the color of its atom but the transparency (dump_modify atrans) of the *other* atom, so the two halves of a bond between an opaque and a transparent atom were rendered inconsistently. Apply color and transparency from the same atom, as the autobond drawing code already does.
8 scenes: the frame, filled (with open faces), transparent, and points draw styles across six region geometries (block, sphere, ellipsoid, cylinder, cone, prism), union and intersect regions drawn via the points style, and dynamic regions - the move and rotate keywords driven by equal-style variables and variable-driven radii - rendered after a 100-step run so that the compared image shows the displaced, rotated, or grown region (which also exercises comparing the last frame of a multi-frame dump).
The test_min_style tester sets up the fourmol molecular test system like the fix-timestep tester, applies min_style, min_modify, and optional fix commands from the post_commands block of the test YAML file, and runs a minimization with a fixed iteration budget (100 iterations, tolerances zero) so that the reference data is deterministic. It compares the potential energy before and after the minimization, the total force norm, all six box dimensions and tilt factors (so changes from fix box/relax are captured and an unchanged box is verified everywhere else), and the per-atom positions. It also checks that the minimization lowered the energy. The tester doubles as the reference generator (-u/-g). 12 tests: cg with the quadratic (default), backtrack, and forcezero line searches and a dmax setting, sd, hftn, quickmin, fire with default settings, verlet integrator, and in abcfire mode, and fix box/relax in iso and aniso mode with min_style cg.
The test_output_style tester runs a test system (the fourmol molecular system or the in.metal template) for 10 MD steps and then collects and compares all output data provided by the compute or fix with the ID 'test' defined in the post_commands block of the test YAML file. It introspects the style's output flags and handles global scalars and vectors (existing YAML keys), global arrays including variable-size per-chunk arrays, per-atom vectors and arrays, and local vectors and arrays (three new additive keys in the shared test configuration: global_array, peratom_data with the atom tag as first column, and local_data). The driver adds its own nve integrator - unless the test defines a time-integrating fix - so that history-dependent styles (msd, vacf, ave/time, ave/atom) report real data. The tester doubles as the reference generator (-u/-g). 23 initial tests covering all output categories: gyration, com, dipole, msd, vacf, reduce, slice, temp/profile, rdf, com/chunk, coord/atom, ke/atom, displace/atom, property/atom, property/local, bond/local, angle/local, pair/local, cna/atom, centro/atom, orientorder/atom, and the fixes ave/time and ave/atom through the same driver. Not yet supported: computes requiring per-atom energy or virial tallies during the run (pe/atom, stress/atom); these need consumption during the run or additional driver support.
Broad sweep over compute styles using the existing test systems: - the chunk family (chunk/atom, gyration/angmom/torque/vcm/omega/inertia/ dipole/msd/temp/property/reduce/com chunk computes, chunk/spread/atom, global/atom) using per-molecule chunks - temperature/pressure/energy variants (temp, temp/com, temp/partial, temp/deform, temp/ramp, temp/region, ke, pe, pressure, pair, group/group, momentum, msd/nongauss, gyration/shape, viscosity/cos) - connectivity (cluster/atom, fragment/atom, aggregate/atom) and bonded data (dihedral/local, improper/local, hbond/local) - profile computes (adf, stress/mop, stress/mop/profile, stress/cartesian) - the TALLY package computes (pe, pe/mol, force, stress, heat/flux) whose callbacks accumulate during the run - rigid body computes (rigid/local, erotate/rigid, ke/rigid via fix rigid/small, using the driver's integrator detection) - lattice analysis on the metal system (ackland, basal, entropy, hexorder, sna/atom, voronoi) and diffraction (xrd, saed) - aspherical particle computes (temp/asphere, erotate/asphere) on the ellipsoid system - reporting fixes (ave/chunk, ave/histo, ave/histo/weight, ave/correlate, numdiff, numdiff/virial) The driver now also supports variable-size global arrays for fixes (e.g. fix ave/chunk). Dropped as unsuitable for the current driver: compute angle/bond/dihedral/improper (require hybrid bonded styles).
restore_lighting() called std::clamp(0.0, 1.0, value) instead of std::clamp(value, 0.0, 1.0). Since the 'high' bound is then smaller than the 'low' bound, the behavior is undefined by the C++ standard and actually differs between standard library implementations: with GNU libstdc++ the swapped call happens to return the intended value, but with libc++ (macOS) and MSVC (Windows) all light intensities were set to 1.0 whenever dump_modify lights or the internal lighting save/restore around flat-surface drawing was used, producing visibly different renderings. Found by the dump-modify-lights unit test failing on the macOS and Windows CI while passing on Linux. Reference data is unaffected since it was generated on Linux where the swapped call produced the intended values.
restore_lighting() called std::clamp(0.0, 1.0, value) instead of std::clamp(value, 0.0, 1.0). Since the 'high' bound is then smaller than the 'low' bound, the behavior is undefined by the C++ standard and actually differs between standard library implementations: with GNU libstdc++ the swapped call happens to return the intended value, but with libc++ (macOS) and MSVC (Windows) all light intensities were set to 1.0 whenever dump_modify lights or the internal lighting save/restore around flat-surface drawing was used, producing visibly different renderings. Found by the dump-modify-lights unit test failing on the macOS and Windows CI while passing on Linux. Reference data is unaffected since it was generated on Linux where the swapped call produced the intended values.
Replace the std::stringstream, sscanf(), atof(), and atoi() based parsing in TestConfigReader with the Tokenizer and ValueTokenizer classes and utils::split_words(), and the stringstream in TestConfig::tags_line() with plain string concatenation. The repeated per-atom and scalar parsing code is consolidated into shared helper functions. Unlike sscanf(), malformed fields now abort with a clear message instead of silently producing garbage reference data, and atom tags are checked against the natoms range. Verified: full force-styles test suite passes and regenerating a yaml file with TEST_ARGS=-u remains idempotent. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RZmFEmpx4SmV2vaUWBF4Yk
Convert the parsing of the global_array, peratom_data, and local_data reference blocks added for the output-style tester from stringstreams to the Tokenizer/ValueTokenizer idiom of the preceding refactor, using a static helper function like the other block parsers. Malformed fields now abort with a clear message like everywhere else. Verified: all 487 force-styles tests pass and regenerating reference data remains idempotent.
The Windows CI runs for the minimizer tests showed that the state after a fixed-iteration minimization is not reproducible across compilers and math libraries: the line search logic branches on floating-point comparisons, so last-bit force differences send the descent along a different but equally valid path. All paths still land in the same basin, so the final potential energies agree to about 1e-4 while the per-atom positions and the force norm can differ by large amounts (up to 92% for hftn positions, 91% for the quickmin force norm). Thus compare only observables that are stable against such path changes: keep the energy and box dimension checks with the per-file epsilon loosened to 1e-3 (10x the largest observed cross-platform drift, still well below the differences between minimizer styles), turn the force norm check into an upper bound (10x the reference), and drop the per-atom position references entirely. Corrupted-reference checks confirm that broken minimizers are still detected.
Add the global_array, peratom_data, and local_data reference keys and oneOf branches for the test_min_style and test_output_style programs. Also catch up with previously missing keys: timestep and the magnetic reference data of the SPIN tests (init_mag_forces, run_mag_forces, run_spin), and no longer require input_coeffs for test_fix_timestep (the ILVES references use a self-contained input template). All 714 YAML reference files now validate.
Describe the two new test programs in the force-styles section of Developer_unittest.rst: what they set up, what they compare and why (including the portability considerations of the minimizer tests), and how reference files are generated. Update the configuration key table with the new and newly shared keys, fix the copy-and-paste error in the manybody pair style test category, and note that the granular tests require the GRANULAR package.
Add a section on the unittest/graphics test suite to Developer_unittest.rst: how scenes are described by the YAML files, how the sampled pixel data (row/column mean RGB projections and pixel block samples) is stored in the same file and compared, why the reference data depends on the MPI rank count, and how to author and generate new tests. Two figures rendered from an actual test scene illustrate the sampling scheme and how a rendering change is detected.
Coverity Scan triage part 1: bug fixes
Coverity Scan triage part 2: defensive hardening
Coverity Scan triage part 3: regression tests
Add force-styles testers for minimizer styles and compute/fix output data
Add YAML-based test facility for dump image and GRAPHICS package rendering
Overhaul regression test reporting and add result merging tool
Collected small changes and fixes
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.
1 participant
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 : )