-
Notifications
You must be signed in to change notification settings - Fork 158
improve ASE traj #633
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
Changes from all commits
820ca84
4594981
b055b88
bde8b7b
0377b27
da3daa8
70f7813
65d2797
42ff2b0
7f1a398
59e3247
a381f89
1585ff8
2fefd7f
793745c
76895a7
a12c374
0a77be5
ea85559
9dbcbd8
0b27552
c09894b
67e7876
2afdc4f
8771f8c
fe5163a
d41573b
17fcb16
e05c9ce
95939bd
52ef114
60212f6
57d938e
3c95376
c3c45ef
46171fc
1efef9b
b669ae1
33c5084
790d4b5
f0c5c7d
551920e
d7c98f9
e0b53f7
81f402a
9248833
8f4684d
eb66571
9893423
77e8ef4
02b4dac
45c8018
71d0e70
e3909e7
b49a4e2
4e0c083
0e22d30
f0febca
4a07590
a65144b
3eb842c
8918a90
6e2df4d
1a322ce
188f945
5d27076
1a466e3
6ffe27f
3ba5933
912d78c
aeb569c
9331a04
bb4d75d
c5fb9ba
0dadef3
b5a9171
a57f6d7
4c7b3ef
7f424fc
b4a9d05
aa96933
d1f02a3
bdcb887
1c1ebdc
ca474a9
b304fc9
7a0530b
1f8113e
14e48f8
7315d77
be33545
af1c94b
a222e8e
62fe922
bb0640f
77fb18a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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 | ||||||||||||
|
|
||||||||||||
|
|
@@ -94,7 +95,7 @@ def from_labeled_system(self, atoms: ase.Atoms, **kwargs) -> dict: | |||||||||||
| "forces": np.array([forces]), | ||||||||||||
| } | ||||||||||||
| try: | ||||||||||||
| stress = atoms.get_stress(False) | ||||||||||||
| stress = atoms.get_stress(voigt=False) | ||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider handling While the current implementation passes silently if |
||||||||||||
| except PropertyNotImplementedError: | ||||||||||||
| pass | ||||||||||||
| else: | ||||||||||||
|
|
@@ -110,7 +111,7 @@ def from_multi_systems( | |||||||||||
| step: int | None = None, | ||||||||||||
| ase_fmt: str | None = None, | ||||||||||||
| **kwargs, | ||||||||||||
| ) -> ase.Atoms: | ||||||||||||
| ) -> Generator[ase.Atoms, None, None]: | ||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use Python's typing module recommends using |
||||||||||||
| """Convert a ASE supported file to ASE Atoms. | ||||||||||||
|
|
||||||||||||
| It will finally be converted to MultiSystems. | ||||||||||||
|
|
@@ -140,7 +141,7 @@ def from_multi_systems( | |||||||||||
| frames = ase.io.read(file_name, format=ase_fmt, index=slice(begin, end, step)) | ||||||||||||
| yield from frames | ||||||||||||
thangckt marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||
|
|
||||||||||||
| def to_system(self, data, **kwargs): | ||||||||||||
| def to_system(self, data, **kwargs) -> list[ase.Atoms]: | ||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Update type annotations from - 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 Also applies to: 162-162
|
||||||||||||
| """Convert System to ASE Atom obj.""" | ||||||||||||
| from ase import Atoms | ||||||||||||
|
|
||||||||||||
|
|
@@ -158,7 +159,7 @@ def to_system(self, data, **kwargs): | |||||||||||
|
|
||||||||||||
| 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 | ||||||||||||
|
|
@@ -300,6 +301,46 @@ def from_labeled_system( | |||||||||||
|
|
||||||||||||
| return dict_frames | ||||||||||||
|
|
||||||||||||
| def to_system(self, data, file_name: str = "confs.traj", **kwargs) -> None: | ||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ensure proper file handling in The methods Also applies to: 323-323 |
||||||||||||
| """Convert System to ASE Atoms object. | ||||||||||||
|
|
||||||||||||
| Parameters | ||||||||||||
| ---------- | ||||||||||||
| file_name : str | ||||||||||||
| path to file | ||||||||||||
| """ | ||||||||||||
| from ase.io import Trajectory | ||||||||||||
|
|
||||||||||||
| 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add unit tests for the new methods 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 | ||||||||||||
| """ | ||||||||||||
| from ase.io import Trajectory | ||||||||||||
|
|
||||||||||||
| 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 | ||||||||||||
|
|
||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Review the use of default parameters in The constructor of - optimizer_kwargs: dict = {}
+ optimizer_kwargs: dict | None = None
if optimizer_kwargs is None:
optimizer_kwargs = {}Committable suggestion
Suggested change
|
||||||||||||
|
|
||||||||||||
| @Driver.register("ase") | ||||||||||||
| class ASEDriver(Driver): | ||||||||||||
|
|
||||||||||||
Uh oh!
There was an error while loading. Please reload this page.