Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improve ASE traj #633

Merged
merged 96 commits into from
May 29, 2024
Merged
Show file tree
Hide file tree
Changes from 94 commits
Commits
Show all changes
96 commits
Select commit Hold shift + click to select a range
820ca84
add ASE's traj
thangckt Mar 13, 2024
4594981
Update ase.py
thangckt Mar 13, 2024
b055b88
Update ase.py
thangckt Mar 13, 2024
bde8b7b
Update ase.py
thangckt Mar 13, 2024
0377b27
Update ase.py
thangckt Mar 13, 2024
da3daa8
finish add ASE's traj support
thangckt Mar 13, 2024
70f7813
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Mar 13, 2024
65d2797
Update ase.py
thangckt Mar 13, 2024
42ff2b0
Merge branch 'tha_devel' of https://github.com/thangckt/dpdata into t…
thangckt Mar 13, 2024
7f1a398
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Mar 13, 2024
59e3247
Update ase.py
thangckt Mar 13, 2024
a381f89
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Mar 13, 2024
1585ff8
Update ase.py
thangckt Mar 14, 2024
2fefd7f
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Mar 14, 2024
793745c
Update ase.py
thangckt Mar 14, 2024
76895a7
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Mar 14, 2024
a12c374
Update ase.py
thangckt Mar 14, 2024
0a77be5
Update test_ase_traj.py
thangckt Mar 14, 2024
ea85559
u
thangckt Mar 14, 2024
9dbcbd8
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Mar 14, 2024
0b27552
Update test_ase_traj.py
thangckt Mar 14, 2024
c09894b
Merge branch 'tha_devel' of https://github.com/thangckt/dpdata into t…
thangckt Mar 14, 2024
67e7876
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Mar 14, 2024
2afdc4f
up
thangckt Mar 14, 2024
8771f8c
u
thangckt Mar 14, 2024
fe5163a
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Mar 14, 2024
d41573b
Update test_ase_traj.py
thangckt Mar 14, 2024
17fcb16
Merge branch 'tha_devel' of https://github.com/thangckt/dpdata into t…
thangckt Mar 14, 2024
e05c9ce
u
thangckt Mar 14, 2024
95939bd
Update ase.py
thangckt Mar 14, 2024
52ef114
Update ase.py
thangckt Mar 14, 2024
60212f6
Update ase.py
thangckt Mar 14, 2024
57d938e
Update test_ase_traj.py
thangckt Mar 14, 2024
3c95376
Update test_ase_traj.py
thangckt Mar 14, 2024
c3c45ef
Update ase.py
thangckt Mar 15, 2024
46171fc
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Mar 15, 2024
1efef9b
Update test_ase_traj.py
thangckt Mar 15, 2024
b669ae1
Merge branch 'tha_devel' of https://github.com/thangckt/dpdata into t…
thangckt Mar 15, 2024
33c5084
Update ase.py
thangckt Mar 15, 2024
790d4b5
u
thangckt Mar 15, 2024
f0c5c7d
u
thangckt Mar 15, 2024
551920e
u
thangckt Mar 15, 2024
d7c98f9
u
thangckt Mar 15, 2024
e0b53f7
u
thangckt Mar 15, 2024
81f402a
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Mar 15, 2024
9248833
u
thangckt Mar 15, 2024
8f4684d
u
thangckt Mar 16, 2024
eb66571
u
thangckt Mar 16, 2024
9893423
Update test_ase_traj.py
thangckt Mar 16, 2024
77e8ef4
u
thangckt Mar 17, 2024
02b4dac
u
thangckt Mar 18, 2024
45c8018
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Mar 18, 2024
71d0e70
Update ase.py
thangckt Mar 18, 2024
e3909e7
add a test for unlabeled system
njzjz Mar 19, 2024
b49a4e2
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Mar 19, 2024
4e0c083
Merge pull request #1 from deepmodeling/devel
thangckt Mar 27, 2024
0e22d30
add to_system in ASETrajFormat
thangckt Apr 2, 2024
f0febca
Merge pull request #2 from deepmodeling/devel
thangckt Apr 4, 2024
4a07590
Update ase.py
thangckt Apr 4, 2024
a65144b
Update ase.py
thangckt Apr 4, 2024
3eb842c
Merge branch 'PR' into devel
thangckt Apr 4, 2024
8918a90
Merge pull request #4 from thangckt/devel
thangckt Apr 4, 2024
6e2df4d
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Apr 4, 2024
1a322ce
Update ase.py
thangckt Apr 5, 2024
188f945
Merge branch 'PR' into devel
thangckt Apr 5, 2024
5d27076
Merge pull request #5 from thangckt/devel
thangckt Apr 5, 2024
1a466e3
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Apr 5, 2024
6ffe27f
Update ase.py
thangckt Apr 5, 2024
3ba5933
Merge pull request #6 from thangckt/devel
thangckt Apr 5, 2024
912d78c
Update ase.py
thangckt Apr 5, 2024
aeb569c
Merge pull request #7 from thangckt/devel
thangckt Apr 5, 2024
9331a04
Merge pull request #8 from deepmodeling/devel
thangckt May 2, 2024
bb4d75d
Merge pull request #9 from deepmodeling/devel
thangckt May 9, 2024
c5fb9ba
Merge branch 'devel' into devel_thang
thangckt May 19, 2024
0dadef3
Update ase.py
thangckt May 19, 2024
b5a9171
Merge pull request #11 from deepmodeling/devel
thangckt May 27, 2024
a57f6d7
Merge remote-tracking branch 'upstream/devel' into PR
thangckt May 27, 2024
4c7b3ef
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] May 27, 2024
7f424fc
Update test_ase_traj.py
thangckt May 27, 2024
b4a9d05
Merge branch 'PR' of https://github.com/thangckt/dpdata into PR
thangckt May 27, 2024
aa96933
Update test_ase_traj.py
thangckt May 27, 2024
d1f02a3
Update test_ase_traj.py
thangckt May 27, 2024
bdcb887
Update test_ase_traj.py
thangckt May 27, 2024
1c1ebdc
Update test_ase_traj.py
thangckt May 27, 2024
ca474a9
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] May 27, 2024
b304fc9
Update ase.py
thangckt May 27, 2024
7a0530b
u
thangckt May 27, 2024
1f8113e
u
thangckt May 27, 2024
14e48f8
Update test.yml
thangckt May 27, 2024
7315d77
u
thangckt May 27, 2024
be33545
u
thangckt May 27, 2024
af1c94b
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] May 27, 2024
a222e8e
Update test.yml
thangckt May 27, 2024
62fe922
Merge branch 'devel' into PR
thangckt May 27, 2024
bb0640f
Update ase.py
thangckt May 29, 2024
77fb18a
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] May 29, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 47 additions & 5 deletions dpdata/plugins/ase.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from __future__ import annotations

from typing import TYPE_CHECKING
import os
from typing import TYPE_CHECKING, Generator

import numpy as np

Expand All @@ -12,6 +13,11 @@
import ase
from ase.optimize.optimize import Optimizer

try:
from ase.io import Trajectory
except ImportError:
pass

Check warning on line 19 in dpdata/plugins/ase.py

View check run for this annotation

Codecov / codecov/patch

dpdata/plugins/ase.py#L18-L19

Added lines #L18 - L19 were not covered by tests


@Format.register("ase/structure")
class ASEStructureFormat(Format):
Expand Down Expand Up @@ -94,7 +100,7 @@
"forces": np.array([forces]),
}
try:
stress = atoms.get_stress(False)
stress = atoms.get_stress(voigt=False)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider handling PropertyNotImplementedError more explicitly.

While the current implementation passes silently if PropertyNotImplementedError is raised, it might be beneficial to log this occurrence or handle it in a way that does not silently ignore potential issues. Consider adding logging or another form of error handling.

except PropertyNotImplementedError:
pass
else:
Expand All @@ -110,7 +116,7 @@
step: int | None = None,
ase_fmt: str | None = None,
**kwargs,
) -> ase.Atoms:
) -> Generator[ase.Atoms, None, None]:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use Generator from typing instead of typing.Generator.

Python's typing module recommends using Generator directly for type hints. This aligns with modern Python practices and improves readability.

"""Convert a ASE supported file to ASE Atoms.

It will finally be converted to MultiSystems.
Expand Down Expand Up @@ -140,7 +146,7 @@
frames = ase.io.read(file_name, format=ase_fmt, index=slice(begin, end, step))
yield from frames

def to_system(self, data, **kwargs):
def to_system(self, data, **kwargs) -> list[ase.Atoms]:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update type annotations from List to list.

- def to_system(self, data, **kwargs) -> List[ase.Atoms]:
+ def to_system(self, data, **kwargs) -> list[ase.Atoms]:

- def to_labeled_system(self, data, *args, **kwargs) -> List[ase.Atoms]:
+ def to_labeled_system(self, data, *args, **kwargs) -> list[ase.Atoms]:

Using list instead of List for type annotations is recommended as of Python 3.9 for better readability and alignment with modern Python practices.

Also applies to: 162-162

Committable suggestion was skipped due low confidence.

"""Convert System to ASE Atom obj."""
from ase import Atoms

Expand All @@ -158,7 +164,7 @@

return structures

def to_labeled_system(self, data, *args, **kwargs):
def to_labeled_system(self, data, *args, **kwargs) -> list[ase.Atoms]:
"""Convert System to ASE Atoms object."""
from ase import Atoms
from ase.calculators.singlepoint import SinglePointCalculator
Expand Down Expand Up @@ -300,6 +306,42 @@

return dict_frames

def to_system(self, data, file_name: str = "confs.traj", **kwargs) -> None:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure proper file handling in to_system and to_labeled_system.

The methods to_system and to_labeled_system perform file operations such as checking if a file exists and removing it. It's crucial to ensure that these operations are safe and do not lead to unintended data loss. Consider adding checks or warnings to inform the user about the file deletion.

Also applies to: 323-323

"""Convert System to ASE Atoms object.

Parameters
----------
file_name : str
path to file
"""
if os.path.isfile(file_name):
os.remove(file_name)

list_atoms = ASEStructureFormat().to_system(data, **kwargs)
traj = Trajectory(file_name, "a")
_ = [traj.write(atom) for atom in list_atoms]
traj.close()
return
Comment on lines +317 to +321
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add unit tests for the new methods to_system and to_labeled_system.

These methods are critical for the functionality added in this PR but are not covered by tests according to the static analysis. Would you like assistance in creating these tests?

Also applies to: 328-332


def to_labeled_system(
self, data, file_name: str = "labeled_confs.traj", *args, **kwargs
) -> None:
"""Convert System to ASE Atoms object.

Parameters
----------
file_name : str
path to file
"""
if os.path.isfile(file_name):
os.remove(file_name)

list_atoms = ASEStructureFormat().to_labeled_system(data, *args, **kwargs)
traj = Trajectory(file_name, "a")
_ = [traj.write(atom) for atom in list_atoms]
traj.close()
return

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review the use of default parameters in ASEMinimizer.

The constructor of ASEMinimizer uses mutable default arguments (optimizer_kwargs). This is a common Python pitfall, as default mutable arguments can lead to unexpected behavior if the object is modified. Consider using a default value of None and then setting the value inside the method if it is None.

- optimizer_kwargs: dict = {}
+ optimizer_kwargs: dict | None = None
  if optimizer_kwargs is None:
      optimizer_kwargs = {}

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
optimizer_kwargs: dict | None = None
if optimizer_kwargs is None:
optimizer_kwargs = {}


@Driver.register("ase")
class ASEDriver(Driver):
Expand Down
24 changes: 24 additions & 0 deletions tests/test_ase_traj.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,5 +67,29 @@ def setUp(self):
self.v_places = 4


@unittest.skipIf(skip_ase, "skip ase related test. install ase to fix")
class TestASEtraj4(unittest.TestCase, CompSys, IsPBC):
def setUp(self):
self.system_1 = dpdata.System("ase_traj/MoS2", fmt="deepmd")
self.system_1.to(file_name="ase_traj/tmp.traj", fmt="ase/traj")
self.system_2 = dpdata.System("ase_traj/tmp.traj", fmt="ase/traj")
self.places = 6
self.e_places = 6
self.f_places = 6
self.v_places = 4


@unittest.skipIf(skip_ase, "skip ase related test. install ase to fix")
class TestASEtraj4Labeled(unittest.TestCase, CompLabeledSys, IsPBC):
def setUp(self):
self.system_1 = dpdata.LabeledSystem("ase_traj/MoS2", fmt="deepmd")
self.system_1.to(file_name="ase_traj/tmp1.traj", fmt="ase/traj")
self.system_2 = dpdata.LabeledSystem("ase_traj/tmp1.traj", fmt="ase/traj")
self.places = 6
self.e_places = 6
self.f_places = 6
self.v_places = 4


if __name__ == "__main__":
unittest.main()