JDFTXOutfileSlice.trajectory revision#4425
Conversation
…es` for `Trajectory` in `JDFTXOutfileSlice.trajectory`
|
I think way too much complexity is being introduced here. Surely the slicing can be integrated as part of the usual out file parsing? E.g., Vasprun just integrates the ionic_step_skip rather than having one special module and class to do it. Not the mention that there is a lot of duplicated code. |
@shyuep For the duplicated code are you referring to the data stored in I'm also not sure which slicing you're referring to (unless you're referring to the pre-existing |
|
@shyuep Sorry I think I understand what you mean now - I think |
| generic_lines.append(line_text) | ||
| return generic_lines, collecting, collected | ||
|
|
||
| def _fill_properties(self) -> None: |
There was a problem hiding this comment.
This is so that JOutStructure.properties gets set automatically, e.g. so JOutStructure.as_dict() saves everything you want to be saved?
There was a problem hiding this comment.
@mkhorton Right now the purpose of JOutStructure_fill_properties is to fill JOutStructure.properties for using as frame properties in Trajectory construction inJDFTXOutfileSlice._set_trajectory. I am also hoping to fully move all possible JOutStructure attributes into JOutStructure.properties, so this function is a placeholder to allow JOutStructures and JDFTXOutfileSlice to fetch data from JOutStructure.properties instead of JOutStructure directly until a future PR where these current JOutStructure attributes are saved under JOutStructure.properties instead. I am hoping that using JOutStructure.properties will make the class a lot cleaner, while also making the process of adding new properties down the road a lot easier.
| """ | ||
| if len(self) == 0: | ||
| return [] | ||
| return [getattr(s, prop, None) for s in self.slices] |
There was a problem hiding this comment.
Since the list of properties is known, why allow this to fail and return None?
There was a problem hiding this comment.
@mkhorton Sorry this is my bad - JOutStructures.get_frame_property_list is a vestigial function from an earlier version of this PR before I realized constructing frame properties for a Trajectory was a lot easier to do using Structure.properties. This function needs to be deleted.
Summary
Major changes:
JDFTXOutfileSlice.trajectoryis now initialized withframe_propertiesset--
JOutStructure.propertiesfilled with relevant data forframe_properties-- More properties added to
JOutStructure.site_propertiesTodos
JOutStructurenow redundant to data stored inJOutStructure.propertiesandJOutStructure.site_properties