This issue comes from a Codex global scan of deepmodeling/deepmd-kit at commit 73de44b1f94471b2e3bdb6b11f57b34d7bc791bb.
Problem
TF1 inference loses the non-PBC flag when an ASE neighbor list is supplied.
DeepEval._prepare_feed_dict detects cells is None, sets pbc = False, and then replaces cells with an identity cell so the TensorFlow feed still has a box:
|
if cells is None: |
|
pbc = False |
|
# make cells to work around the requirement of pbc |
|
cells = np.tile(np.eye(3), [nframes, 1]).reshape([nframes, 9]) # pylint: disable=no-explicit-dtype |
The normal no-neighbor-list path uses that separate pbc flag when building the default mesh:
|
# make natoms_vec and default_mesh |
|
if self.neighbor_list is None: |
|
natoms_vec = self.make_natoms_vec(atom_types) |
|
assert natoms_vec[0] == natoms |
|
mesh = make_default_mesh(pbc, not self._check_mixed_types(atom_types)) |
|
ghost_map = None |
But the ASE neighbor-list path passes the now non-None identity cells into build_neighbor_list:
|
else: |
|
if nframes > 1: |
|
raise NotImplementedError( |
|
"neighbor_list does not support multiple frames" |
|
) |
|
( |
|
natoms_vec, |
|
coords, |
|
atom_types, |
|
mesh, |
|
imap, |
|
ghost_map, |
|
) = self.build_neighbor_list( |
|
coords, |
|
cells if cells is not None else None, |
|
atom_types, |
|
imap, |
|
self.neighbor_list, |
|
) |
build_neighbor_list recomputes periodicity as cell is not None, so the open-boundary input is treated as periodic with an identity box:
|
pbc = np.repeat(cell is not None, 3) |
|
cell = cell.reshape(3, 3) |
|
positions = coords.reshape(-1, 3) |
|
neighbor_list.bothways = True |
|
neighbor_list.self_interaction = False |
|
if neighbor_list.update(pbc, cell, positions): |
|
neighbor_list.build(pbc, cell, positions) |
DeepTensor repeats the same pattern for tensor evaluation:
|
if cells is None: |
|
pbc = False |
|
cells = np.tile(np.eye(3), [nframes, 1]).reshape([nframes, 9]) # pylint: disable=no-explicit-dtype |
|
else: |
|
pbc = True |
|
cells = np.array(cells).reshape([nframes, 9]) |
|
if self.neighbor_list is None: |
|
natoms_vec = self.make_natoms_vec(atom_types, mixed_type=mixed_type) |
|
assert natoms_vec[0] == natoms |
|
mesh = make_default_mesh(pbc, mixed_type) |
|
else: |
|
if nframes > 1: |
|
raise NotImplementedError( |
|
"neighbor_list does not support multiple frames" |
|
) |
|
( |
|
natoms_vec, |
|
coords, |
|
atom_types, |
|
mesh, |
|
imap, |
|
_, |
|
) = self.build_neighbor_list( |
|
coords, |
|
cells if cells is not None else None, |
|
atom_types, |
|
imap, |
|
self.neighbor_list, |
|
) |
|
if cells is None: |
|
pbc = False |
|
cells = np.tile(np.eye(3), [nframes, 1]).reshape([nframes, 9]) # pylint: disable=no-explicit-dtype |
|
else: |
|
pbc = True |
|
cells = np.array(cells).reshape([nframes, 9]) |
|
if self.neighbor_list is None: |
|
natoms_vec = self.make_natoms_vec(atom_types, mixed_type=mixed_type) |
|
assert natoms_vec[0] == natoms |
|
mesh = make_default_mesh(pbc, mixed_type) |
|
ghost_map = None |
|
else: |
|
if nframes > 1: |
|
raise NotImplementedError( |
|
"neighbor_list does not support multiple frames" |
|
) |
|
( |
|
natoms_vec, |
|
coords, |
|
atom_types, |
|
mesh, |
|
imap, |
|
ghost_map, |
|
) = self.build_neighbor_list( |
|
coords, |
|
cells if cells is not None else None, |
|
atom_types, |
|
imap, |
|
self.neighbor_list, |
|
) |
Impact
Open-boundary TF1 inference with a user-provided ASE neighbor list can include periodic images and ghost atoms that should not exist. This changes energies, forces, tensor values, and virials for non-PBC systems.
Suggested fix
Keep the original PBC decision separate from the feed-box workaround. Pass pbc into build_neighbor_list, or pass None for cell when the original input used non-PBC and make build_neighbor_list handle that case without forcing cell.reshape(3, 3). Add a non-PBC ASE neighbor-list regression test for both DeepEval and DeepTensor.
This issue comes from a Codex global scan of
deepmodeling/deepmd-kitat commit73de44b1f94471b2e3bdb6b11f57b34d7bc791bb.Problem
TF1 inference loses the non-PBC flag when an ASE neighbor list is supplied.
DeepEval._prepare_feed_dictdetectscells is None, setspbc = False, and then replacescellswith an identity cell so the TensorFlow feed still has a box:deepmd-kit/deepmd/tf/infer/deep_eval.py
Lines 790 to 793 in 73de44b
The normal no-neighbor-list path uses that separate
pbcflag when building the default mesh:deepmd-kit/deepmd/tf/infer/deep_eval.py
Lines 847 to 852 in 73de44b
But the ASE neighbor-list path passes the now non-
Noneidentitycellsintobuild_neighbor_list:deepmd-kit/deepmd/tf/infer/deep_eval.py
Lines 853 to 871 in 73de44b
build_neighbor_listrecomputes periodicity ascell is not None, so the open-boundary input is treated as periodic with an identity box:deepmd-kit/deepmd/tf/infer/deep_eval.py
Lines 1515 to 1521 in 73de44b
DeepTensorrepeats the same pattern for tensor evaluation:deepmd-kit/deepmd/tf/infer/deep_tensor.py
Lines 199 to 204 in 73de44b
deepmd-kit/deepmd/tf/infer/deep_tensor.py
Lines 212 to 234 in 73de44b
deepmd-kit/deepmd/tf/infer/deep_tensor.py
Lines 343 to 348 in 73de44b
deepmd-kit/deepmd/tf/infer/deep_tensor.py
Lines 357 to 380 in 73de44b
Impact
Open-boundary TF1 inference with a user-provided ASE neighbor list can include periodic images and ghost atoms that should not exist. This changes energies, forces, tensor values, and virials for non-PBC systems.
Suggested fix
Keep the original PBC decision separate from the feed-box workaround. Pass
pbcintobuild_neighbor_list, or passNoneforcellwhen the original input used non-PBC and makebuild_neighbor_listhandle that case without forcingcell.reshape(3, 3). Add a non-PBC ASE neighbor-list regression test for bothDeepEvalandDeepTensor.