Including Forcefield methods

[JaGeo]

Currently, the forcefields rely on the TrajectoryObserver from chgnet and megnet (it looks extremely similar in both codes). This, however, means that we would need to rely on it (or a similar class) for any kind of forcefield in the future. It also always includes magnetic moments by default (I don't think all force fields consider them; I ran into an issue with one ML potential). Any ideas how to solve this? Should we include our own TrajectoryObserver into atomate2?

Currently, we also don't have any ForcefieldBaseMaker or similar.

I plan to write a RelaxMaker for EMT to test this in more detail.

Any ideas, thoughts? @utf @matthewkuner @arosen93 @janosh ?

[Andrew-S-Rosen]

Strictly speaking, TrajectoryObserver should only be needed if the force field method is being used as an ASE Calculator, right? So, presumably, if one made an Atomate2 recipe using (for example) GULP-based force fields using the GULP internal relaxation engine, then there would be no need for a TrajectoryObserver for that code in practice.

It's also worth noting that TrajectoryObserver does the exact same thing as the regular TrajectoryWriter in ASE except in the case of the former it stores it in memory instead of writing out the trajectory file to the disk. This can be helpful to speed up calculations since the steps are quick and file I/O would be the bottleneck, which is why it's used.

[JaGeo]

@arosen93 thanks for the clarification with regard to the TrajectoryObserver. Maybe worth adding it to ase as well?

[Andrew-S-Rosen]

@JaGeo: Yes, I think that it is probably worth adding TrajectoryObserver to ASE as a class alongside TrajectoryReader and TrajectoryWriter. Someone with knowledge of ASE would have to open a PR for it of course. I'll probably get to it one day if nobody else does, but it's not high on my priority list...

[JaGeo]

@arosen93 Thanks! For now, I would probably need to include it into atomate2 to add further ase-based force fields and ML potentials.

We probably also need to add a BaseForceFieldMaker to simplify writing workflows for all forcefields...

I will then include it into my on-going PR of the phonon workflow for forcefields. #398

[Andrew-S-Rosen]

@JaGeo: Makes sense. In the meantime, I've opened an ASE issue here for the sake of bookkeeping: https://gitlab.com/ase/ase/-/issues/1293.

[Andrew-S-Rosen]

@JaGeo: The lead dev of ASE responded to the above issue --- there isn't actually ever a need for TrajectoryObserver in practice. The same can be done just by passing a trajectory kwarg to the ASE Optimizer class (see here) and then adjusting the IO buffer size (as noted here) so that it doesn't flush to disk every step. In the meantime, of course it might be easier to just copy the TrajectoryObserver to atomate2 directly, but longer term I think it's a class that doesn't really have to exist in the first place.

[matthewkuner]

@JaGeo Do you mean that the TrajectoryObserver always includes magmoms, or the ForceFieldTaskDocument? I'm unfamiliar with the larger pros/cons of storing this data in different forms and outsourced that opinion when I wrote the original forcefield code for Atomate2.
In the ForceFieldTaskDocument, I convert the TrajectoryObserver to a IonicStep object as well. So we would just need a way to convert other code's convention for storing trajectories to this IonicStep object if we want to implement other forcefields, no?

[JaGeo]

I think both. It caused an issues for me in case they were not defined. I will try to adapt the whole code for this case.

I will try to come up with something, I think. I mainly wanted to hear opinions on the TrajectoryObserver.

[utf]

This has been resolved in #398