[Code scan] System slicing and first append share mutable data with the source

[njzjz]

This issue is part of a Codex global repository code scan.

System.sub_system() and the first append into an empty System can return/store shared mutable objects from the source system. Mutating the derived system can unexpectedly mutate the original, and mutating the original after an initial append can mutate the appended target.

Affected code:

dpdata/dpdata/system.py

Lines 456 to 465 in a7a50bf

	if tt.shape is not None and Axis.NFRAMES in tt.shape:
	axis_nframes = tt.shape.index(Axis.NFRAMES)
	new_shape: list[slice | np.ndarray | list] = [
	slice(None) for _ in self.data[tt.name].shape
	]
	new_shape[axis_nframes] = f_idx
	tmp.data[tt.name] = self.data[tt.name][tuple(new_shape)]
	else:
	# keep the original data
	tmp.data[tt.name] = self.data[tt.name]

dpdata/dpdata/system.py

Lines 476 to 482 in a7a50bf

	if not len(system.data["atom_numbs"]):
	# skip if the system to append is non-converged
	return False
	elif not len(self.data["atom_numbs"]):
	# this system is non-converged but the system to append is converged
	self.data = system.data.copy()
	return False

Minimal reproducer:

import numpy as np
import dpdata

data = {
    "atom_names": ["O", "H"],
    "atom_numbs": [1, 2],
    "atom_types": np.array([1, 0, 1]),
    "orig": np.zeros(3),
    "cells": np.arange(18.0).reshape(2, 3, 3) + np.eye(3),
    "coords": np.arange(18.0).reshape(2, 3, 3),
}

s = dpdata.System(data={k: (v.copy() if hasattr(v, "copy") else list(v)) for k, v in data.items()})
sub = s[:1]
sub.data["coords"][0, 0, 0] = 123
print(s.data["coords"][0, 0, 0])

src = dpdata.System(data={k: (v.copy() if hasattr(v, "copy") else list(v)) for k, v in data.items()})
tgt = dpdata.System()
tgt.append(src)
src.data["coords"][0, 0, 0] = 7
src.data["atom_names"][0] = "X"
print(tgt.data["coords"][0, 0, 0], tgt.data["atom_names"])

Current output shows aliasing:

123.0
7.0 ['X', 'H']

sub_system() should return independent data, and the initial append path should deep-copy the source data consistently with other append paths.

[dosubot]

Good catch. Looking at the code, there are two distinct aliasing paths:

1. sub_system() — non-frame data is assigned by reference [1]

For data items that don't have NFRAMES in their shape (like atom_names, atom_numbs, atom_types, orig), the code does a bare assignment:

tmp.data[tt.name] = self.data[tt.name]

Frame-based data (e.g. coords, cells) uses fancy indexing which already produces a copy, so it's only the non-frame items that alias.

2. append() into an empty system — shallow dict copy [2]

self.data = system.data.copy()

This is a shallow dictionary .copy(), so all values (NumPy arrays and lists alike) remain shared references.

The codebase already has deepcopy usage in the canonical System.copy() method [3] and in the reorder path of append() [4], so the fix is straightforward:

from copy import deepcopy

# sub_system: replace the else branch
else:
    tmp.data[tt.name] = deepcopy(self.data[tt.name])

# append: replace the empty-system branch
elif not len(self.data["atom_numbs"]):
    self.data = deepcopy(system.data)
    return False

This is consistent with the pattern established in prior fixes for similar aliasing bugs — PR #483 [4], PR #671 [5], and the MultiSystems fix in PR #30 [6].

To reply, just mention @dosu.

---

Share context across your team and agents. Try Dosu.