-
Notifications
You must be signed in to change notification settings - Fork 133
Description
While running MLFF calculation with ASE based jobs I encountered two problems with the data being stored in the ionic steps.
The first is that the base AseRelaxMaker has a ionic_step_data attribute that contains "mol_or_struct" in its default:
atomate2/src/atomate2/ase/jobs.py
Lines 84 to 90 in 1822f9d
| ionic_step_data: tuple[str, ...] | None = ( | |
| "energy", | |
| "forces", | |
| "magmoms", | |
| "stress", | |
| "mol_or_struct", | |
| ) |
Based on this and on the docstrings, I assumed that if I provided a list excluding "mol_or_struct" the structure would have not been saved. However, in the code, as long as there are some data for ionic steps stored, the mol_or_struct element is always stored:
atomate2/src/atomate2/ase/schemas.py
Line 530 in 1822f9d
| mol_or_struct=atoms, |
I think that it would be correct to exclude mol_or_struct from the output if not present in ionic_step_data, but if not, at least this should be made clear in the docstrings and mol_or_struct removed from the default.
The second issue is that in IonicStep the structure and molecule are automatically filled in based on mol_or_struct:
atomate2/src/atomate2/ase/schemas.py
Line 124 in 1822f9d
| def model_post_init(self, context: Any, /) -> None: |
However, this makes so that when the output is serialized it contains the structure twice for each ionic step. Is there any reason for
structure and molecule not being @property? Or at least they could be set as Field(..., excluded=True) so that the same data will not be stored twice.
I can open a PR to address these issues if needed, but I first wanted to check if these were the result of a specific choice/need and if there is any preference about how to address them.