Repository navigation
[BUG] dpa4spin accepts non-metal units but uses metal-unit hbar #5993
Description
Summary
The dpa4spin and dpa4spin/kk pair styles accept non-metal physical unit styles, but their magnetic-force conversion always divides by a hard-coded reduced Planck constant in metal units (eV·ps). Magnetic dynamics are therefore unit-incorrect for accepted styles such as real and si.
This is present on origin/master at 8cfd46e37448.
Evidence
The constructor rejects only units lj and explicitly tells users that real is acceptable:
if (strcmp(update->unit_style, "lj") == 0) {
// ... use a physical unit style such as metal or real.
}Both implementations then use:
constexpr double kHBar = 6.5821191e-04; // eV.psand scale the magnetic force with force_unit_cvt_factor / kHBar:
source/lmp/pair_dpa4spin.cpp:113,543-544source/lmp/pair_dpa4spin_kokkos.cpp:24,336
For example, LAMMPS real units use force->hplanck = 95.306976368 in kcal/mol·fs, so the numerator is converted while the denominator remains in eV·ps. The result is not the metal-unit result expressed in real units.
The repository's spin test helpers already state that only metal is valid, and LAMMPS's PairSpin::settings() rejects every non-metal unit style.
Expected behavior
The safest fix is to reject strcmp(update->unit_style, "metal") != 0 in dpa4spin, which also covers dpa4spin/kk through its base constructor. Alternatively, all magnetic-force dimensions would need a complete active-unit conversion based on force->hplanck / (2*pi) and the appropriate energy conversion.
A regression test should verify that units real fails at pair-style initialization with a clear error.
Coding agent: Codex
Codex version: codex-cli 0.149.0
Model: gpt-5.6-sol
Reasoning effort: xhigh
Activity
LOGO127 commentedon Sep 8, 2026
Thanks for the diagnosis. I reproduced the constructor accepting
realandsiagainst master28b7d06, using the actual compiledPairDPA4Spinclass, and prepared the metal-only guard suggested here.One integration prerequisite surfaced: the runtime plugin currently omits the host
dpa4spinregistration, so the unmodified plugin reports an unrecognized style before reaching this guard. I opened #6012 separately for that omission. This unit-policy change is kept separate.With host registration held constant, the same eight Python-binding/real-LAMMPS-command tests produce 7 failed / 1 passed without the unit guard, versus 8 passed with it. The
ljdifference is the updated diagnostic; the other six non-metal physical styles are newly rejected. The metal control reaches the expected missing-artifact-argument error, not successful inference. The registration test also passes. Tested on LAMMPSstable_22Jul2025_update6, serial CPU, with independently built C API and C++ API backend-neutral plugins; no GPU/Kokkos or magnetic-force numerical validation claimed.Would you prefer the unit fix after #6012, or a built-in-package test path independent of runtime registration? I can keep the change to the guard, explanatory hbar comment and focused tests. AI assistance was used for implementation and validation; the original diagnosis is yours.
njzjz-bot commentedon Sep 29, 2026
Please keep this unit-policy fix independent of #6012 and do not wait for the runtime-plugin registration change.
The current source still accepts every physical unit style except
lj, while the conversion uses the hard-codedkHBar = 6.5821191e-04in eV·ps. Until the full magnetic-force conversion is made unit-aware, the safe contract is therefore metal-only.A focused PR can simply:
strcmp(update->unit_style, "metal") != 0with a clear diagnostic;units real(and preferably another non-metal physical style) fails at pair-style initialization;metalcontrol that reaches the next expected validation stage.Use a built-in-package test path if that avoids depending on #6012. The plugin-registration PR and this numerical-units correctness fix should remain independently reviewable.
Agent: ChatGPT
Model: GPT-5.6 Sol