ASE <---> pymatgen trajectory conversion #4253
Conversation
…he ASE traj interface
…nctinoality requested
esoteric-ephemera
changed the title
src/pymatgen/core/trajectory.py
Outdated
| @@ -734,3 +732,160 @@ def _get_site_props(self, frames: ValidIndex) -> SitePropsType | None: | |||
| return [self.site_properties[idx] for idx in frames] | |||
| raise ValueError("Unexpected frames type.") | |||
| raise ValueError("Unexpected site_properties type.") | |||
|
|
|||
| def to_ase_trajectory(self, **kwargs) -> AseTrajectory: | |||
There was a problem hiding this comment.
I suggest we just call it to_ase. Given this class is already called Trajectory, I don't think it is a stretch to imagine to_ase meaning the ASE format for trajectory.
src/pymatgen/core/trajectory.py
Outdated
| raise ImportError("ASE is required to write .traj files. pip install ase") | ||
|
|
||
| @classmethod | ||
| def ase_to_pmg_trajectory( |
There was a problem hiding this comment.
This should be just called from_ase.
src/pymatgen/core/trajectory.py
Outdated
| ) | ||
|
|
||
| @staticmethod | ||
| def pmg_to_ase_trajectory( |
There was a problem hiding this comment.
I don't think we need a separate method from to_ase?
There was a problem hiding this comment.
Thanks @shyuep - the intent was to have two methods that don't depend on the Trajectory class being instantiated to use as general converters. Similar design as AseAtomsAdaptor(get_atoms and get_structure are both staticmethods)
I'm OK changing this if you'd rather just have the to_ase method
There was a problem hiding this comment.
I don't think we need a method that is independent of Trajectory.
|
Looks good. Thanks. Merged. |
Adds feature to convert between ASE and pymatgen trajectories via
from_aseandto_asemethods onTrajectory. Allows for carrying energies, forces, stresses, etc. between ASE and pymatgen trajectories, rather than just structural info as previously implemented.Merging would close #4252. From the discussion there: this PR ensures that any calculator attr's that are set by
AseAtomsAdaptorare not lost in the trajectory conversion processs.To-do:
- Tests