-
Notifications
You must be signed in to change notification settings - Fork 87
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
NSW=1 --> NSW=0 in static, Cf in POTCARs #10
Conversation
Hi @arosen93, great to have someone looking over atomate2. It is obviously still forming so any and all suggestions/PR are very welcome. Thanks for catching the Cf pseudo potential. As for Before I merge, can you add |
Important for DFPT that NSW = 1, not NSW = 0
@utf, interesting! I didn't know that about DFPT. I see the thought process there, although my personal opinion is that it's better to set |
I figured that might break something along the way. Sorry for not running tests locally. I'll get the testing stuff set up tomorrow and can fix then. :) |
Ah, the test data is now inconsistent. Should be as simple as editing the INCAR in the Also, to be consistent, we should probably also adjust NSW in the HSEStaticSetGenerator. If I have a chance, I can take a look at this later. |
Hi again @utf. That should do it now. Tests passed locally. I had to change the INCARs in the following:
|
For the lint error. The easiest thing to do is to run |
@utf: I guess I must have run the wrong I wanted to run a few things by you here. Which INCARs should have |
Quick Hello
Hi @utf! I'm opening this very minor PR in part to say hello and that I think
atomate2
looks fantastic! I am really excited to start using it. Also a few small things.Summary of Proposed Changes
Cf: Cf
to the list of default pseudopotentials since it's available with the VASP PAW_PBE_54 potentials. It probably won't be relevant to anyone, but why not aim for completeness. You got all the other ones!NSW=1
toNSW=0
in yourget_incar_updates()
forStaticSetGenerator()
. Was there a specific reason to useNSW=1
for the static calculation? One of my concerns is that other codes might look atNSW
and think that it's doing a relaxation even though it's a static calculation.DriftErrorHandler()
in Custodian, for example. It checks to see ifEDIFFG >= 0 or NSW == 0
. If this condition is met, it doesn't run the drift error checker; otherwise, it does. However, here,NSW=1
would make Custodian think it's a relaxation. Granted, Custodian is unlikely to actually do anything here, but it's more the premise that this might influence other codes in unexpected ways.