Add VASP input set `MatPESStaticSet` by SophiaRuan · Pull Request #3254 · materialsproject/pymatgen

SophiaRuan · 2023-08-19T00:05:50Z

Summary

Major changes:

feature 1: add MatPESStaticSet class

Todos

If this is work in progress, what else needs to be done?

make sure all the input set default settings are right.
write unit tests

Checklist

Google format doc strings added. Check with ruff.
Type annotations included. Check with mypy.
Tests added for new features/fixes.
If applicable, new classes/functions/modules have duecredit @due.dcite decorators to reference relevant papers by DOI (example)

Tip: Install pre-commit hooks to auto-check types and linting before every commit:

pip install -U pre-commit
pre-commit install

prev: incar = Incar(self.prev_incar) if self.prev_incar is not None else Incar(parent_incar)

shyuep · 2023-08-19T15:07:00Z

+# using the Regularized-Restored Strongly Constrained and Appropriately
+# Normed functional (PBE, PBE+U or r2SCAN).
+INCAR:
+  ALGO: ALL


Default ALGO should be Normal for PBE. R2SCAN uses All. But our input set is written with PBE as a default.

shyuep · 2023-08-19T15:10:30Z

+    W: 5
+    Yb3+: 1
+POTCAR_FUNCTIONAL: PBE_54
+POTCAR:


I don't think we should be hard coding POTCARs or U values in this input set. The VASP input sets support inheritance. E.g., VASPIncarBase defines U values. We should write a base for PBE54 that incorporates things we feel would be common across all calculations. E.g., POTCAR_FUNCTIONAL, POTCAR and maybe the U values.

I felt confused about how we make sure all the MatPES static calculations have the same PSP, so that's why I incorporate the POTCAR values here. When you mentioned write a base for PBE54, do you mean to include another PBE54Base.yaml used for all the PBE54 POTCARs, POTCAR_FUNCTIONAL, and U values?

same for def kpoint()

janosh · 2023-08-24T18:49:02Z


+class TestMatPESStaticSet(PymatgenTest):
+    def setUp(self):
+        self.tmp = tempfile.mkdtemp()


Not needed, just use self.tmp_path inherited from PymatgenTest.

janosh · 2023-08-24T18:50:57Z

+        assert vis.incar["NSW"] == 0
+        assert vis.incar["NELM"] == 200
+        assert vis.incar["ENCUT"] == 680
+        assert vis.incar["ENAUG"] == 1360
+        assert vis.incar["EDIFF"] == 1.0e-05
+        assert vis.incar["LREAL"] is False
+        assert vis.incar["LORBIT"] == 11
+        assert vis.incar["LVHAR"]
+        assert vis.incar["ISMEAR"] == 0
+        assert vis.incar["KSPACING"] == 0.22
+        assert vis.incar["GGA"] == "PE"


No need to assert keys one by one. If you want to make sure they match the values in pymatgen/io/vasp/MatPESStaticSet.yaml, you can load that file into a dict and compare vis.incar against that dict.

…hecks

shyuep · 2023-08-25T21:46:36Z

@SophiaRuan This looks good. But I would like @JiQi535 to go through it carefully and make sure everything is correct.

esoteric-ephemera · 2023-08-25T23:11:20Z

A few remarks given the updates to MP's GGA and r2SCAN WFs:

LREAL should be False for statics (consistent with how we do MP statics)
It says this is set is intended to r2SCAN, but the default is PBE? Otherwise, GGA = None and METAGGA = R2SCAN is needed (the GGA = None is needed for the more recent versions of VASP)
LAECHG should be False unless you plan on using the AECCAR* files
Same with LCHARG and the CHG/CHGCAR files
Same with LVTOT and the LOCPOT / POT files

If you want to contribute the statics to MP-core, then unfortunately you need LAECHG, LCHARG, and LOCPOT set to true for consistency with the r2SCAN WF. It'll just slow things down slightly with the file I/O

We're working on independently testing the differences in energies / forces / stresses for ISMEAR = 0 and -5. We may ultimately stay with ISMEAR = -5 for DOS / bandstructures / thermo properties, but that does mean incorrect forces for metals

shyuep · 2023-08-25T23:15:06Z

@esoteric-ephemera this input set is for MatPES (as the name implies). While we are happy to contribute to MPstatics if MP wants them, forces are a priority here. Perhaps more so than even energies.

I am unsure why MP would want to stick to ISMEAR -5 for all statics. The evidence is quite conclusive than 0 works as well for insulators and certainly better for metals. So what is the advantage of sticking with -5?

esoteric-ephemera · 2023-08-25T23:26:39Z

@shyuep I saw the work from your group for these differences, my only concern is that the average absolute energy differences between the two smearing methods is ~1 eV, which can be large on the scale of energy differences relevant for thermoprops (~5-10 meV/atom). Would be good to see these differences in eV/atom instead of eV

I'm also not sure what the effect of ISMEAR is on the DOS / KS eigenvalues / bandgap. We usually assume ISMEAR = -5 gives a better description of the actual electronic structure, but we're testing this now

shyuep · 2023-08-25T23:28:31Z

@esoteric-ephemera ok. Would be good to have independent validation.

JaGeo · 2023-08-25T23:34:31Z

-5 should give a much better band gap based on DOS.

shyuep · 2023-08-26T00:00:24Z

@esoteric-ephemera This is just my intuition. But if you use a small enough SIGMA, the DOS effect is small. After all, a smearing of 0.02 can't lead to more than a 0.04 eV error in band edges. In the grand scheme of DFT errors in band gaps, it is practically nothing.

JiQi535 · 2023-08-26T03:15:31Z

@SophiaRuan This looks good. But I would like @JiQi535 to go through it carefully and make sure everything is correct.

I have checked @SophiaRuan's implementation of MatPESStaticSet, and it looks nice to me. I have below questions.

It seems to me that +U is by default not added. Isn’t this conflicting with the default in MPRelaxSet and MPStaticSet?
In MPRelaxSet, there are 89 elements, same as the current coverage by MP. While for the new set, we have 97 elements. Some of the extra elements are Fr, Ra, Rn. @janosh I’m curious if and why they are added 😃
LVTOT=True. We don’t have this in MPStaticSet. Is MP storing this data? @janosh

JaGeo · 2023-08-26T08:16:32Z

Just adding this paper here for my comment on the DOS computations (a smearing of 0.05 was used):
https://www.sciencedirect.com/science/article/pii/S277294942200002X

janosh · 2023-08-26T14:53:00Z

2 In MPRelaxSet, there are 89 elements, same as the current coverage by MP. While for the new set, we have 97 elements. Some of the extra elements are Fr, Ra, Rn. @janosh I’m curious if and why they are added 😃

@JiQi535 They were copied from MPSCANRelaxSet IIRC.

3 LVTOT=True. We don’t have this in MPStaticSet. Is MP storing this data? @janosh

No.

JiQi535 · 2023-08-26T15:53:52Z

+  LMIXTAU: True
+  LORBIT: 11
+  LREAL: Auto
+  LVTOT: True


This line should be removed. The LOCPOT file is going to consume storage space.

JiQi535 · 2023-08-26T15:58:47Z

+    def __init__(
+        self,
+        structure: str,
+        functional: Literal["R2SCAN", "R2SCAN+U", "PBE", "PBE+U"] = "PBE",


@SophiaRuan Here, I think we should have PBE+U as default, inline with default of MPRelaxSet and MPStaticSet. Also, I don't think we should distinguish PBE and PBE+U as two functionals. Instead, +U can be disabled in the same way as in MPRelaxSet and MPStaticSet with user_incar_settings={"LDAU": False}.

@shyuep Please kindly comment if you agree.

shyuep · 2023-08-26T16:15:02Z

No. PBE default. U and r2scan as options. This is not MP.

shyuep · 2023-08-28T15:46:57Z

The lint and test failures are unrelated to the PR. I am merging.

janosh · 2023-08-28T15:53:28Z

They're not unrelated. The 1st TestMatPESStaticSet assert fails:

pytest tests/io/vasp/test_sets.py -k TestMatPESStaticSet -vv

_____________________________ TestMatPESStaticSet.test_init ______________________________

self = <tests.io.vasp.test_sets.TestMatPESStaticSet testMethod=test_init>

    def test_init(self):
        prev_run = f"{TEST_FILES_DIR}/relaxation"
    
        vis = MatPESStaticSet.from_prev_calc(prev_calc_dir=prev_run)
        # Check that INCAR settings were load from yaml, check the default functional is PBE.
        CONFIG = _load_yaml_config("MatPESStaticSet")
>       assert vis.incar == CONFIG["INCAR"]
E       AssertionError: assert {'PREC': 'acc...ORBIT': False} == {'ALGO': 'Nor...5, 'Yb3+': 1}}
E         Omitting 3 identical items, use -vv to show
E         Differing items:
E         {'PREC': 'accurate'} != {'PREC': 'Accurate'}
E         {'ISMEAR': -5} != {'ISMEAR': 0}
E         {'MAGMOM': [0.6]} != {'MAGMOM': {'Ce': 5, 'Ce3+': 1, 'Co': 0.6, 'Co3+': 0.6, 'Co4+': 1, 'Cr': 5, 'Dy3+': 5, 'Er3+': 3, 'Eu': 10, 'Eu2+': 7,...4+': 3, 'Mo': 5, 'Nd3+': 3, 'Ni': 5, 'Pm3+': 4, 'Pr3+': 2, 'Sm3+': 5, 'Tb3+': 6, 'Tm3+': 2, 'V': 5, 'W': 5, 'Yb3+': 1}}
E         {'SIGMA': 0.2} != {'SIGMA': 0.05}
E         {'LORBIT': False} != {'LORBIT': 11}...

shyuep · 2023-08-28T15:53:43Z

I just noticed one problem. Why is the INCAR written as return Incar(self.prev_incar or parent_incar)? This is wrong. If the prev_incar is a relaxation, this will make it a relaxation calculator too.

SophiaRuan and others added 6 commits August 18, 2023 17:02

add MatPESStaticSet to sets.py

deac470

pre-commit auto-fixes

704eff5

Merge branch 'master' into matpes_static

14f4fe2

add MatPESStaticSet type hints

9b9a361

simplify to Incar(self.prev_incar or parent_incar)

d7c4ea9

prev: incar = Incar(self.prev_incar) if self.prev_incar is not None else Incar(parent_incar)

handle functional="PBE" with no U correctly

29cdaa3

janosh added enhancement A new feature or improvement to an existing one io Input/output functionality vasp Vienna Ab initio Simulation Package labels Aug 19, 2023

janosh changed the title ~~add MatPESStaticSet to sets.py (WIP)~~ Add MatPESStaticSet to VASP input set Aug 19, 2023

janosh changed the title ~~Add MatPESStaticSet to VASP input set~~ Add VASP input set MatPESStaticSet Aug 19, 2023

SophiaRuan force-pushed the matpes_static branch from e045029 to e5cf479 Compare August 19, 2023 02:29

shyuep reviewed Aug 19, 2023

View reviewed changes

janosh added 2 commits August 19, 2023 08:32

fix MatPESStaticSet.incar() not returning anything

34a637d

add return type hint def incar(self) -> Incar:

a1e57a0

same for def kpoint()

janosh force-pushed the matpes_static branch from 38b8195 to a1e57a0 Compare August 19, 2023 15:33

SophiaRuan and others added 9 commits August 21, 2023 10:10

Merge branch 'materialsproject:master' into matpes_static

4dec6ac

Merge branch 'materialsproject:master' into matpes_static

359be66

change formats

0cef1e3

add pymatgen/io/vasp/PBE54Base.yaml

0ca6d34

add pymatgen/io/vasp/PBE54Base.yaml

fb1cfc0

pre-commit auto-fixes

f4c0cfc

change Normal=All for R2SCAN

4ff0cd9

add PARENT to MatPESStaticSet PBE54Base

bc728ae

add test for MatPESStaticSet, fix typo in sets.py

7a2e639

janosh suggested changes Aug 24, 2023

View reviewed changes

janosh and others added 2 commits August 24, 2023 11:51

Merge branch 'master' into matpes_static

7a9931d

remove setup() in TestMatPESStaticSet, remove unnecessary incar key c…

73b523e

…hecks

JiQi535 reviewed Aug 26, 2023

View reviewed changes

Merge branch 'master' into matpes_static

5f30370

shyuep merged commit 5a1a9d2 into materialsproject:master Aug 28, 2023

SophiaRuan mentioned this pull request Aug 28, 2023

[WIP] revise MatPESStaticSet, inherit from DictSet #3278

Closed

4 tasks

JiQi535 mentioned this pull request Aug 29, 2023

Corrections for MatPESStaticSet and its tests #3280

Closed

janosh mentioned this pull request Sep 26, 2023

Add MatPES GGA and r2SCAN static makers materialsproject/atomate2#532

Merged

janosh mentioned this pull request Nov 12, 2023

Define PBE64Base.yaml for new VASP PBE_64 POTCARs #3470

Merged

Conversation

SophiaRuan commented Aug 19, 2023

Summary

Todos

Checklist

Uh oh!

Uh oh!

Uh oh!

shyuep Aug 19, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

shyuep Aug 19, 2023

Choose a reason for hiding this comment

Uh oh!

SophiaRuan Aug 21, 2023

Choose a reason for hiding this comment

Uh oh!

shyuep Aug 21, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

janosh Aug 24, 2023

Choose a reason for hiding this comment

Uh oh!

janosh Aug 24, 2023

Choose a reason for hiding this comment

Uh oh!

shyuep commented Aug 25, 2023

Uh oh!

esoteric-ephemera commented Aug 25, 2023

Uh oh!

shyuep commented Aug 25, 2023

Uh oh!

esoteric-ephemera commented Aug 25, 2023

Uh oh!

shyuep commented Aug 25, 2023 • edited by janosh Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JaGeo commented Aug 25, 2023

Uh oh!

shyuep commented Aug 26, 2023

Uh oh!

JiQi535 commented Aug 26, 2023

Uh oh!

JaGeo commented Aug 26, 2023

Uh oh!

janosh commented Aug 26, 2023

Uh oh!

JiQi535 Aug 26, 2023

Choose a reason for hiding this comment

Uh oh!

JiQi535 Aug 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shyuep commented Aug 26, 2023

Uh oh!

shyuep commented Aug 28, 2023

Uh oh!

janosh commented Aug 28, 2023

Uh oh!

shyuep commented Aug 28, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

shyuep commented Aug 25, 2023 •

edited by janosh

Loading

JiQi535 Aug 26, 2023 •

edited

Loading