Skip to content

Commit 5902263

Browse files
authored
fix(jax): materialize default fparam in cxx api (deepmodeling#5849)
Closes deepmodeling#5658. ## Summary - load and validate `get_default_fparam` when initializing a JAX SavedModel in the C++ API - materialize either caller-provided or stored frame parameters, including one-frame-to-multiframe broadcasting, in both direct and neighbor-list paths - reject inconsistent tensor data/shape combinations, cast tensor rank explicitly, and report TensorFlow C API allocation failure before accessing tensor storage - generate a JAX default-fparam fixture and extend the shared C++ regression matrix with default, distinct explicit override, float/double, neighbor-list, multiframe, and invalid-size coverage ## Why existing tests missed this The shared C++ default-fparam suite only included TorchScript and PT2 artifacts. Existing JAX C++ fixtures either had `dim_fparam == 0` or supplied frame parameters explicitly, while Python JAX inference materializes the saved default before invoking the model. Serialization coverage therefore proved that the flag/getter were exported, but never exercised C++ consumption of an omitted JAX `fparam`. The explicit regression now uses `fparam=[0.5]`, distinct from the stored default `[0.25852028]`, so it also detects an implementation that ignores caller overrides. ## Validation - `ruff format .` - `ruff check .` - `clang-format --dry-run --Werror` on all changed C++ files - TensorFlow/JAX-only C++ build of `runUnitTests_cc` and `deepmd_backend_jax`; both targets rebuilt successfully after the allocation-safety follow-up - 11 focused `DefaultFParamDeepPotTest` JAX SavedModel cases passed, covering direct and neighbor-list inference, float/double, stored defaults, distinct explicit overrides, two-frame broadcasting, metadata, and invalid sizes Coding agent: Codex Codex version: codex-cli 0.144.4 Model: gpt-5.6-sol Reasoning effort: xhigh <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Hardened tensor creation with shape validation, overflow checks, and safer zero-byte handling. * Improved JAX frame-parameter behavior, including embedded model defaults, stricter validation, and clear warnings when defaults are unavailable. * Added support for supplying per-frame or full-length frame-parameter inputs, with broadcasting across frames. * Ensures invalid frame-parameter sizes are rejected with better error reporting. * **Tests** * Expanded JAX SavedModel test coverage for overrides, broadcasting, and invalid input sizes. * Updated/added generated JAX SavedModel artifacts and corresponding expected reference outputs. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: njzjz-bot <njzjz.bot@gmail.com> Co-authored-by: njzjz-bot <njzjz-bot@users.noreply.github.com>
1 parent 033a5ca commit 5902263

5 files changed

Lines changed: 249 additions & 10 deletions

File tree

source/api_cc/include/DeepPotJAX.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,10 @@ class DeepPotJAX : public DeepPotBackend {
276276
bool do_message_passing;
277277
// has default fparam
278278
bool has_default_fparam_;
279+
// Frame parameters embedded in the exported model. The JAX SavedModel
280+
// signatures require a concrete tensor, so the C++ API materializes these
281+
// values when callers omit fparam.
282+
std::vector<double> default_fparam_;
279283
// Model-level pair-exclusion keep table (flat (ntypes+1)^2), built once in
280284
// init from the exported ``get_pair_exclude_types``. Empty => no exclusion.
281285
// The exported ``call_lower_*`` consumes a pre-excluded nlist (decision

source/api_cc/src/DeepPotJAX.cc

Lines changed: 111 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include <cstdio>
1313
#include <cstring>
1414
#include <iostream>
15+
#include <limits>
1516
#include <numeric>
1617
#include <ostream>
1718
#include <stdexcept>
@@ -439,10 +440,38 @@ inline std::vector<std::string> get_vector_string(
439440
template <typename T>
440441
inline TF_Tensor* create_tensor(const std::vector<T>& data,
441442
const std::vector<int64_t>& shape) {
443+
size_t element_count = 1;
444+
for (const int64_t dim : shape) {
445+
if (dim < 0) {
446+
throw deepmd::deepmd_exception(
447+
"Cannot create a tensor with a negative dimension.");
448+
}
449+
const size_t size_dim = static_cast<size_t>(dim);
450+
if (size_dim != 0 &&
451+
element_count > std::numeric_limits<size_t>::max() / size_dim) {
452+
throw deepmd::deepmd_exception("Tensor element count overflows size_t.");
453+
}
454+
element_count *= size_dim;
455+
}
456+
if (element_count != data.size()) {
457+
throw deepmd::deepmd_exception(
458+
"Tensor shape requires " + std::to_string(element_count) +
459+
" values, but " + std::to_string(data.size()) + " were provided.");
460+
}
461+
if (element_count > std::numeric_limits<size_t>::max() / sizeof(T)) {
462+
throw deepmd::deepmd_exception("Tensor byte size overflows size_t.");
463+
}
464+
const size_t byte_size = element_count * sizeof(T);
442465
TF_Tensor* tensor =
443-
TF_AllocateTensor(get_data_tensor_type(data), shape.data(), shape.size(),
444-
data.size() * sizeof(T));
445-
memcpy(TF_TensorData(tensor), data.data(), TF_TensorByteSize(tensor));
466+
TF_AllocateTensor(get_data_tensor_type(data), shape.data(),
467+
static_cast<int>(shape.size()), byte_size);
468+
if (tensor == nullptr) {
469+
throw deepmd::deepmd_exception(
470+
"TensorFlow failed to allocate an input tensor.");
471+
}
472+
if (byte_size != 0) {
473+
memcpy(TF_TensorData(tensor), data.data(), byte_size);
474+
}
446475
return tensor;
447476
}
448477

@@ -509,6 +538,60 @@ inline std::vector<double> make_charge_spin_input(
509538
std::to_string(nframes) + " frames).");
510539
}
511540

541+
inline std::vector<double> make_fparam_input(
542+
const std::vector<double>& fparam,
543+
const int dfparam,
544+
const int nframes,
545+
const bool has_default_fparam,
546+
const std::vector<double>& default_fparam) {
547+
if (dfparam == 0) {
548+
if (!fparam.empty()) {
549+
std::cerr << "WARNING: fparam was provided, but this model has "
550+
"dim_fparam=0. The provided fparam will be ignored."
551+
<< std::endl;
552+
}
553+
return {};
554+
}
555+
556+
const std::vector<double>* source = nullptr;
557+
if (!fparam.empty()) {
558+
source = &fparam;
559+
} else if (!default_fparam.empty()) {
560+
source = &default_fparam;
561+
} else if (has_default_fparam) {
562+
throw deepmd::deepmd_exception(
563+
"fparam was omitted, but the model's default_fparam values are not "
564+
"available. Regenerate the JAX SavedModel with an updated version of "
565+
"deepmd-kit or provide fparam explicitly.");
566+
} else {
567+
throw deepmd::deepmd_exception(
568+
"fparam is required for this model but was not provided, and no "
569+
"default_fparam is stored in the model.");
570+
}
571+
572+
const size_t dim = static_cast<size_t>(dfparam);
573+
if (static_cast<size_t>(nframes) > std::numeric_limits<size_t>::max() / dim) {
574+
throw deepmd::deepmd_exception("fparam element count overflows size_t.");
575+
}
576+
const size_t expected = static_cast<size_t>(nframes) * dim;
577+
if (source->size() == expected) {
578+
return *source;
579+
}
580+
if (source->size() == dim) {
581+
std::vector<double> result(expected);
582+
for (int ff = 0; ff < nframes; ++ff) {
583+
std::copy(source->begin(), source->end(),
584+
result.begin() + static_cast<size_t>(ff) * dim);
585+
}
586+
return result;
587+
}
588+
throw deepmd::deepmd_exception(
589+
"fparam has " + std::to_string(source->size()) +
590+
" values but the model expects dim_fparam=" + std::to_string(dfparam) +
591+
" (per frame) or " + std::to_string(expected) + " (for " +
592+
std::to_string(nframes) + " frames).");
593+
}
594+
512595
template <typename T>
513596
inline void tensor_to_vector(std::vector<T>& result,
514597
TFE_TensorHandle* retval,
@@ -675,6 +758,27 @@ void deepmd::DeepPotJAX::init(const std::string& model,
675758
} catch (tf_function_not_found& e) {
676759
has_default_fparam_ = false;
677760
}
761+
if (dfparam > 0 && has_default_fparam_) {
762+
try {
763+
default_fparam_ = get_vector<double>(ctx, "get_default_fparam",
764+
func_vector, device, status);
765+
if (static_cast<int>(default_fparam_.size()) != dfparam) {
766+
throw deepmd::deepmd_exception(
767+
"default_fparam length (" +
768+
std::to_string(default_fparam_.size()) +
769+
") does not match dim_fparam (" + std::to_string(dfparam) + ").");
770+
}
771+
} catch (tf_function_not_found& e) {
772+
default_fparam_.clear();
773+
std::cerr << "WARNING: Model has has_default_fparam=true but the "
774+
"get_default_fparam function is missing. Empty fparam "
775+
"will not be substituted. Please regenerate the JAX "
776+
"SavedModel with an updated version of deepmd-kit."
777+
<< std::endl;
778+
}
779+
} else {
780+
default_fparam_.clear();
781+
}
678782
try {
679783
// Model-level pair_exclude_types, exported flat [ti0, tj0, ti1, tj1,
680784
// ...]. Fold exclusion into the LAMMPS nlist at ingestion (decision
@@ -779,6 +883,8 @@ void deepmd::DeepPotJAX::compute(std::vector<ENERGYTYPE>& ener,
779883
std::vector<double> coord_double(coord.begin(), coord.end());
780884
std::vector<double> box_double(box.begin(), box.end());
781885
std::vector<double> fparam_double(fparam.begin(), fparam.end());
886+
fparam_double = make_fparam_input(fparam_double, dfparam, nframes,
887+
has_default_fparam_, default_fparam_);
782888
std::vector<double> aparam_double(aparam.begin(), aparam.end());
783889
std::vector<double> charge_spin_double =
784890
make_charge_spin_input(charge_spin, dchgspin, nframes, default_chg_spin_);
@@ -942,6 +1048,8 @@ void deepmd::DeepPotJAX::compute(std::vector<ENERGYTYPE>& ener,
9421048
// float model interface
9431049
std::vector<double> coord_double(coord.begin(), coord.end());
9441050
std::vector<double> fparam_double(fparam.begin(), fparam.end());
1051+
fparam_double = make_fparam_input(fparam_double, dfparam, nframes,
1052+
has_default_fparam_, default_fparam_);
9451053
std::vector<double> aparam_double(aparam.begin(), aparam.end());
9461054
std::vector<double> charge_spin_double =
9471055
make_charge_spin_input(charge_spin, dchgspin, nframes, default_chg_spin_);

source/api_cc/tests/deeppot_universal_test_common.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,11 @@ inline std::vector<FParamAParamCase> fparam_aparam_cases() {
148148
"../../tests/infer/fparam_aparam.expected", 1e-7, 1e-4,
149149
/*supports_float=*/true},
150150
{"pytorch_pt2", Backend::PTExpt, "../../tests/infer/fparam_aparam.pt2",
151+
/*convert_pbtxt=*/false, nullptr,
152+
"../../tests/infer/fparam_aparam.expected", 1e-7, 1e-4,
153+
/*supports_float=*/true},
154+
{"jax_savedmodel", Backend::JAX,
155+
"../../tests/infer/fparam_aparam.savedmodel",
151156
/*convert_pbtxt=*/false, nullptr,
152157
"../../tests/infer/fparam_aparam.expected", 1e-7, 1e-4,
153158
/*supports_float=*/true}};

source/api_cc/tests/test_deeppot_universal.cc

Lines changed: 100 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
#include <exception>
88
#include <string>
99
#include <type_traits>
10+
#include <utility>
1011
#include <vector>
1112

1213
#include "DeepPot.h"
@@ -359,6 +360,10 @@ std::vector<DefaultFParamCase> default_fparam_cases() {
359360
{"pytorch_pt2", Backend::PTExpt,
360361
"../../tests/infer/fparam_aparam_default.pt2",
361362
"../../tests/infer/fparam_aparam_default.expected", 1e-7, 1e-4,
363+
/*supports_float=*/true},
364+
{"jax_savedmodel", Backend::JAX,
365+
"../../tests/infer/fparam_aparam_default.savedmodel",
366+
"../../tests/infer/fparam_aparam_default.expected", 1e-7, 1e-4,
362367
/*supports_float=*/true}};
363368
}
364369

@@ -532,6 +537,8 @@ class DefaultFParamDeepPotTest
532537
protected:
533538
deepmd::DeepPot dp;
534539
deepmd_test::DeepPotRef ref;
540+
deepmd_test::DeepPotRef override_ref;
541+
const std::vector<double> override_fparam = {0.5};
535542

536543
void SetUp() override {
537544
const auto& param = GetParam();
@@ -543,6 +550,7 @@ class DefaultFParamDeepPotTest
543550
ASSERT_TRUE(path_exists(param.ref_path))
544551
<< "Reference artifact is not available: " << param.ref_path;
545552
ref = load_fparam_ref(param.ref_path);
553+
override_ref = load_expected_ref(param.ref_path, "override");
546554
ref.has_default_fparam = true;
547555
dp.init(param.model_path);
548556
}
@@ -1918,6 +1926,22 @@ TEST_P(FParamAParamDeepPotTest, Metadata) {
19181926
EXPECT_FALSE(dp.has_default_fparam());
19191927
}
19201928

1929+
TEST_P(FParamAParamDeepPotTest, JAXRejectsMissingRequiredFParam) {
1930+
if (GetParam().backend != Backend::JAX) {
1931+
GTEST_SKIP() << "This regression targets JAX SavedModel tensor inputs.";
1932+
}
1933+
const std::vector<double> coord = deepmd_test::deeppot_coord();
1934+
const std::vector<int> atype = deepmd_test::fparam_aparam_atype();
1935+
const std::vector<double> box = deepmd_test::deeppot_box();
1936+
const std::vector<double> aparam = deepmd_test::aparam_value();
1937+
double energy = 0.0;
1938+
std::vector<double> force, virial;
1939+
1940+
EXPECT_THROW(dp.compute(energy, force, virial, coord, atype, box,
1941+
std::vector<double>{}, aparam),
1942+
deepmd::deepmd_exception);
1943+
}
1944+
19211945
TEST_P(FParamAParamDeepPotTest, ComputeDouble) {
19221946
check_fparam_compute_atomic<double>(dp, *ref, GetParam().double_tol,
19231947
deepmd_test::fparam_value());
@@ -2007,17 +2031,17 @@ TEST_P(DefaultFParamDeepPotTest, ComputeWithEmptyFParamFloat) {
20072031
}
20082032

20092033
TEST_P(DefaultFParamDeepPotTest, ComputeWithExplicitFParamDouble) {
2010-
check_fparam_compute_simple<double>(dp, ref, GetParam().double_tol,
2011-
deepmd_test::fparam_value());
2034+
check_fparam_compute_simple<double>(dp, override_ref, GetParam().double_tol,
2035+
override_fparam);
20122036
}
20132037

20142038
TEST_P(DefaultFParamDeepPotTest, ComputeWithExplicitFParamFloat) {
20152039
if (!GetParam().supports_float) {
20162040
GTEST_SKIP() << backend_name(GetParam().backend)
20172041
<< " does not provide float inference coverage.";
20182042
}
2019-
check_fparam_compute_simple<float>(dp, ref, GetParam().float_tol,
2020-
deepmd_test::fparam_value());
2043+
check_fparam_compute_simple<float>(dp, override_ref, GetParam().float_tol,
2044+
override_fparam);
20212045
}
20222046

20232047
TEST_P(DefaultFParamDeepPotTest, LmpNlistWithEmptyFParamDouble) {
@@ -2034,6 +2058,78 @@ TEST_P(DefaultFParamDeepPotTest, LmpNlistWithEmptyFParamFloat) {
20342058
1);
20352059
}
20362060

2061+
TEST_P(DefaultFParamDeepPotTest, LmpNlistWithExplicitFParamDouble) {
2062+
check_fparam_lmp_nlist<double>(dp, override_ref, GetParam().double_tol,
2063+
override_fparam, false, 1.0, 1);
2064+
}
2065+
2066+
TEST_P(DefaultFParamDeepPotTest, LmpNlistWithExplicitFParamFloat) {
2067+
if (!GetParam().supports_float) {
2068+
GTEST_SKIP() << backend_name(GetParam().backend)
2069+
<< " does not provide float inference coverage.";
2070+
}
2071+
check_fparam_lmp_nlist<float>(dp, override_ref, GetParam().float_tol,
2072+
override_fparam, false, 1.0, 1);
2073+
}
2074+
2075+
TEST_P(DefaultFParamDeepPotTest, JAXBroadcastsFParamAcrossFrames) {
2076+
if (GetParam().backend != Backend::JAX) {
2077+
GTEST_SKIP() << "This regression targets JAX SavedModel tensor inputs.";
2078+
}
2079+
const int nframes = 2;
2080+
const std::vector<double> coord =
2081+
repeat_values(deepmd_test::deeppot_coord(), nframes);
2082+
const std::vector<int> atype = deepmd_test::fparam_aparam_atype();
2083+
const std::vector<double> box =
2084+
repeat_values(deepmd_test::deeppot_box(), nframes);
2085+
const std::vector<double> aparam =
2086+
repeat_values(deepmd_test::aparam_value(), nframes);
2087+
const std::vector<
2088+
std::pair<std::vector<double>, const deepmd_test::DeepPotRef*>>
2089+
fparam_cases = {{{}, &ref}, {override_fparam, &override_ref}};
2090+
for (const auto& [fparam, expected_ref] : fparam_cases) {
2091+
SCOPED_TRACE(fparam.empty() ? "stored default" : "explicit override");
2092+
const std::vector<double> expected_virial =
2093+
deepmd_test::total_virial(*expected_ref);
2094+
std::vector<double> energy, force, virial;
2095+
dp.compute(energy, force, virial, coord, atype, box, fparam, aparam);
2096+
2097+
ASSERT_EQ(energy.size(), static_cast<size_t>(nframes));
2098+
ASSERT_EQ(force.size(), expected_ref->force.size() * nframes);
2099+
ASSERT_EQ(virial.size(), 9U * nframes);
2100+
for (int ff = 0; ff < nframes; ++ff) {
2101+
EXPECT_NEAR(energy[ff], deepmd_test::total_energy(*expected_ref),
2102+
GetParam().double_tol);
2103+
for (size_t ii = 0; ii < expected_ref->force.size(); ++ii) {
2104+
EXPECT_NEAR(
2105+
force[static_cast<size_t>(ff) * expected_ref->force.size() + ii],
2106+
expected_ref->force[ii], GetParam().double_tol);
2107+
}
2108+
for (size_t ii = 0; ii < expected_virial.size(); ++ii) {
2109+
EXPECT_NEAR(virial[static_cast<size_t>(ff) * 9 + ii],
2110+
expected_virial[ii], GetParam().double_tol);
2111+
}
2112+
}
2113+
}
2114+
}
2115+
2116+
TEST_P(DefaultFParamDeepPotTest, JAXRejectsInvalidFParamSize) {
2117+
if (GetParam().backend != Backend::JAX) {
2118+
GTEST_SKIP() << "This regression targets JAX SavedModel tensor inputs.";
2119+
}
2120+
const std::vector<double> coord = deepmd_test::deeppot_coord();
2121+
const std::vector<int> atype = deepmd_test::fparam_aparam_atype();
2122+
const std::vector<double> box = deepmd_test::deeppot_box();
2123+
const std::vector<double> aparam = deepmd_test::aparam_value();
2124+
const std::vector<double> invalid_fparam = {0.1, 0.2};
2125+
double energy = 0.0;
2126+
std::vector<double> force, virial;
2127+
2128+
EXPECT_THROW(dp.compute(energy, force, virial, coord, atype, box,
2129+
invalid_fparam, aparam),
2130+
deepmd::deepmd_exception);
2131+
}
2132+
20372133
INSTANTIATE_TEST_SUITE_P(
20382134
AvailableBackends,
20392135
UniversalDeepPotTest,

0 commit comments

Comments
 (0)