Skip to content

Commit d798a8a

Browse files
njzjznjzjz-bot
andauthored
fix(dpmodel): preserve model dictionaries during serialization (#5788)
## Summary - preserve the caller-owned model dictionary while save_dp_model() rewrites variables into YAML or HDF5 references; - copy only the nested dict and list containers that traversal mutates, so large arrays are not duplicated; - retain support for non-copyable variable objects such as h5py.Dataset; - add regressions for nested YAML/native serialization and an HDF5-backed variable. ## Why existing tests missed this The existing save/load tests pass deepcopy(self.model_dict) into save_dp_model() and then validate only the serialized round trip. That disposable copy hides mutations of the object supplied by a real caller, so the shallow-copy bug was never observed. The new tests pass a caller-owned nested structure directly, assert that its containers and variables remain unchanged, and cover both serialization implementations. The HDF5 dataset case also prevents a memory-heavy full deepcopy from becoming the fix. ## Validation - targeted serialization tests and existing save/load tests: 6 passed; - ruff format .; - ruff check .; - git diff --check. Closes #5639. Coding agent: Codex Codex version: codex-cli 0.144.1 Model: gpt-5.6-sol Reasoning effort: xhigh <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Saving models no longer alters the original nested data structures. * Preserves shared and non-copyable objects, including HDF5-backed datasets, during serialization. * Ensures saved models retain expected data when loaded across supported formats. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: njzjz-bot <njzjz-bot@users.noreply.github.com>
1 parent 1c5ff68 commit d798a8a

2 files changed

Lines changed: 93 additions & 2 deletions

File tree

deepmd/dpmodel/utils/serialization.py

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,22 @@ def traverse_model_dict(
6363
return model_obj
6464

6565

66+
def _copy_model_containers(model_obj: Any) -> Any:
67+
"""Copy only containers that :func:`traverse_model_dict` mutates.
68+
69+
Arrays and other variable objects can be large or may not support ``deepcopy``.
70+
They remain shared because traversal replaces their entries in parent containers
71+
without modifying the variable objects themselves.
72+
"""
73+
if isinstance(model_obj, dict):
74+
if model_obj.get("@is_variable", False):
75+
return model_obj
76+
return {key: _copy_model_containers(value) for key, value in model_obj.items()}
77+
if isinstance(model_obj, list):
78+
return [_copy_model_containers(value) for value in model_obj]
79+
return model_obj
80+
81+
6682
class Counter:
6783
"""A callable counter.
6884
@@ -91,9 +107,12 @@ def save_dp_model(filename: str, model_dict: dict) -> None:
91107
filename : str
92108
The filename to save to.
93109
model_dict : dict
94-
The model dict to save.
110+
The model dict to save. The caller retains ownership, and this function does
111+
not modify it.
95112
"""
96-
model_dict = model_dict.copy()
113+
# Give traversal an independent container tree while retaining references to
114+
# potentially large or non-copyable variable objects.
115+
model_dict = _copy_model_containers(model_dict)
97116
filename_extension = Path(filename).suffix
98117
extra_dict = {
99118
"software": "deepmd-kit",
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
# SPDX-License-Identifier: LGPL-3.0-or-later
2+
from copy import (
3+
deepcopy,
4+
)
5+
from pathlib import (
6+
Path,
7+
)
8+
9+
import h5py
10+
import numpy as np
11+
import pytest
12+
13+
from deepmd.dpmodel.utils.serialization import (
14+
load_dp_model,
15+
save_dp_model,
16+
)
17+
18+
19+
@pytest.mark.parametrize(
20+
"suffix",
21+
[
22+
pytest.param(".yaml", id="yaml"),
23+
pytest.param(".dp", id="native-hdf5"),
24+
],
25+
)
26+
def test_save_dp_model_preserves_nested_input(tmp_path: Path, suffix: str) -> None:
27+
"""Saving must not modify caller-owned containers or variables."""
28+
weights = np.arange(6, dtype=np.float64).reshape(2, 3)
29+
model_dict = {
30+
"model": {
31+
"layers": [
32+
{
33+
"@variables": {
34+
"weights": weights,
35+
"bias": np.array([0.25, -0.5], dtype=np.float64),
36+
}
37+
}
38+
],
39+
"metadata": {"labels": ["energy", "force"]},
40+
}
41+
}
42+
expected = deepcopy(model_dict)
43+
44+
save_dp_model(str(tmp_path / f"model{suffix}"), model_dict)
45+
46+
np.testing.assert_equal(model_dict, expected)
47+
assert model_dict["model"]["layers"][0]["@variables"]["weights"] is weights
48+
49+
50+
def test_save_dp_model_accepts_hdf5_dataset_without_mutation(tmp_path: Path) -> None:
51+
"""Native saving must preserve non-copyable HDF5 variable objects."""
52+
values = np.arange(6, dtype=np.float64).reshape(2, 3)
53+
with h5py.File(tmp_path / "variables.h5", "w") as variable_file:
54+
weights = variable_file.create_dataset("weights", data=values)
55+
variables = {"weights": weights}
56+
layer = {"@variables": variables}
57+
layers = [layer]
58+
model_dict = {"model": {"layers": layers}}
59+
60+
output = tmp_path / "model.dp"
61+
save_dp_model(str(output), model_dict)
62+
63+
assert model_dict["model"]["layers"] is layers
64+
assert layers[0] is layer
65+
assert layer["@variables"] is variables
66+
assert variables["weights"] is weights
67+
np.testing.assert_equal(weights[()], values)
68+
69+
loaded_model = load_dp_model(str(output))
70+
np.testing.assert_equal(
71+
loaded_model["model"]["layers"][0]["@variables"]["weights"], values
72+
)

0 commit comments

Comments
 (0)