[WIP] MP-compatible r2SCAN workflow (and a few general INCAR improvements) #362

Andrew-S-Rosen · 2023-06-02T20:12:48Z

Closes #352. Closes #363. Closes #364.

Port SCAN workflows to atomate2 #352
BUG: LMAXTAU should be auto-set when there are f electrons and LASPH is True for meta-GGA calculations #363
BUG: LMAXMIX should be set based on the presence of s, p, d, f electrons not based on Z #364

R2SCAN Workflow

Note: This is in close collaboration with @janosh and @esoteric-ephemera.

This PR adds MP-compatible r2SCAN workflows to Atomate2 (#352). We are still adding tests for now, hence the WIP.

VASP INCAR Improvements

I also baked in fixes for several VASP INCAR issues I brought up (#363, #364, #365).

Review Request

@utf: Mind giving this a quick look to make sure we didn't do anything drastically wrong before we go and write all the tests? Thanks!

Also tagging @munrojm, @rkingsbury, and @mkhorton in case they're interested in following this.

ToDo

Finish adding unit tests
Only copy the CHGCAR (not all the files) from the PBEsol job to the R2SCAN job in order to remain truthful to the original workflow.

…not needed

…project

codecov · 2023-06-02T20:36:15Z

Codecov Report

Merging #362 (dd673cf) into main (59f47ee) will decrease coverage by 10.35%.
Report is 2 commits behind head on main.
The diff coverage is 57.57%.

❗ Current head dd673cf differs from pull request most recent head 233b976. Consider uploading reports for the commit 233b976 to get more accurate results

Additional details and impacted files

@@             Coverage Diff             @@
##             main     #362       +/-   ##
===========================================
- Coverage   75.09%   64.75%   -10.35%     
===========================================
  Files          74       76        +2     
  Lines        6570     7271      +701     
  Branches      952      958        +6     
===========================================
- Hits         4934     4708      -226     
- Misses       1330     2259      +929     
+ Partials      306      304        -2

Files Changed	Coverage Δ
src/atomate2/common/jobs/defect.py	`83.48% <0.00%> (-2.23%)`	⬇️
src/atomate2/vasp/flows/core.py	`90.59% <ø> (ø)`
src/atomate2/vasp/sets/core.py	`81.59% <0.00%> (ø)`
src/atomate2/vasp/flows/mp.py	`35.13% <35.13%> (ø)`
src/atomate2/vasp/sets/base.py	`69.39% <52.94%> (-0.09%)`	⬇️
src/atomate2/vasp/jobs/mp.py	`82.05% <82.05%> (ø)`
src/atomate2/cp2k/schemas/task.py	`82.72% <100.00%> (ø)`
src/atomate2/vasp/jobs/base.py	`90.00% <100.00%> (-6.56%)`	⬇️
src/atomate2/vasp/jobs/elastic.py	`78.31% <100.00%> (-3.74%)`	⬇️

... and 33 files with indirect coverage changes

munrojm · 2023-06-02T20:39:06Z

This is great! Looking forward to having access to it.

…s/phonons.py

wrong key user_incar_settings -> INCAR and raises ruamel/yaml/comments.py:881: in update self._ok.add(*kw.keys()) E TypeError: set.add() takes exactly one argument (3 given)

for consistency with MPPreRelaxMaker

…elaxMaker was passed to __init__, needs to be passed to make()

…ests just test_MP2023RelaxMaker_default_values() for now

janosh · 2023-06-03T02:31:15Z

Linking materialsproject/pymatgen#2325 as a TODO as suggested by @arosen93 so we don't forget about it.

Andrew-S-Rosen · 2023-06-03T02:33:04Z

Note to self while typing on mobile: this might be a swap to add in base.py of vasp/sets.

Edit: Just patched it in!

rkingsbury

This is fantastic, thanks so much @arosen93 and @janosh! I have a postdoc starting in about a month who will likely want to make use of this, too. I left a few minor comments but overall it looks good to me (with the caveat that I am still not terribly familiar with atomate2 VASP flows in general).

One enhancement to consider - in Figure 3 of the mixing scheme paper (below), we describe different levels of calculation coverage, and note that sometimes (for large structures) it is useful / necessary to just do a static r2SCAN calculation. This gives you an energy that allows you to apply the mixing scheme even though the structure itself is probably too large to relax with r2SCAN.

If MP wants to adopt this strategy, I suggest breaking the 2nd relaxation into two Job - a static calculation followed by a relaxation. So the overall workflow would be:

PBESol relax --- CHGCAR----> r2SCAN static ----CHGCAR+WAVECAR----> r2SCAN relax

You could pass everything (CHGCAR and WAVECAR ) on to the final relaxation so it shouldn't cost much additional time, but this would give you the flexibility to stop after the static calc if you have large structures.

Just food for thought. The argument against this is that it will deviate slightly from what is described in the workflow paper, creating potential for confusion.

src/atomate2/vasp/jobs/mp.py

src/atomate2/vasp/flows/mp.py

src/atomate2/vasp/sets/BaseMPR2SCANRelaxSet.yaml

fix bandgap doc str in the wrong place

utf

Hi @janosh, I've added some comments.

One potential reason for the tests failing could be that module imports for the makers is not inside the test functions themselves. This is necessary for the monkey patching to work properly. I added a comment about this with more details.

src/atomate2/vasp/flows/mp.py

utf · 2023-08-17T14:59:07Z

src/atomate2/vasp/flows/mp.py

+    """
+
+    name: str = "MP GGA Relax"
+    initial_maker: BaseVaspMaker | None = field(default_factory=MPGGARelaxMaker)


I think we should add a MPDoubleRelaxMaker and just use that here instead. This will be in keeping with the other non-mp workflows (and also for Abinit and CP2K).

What do you think about having a more versatile mp_compat_converter() function that takes in any existing atomate2 Maker and applies a config_dict of the user's choosing which defaults to src/atomate2/vasp/sets/BaseMPGGASet.yaml?

I think that would be a nice feature. I think I suggested that at some point. Although I think for the canonical MP workflows we should define them explicitly. I image the function you describe (I think it should be a powerup) would be available for other people to convert non-MP workflows into MP compatible ones. If we use it for defining the canonical workflows it will be a bit confusing for users reading the code trying to understand what is going on.

src/atomate2/vasp/flows/mp.py

utf · 2023-08-17T15:09:54Z

src/atomate2/vasp/sets/mp.py

+        dict
+            A dictionary of updates to apply.
+        """
+        return {}


You need to set these properly so that it will configure the necessary settings for a relaxation. E.g., see here:

atomate2/src/atomate2/vasp/sets/core.py

Line 71 in 18d014a

return {"NSW": 99, "LCHARG": False, "ISIF": 3, "IBRION": 2}

Although if using the subclassing approach detailed in my previous comment this shouldn't be necessary.

utf · 2023-08-17T15:10:36Z

src/atomate2/vasp/sets/mp.py

+        rmin = 25.22 - 2.87 * bandgap
+        kspacing = 2 * np.pi * 1.0265 / (rmin - 1.0183)
+        return {
+            "KSPACING": np.clip(kspacing, 0.22, 0.44),


Again, this is missing the tags to actually make this a relaxation if continuing from a static calculation.

src/atomate2/vasp/sets/mp.py

utf · 2023-08-17T15:17:45Z

tests/vasp/flows/test_mp.py

+from atomate2.vasp.flows.mp import MPMetaGGARelax
+from atomate2.vasp.jobs.mp import (
+    MPMetaGGARelaxMaker,
+    MPPreRelaxMaker,
+)
+


All imports need to be within the tests themselves otherwise it breaks the tests. This is mentioned at the bottom of this page but perhaps we need to make this more clear elsewhere: https://materialsproject.github.io/atomate2/dev/vasp_tests.html

I tried moving all imports except pytest into the test function yesterday which made no difference, tests were failing either way.

@utf

Added generic 3-step flow to avoid duplication of code. New / modified WFs: - MPGGARelax now permits GGA and GGA + U calcs via kwarg to maker (previously only did GGA + U) - MPGGADoubleRelaxMaker and MPMetaGGADoubleRelaxMaker per request from @utf. These are the same as MPGGARelax and MPMetaGGARelax excluding final statics

Ensure pre-relax always uses Gaussian smearing for better generality

utf · 2023-08-18T20:45:45Z

src/atomate2/vasp/flows/mp.py

+
+
+@dataclass
+class _MPGenericThreeStep(Maker):


Personally, I would be against defining the flows like this. It is quite complicated for users trying to understand what is going on. I don't feel that there is much boilerplate in a DoubleRelax + Static Maker.

I've rewritten these: the hidden generic maker has been replaced by MPGGADoubleRelaxMaker. This maker features an optional final static, which is disabled by default. The other flows (MPGGARelax, MPMetaGGARelax, and MPMetaGGADoubleRelaxMaker) all inherit from MPGGADoubleRelaxMaker. Let us (cc. @janosh) know if these are more what you had in mind, @utf?

@utf

Updating MP flows consistent with feedback from @utf: - Removed hidden generic maker, replaced this with MPGGADoubleRelaxMaker. This maker has an optional final static which is disabled by default - All subsequent flows inherit from MPGGADoubleRelaxMaker - MPGGARelax = MPGGADoubleRelaxMaker + final GGA static - MPMetaGGADoubleRelaxMaker = MPGGADoubleRelaxMaker with meta-GGA params - MPMetaGGARelax = MPMetaGGADoubleRelaxMaker + final meta-GGA static

field has no args, just kwargs. testing threw this error: `TypeError: field() takes 0 positional arguments but 1 was given`

Iterate over makers and not jobs when applying +U kwarg (job has no attr input_set_generator, but the maker itself does)

Correct ordering of application of GGA + U tag

…grows larger than 8.78 eV

janosh · 2023-08-31T13:59:16Z

@arosen93 @esoteric-ephemera I just noticed kspacing jumps from its max value of 0.44 for large band gaps down to 0.22 if the band gap exceeds 8.78 eV where we actually want it to stay at 0.44. Could you confirm the fix in 4bf816a?

esoteric-ephemera · 2023-08-31T18:23:53Z

It should work correctly now. Also setting GGA = None here is still unfortunately overriden by the previous INCAR, so we should update it in the relax and static meta-GGA makers through user_incar_settings

I'm not sure if the following is more transparent, basically defining the kspacing as a discontinuous piecewise function (KSPACING = 0.22 for gap < bandgap_tol, but 0.26 for gap = bandgap_tol):

def kspacing_from_gap(bandgap,
    bandgap_tol = 1.e-4,
    min_kspacing = 0.22,
    max_kspacing = 0.44
):
    
    rmin_intcpt = 25.22
    rmin_slope = - 2.87
    rmin_b = 1.0183
    rmin_a = 2*np.pi*1.0265

    max_bandgap = (rmin_a/max_kspacing + rmin_b - rmin_intcpt)/rmin_slope

    updt_pars = {"KSPACING": 0.44, "ISMEAR": 0, "SIGMA": 0.05, "GGA": None}
    
    if bandgap < bandgap_tol:
        updt_pars = {"KSPACING": min_kspacing, "ISMEAR": 2, "SIGMA": 0.2, "GGA": None}
    elif bandgap < max_bandgap - bandgap_tol:
        rmin = rmin_intcpt + rmin_slope * bandgap
        updt_pars['KSPACING'] = rmin_a / (rmin - rmin_b)
    else:
        updt_pars['KSPACING'] = max_kspacing

    return updt_pars

utf · 2023-08-31T18:26:04Z

Rather than setting things in the user_incar_settings, can you set them in the actual get_incar_updates function on the input set generators themselves. I made some comments about it on the PR which should still be visible.

janosh · 2023-10-02T16:37:26Z

@utf #504 and this PR were mostly overlapping, i.e. only only should be merged. I think we need to revert this merge.

utf · 2023-10-02T16:39:56Z

Ok, are you able to handle the unmerging?

janosh · 2023-10-02T16:42:30Z

Yes, do you prefer a revert commit or rewriting git history like it never happened? The latter would require people who pulled main in the last 20 minutes to do a git reset --hard @{u}.

utf · 2023-10-02T16:48:45Z

Can you overwrite the history? Should be ok I think.

Andrew-S-Rosen and others added 12 commits June 2, 2023 12:53

Initial attempt at MP workflows

0d30189

First attempt

892c911

Remove MAGMOM.yaml

2dab928

drop added **kwargs from RelaxSetGenerator.get_incar_updates() since …

230e504

…not needed

black .

56b0fa2

Merge remote-tracking branch 'origin/materialsproject' into materials…

aceeb11

…project

Remove unused function

a7a3b8f

Fix __all__

7e47522

Remove sets/mp.py

c08f549

Fix ISMEAR and SIGMA

18c0d20

Fix imports

0003b28

More robust KSPACING

01f2000

Andrew-S-Rosen and others added 9 commits June 2, 2023 13:54

Fix linting

9323435

Fix linting

1fcff17

fix ruff

6692dfe

mv tests/vasp/flows/test_phonon{,s}.py to match module names vasp/job…

5927eed

…s/phonons.py

fix self.input_set_generator.config_dict["user_incar_settings"].update()

493b4c6

wrong key user_incar_settings -> INCAR and raises ruamel/yaml/comments.py:881: in update self._ok.add(*kw.keys()) E TypeError: set.add() takes exactly one argument (3 given)

rename prerelax_maker to pre_relax_maker

cea4d28

for consistency with MPPreRelaxMaker

fix MPRelaxR2SCANGenerator arg type

4eb86b5

fix passing bandgap estimate for kspacing from MPPreRelaxMaker to MPR…

c7bcb82

…elaxMaker was passed to __init__, needs to be passed to make()

add module tests/vasp/flows/test_mp.py for soon-to-come MP workflow t…

c98cf7c

…ests just test_MP2023RelaxMaker_default_values() for now

janosh mentioned this pull request Jun 2, 2023

Port SCAN workflows to atomate2 #352

Closed

rkingsbury reviewed Jun 3, 2023

View reviewed changes

src/atomate2/vasp/jobs/mp.py Outdated Show resolved Hide resolved

src/atomate2/vasp/jobs/mp.py Outdated Show resolved Hide resolved

src/atomate2/vasp/flows/mp.py Outdated Show resolved Hide resolved

src/atomate2/vasp/sets/BaseMPR2SCANRelaxSet.yaml Show resolved Hide resolved

janosh added 2 commits June 3, 2023 07:13

add BaseVaspMaker.make() return type and doc str

cbab233

add bandgap_tol: float = 1e-4 to MPRelaxMaker.make()

9374e93

fix bandgap doc str in the wrong place

janosh mentioned this pull request Jun 3, 2023

Add forcefield schemas/makers to atomate2 #322

Merged

janosh added 2 commits August 16, 2023 17:55

check_poscar() print paths and contents of inconsistent POSCARs

876ff08

update reference files

1f130fc

utf reviewed Aug 17, 2023

View reviewed changes

janosh and others added 6 commits August 17, 2023 09:39

add copy_vasp_files to MP[Meta]GGARelax doc str

2493567

rename MP[Meta]GGARelax->MP[Meta]GGADoubleRelaxStatic,

46d661f

rename MP[Meta]GGARelax{''->Set}Generator

1793bc3

add bandgap_tol bandgap_override to MPMetaGGARelaxSetGenerator doc str

25f4c5d

Update BaseMPR2SCANRelaxSet.yaml

15e301b

Ensure pre-relax always uses Gaussian smearing for better generality

utf reviewed Aug 18, 2023

View reviewed changes

esoteric-ephemera added 2 commits August 18, 2023 16:45

Update MP jobs

ba9c1dc

field has no args, just kwargs. testing threw this error: `TypeError: field() takes 0 positional arguments but 1 was given`

Andrew-S-Rosen mentioned this pull request Aug 22, 2023

Move inputsets to pymatgen? #486

Closed

esoteric-ephemera and others added 3 commits August 23, 2023 11:29

Update MP flow

208d92f

Iterate over makers and not jobs when applying +U kwarg (job has no attr input_set_generator, but the maker itself does)

Update mp flow

b99e47b

Correct ordering of application of GGA + U tag

fix kspacing falsely jumps from 0.44 (expected) to 0.22 when bandgap …

4bf816a

…grows larger than 8.78 eV

utf removed the flows label Sep 1, 2023

janosh added 4 commits September 1, 2023 14:16

Merge branch 'main' into materialsproject

9121371

fix typo (inital->initial)_relax_maker

4a2c5f4

improve check_kpoints() inconsistent err msg

c8e10e2

add tests/vasp/jobs/test_mp.py

233b976

janosh mentioned this pull request Sep 1, 2023

Materials Project GGA and meta-GGA workflows #504

Merged

utf merged commit 233b976 into materialsproject:main Oct 2, 2023
2 of 6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] MP-compatible r2SCAN workflow (and a few general INCAR improvements) #362

[WIP] MP-compatible r2SCAN workflow (and a few general INCAR improvements) #362

Andrew-S-Rosen commented Jun 2, 2023 •

edited by janosh

codecov bot commented Jun 2, 2023 •

edited

munrojm commented Jun 2, 2023

janosh commented Jun 3, 2023

Andrew-S-Rosen commented Jun 3, 2023 •

edited

rkingsbury left a comment

utf left a comment

utf Aug 17, 2023

janosh Aug 17, 2023

utf Aug 18, 2023

utf Aug 17, 2023

utf Aug 17, 2023

utf Aug 17, 2023 •

edited by janosh

janosh Aug 17, 2023

utf Aug 18, 2023

esoteric-ephemera Aug 18, 2023 •

edited by janosh

janosh commented Aug 31, 2023

esoteric-ephemera commented Aug 31, 2023

utf commented Aug 31, 2023

janosh commented Oct 2, 2023

utf commented Oct 2, 2023

janosh commented Oct 2, 2023

utf commented Oct 2, 2023



		@dataclass
		class _MPGenericThreeStep(Maker):

[WIP] MP-compatible r2SCAN workflow (and a few general INCAR improvements) #362

[WIP] MP-compatible r2SCAN workflow (and a few general INCAR improvements) #362

Conversation

Andrew-S-Rosen commented Jun 2, 2023 • edited by janosh

R2SCAN Workflow

VASP INCAR Improvements

Review Request

ToDo

codecov bot commented Jun 2, 2023 • edited

Codecov Report

munrojm commented Jun 2, 2023

janosh commented Jun 3, 2023

Andrew-S-Rosen commented Jun 3, 2023 • edited

rkingsbury left a comment

Choose a reason for hiding this comment

utf left a comment

Choose a reason for hiding this comment

utf Aug 17, 2023

Choose a reason for hiding this comment

janosh Aug 17, 2023

Choose a reason for hiding this comment

utf Aug 18, 2023

Choose a reason for hiding this comment

utf Aug 17, 2023

Choose a reason for hiding this comment

utf Aug 17, 2023

Choose a reason for hiding this comment

utf Aug 17, 2023 • edited by janosh

Choose a reason for hiding this comment

janosh Aug 17, 2023

Choose a reason for hiding this comment

utf Aug 18, 2023

Choose a reason for hiding this comment

esoteric-ephemera Aug 18, 2023 • edited by janosh

Choose a reason for hiding this comment

janosh commented Aug 31, 2023

esoteric-ephemera commented Aug 31, 2023

utf commented Aug 31, 2023

janosh commented Oct 2, 2023

utf commented Oct 2, 2023

janosh commented Oct 2, 2023

utf commented Oct 2, 2023

Andrew-S-Rosen commented Jun 2, 2023 •

edited by janosh

codecov bot commented Jun 2, 2023 •

edited

Andrew-S-Rosen commented Jun 3, 2023 •

edited

utf Aug 17, 2023 •

edited by janosh

esoteric-ephemera Aug 18, 2023 •

edited by janosh