-
Notifications
You must be signed in to change notification settings - Fork 96
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
BUG: INCAR parameter inheritance with prev_dir
has changed for MPGGAStaticMaker
#995
Comments
prev_dir
has changed (bug?)prev_dir
has changed (bug?)
prev_dir
has changed (bug?)prev_dir
has changed
prev_dir
has changedprev_dir
has changed. Intentional or not?
prev_dir
has changed. Intentional or not?prev_dir
has changed
prev_dir
has changedprev_dir
has changed
I think you might need to pass 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 |
Thanks for your comment, @esoteric-ephemera!
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 That said, based on your comment, I tested the minimal example for |
prev_dir
has changedprev_dir
has changed for MPGGAStaticMaker
prev_dir
has changed for MPGGAStaticMaker
prev_dir
has changed for MPGGAStaticMaker
The proposed fix would be to set atomate2/src/atomate2/vasp/jobs/mp.py Line 93 in 9600cef
This would match the behavior of older versions of atomate2 and also the current behavior of |
Ah my bad - yes some of the |
Didn't mean to imply any blame on your part! Just thought it might be relevant to the migration :) |
Yes, I think it is just natural that bugs happen if such large migrations are done. 😊 |
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:
test.zip
The above example is generating an
MPStaticSet
of parameters while building upon a prior directory's run via theprev_dir
keyword argument. The prior directory in this case (test
) has some parameters that are intentionally distinct from theMPStaticSet
for the sake of demonstration. For instance, let's track ENCUT, which is set to 123 intest/vasprun.xml
whereasMPGGAStaticMaker
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 ifinherit_incar=True
inVaspInputSetGenerator
, which isFalse
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.
The text was updated successfully, but these errors were encountered: