Handle numpy array for selective dynamics in Structure#4461
Merged
shyuep merged 14 commits intomaterialsproject:masterfrom Jul 29, 2025
Merged
Handle numpy array for selective dynamics in Structure#4461shyuep merged 14 commits intomaterialsproject:masterfrom
Structure#4461shyuep merged 14 commits intomaterialsproject:masterfrom
Conversation
3e6f369 to
5943661
Compare
DanielYang59
commented
Jul 22, 2025
| dim = value.shape | ||
| if dim[1] != 3 or dim[0] != len(self.structure): | ||
| raise ValueError(f"{name} array must be same length as the structure.") | ||
| value = value.tolist() |
Contributor
Author
There was a problem hiding this comment.
Looks like this tolist is not necessary as value for both selective_dynamics and velocities are converted to numpy array if not via __setattr__?
pymatgen/src/pymatgen/io/vasp/inputs.py
Lines 143 to 151 in f9d9fe8
8847f18 to
9065b33
Compare
398c9ad to
8015819
Compare
Member
|
Looks good to me. |
8015819 to
53de5c2
Compare
53de5c2 to
3719026
Compare
DanielYang59
commented
Jul 26, 2025
|
|
||
| elif fmt == "json" or fnmatch(filename, "*.json*") or fnmatch(filename, "*.mson*"): | ||
| json_str = orjson.dumps(self.as_dict()).decode() | ||
| json_str = orjson.dumps(self.as_dict(), option=orjson.OPT_SERIALIZE_NUMPY).decode() |
Contributor
Author
There was a problem hiding this comment.
For IMolecule, I'm a bit unsure if there's any case where a property need to be numpy array, but now that we allow arbitrary properties, I guess it's safer to turn on OPT_SERIALIZE_NUMPY by default:
pymatgen/src/pymatgen/core/structure.py
Lines 3745 to 3754 in f9d9fe8
Also as far as I could tell there's no significant performance penalty (to turn it on when there's no numpy array) unlike the case for OPT_SORT_KEYS:
import timeit
import orjson
from pymatgen.core.structure import Structure
# Create a small structure (no NumPy arrays)
structure = Structure(
lattice=[[10, 0, 0], [0, 10, 0], [0, 0, 10]],
species=["H", "O"],
coords=[[0, 0, 0], [0.5, 0.5, 0.5]],
site_properties={"selective_dynamics": [[True, True, True], [False, False, False]]}
)
structure_dict = structure.as_dict()
N = 10_000
plain_time = timeit.timeit(
stmt='orjson.dumps(structure_dict).decode()',
globals={"orjson": orjson, "structure_dict": structure_dict},
number=N
)
numpy_time = timeit.timeit(
stmt='orjson.dumps(structure_dict, option=orjson.OPT_SERIALIZE_NUMPY).decode()',
globals={"orjson": orjson, "structure_dict": structure_dict},
number=N
)
print(f"orjson (plain): {plain_time * 1000:.3f} ms for {N} runs")
print(f"orjson (+NUMPY): {numpy_time * 1000:.3f} ms for {N} runs")
print(f"Overhead ratio: {numpy_time / plain_time:.4f}x")Gives:
orjson (plain): 8.585 ms for 10000 runs
orjson (+NUMPY): 8.808 ms for 10000 runs
Overhead ratio: 1.0260x
1de3fce to
f66de36
Compare
DanielYang59
commented
Jul 26, 2025
This reverts commit 409d954.
bec7265 to
384c2f2
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Structure, to fixError: Type is not JSON serializablewhen Exporting POSCAR Structure to JSON #4460jsonandorjson, also check round trip?