Skip to content

Commit 0416b54

Browse files
authored
fix(deepmd/hdf5): skip empty optional frame arrays on dump (#996) (#1011)
Fixes #996. The `deepmd/hdf5` writer reshapes and writes every optional frame array, so an empty optional property (e.g. `forces`/`virials` when `cal_force`/`cal_stress` is disabled) is stored as a `(nframes, 0)` dataset. On load, `to_system_data` tries to reshape that zero-width dataset back to `(nframes, natoms, 3)` and raises `ValueError: cannot reshape array of size 0`. Reproducer (fails on current `master`): ```python import dpdata, numpy as np s = dpdata.LabeledSystem(data={ "atom_names": ["H"], "atom_numbs": [1], "atom_types": np.array([0]), "cells": np.eye(3).reshape(1, 3, 3), "coords": np.zeros((1, 1, 3)), "energies": np.zeros(1), "orig": np.zeros(3), "forces": np.zeros((1, 0, 3)), }) s.to("deepmd/hdf5", "t.h5") dpdata.LabeledSystem("t.h5", fmt="deepmd/hdf5") # ValueError before this change ``` The `deepmd/raw` and `deepmd/npy` writers already skip this case; this makes the HDF5 writer consistent with them by skipping an optional array whose size is 0 while the system still has frames. Non-empty arrays are written and reloaded exactly as before. Added `TestHDF5EmptyOptionalArray` covering both the previously-failing empty round-trip and a non-empty round-trip to guard against regression. The `deepmd` test suites pass (`test_deepmd_hdf5`, `test_deepmd_raw`, `test_deepmd_comp`, `test_deepmd_mixed`) and `ruff` is clean. *Disclosure: written with AI assistance; reviewed and tested before submitting.* <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved HDF5 export/import handling for optional empty per-frame arrays, avoiding invalid data files that could fail to load. * Empty optional fields are now skipped during save, while valid data continues to round-trip correctly. * **Tests** * Added regression coverage for empty optional array handling. * Added verification that real force data is still preserved after save and reload. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent 221f254 commit 0416b54

2 files changed

Lines changed: 54 additions & 0 deletions

File tree

dpdata/formats/deepmd/hdf5.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,13 @@ def dump(
201201
for dt, prop in data_types.items():
202202
if dt in data:
203203
if prop["dump"]:
204+
if nframes > 0 and np.asarray(data[dt]).size == 0:
205+
# An optional frame property (e.g. forces/virials when
206+
# cal_force/cal_stress is disabled) may be empty while the
207+
# system still has frames. Skip it instead of writing a
208+
# (nframes, 0) dataset that cannot be reshaped on load. This
209+
# matches the behaviour of the deepmd/raw and deepmd/npy writers.
210+
continue
204211
ddata = np.reshape(data[dt], prop["shape"])
205212
if np.issubdtype(ddata.dtype, np.floating):
206213
ddata = ddata.astype(comp_prec)

tests/test_deepmd_hdf5.py

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -300,3 +300,50 @@ def test_regular_hdf5_groups_are_not_mixed(self):
300300
)
301301

302302
self.assertEqual(len(systems), 0)
303+
304+
305+
class TestHDF5EmptyOptionalArray(unittest.TestCase):
306+
"""Regression test for #996.
307+
308+
An empty optional frame array (e.g. forces when they are not computed) must not
309+
break the deepmd/hdf5 write/read round-trip. Previously the writer stored a
310+
``(nframes, 0)`` dataset that could not be reshaped on load.
311+
"""
312+
313+
def tearDown(self):
314+
if os.path.isfile("tmp.deepmd.empty.hdf5"):
315+
os.remove("tmp.deepmd.empty.hdf5")
316+
317+
@staticmethod
318+
def _make(forces):
319+
return dpdata.LabeledSystem(
320+
data={
321+
"atom_names": ["H"],
322+
"atom_numbs": [1],
323+
"atom_types": np.array([0]),
324+
"cells": np.eye(3).reshape(1, 3, 3),
325+
"coords": np.zeros((1, 1, 3)),
326+
"energies": np.zeros(1),
327+
"orig": np.zeros(3),
328+
"forces": forces,
329+
}
330+
)
331+
332+
def test_empty_forces_round_trip(self):
333+
system = self._make(np.zeros((1, 0, 3)))
334+
system.to("deepmd/hdf5", "tmp.deepmd.empty.hdf5")
335+
reloaded = dpdata.LabeledSystem("tmp.deepmd.empty.hdf5", fmt="deepmd/hdf5")
336+
# The empty optional array is skipped rather than written as (nframes, 0).
337+
self.assertNotIn("forces", reloaded.data)
338+
np.testing.assert_allclose(reloaded["coords"], system["coords"])
339+
340+
def test_real_forces_preserved(self):
341+
forces = np.arange(3, dtype=float).reshape(1, 1, 3)
342+
system = self._make(forces)
343+
system.to("deepmd/hdf5", "tmp.deepmd.empty.hdf5")
344+
reloaded = dpdata.LabeledSystem("tmp.deepmd.empty.hdf5", fmt="deepmd/hdf5")
345+
np.testing.assert_allclose(reloaded["forces"], forces)
346+
347+
348+
if __name__ == "__main__":
349+
unittest.main()

0 commit comments

Comments
 (0)