JDFTXOutfileSlice.trajectory revision

[benrich37]

Summary

Major changes:

• feature 1: JDFTXOutfileSlice.trajectory is now initialized with frame_properties set
  -- JOutStructure.properties filled with relevant data for frame_properties
  -- More properties added to JOutStructure.site_properties

Todos

• Remove class attributes in JOutStructure now redundant to data stored in JOutStructure.properties and JOutStructure.site_properties

[shyuep]

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.

[benrich37]

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 insite_properties and properties that are also stored as regular io.jdftx.joutstructure.JOutStructure attributes? I was putting off removing the attributes as I believe some are referenced in atomate2, but I can do my due diligence / update those references in atomate2 to use the site_properties/properties instead.

I'm also not sure which slicing you're referring to (unless you're referring to the pre-existing none_slice_bools functionality in io.jdftx.outputs.JDFTXOutfile.from_file which is admittedly a bit overly convoluted)

[benrich37]

@shyuep Sorry I think I understand what you mean now - I think JOutStructure can be entirely replaced by a regular Structure by moving the parsing functions to JOutStructures and all attributes to Structure.properties and Structure.site_properties. I just want to confirm this is what you mean before starting as this might take a bit of work

[mkhorton]

This is so that JOutStructure.properties gets set automatically, e.g. so JOutStructure.as_dict() saves everything you want to be saved?

[benrich37]

@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.

[mkhorton]

Since the list of properties is known, why allow this to fail and return None?

[benrich37]

@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.