Skip to content

Commit dd0e511

Browse files
njzjz-botnjzjz-bot
andauthored
fix(tf): honor NVNMD tensor neighbor lists (#5845)
## Summary - parse mode-4 neighbor lists stored in NVNMD mesh tensors instead of rebuilding them from coordinates - validate the tensor layout, reject local or neighbor indices outside the current frame, and own copied neighbor rows for the duration of the op - cover both standard and mixed-type NVNMD env-mat operators, including malformed input ## Why existing tests missed this Existing NVNMD entrypoint tests only exercised self-built neighbor lists with 0-, 1-, 6-, or 7-element meshes. No NVNMD env-mat test passed a mesh larger than 16 elements, no malformed test supplied a nonnegative index beyond `nall`, and no caller-provided list intentionally differed from the distance-built list. In normal inference paths those two lists are usually identical, so rebuilding the list produced plausible output and hid the bug. ## Validation - `cmake --build source/build --target deepmd_op -j2` - fresh build-tree TensorFlow op: `TestOpProdEnvMatNvnmdTensorNlist` (5 passed), including a positive out-of-range neighbor index - `ruff format .` - `ruff check .` - `git diff --check` Fixes #5652 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 * **New Features** * Added support for using caller-supplied neighbor lists embedded in mesh tensors for NVNMD environment-matrix operations (standard and mixed-type). * **Bug Fixes** * Strengthened input validation for mesh-embedded neighbor data, including truncation, out-of-range neighbor/local indices, duplicate local indices, invalid neighbor counts, and neighbor-count overflow beyond the provided payload. * **Tests** * Added new tests covering correct consumption of tensor neighbor lists and the above failure scenarios for both operation variants. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: njzjz-bot <njzjz.bot@gmail.com>
1 parent 2f419f4 commit dd0e511

2 files changed

Lines changed: 255 additions & 59 deletions

File tree

source/op/tf/prod_env_mat_multi_device_nvnmd.cc

Lines changed: 142 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ date: 2021-12-6
1616
1717
*/
1818

19+
#include <cstring>
20+
1921
#include "coord.h"
2022
#include "custom_op.h"
2123
#include "errors.h"
@@ -112,28 +114,29 @@ static void _map_nei_info_cpu(int* nlist,
112114
const bool& b_nlist_map);
113115

114116
template <typename FPTYPE>
115-
static void _prepare_coord_nlist_cpu(OpKernelContext* context,
116-
FPTYPE const** coord,
117-
std::vector<FPTYPE>& coord_cpy,
118-
int const** type,
119-
std::vector<int>& type_cpy,
120-
std::vector<int>& idx_mapping,
121-
deepmd::InputNlist& inlist,
122-
std::vector<int>& ilist,
123-
std::vector<int>& numneigh,
124-
std::vector<int*>& firstneigh,
125-
std::vector<std::vector<int>>& jlist,
126-
int& new_nall,
127-
int& mem_cpy,
128-
int& mem_nnei,
129-
int& max_nbor_size,
130-
const FPTYPE* box,
131-
const int* mesh_tensor_data,
132-
const int& nloc,
133-
const int& nei_mode,
134-
const float& rcut_r,
135-
const int& max_cpy_trial,
136-
const int& max_nnei_trial);
117+
static tensorflow::Status _prepare_coord_nlist_cpu(
118+
FPTYPE const** coord,
119+
std::vector<FPTYPE>& coord_cpy,
120+
int const** type,
121+
std::vector<int>& type_cpy,
122+
std::vector<int>& idx_mapping,
123+
deepmd::InputNlist& inlist,
124+
std::vector<int>& ilist,
125+
std::vector<int>& numneigh,
126+
std::vector<int*>& firstneigh,
127+
std::vector<std::vector<int>>& jlist,
128+
int& new_nall,
129+
int& mem_cpy,
130+
int& mem_nnei,
131+
int& max_nbor_size,
132+
const FPTYPE* box,
133+
const int* mesh_tensor_data,
134+
const int_64 mesh_tensor_size,
135+
const int& nloc,
136+
const int& nei_mode,
137+
const float& rcut_r,
138+
const int& max_cpy_trial,
139+
const int& max_nnei_trial);
137140

138141
// instance of function
139142

@@ -228,58 +231,132 @@ static void _map_nei_info_cpu(int* nlist,
228231
ntypes, b_nlist_map);
229232
}
230233

234+
static tensorflow::Status _prepare_mesh_nlist_cpu(
235+
deepmd::InputNlist& inlist,
236+
std::vector<int>& ilist,
237+
std::vector<int>& numneigh,
238+
std::vector<int*>& firstneigh,
239+
std::vector<std::vector<int>>& jlist,
240+
int& max_nbor_size,
241+
const int* mesh_tensor_data,
242+
const int_64 mesh_tensor_size,
243+
const int nloc,
244+
const int nall) {
245+
// Tensor-stored lists use a 16-int header followed by ilist, numneigh,
246+
// and the concatenated neighbor rows. Validate every offset before copying
247+
// because this input can be constructed directly by callers.
248+
const int_64 header_size = 16 + static_cast<int_64>(2) * nloc;
249+
if (mesh_tensor_size < header_size) {
250+
return errors::InvalidArgument("invalid mesh tensor");
251+
}
252+
253+
const int* ilist_in = mesh_tensor_data + 16;
254+
const int* numneigh_in = mesh_tensor_data + 16 + nloc;
255+
const int* neighbors_in = mesh_tensor_data + header_size;
256+
std::vector<int_64> neighbor_offset(nloc + 1, 0);
257+
int supplied_max_nbor_size = 0;
258+
for (int ii = 0; ii < nloc; ++ii) {
259+
const int_64 neighbor_count = numneigh_in[ii];
260+
if (neighbor_count < 0 ||
261+
neighbor_offset[ii] > mesh_tensor_size - header_size - neighbor_count) {
262+
return errors::InvalidArgument("invalid mesh tensor");
263+
}
264+
neighbor_offset[ii + 1] = neighbor_offset[ii] + neighbor_count;
265+
supplied_max_nbor_size = std::max(supplied_max_nbor_size, numneigh_in[ii]);
266+
}
267+
268+
std::vector<unsigned char> seen(static_cast<size_t>(nloc), 0);
269+
for (int ii = 0; ii < nloc; ++ii) {
270+
const int i_idx = ilist_in[ii];
271+
if (i_idx < 0 || i_idx >= nloc || seen[static_cast<size_t>(i_idx)]) {
272+
return errors::InvalidArgument("invalid mesh tensor");
273+
}
274+
seen[static_cast<size_t>(i_idx)] = 1;
275+
for (int_64 jj = neighbor_offset[ii]; jj < neighbor_offset[ii + 1]; ++jj) {
276+
// Downstream environment-matrix kernels index both coordinates and
277+
// atom types with this value, so accept only live frame atoms.
278+
if (neighbors_in[jj] < 0 || neighbors_in[jj] >= nall) {
279+
return errors::InvalidArgument("invalid mesh tensor");
280+
}
281+
}
282+
}
283+
284+
for (int ii = 0; ii < nloc; ++ii) {
285+
ilist[ii] = ilist_in[ii];
286+
numneigh[ii] = numneigh_in[ii];
287+
jlist[ii].assign(neighbors_in + neighbor_offset[ii],
288+
neighbors_in + neighbor_offset[ii + 1]);
289+
firstneigh[ii] = jlist[ii].data();
290+
}
291+
inlist = deepmd::InputNlist(nloc, ilist.data(), numneigh.data(),
292+
firstneigh.data());
293+
max_nbor_size = std::max(max_nbor_size, supplied_max_nbor_size);
294+
return tensorflow::Status();
295+
}
296+
231297
template <typename FPTYPE>
232-
static void _prepare_coord_nlist_cpu(OpKernelContext* context,
233-
FPTYPE const** coord,
234-
std::vector<FPTYPE>& coord_cpy,
235-
int const** type,
236-
std::vector<int>& type_cpy,
237-
std::vector<int>& idx_mapping,
238-
deepmd::InputNlist& inlist,
239-
std::vector<int>& ilist,
240-
std::vector<int>& numneigh,
241-
std::vector<int*>& firstneigh,
242-
std::vector<std::vector<int>>& jlist,
243-
int& new_nall,
244-
int& mem_cpy,
245-
int& mem_nnei,
246-
int& max_nbor_size,
247-
const FPTYPE* box,
248-
const int* mesh_tensor_data,
249-
const int& nloc,
250-
const int& nei_mode,
251-
const float& rcut_r,
252-
const int& max_cpy_trial,
253-
const int& max_nnei_trial) {
298+
static tensorflow::Status _prepare_coord_nlist_cpu(
299+
FPTYPE const** coord,
300+
std::vector<FPTYPE>& coord_cpy,
301+
int const** type,
302+
std::vector<int>& type_cpy,
303+
std::vector<int>& idx_mapping,
304+
deepmd::InputNlist& inlist,
305+
std::vector<int>& ilist,
306+
std::vector<int>& numneigh,
307+
std::vector<int*>& firstneigh,
308+
std::vector<std::vector<int>>& jlist,
309+
int& new_nall,
310+
int& mem_cpy,
311+
int& mem_nnei,
312+
int& max_nbor_size,
313+
const FPTYPE* box,
314+
const int* mesh_tensor_data,
315+
const int_64 mesh_tensor_size,
316+
const int& nloc,
317+
const int& nei_mode,
318+
const float& rcut_r,
319+
const int& max_cpy_trial,
320+
const int& max_nnei_trial) {
254321
inlist.inum = nloc;
255-
if (nei_mode != 3) {
322+
if (nei_mode != 3 && nei_mode != 4) {
256323
// build nlist by myself
257324
// normalize and copy coord
258325
if (nei_mode == 1) {
259326
int copy_ok = _norm_copy_coord_cpu(coord_cpy, type_cpy, idx_mapping,
260327
new_nall, mem_cpy, *coord, box, *type,
261328
nloc, max_cpy_trial, rcut_r);
262-
OP_REQUIRES(context, copy_ok,
263-
errors::Aborted("cannot allocate mem for copied coords"));
329+
if (!copy_ok) {
330+
return errors::Aborted("cannot allocate mem for copied coords");
331+
}
264332
*coord = &coord_cpy[0];
265333
*type = &type_cpy[0];
266334
}
267335
// build nlist
268336
int build_ok = _build_nlist_cpu(ilist, numneigh, firstneigh, jlist,
269337
max_nbor_size, mem_nnei, *coord, nloc,
270338
new_nall, max_nnei_trial, rcut_r);
271-
OP_REQUIRES(context, build_ok,
272-
errors::Aborted("cannot allocate mem for nlist"));
339+
if (!build_ok) {
340+
return errors::Aborted("cannot allocate mem for nlist");
341+
}
273342
inlist.ilist = &ilist[0];
274343
inlist.numneigh = &numneigh[0];
275344
inlist.firstneigh = &firstneigh[0];
345+
} else if (nei_mode == 4) {
346+
tensorflow::Status status = _prepare_mesh_nlist_cpu(
347+
inlist, ilist, numneigh, firstneigh, jlist, max_nbor_size,
348+
mesh_tensor_data, mesh_tensor_size, nloc, new_nall);
349+
if (!status.ok()) {
350+
return status;
351+
}
276352
} else {
277353
// copy pointers to nlist data
278354
memcpy(&inlist.ilist, 4 + mesh_tensor_data, sizeof(int*));
279355
memcpy(&inlist.numneigh, 8 + mesh_tensor_data, sizeof(int*));
280356
memcpy(&inlist.firstneigh, 12 + mesh_tensor_data, sizeof(int**));
281357
max_nbor_size = max_numneigh(inlist);
282358
}
359+
return tensorflow::Status();
283360
}
284361

285362
/*
@@ -506,11 +583,14 @@ class ProdEnvMatANvnmdQuantizeOp : public OpKernel {
506583
std::vector<int> type_cpy;
507584
int frame_nall = nall;
508585
// prepare coord and nlist
509-
_prepare_coord_nlist_cpu<FPTYPE>(
510-
context, &coord, coord_cpy, &type, type_cpy, idx_mapping, inlist,
511-
ilist, numneigh, firstneigh, jlist, frame_nall, mem_cpy, mem_nnei,
512-
max_nbor_size, box, mesh_tensor.flat<int>().data(), nloc, nei_mode,
513-
rcut_r, max_cpy_trial, max_nnei_trial);
586+
OP_REQUIRES_OK(
587+
context,
588+
_prepare_coord_nlist_cpu<FPTYPE>(
589+
&coord, coord_cpy, &type, type_cpy, idx_mapping, inlist, ilist,
590+
numneigh, firstneigh, jlist, frame_nall, mem_cpy, mem_nnei,
591+
max_nbor_size, box, mesh_tensor.flat<int>().data(),
592+
mesh_tensor.NumElements(), nloc, nei_mode, rcut_r,
593+
max_cpy_trial, max_nnei_trial));
514594
// launch the cpu compute function
515595
deepmd::prod_env_mat_a_nvnmd_quantize_cpu(
516596
em, em_deriv, rij, nlist, coord, type, inlist, max_nbor_size, avg,
@@ -789,11 +869,14 @@ class ProdEnvMatAMixNvnmdQuantizeOp : public OpKernel {
789869
std::vector<int> type_cpy;
790870
int frame_nall = nall;
791871
// prepare coord and nlist
792-
_prepare_coord_nlist_cpu<FPTYPE>(
793-
context, &coord, coord_cpy, &f_type, type_cpy, idx_mapping, inlist,
794-
ilist, numneigh, firstneigh, jlist, frame_nall, mem_cpy, mem_nnei,
795-
max_nbor_size, box, mesh_tensor.flat<int>().data(), nloc, nei_mode,
796-
rcut_r, max_cpy_trial, max_nnei_trial);
872+
OP_REQUIRES_OK(
873+
context,
874+
_prepare_coord_nlist_cpu<FPTYPE>(
875+
&coord, coord_cpy, &f_type, type_cpy, idx_mapping, inlist,
876+
ilist, numneigh, firstneigh, jlist, frame_nall, mem_cpy,
877+
mem_nnei, max_nbor_size, box, mesh_tensor.flat<int>().data(),
878+
mesh_tensor.NumElements(), nloc, nei_mode, rcut_r,
879+
max_cpy_trial, max_nnei_trial));
797880
// launch the cpu compute function
798881
deepmd::prod_env_mat_a_nvnmd_quantize_cpu(
799882
em, em_deriv, rij, nlist, coord, type, inlist, max_nbor_size, avg,

source/tests/tf/test_nvnmd_op.py

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -421,5 +421,118 @@ def test_op(self) -> None:
421421
tf.reset_default_graph()
422422

423423

424+
class TestOpProdEnvMatNvnmdTensorNlist(tf.test.TestCase):
425+
"""Verify NVNMD env-mat ops honor neighbor lists stored in mesh tensors."""
426+
427+
def setUp(self) -> None:
428+
self.coord = np.array([[0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 1.0, 0.0]])
429+
self.atype = np.array([[0, 0, 0]], dtype=np.int32)
430+
self.natoms = np.array([3, 3, 3], dtype=np.int32)
431+
self.box = np.array([[10.0, 0.0, 0.0, 0.0, 10.0, 0.0, 0.0, 0.0, 10.0]])
432+
self.sel = [2]
433+
self.ndescrpt = 4 * sum(self.sel)
434+
self.avg = np.zeros((1, self.ndescrpt))
435+
self.std = np.ones((1, self.ndescrpt))
436+
437+
# All atoms are mutually within rcut, but this caller-provided list
438+
# deliberately retains only one cyclic neighbor per local atom.
439+
header = np.zeros(16, dtype=np.int32)
440+
ilist = np.array([0, 1, 2], dtype=np.int32)
441+
numneigh = np.ones(3, dtype=np.int32)
442+
jlist = np.array([2, 0, 1], dtype=np.int32)
443+
self.mesh = np.concatenate((header, ilist, numneigh, jlist))
444+
self.expected_nlist = np.array([[2, -1, 0, -1, 1, -1]], dtype=np.int32)
445+
446+
def _run_op(self, mixed_types: bool, mesh: np.ndarray | None = None) -> np.ndarray:
447+
op = (
448+
op_module.prod_env_mat_a_mix_nvnmd_quantize
449+
if mixed_types
450+
else op_module.prod_env_mat_a_nvnmd_quantize
451+
)
452+
outputs = op(
453+
tf.constant(self.coord, dtype=tf.float64),
454+
tf.constant(self.atype),
455+
tf.constant(self.natoms),
456+
tf.constant(self.box, dtype=tf.float64),
457+
tf.constant(self.mesh if mesh is None else mesh),
458+
tf.constant(self.avg, dtype=tf.float64),
459+
tf.constant(self.std, dtype=tf.float64),
460+
rcut_a=-1.0,
461+
rcut_r=3.0,
462+
rcut_r_smth=0.5,
463+
sel_a=self.sel,
464+
sel_r=[0],
465+
)
466+
with self.cached_session() as sess:
467+
return sess.run(outputs[3])
468+
469+
def test_standard_op_uses_tensor_neighbor_list(self) -> None:
470+
np.testing.assert_array_equal(self._run_op(False), self.expected_nlist)
471+
472+
def test_mixed_type_op_uses_tensor_neighbor_list(self) -> None:
473+
np.testing.assert_array_equal(self._run_op(True), self.expected_nlist)
474+
475+
def test_rejects_truncated_tensor_neighbor_list(self) -> None:
476+
"""Reject mode-4 tensors before reading an incomplete list header."""
477+
with self.assertRaisesRegex(
478+
tf.errors.InvalidArgumentError, "invalid mesh tensor"
479+
):
480+
self._run_op(False, self.mesh[:17])
481+
482+
def test_rejects_out_of_range_tensor_neighbor_index(self) -> None:
483+
"""Reject neighbors that would index beyond this frame's atoms."""
484+
invalid_mesh = self.mesh.copy()
485+
invalid_mesh[-1] = self.natoms[1]
486+
with self.assertRaisesRegex(
487+
tf.errors.InvalidArgumentError, "invalid mesh tensor"
488+
):
489+
self._run_op(False, invalid_mesh)
490+
491+
def test_rejects_duplicate_tensor_local_index(self) -> None:
492+
"""Each local atom must occur exactly once in the caller ilist."""
493+
invalid_mesh = self.mesh.copy()
494+
invalid_mesh[17] = invalid_mesh[16]
495+
with self.assertRaisesRegex(
496+
tf.errors.InvalidArgumentError, "invalid mesh tensor"
497+
):
498+
self._run_op(False, invalid_mesh)
499+
500+
def test_rejects_out_of_range_tensor_local_index(self) -> None:
501+
"""Reject ilist rows that do not identify a live local atom."""
502+
invalid_mesh = self.mesh.copy()
503+
invalid_mesh[16] = self.natoms[0]
504+
with self.assertRaisesRegex(
505+
tf.errors.InvalidArgumentError, "invalid mesh tensor"
506+
):
507+
self._run_op(False, invalid_mesh)
508+
509+
def test_rejects_negative_tensor_neighbor_count(self) -> None:
510+
"""A negative row length must never reach neighbor-row copying."""
511+
invalid_mesh = self.mesh.copy()
512+
invalid_mesh[16 + self.natoms[0]] = -1
513+
with self.assertRaisesRegex(
514+
tf.errors.InvalidArgumentError, "invalid mesh tensor"
515+
):
516+
self._run_op(False, invalid_mesh)
517+
518+
def test_rejects_tensor_neighbor_count_beyond_payload(self) -> None:
519+
"""Declared row lengths must fit in the remaining mesh payload."""
520+
invalid_mesh = self.mesh.copy()
521+
invalid_mesh[16 + self.natoms[0]] += 1
522+
with self.assertRaisesRegex(
523+
tf.errors.InvalidArgumentError, "invalid mesh tensor"
524+
):
525+
self._run_op(False, invalid_mesh)
526+
527+
def test_mixed_type_op_rejects_invalid_tensor_neighbor_list(self) -> None:
528+
"""The mixed-type kernel must use the same validated tensor path."""
529+
invalid_mesh = self.mesh.copy()
530+
invalid_mesh[-1] = self.natoms[1]
531+
with self.assertRaisesRegex(
532+
tf.errors.InvalidArgumentError, "invalid mesh tensor"
533+
):
534+
self._run_op(True, invalid_mesh)
535+
536+
424537
if __name__ == "__main__":
425538
unittest.main()

0 commit comments

Comments
 (0)