From 6c92c5afd53549b1894972bc66eabdde1e5bdd50 Mon Sep 17 00:00:00 2001 From: njzjz-bot Date: Fri, 17 Jul 2026 03:53:03 +0800 Subject: [PATCH 1/2] fix(lammps): validate dplr and spin options Reject truncated or keyword-valued DPLR and model-deviation options before reading them. Replace the type_associate assertion with a normal LAMMPS input error and use strict numeric conversion for spin parser values. Coding-Agent: Codex Codex-Version: codex-cli 0.144.4 Model: gpt-5.6-sol Reasoning-Effort: xhigh --- source/lmp/fix_dplr.cpp | 25 ++- source/lmp/pair_deepmd.cpp | 31 ++- source/lmp/pair_deepspin.cpp | 31 ++- .../lmp/tests/test_lammps_option_parsers.py | 182 ++++++++++++++++++ 4 files changed, 253 insertions(+), 16 deletions(-) create mode 100644 source/lmp/tests/test_lammps_option_parsers.py diff --git a/source/lmp/fix_dplr.cpp b/source/lmp/fix_dplr.cpp index 524b2a22f1..2eeafb255a 100644 --- a/source/lmp/fix_dplr.cpp +++ b/source/lmp/fix_dplr.cpp @@ -80,16 +80,21 @@ FixDPLR::FixDPLR(LAMMPS* lmp, int narg, char** arg) error->all(FLERR, "Illegal fix command\nwrong number of parameters\n"); } if (string(arg[iarg]) == string("model")) { - if (iarg + 1 > narg) { - error->all(FLERR, "Illegal fix adapt command"); + if (iarg + 1 >= narg || is_key(arg[iarg + 1])) { + error->all(FLERR, "Illegal fix dplr model option, not provided"); } model = string(arg[iarg + 1]); iarg += 2; } else if (string(arg[iarg]) == string("efield")) { - if (iarg + 3 > narg) { - error->all(FLERR, - "Illegal fix adapt command, efield should be provided 3 " - "float numbers"); + // A following option keyword is not an electric-field component. Check + // all three positions before reading any of them so truncated commands + // cannot access beyond LAMMPS's argument array. + for (int ii = 1; ii <= 3; ++ii) { + if (iarg + ii >= narg || is_key(arg[iarg + ii])) { + error->all(FLERR, + "Illegal fix dplr efield option, three values are " + "required"); + } } if (utils::strmatch(arg[iarg + 1], "^v_")) { xstr = utils::strdup(arg[iarg + 1] + 2); @@ -137,8 +142,12 @@ FixDPLR::FixDPLR(LAMMPS* lmp, int narg, char** arg) break; } } - assert(map_vec.size() % 2 == 0 && - "number of ints provided by type_associate should be even"); + if (map_vec.size() % 2 != 0) { + error->all( + FLERR, + "Illegal fix dplr type_associate option, an even number of atom types " + "is required"); + } // dpt.init(model); // dtm.init("frozen_model.pb"); diff --git a/source/lmp/pair_deepmd.cpp b/source/lmp/pair_deepmd.cpp index 3ce4705e91..893f3996dd 100644 --- a/source/lmp/pair_deepmd.cpp +++ b/source/lmp/pair_deepmd.cpp @@ -25,6 +25,7 @@ #include "neighbor.h" #include "output.h" #include "update.h" +#include "utils.h" #if LAMMPS_VERSION_NUMBER >= 20210831 // in lammps #2902, fix_ttm members turns from private to protected #define USE_TTM 1 @@ -837,23 +838,45 @@ void PairDeepMD::settings(int narg, char** arg) { out_each = 1; iarg += 1; } else if (string(arg[iarg]) == string("relative")) { + if (iarg + 1 >= narg || is_key(arg[iarg + 1])) { + error->all(FLERR, "Illegal relative, not provided"); + } out_rel = 1; - eps = atof(arg[iarg + 1]) / ener_unit_cvt_factor; + eps = utils::numeric(FLERR, arg[iarg + 1], false, lmp) / + ener_unit_cvt_factor; iarg += 2; } else if (string(arg[iarg]) == string("relative_v")) { + if (iarg + 1 >= narg || is_key(arg[iarg + 1])) { + error->all(FLERR, "Illegal relative_v, not provided"); + } out_rel_v = 1; - eps_v = atof(arg[iarg + 1]) / ener_unit_cvt_factor; + eps_v = utils::numeric(FLERR, arg[iarg + 1], false, lmp) / + ener_unit_cvt_factor; iarg += 2; } else if (string(arg[iarg]) == string("virtual_len")) { virtual_len.resize(numb_types_spin); for (int ii = 0; ii < numb_types_spin; ++ii) { - virtual_len[ii] = atof(arg[iarg + ii + 1]); + if (iarg + ii + 1 >= narg || is_key(arg[iarg + ii + 1])) { + char tmp[1024]; + sprintf(tmp, "Illegal virtual_len, the dimension should be %d", + numb_types_spin); + error->all(FLERR, tmp); + } + virtual_len[ii] = + utils::numeric(FLERR, arg[iarg + ii + 1], false, lmp); } iarg += numb_types_spin + 1; } else if (string(arg[iarg]) == string("spin_norm")) { spin_norm.resize(numb_types_spin); for (int ii = 0; ii < numb_types_spin; ++ii) { - spin_norm[ii] = atof(arg[iarg + ii + 1]); + if (iarg + ii + 1 >= narg || is_key(arg[iarg + ii + 1])) { + char tmp[1024]; + sprintf(tmp, "Illegal spin_norm, the dimension should be %d", + numb_types_spin); + error->all(FLERR, tmp); + } + spin_norm[ii] = + utils::numeric(FLERR, arg[iarg + ii + 1], false, lmp); } iarg += numb_types_spin + 1; } diff --git a/source/lmp/pair_deepspin.cpp b/source/lmp/pair_deepspin.cpp index 4de54c9197..91260cd81e 100644 --- a/source/lmp/pair_deepspin.cpp +++ b/source/lmp/pair_deepspin.cpp @@ -23,6 +23,7 @@ #include "neighbor.h" #include "output.h" #include "update.h" +#include "utils.h" #if LAMMPS_VERSION_NUMBER >= 20210831 // in lammps #2902, fix_ttm members turns from private to protected #define USE_TTM 1 @@ -719,23 +720,45 @@ void PairDeepSpin::settings(int narg, char** arg) { out_each = 1; iarg += 1; } else if (string(arg[iarg]) == string("relative")) { + if (iarg + 1 >= narg || is_key(arg[iarg + 1])) { + error->all(FLERR, "Illegal relative, not provided"); + } out_rel = 1; - eps = atof(arg[iarg + 1]) / ener_unit_cvt_factor; + eps = utils::numeric(FLERR, arg[iarg + 1], false, lmp) / + ener_unit_cvt_factor; iarg += 2; } else if (string(arg[iarg]) == string("relative_v")) { + if (iarg + 1 >= narg || is_key(arg[iarg + 1])) { + error->all(FLERR, "Illegal relative_v, not provided"); + } out_rel_v = 1; - eps_v = atof(arg[iarg + 1]) / ener_unit_cvt_factor; + eps_v = utils::numeric(FLERR, arg[iarg + 1], false, lmp) / + ener_unit_cvt_factor; iarg += 2; } else if (string(arg[iarg]) == string("virtual_len")) { virtual_len.resize(numb_types_spin); for (int ii = 0; ii < numb_types_spin; ++ii) { - virtual_len[ii] = atof(arg[iarg + ii + 1]); + if (iarg + ii + 1 >= narg || is_key(arg[iarg + ii + 1])) { + char tmp[1024]; + sprintf(tmp, "Illegal virtual_len, the dimension should be %d", + numb_types_spin); + error->all(FLERR, tmp); + } + virtual_len[ii] = + utils::numeric(FLERR, arg[iarg + ii + 1], false, lmp); } iarg += numb_types_spin + 1; } else if (string(arg[iarg]) == string("spin_norm")) { spin_norm.resize(numb_types_spin); for (int ii = 0; ii < numb_types_spin; ++ii) { - spin_norm[ii] = atof(arg[iarg + ii + 1]); + if (iarg + ii + 1 >= narg || is_key(arg[iarg + ii + 1])) { + char tmp[1024]; + sprintf(tmp, "Illegal spin_norm, the dimension should be %d", + numb_types_spin); + error->all(FLERR, tmp); + } + spin_norm[ii] = + utils::numeric(FLERR, arg[iarg + ii + 1], false, lmp); } iarg += numb_types_spin + 1; } diff --git a/source/lmp/tests/test_lammps_option_parsers.py b/source/lmp/tests/test_lammps_option_parsers.py new file mode 100644 index 0000000000..6a9ffda706 --- /dev/null +++ b/source/lmp/tests/test_lammps_option_parsers.py @@ -0,0 +1,182 @@ +# SPDX-License-Identifier: LGPL-3.0-or-later +"""Regression tests for malformed DeePMD LAMMPS option lists. + +The existing LAMMPS tests exercise complete, valid option lists. These cases +run out of process so a future out-of-bounds parser regression fails one test +without crashing the entire pytest worker. +""" + +import os +import subprocess as sp +import sys +from pathlib import ( + Path, +) + +import pytest +from lammps_test_utils import ( + require_backend, +) +from model_convert import ( + ensure_converted_pb, +) + +pbtxt_file = ( + Path(__file__).parent.parent.parent / "tests" / "infer" / "deepspin_nlist.pbtxt" +) + +_LAMMPS_RUNNER = """ +import os +import sys + +from lammps import lammps + +lmp = lammps(cmdargs=["-log", "none", "-screen", "none"]) +try: + if plugin := os.environ.get("DEEPMD_TEST_PLUGIN"): + lmp.command(f"plugin load {plugin}") + for command in sys.argv[1:]: + lmp.command(command) +finally: + lmp.close() +""" + + +def setup_module() -> None: + require_backend("ENABLE_TENSORFLOW", "TensorFlow") + + +@pytest.fixture(scope="module") +def spin_model(tmp_path_factory: pytest.TempPathFactory) -> Path: + """Build a one-spin-type model used to size spin option value lists.""" + model = tmp_path_factory.mktemp("lammps_parser") / "deepspin.pb" + ensure_converted_pb(pbtxt_file, model) + return model + + +def _run_lammps(*commands: str) -> sp.CompletedProcess[str]: + """Isolate malformed commands that could crash an unfixed LAMMPS parser.""" + return sp.run( + [sys.executable, "-c", _LAMMPS_RUNNER, *commands], + capture_output=True, + text=True, + timeout=60, + check=False, + env=os.environ.copy(), + ) + + +def _assert_lammps_error(result: sp.CompletedProcess[str], message: str) -> None: + """Require the controlled diagnostic, not merely a crashed subprocess.""" + output = result.stdout + result.stderr + assert result.returncode != 0, output + assert f"ERROR: {message} (" in output, output + + +@pytest.mark.parametrize( + ("style", "option", "message"), + [ + ("deepmd", "relative", "Illegal relative, not provided"), + ("deepmd", "relative_v", "Illegal relative_v, not provided"), + ("deepspin", "relative", "Illegal relative, not provided"), + ("deepspin", "relative_v", "Illegal relative_v, not provided"), + ( + "deepspin", + "virtual_len", + "Illegal virtual_len, the dimension should be 1", + ), + ( + "deepspin", + "spin_norm", + "Illegal spin_norm, the dimension should be 1", + ), + ], +) +def test_pair_style_rejects_truncated_option( + spin_model: Path, style: str, option: str, message: str +) -> None: + result = _run_lammps( + "units metal", f"pair_style {style} {spin_model.resolve()} {option}" + ) + _assert_lammps_error(result, message) + + +@pytest.mark.parametrize("style", ["deepmd", "deepspin"]) +@pytest.mark.parametrize( + ("option", "message"), + [ + ("relative", "Illegal relative, not provided"), + ("relative_v", "Illegal relative_v, not provided"), + ], +) +def test_pair_style_rejects_keyword_as_relative_value( + spin_model: Path, style: str, option: str, message: str +) -> None: + result = _run_lammps( + "units metal", + f"pair_style {style} {spin_model.resolve()} {option} atomic", + ) + _assert_lammps_error(result, message) + + +@pytest.mark.parametrize( + ("option", "message"), + [ + ("virtual_len", "Illegal virtual_len, the dimension should be 1"), + ("spin_norm", "Illegal spin_norm, the dimension should be 1"), + ], +) +def test_deepspin_rejects_keyword_as_spin_value( + spin_model: Path, option: str, message: str +) -> None: + result = _run_lammps( + "units metal", + f"pair_style deepspin {spin_model.resolve()} {option} atomic", + ) + _assert_lammps_error(result, message) + + +def test_deepspin_accepts_complete_option_values(spin_model: Path) -> None: + """Keep the valid combined option path covered alongside negative cases.""" + result = _run_lammps( + "units metal", + f"pair_style deepspin {spin_model.resolve()} relative 1.0 relative_v 2.0 " + "virtual_len 0.4 spin_norm 1.2737 atomic", + ) + assert result.returncode == 0, result.stdout + result.stderr + + +@pytest.mark.parametrize( + ("fix_options", "message"), + [ + ("model", "Illegal fix dplr model option, not provided"), + ( + "model efield 0 0 0", + "Illegal fix dplr model option, not provided", + ), + ( + "model {model} efield 0 0", + "Illegal fix dplr efield option, three values are required", + ), + ( + "model {model} efield 0 bond_type 1", + "Illegal fix dplr efield option, three values are required", + ), + ( + "model {model} type_associate 1", + "Illegal fix dplr type_associate option, an even number of atom " + "types is required", + ), + ], +) +def test_fix_dplr_rejects_malformed_options( + spin_model: Path, fix_options: str, message: str +) -> None: + result = _run_lammps( + "units metal", + "atom_style full", + "region box block 0 1 0 1 0 1", + "create_box 1 box", + f"fix 0 all dplr {fix_options.format(model=spin_model.resolve())}", + ) + _assert_lammps_error(result, message) From 5f65a39029f497667685f93dcd0418ac5c0a13b7 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 16 Jul 2026 20:12:22 +0000 Subject: [PATCH 2/2] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- source/lmp/pair_deepmd.cpp | 6 ++---- source/lmp/pair_deepspin.cpp | 6 ++---- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/source/lmp/pair_deepmd.cpp b/source/lmp/pair_deepmd.cpp index 893f3996dd..61cbd5c1cc 100644 --- a/source/lmp/pair_deepmd.cpp +++ b/source/lmp/pair_deepmd.cpp @@ -862,8 +862,7 @@ void PairDeepMD::settings(int narg, char** arg) { numb_types_spin); error->all(FLERR, tmp); } - virtual_len[ii] = - utils::numeric(FLERR, arg[iarg + ii + 1], false, lmp); + virtual_len[ii] = utils::numeric(FLERR, arg[iarg + ii + 1], false, lmp); } iarg += numb_types_spin + 1; } else if (string(arg[iarg]) == string("spin_norm")) { @@ -875,8 +874,7 @@ void PairDeepMD::settings(int narg, char** arg) { numb_types_spin); error->all(FLERR, tmp); } - spin_norm[ii] = - utils::numeric(FLERR, arg[iarg + ii + 1], false, lmp); + spin_norm[ii] = utils::numeric(FLERR, arg[iarg + ii + 1], false, lmp); } iarg += numb_types_spin + 1; } diff --git a/source/lmp/pair_deepspin.cpp b/source/lmp/pair_deepspin.cpp index 91260cd81e..d6ac6d6972 100644 --- a/source/lmp/pair_deepspin.cpp +++ b/source/lmp/pair_deepspin.cpp @@ -744,8 +744,7 @@ void PairDeepSpin::settings(int narg, char** arg) { numb_types_spin); error->all(FLERR, tmp); } - virtual_len[ii] = - utils::numeric(FLERR, arg[iarg + ii + 1], false, lmp); + virtual_len[ii] = utils::numeric(FLERR, arg[iarg + ii + 1], false, lmp); } iarg += numb_types_spin + 1; } else if (string(arg[iarg]) == string("spin_norm")) { @@ -757,8 +756,7 @@ void PairDeepSpin::settings(int narg, char** arg) { numb_types_spin); error->all(FLERR, tmp); } - spin_norm[ii] = - utils::numeric(FLERR, arg[iarg + ii + 1], false, lmp); + spin_norm[ii] = utils::numeric(FLERR, arg[iarg + ii + 1], false, lmp); } iarg += numb_types_spin + 1; }