Repository navigation
Conversation
…evelop # Conflicts: # source/source_io/module_parameter/read_inp_sys.cpp
ABACUS-PR-BOT
left a comment
There was a problem hiding this comment.
The SNAP MD path mixes ghost and owned force units, and accepted model settings can produce inconsistent or non-finite results. Please correct the force handoff and validate or handle those model settings.
Static source and analytic review at 47543b3 against e5ad530, with separate compatibility inspection of develop 5a7e6a6. No build, tests, numerical/MPI runs or CI diagnostics were executed; the author’s reported checks were not independently verified.
Non-blocking: Correct the PR INPUT notes: snap_param_file is newly added, and snap becomes an accepted esolver_type value. The C++ metadata registrations are already present.
ABACUS-PR-BOT
left a comment
There was a problem hiding this comment.
One radial edge case and three nonblocking items remain; the existing radial, chemical-order and argument-style threads contain the follow-ups. Source inspection confirms the ghost-force conversion, pair-cutoff checks, inner switch, common cell accumulator, environment reuse, logging and angular/strain test additions.
Static review at 8fbec3f; no builds, runtime tests or CI diagnostics were run. Author-reported test results were not independently reproduced.
| for (std::size_t index = 0; index < entries_.size(); ++index) | ||
| { | ||
| const Entry& entry = entries_[index]; | ||
| if (entry.j1 == j1 && entry.m1 == m1 && entry.j2 == j2 && entry.m2 == m2 && entry.j == j && entry.m == m) |
There was a problem hiding this comment.
Non-blocking: coefficient() linearly searches entries_, and absent tuples traverse the whole vector. Both bispectrum contractions repeat these angular-index lookups across chemical channels and neighbor-coordinate derivatives. Pre-indexing admissible tuples or caching the contraction plan would avoid those repeated scans.
Reminder
Linked issue
Standalone CPU SNAP implementation; no linked issue.
What changed
Use
SNAP model parameters and chemical-channel ordering are documented in docs/advanced/md.md.
Verification
Commands run:
The Autotest command ran from /tmp/pr8235-regression/04_FF outside the sandbox with all five checked-in SNAP cases copied there. Build passed; 6/6 focused CTest tests passed; 20/20 numerical case comparisons passed. The four quality-scored changed files passed. The governance checker returned no blocking errors; its review warnings are addressed below. The three-atom case gives 54.909561 eV and Pxx -122.909021 kbar, consistent with the LAMMPS reference in its README; its force components and strain derivative are checked by the unit test. A direct UnitCell single-point run gave the same energy, force, and stress.
Governance notes