Breaking: Unify previous directory Maker API

[janosh]

As discussed in today's atomate2 maintainer meeting, it would be great to unify the API between Makers for different codes and forcefields to more easily swap, say, a PBEsol pre-relax maker for a forcefield pre-relax maker and not have to worry about the flow that stitches the makers together passing in an unrecognized keyword like prev_vasp_dir into the forcefield maker.

[codecov]

Codecov Report

Merging #593 (3d25536) into main (d464012) will decrease coverage by 0.01%.
Report is 11 commits behind head on main.
The diff coverage is 71.57%.

❗ Current head 3d25536 differs from pull request most recent head 47f564c. Consider uploading reports for the commit 47f564c to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #593      +/-   ##
==========================================
- Coverage   75.57%   75.57%   -0.01%     
==========================================
  Files          83       84       +1     
  Lines        6793     6804      +11     
  Branches     1001     1002       +1     
==========================================
+ Hits         5134     5142       +8     
- Misses       1348     1350       +2     
- Partials      311      312       +1     

Files	Coverage Δ
src/atomate2/common/flows/defect.py	81.57% <ø> (ø)
src/atomate2/common/flows/elastic.py	77.55% <ø> (ø)
src/atomate2/common/jobs/defect.py	80.95% <100.00%> (ø)
src/atomate2/common/jobs/phonons.py	84.70% <100.00%> (ø)
src/atomate2/common/utils.py	78.94% <ø> (ø)
src/atomate2/forcefields/flows/relax.py	63.33% <100.00%> (+1.26%)	⬆️
src/atomate2/forcefields/jobs.py	92.37% <100.00%> (ø)
src/atomate2/vasp/flows/elastic.py	91.66% <100.00%> (ø)
src/atomate2/vasp/flows/lobster.py	86.95% <100.00%> (ø)
src/atomate2/vasp/flows/matpes.py	83.33% <100.00%> (ø)
... and 15 more

[JaGeo]

Sounds like a good change. Do the forcefield makers need a prev_dir? No files are currently written (should we write some logs?)

In addition:
Should we include a tight-binding based approach to cheaply generate electronic structres?

[janosh]

3d25536 shows we could even go a step further and define a class BaseMaker(Maker, ABC) that inherits from jobflow.Maker and which all other Maker subclasses inherit from to enforce a consistent def make() method signature.

That way, it's not up to the PR reviewer to notice violations. mypy will automatically flag if a Maker has a signature that's incompatible with other makers.

[janosh]

I think we may want to hold off on the abstract BaseMaker to enforce method signature since abstractmethods are covariant, i.e. restrict subclass methods to be more generic than the parent method which is not what we want. We want subclass Maker.make to be able to have additional arguments not defined in the parent class. We just want to ensure the arguments structure and prev_dir are always present. Not sure if there's a good static analysis way of doing this in Python and enforcing it at runtime seems less useful.

@utf Would be great to get your take here.

[janosh]

Either way, here's a one-liner to migrate atomate2 Flows outside this code base to the new prev_dir keyword:

find ./some/directory -type f -name "*.py" -exec sed -i'' -E 's/prev_(vasp|cp2k|amset)_dir/prev_dir/g' {} +

Or to replace any prev_????_dir:

find ./some/directory -type f -name "*.py" -exec sed -i'' -E 's/prev_.+_dir/prev_dir/g' {} +