Skip to content
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

Fixed bug in ScanOptimizeFW PBEsol step #766

Merged
merged 5 commits into from
May 19, 2023

Conversation

MichaelWolloch
Copy link
Contributor

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:

  • Check the METAGGA flag in the vasp_input_set.incar or the vasp_input_set_params an put it in metagga_type.
  • Instead of setting "METAGGA": None, one unsets "METAGGA": metagga_type.

TODO (maybe)

  • Maybe write a unit test for this? But I think this is not easy since one cannot run vasp of course.

@MichaelWolloch
Copy link
Contributor Author

MichaelWolloch commented Dec 21, 2022

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
Copy link
Contributor Author

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
Copy link
Contributor

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.

@Zhuoying Zhuoying closed this Feb 13, 2023
@MichaelWolloch
Copy link
Contributor Author

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
Copy link
Contributor

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.

@Zhuoying Zhuoying reopened this Mar 13, 2023
@MichaelWolloch
Copy link
Contributor Author

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

@Zhuoying Zhuoying merged commit 43298db into hackingmaterials:main May 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants