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 trajectory handling for MD #886

Merged
merged 4 commits into from
Nov 22, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
2 changes: 2 additions & 0 deletions emmet-core/emmet/core/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -963,6 +963,8 @@ def _get_task_files(files, suffix=""):
vol_files.append(file_no_path)
elif file.match(f"*POSCAR.T=*{suffix}*"):
elph_poscars.append(file_no_path)
elif file.match(f"*OSZICAR{suffix}*"):
vasp_files["oszicar_file"] = file_no_path

if len(vol_files) > 0:
# add volumetric files if some were found or other vasp files were found
Expand Down
55 changes: 44 additions & 11 deletions emmet-core/emmet/core/vasp/calculation.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
BSVasprun,
Kpoints,
Locpot,
Oszicar,
Outcar,
Poscar,
Potcar,
Expand Down Expand Up @@ -422,7 +423,7 @@ def from_vasp_outputs(
contcar: Optional[Poscar],
locpot: Optional[Locpot] = None,
elph_poscars: Optional[List[Path]] = None,
store_trajectory: bool = False,
store_trajectory: Union[bool, str] = False,
) -> "CalculationOutput":
"""
Create a VASP output document from VASP outputs.
Expand All @@ -441,8 +442,10 @@ def from_vasp_outputs(
Path to displaced electron-phonon coupling POSCAR files generated using
``PHON_LMC = True``.
store_trajectory
Whether to store ionic steps as a pymatgen Trajectory object. If `True`,
the `ionic_steps` field is left as None.
Whether to store ionic steps as a pymatgen Trajectory object. Can be
True, False or "partial", tuning the amount of data from the ionic_steps
stored in the Trajectory.
If not `False`, the `ionic_steps` field is left as None.

Returns
-------
Expand Down Expand Up @@ -610,6 +613,7 @@ def from_vasp_files(
contcar_file: Union[Path, str],
volumetric_files: List[str] = None,
elph_poscars: List[Path] = None,
oszicar_file: Optional[Union[Path, str]] = None,
parse_dos: Union[str, bool] = False,
parse_bandstructure: Union[str, bool] = False,
average_locpot: bool = True,
Expand All @@ -618,7 +622,7 @@ def from_vasp_files(
strip_bandstructure_projections: bool = False,
strip_dos_projections: bool = False,
store_volumetric_data: Optional[Tuple[str]] = None,
store_trajectory: bool = False,
store_trajectory: Union[bool, str] = False,
Copy link
Member

@munrojm munrojm Nov 14, 2023

Choose a reason for hiding this comment

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

Instead of mixing bool and str inputs to support a third "partial" option, I would vote change this to be an enum value or string representation of an enum value. That way the type and options are more well-defined. Even expecting a string and type hinting with a literal would work well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I switched to the Enum. Passing the string value is also fine. Let me know if you have other suggestions or better naming conventions.

Copy link
Member

Choose a reason for hiding this comment

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

Looks fine to me, let me know when you are happy to have it merged.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If it is fine, you can proceed to merge. Thanks!

vasprun_kwargs: Optional[Dict] = None,
) -> Tuple["Calculation", Dict[VaspObject, Dict]]:
"""
Expand All @@ -641,6 +645,8 @@ def from_vasp_files(
elph_poscars
Path to displaced electron-phonon coupling POSCAR files generated using
``PHON_LMC = True``, given relative to dir_name.
oszicar_file
Path to the OSZICAR file, relative to dir_name
parse_dos
Whether to parse the DOS. Can be:

Expand Down Expand Up @@ -680,9 +686,17 @@ def from_vasp_files(
store_volumetric_data
Which volumetric files to store.
store_trajectory
Whether to store the ionic steps in a pymatgen Trajectory object. if `True`,
:obj:'.CalculationOutput.ionic_steps' is set to None to reduce duplicating
information.
Whether to store the ionic steps in a pymatgen Trajectory object and the
amount of data to store from the ionic_steps. Can be:
- True: Store the Trajectory. All the properties from the ionic_steps
are stored in the frame_properties except for the Structure, to
avoid redundancy.
- "partial": Store the Trajectory. All the properties from the ionic_steps
are stored in the frame_properties except from Structure and
ElectronicStep
- False: Trajectory is not Stored.
If not `False`, :obj:'.CalculationOutput.ionic_steps' is set to None
to reduce duplicating information.
vasprun_kwargs
Additional keyword arguments that will be passed to the Vasprun init.

Expand Down Expand Up @@ -728,7 +742,9 @@ def from_vasp_files(
ddec6 = None
if run_ddec6 and VaspObject.CHGCAR in output_file_paths:
densities_path = run_ddec6 if isinstance(run_ddec6, (str, Path)) else None
ddec6 = ChargemolAnalysis(path=dir_name, atomic_densities_path=densities_path).summary
ddec6 = ChargemolAnalysis(
path=dir_name, atomic_densities_path=densities_path
).summary

locpot = None
if average_locpot:
Expand All @@ -749,11 +765,28 @@ def from_vasp_files(
store_trajectory=store_trajectory,
)
if store_trajectory:
exclude_from_trajectory = ["structure"]
if store_trajectory == "partial":
exclude_from_trajectory.append("electronic_steps")
frame_properties = [
IonicStep(**x).model_dump(exclude=exclude_from_trajectory)
for x in vasprun.ionic_steps
]
if oszicar_file:
try:
oszicar = Oszicar(oszicar_file)
if "T" in oszicar.ionic_steps[0]:
for frame_property, oszicar_is in zip(
frame_properties, oszicar.ionic_steps
):
frame_property["temperature"] = oszicar_is.get("T")
except ValueError:
# there can be errors in parsing the floats from OSZICAR
pass

traj = Trajectory.from_structures(
[d["structure"] for d in vasprun.ionic_steps],
frame_properties=[
IonicStep(**x).model_dump() for x in vasprun.ionic_steps
],
frame_properties=frame_properties,
constant_lattice=False,
)
vasp_objects[VaspObject.TRAJECTORY] = traj # type: ignore
Expand Down
Loading