BUG: INCAR parameter inheritance with `prev_dir` has changed for `MPGGAStaticMaker`

[Andrew-S-Rosen]

Describe the bug

There is a change in behavior for how INCAR parameters are inherited from previous directories. I believe it to be potentially undesired.

Take the following example:

from atomate2.vasp.jobs.mp import MPGGAStaticMaker
from pymatgen.core import Structure

structure = Structure.from_file("test/POSCAR")
input_set = MPGGAStaticMaker().input_set_generator.get_input_set(structure=structure, potcar_spec=True, prev_dir="test")

print(input_set.incar["ENCUT"])

test.zip

The above example is generating an MPStaticSet of parameters while building upon a prior directory's run via the prev_dir keyword argument. The prior directory in this case (test) has some parameters that are intentionally distinct from the MPStaticSet for the sake of demonstration. For instance, let's track ENCUT, which is set to 123 in test/vasprun.xml whereas MPGGAStaticMaker has it set to 520.

In atomate2 0.0.14, I get ENCUT = 520. In other words, the MPGGAStaticMaker parameters are taking priority here.

In atomate2 0.0.15+, I get ENCUT = 123. In other words, the previous directory takes priority and overrides the settings by the MPGGAStaticMaker.

My question is: is this intentional? My gut feeling is no. For instance, if you are continuing a calculation from a previous directory but are now running with tighter settings, those tighter settings will get ignored. We can compare with what was in atomate2 <=0.0.14. As you can see in the linked section of the codebase, prev_dir previously did not update the INCAR parameters by default and instead was only used to update things such as KSPACING due to the prior calculation's band gap. The only time it would update the INCAR parameters was if inherit_incar=True in VaspInputSetGenerator, which is False by default.

This is ultimately due to the switch to pymatgen-based input sets, but then why is nothing breaking in CI as a result? CC'ing @esoteric-ephemera.

[esoteric-ephemera]

I think you might need to pass inherit_incar=True to the input set to get this behavior now - otherwise, INCAR settings are solely set by the input set and not previous calcs

There was some discussion a while ago about whether this behavior should be the default - it may have mistakenly been set to True for the MP GGA static set prior to transitioning to pmg sets

[Andrew-S-Rosen]

Thanks for your comment, @esoteric-ephemera!

I think you might need to pass inherit_incar=True to the input set to get this behavior now - otherwise, INCAR settings are solely set by the input set and not previous calcs

I believe the opposite is occurring right now. Namely, right now with atomate2==0.0.16, it is always inheriting by default. I would need to set inherit_incar=False to return to the old behavior.

That said, based on your comment, I tested the minimal example for MPMetaGGAStaticMaker and see that everything behaves as expected. It only appears to be an issue for MPGGAStaticMaker since inherit_incar is not set there in atomate2 but it defaults to True in pymatgen.

[Andrew-S-Rosen]

The proposed fix would be to set inherit_incar=False in the line below:

https://github.com/materialsproject/atomate2/blob/9600cef746957aae34dfa4a4df1f1bb68deca5cf/src/atomate2/vasp/jobs/mp.py#L93

This would match the behavior of older versions of atomate2 and also the current behavior of MPMetaGGAStaticMaker where that flag is set.

[esoteric-ephemera]

Ah my bad - yes some of the inherit_incar tags are set to True in the pymatgen sets but probably weren't in the atomate2 sets. I don't know that I changed that behavior, but agree that this should probably be changed on the atomate2 side to avoid issues with pmg

[Andrew-S-Rosen]

Didn't mean to imply any blame on your part! Just thought it might be relevant to the migration :)

[JaGeo]

Yes, I think it is just natural that bugs happen if such large migrations are done. 😊