hackingmaterials / atomate

atomate is a powerful software for computational materials science and contains pre-built workflows.
https://hackingmaterials.github.io/atomate
Other
245 stars 175 forks source link

Fixed bug in ScanOptimizeFW PBEsol step #766

Closed MichaelWolloch closed 1 year ago

MichaelWolloch commented 1 year ago

Summary

I am using my own fork of atomate to enable better copying of vdW kernels and better chaining of OptimizeFW and StaticFW. I updated my fork recently and found that the PBEsol step of the ScanOptimizeFW is broken now. Specifically the removal of the METAGGA tag for that step is not working correctly.

Setting METAGGA = None in the INCAR (which one would assume is the default according to the Wiki) results in an error, at least for vasp 6.3:

running on    6 total cores
 distrk:  each k-point on    3 cores,    2 groups
 distr:  one band on    3 cores,    1 groups
 vasp.6.3.0 20Jan22 (build Mar 10 2022 16:20:19) complex                        

 POSCAR found type information on POSCAR NaCl
 POSCAR found :  2 types and       6 ions
 Reading from existing POTCAR
 scaLAPACK will be used
 Reading from existing POTCAR
 -----------------------------------------------------------------------------
|                                                                             |
|               ----> ADVICE to this user running VASP <----                  |
|                                                                             |
|     You enforced a specific xc type in the INCAR file but a different       |
|     type was found in the POTCAR file.                                      |
|     I HOPE YOU KNOW WHAT YOU ARE DOING!                                     |
|                                                                             |
 -----------------------------------------------------------------------------

 -----------------------------------------------------------------------------
|                                                                             |
|     EEEEEEE  RRRRRR   RRRRRR   OOOOOOO  RRRRRR      ###     ###     ###     |
|     E        R     R  R     R  O     O  R     R     ###     ###     ###     |
|     E        R     R  R     R  O     O  R     R     ###     ###     ###     |
|     EEEEE    RRRRRR   RRRRRR   O     O  RRRRRR       #       #       #      |
|     E        R   R    R   R    O     O  R   R                               |
|     E        R    R   R    R   O     O  R    R      ###     ###     ###     |
|     EEEEEEE  R     R  R     R  OOOOOOO  R     R     ###     ###     ###     |
|                                                                             |
|     Error: This functional is not implemented.                              |
|                                                                             |
|       ---->  I REFUSE TO CONTINUE WITH THIS SICK JOB ... BYE!!! <----       |
|                                                                             |
 -----------------------------------------------------------------------------

It is better to remove the tag altogether, which is not hard to do with the following steps:

TODO (maybe)

MichaelWolloch commented 1 year ago

I initially forgot to update the tests and the reference INCARs for the PBEsol precalculations. Double checked that those INCARs with METAGGA = None indeed show the same "functional not implemented" error as discussed before.

For some reason when I use pytest locally, everything in the relevant module (atomate/vasp/workflows/tests/test_vasp_workflows.py) gets skipped.

MichaelWolloch commented 1 year ago

OK, I do not really get why the tests fail still. Apparently there is still an INCAR written with a METAGGA tag, which is then not found in the reference file (because I deleted it). I am not sure why, since in testing this myself, I do not get this problem.

I cannot debug the tests, since they are all skipped locally.

Maybe there is some interaction with my other changes?

Anyhow, I can confirm that METAGGA = None works for vasp.5.4.4, but fails for vasp.6.3.0. It is for sure safer to remove the flag from the INCAR of the PBEsol calculation, but maybe my metagga_type idea to find the correct value is flawed if the parameter is not explicitly set in the vasp input set or the params?

Zhuoying commented 1 year ago

Hi, @MichaelWolloch, thanks for your contribution. If you want to enable the compatibility of metagga for both VASP 5.4.4 and VASP 6.3.0, you may consider the case of value not being explicitly set in vasp input. So I will close this PR for now, if you fix it later, pls let me know and I can reopen this PR.

MichaelWolloch commented 1 year ago

Hi, @MichaelWolloch, thanks for your contribution. If you want to enable the compatibility of metagga for both VASP 5.4.4 and VASP 6.3.0, you may consider the case of value not being explicitly set in vasp input. So I will close this PR for now, if you fix it later, pls let me know and I can reopen this PR.

Hi @Zhuoying , I am not sure what you mean. Why should my approach not work for vasp 5.4.4? And why should the metagga flag not be set in the ScanOptimizeFW? It terminates if any other than MPScanRelaxSet is passed (note that a UserWarning is raised, so execution stops). If only a structure is passed, vasp_input_set is set to MPScanRelaxSet if not explicitly set anyhow. And the following lines look quite robust to me also:

metagga_type = vasp_input_set.incar.get("METAGGA",
                       vasp_input_set_params.get("METAGGA", "R2scan"))

Note that my changes ONLY apply if there is a structure passed.

As I said, I am using a custom fork, so I do not care that much if this is merged or not, but I do not really understand the problem with this PR. That of course does not mean that there are none, but please take the time to explain a bit more what your issues are.

Zhuoying commented 1 year ago

Hi @MichaelWolloch, Thanks for your detailed explanation. I thought the previous tests failed the PR due to a compatibility issue (maybe not, I am reopening the PR and taking a look). I just want to ensure the fix works for both 5.4.4 and 6.3.0. Then I have no problem merging it. Sorry for the misunderstanding, if any. BTW: I have merged some PRs since then. Pls remember to git pull them first. Thanks.

MichaelWolloch commented 1 year ago

Hi @Zhuoying, thanks for opening this back up. I pulled the changes, and now fixed a bug for vdw SCAN. I hope all the tests pass now. Cheers, Michael