Update rVV10 parameters for r2SCAN+rVV10 in MPScanRelaxSet() #4504
Conversation
-Add the code to update rVV10 parameters ("BPARAM"=11.95) for r2SCAN+rVV10 if vdw is set to "rvv10" and METAGGA is "R2SCAN"
|
Hey @junchichen21 can you take a look at how this is already done in MP24RelaxSet and perhaps merge the incar updates into a common function for both the older |
|
Also, unittests are needed. Thanks. |
Hi @esoteric-ephemera, I somehow missed your message last week. Sorry about that. No problem, I will update it soon. |
…Set for the given XC functional and dispersion correction
…eters; streamline vdw handling with _config_updates function.
…ling with _config_updates function
…correction validation
…n version compatibility
…ove dispersion handling
|
Hi @esoteric-ephemera and @shyuep, I have made some updates as shown below: Refactoring and code unification:
API and interface changes:
Validation and error handling improvements:
Test updates and additions:
Please let me know whether the above refactoring and interface changes align with the intended update policy. |
|
Looks ok to me. @esoteric-ephemera To review. |
mkhorton
left a comment
mkhorton
left a comment
There was a problem hiding this comment.
Thanks @junchichen21 for this. Added a brief review just to make this more robust/future-proof.
@esoteric-ephemera if you don't have any objections to this will merge after these changes.
| try: | ||
| config_updates |= rvv10_params_by_xc[xc] | ||
| except KeyError: | ||
| raise ValueError( |
There was a problem hiding this comment.
This line can be removed: as of VASP 6.4.0 you can use rVV10 with other functionals including GGAs. PBE is explicitly supported with a BPARAM of 10.
There was a problem hiding this comment.
May be safer to also add "IVDW_NL": 2 too, as well as "LASPH": True (the latter should already be set, but should also be harmless to specify just in case).
There was a problem hiding this comment.
Maybe raising a warning instead if a GGA is requested? Still see lots of works with older VASP versions
| if xc not in d4_pars: | ||
| raise ValueError(f"D4 parameters for XC functional '{xc_functional}' are not defined yet.") | ||
| pars = d4_pars[xc] | ||
| config_updates |= {"IVDW": 13, **{f"VDW_{k}": v for k, v in pars.items()}} |
There was a problem hiding this comment.
We probably want some kind of guard here: if setting e.g. D4, other keys like LUSE_VDW if already present should be un-set.
In principle if a user had already set some INCAR parameters manually, then this function is called, it would be possible that the INCAR file would either end up with contradictory or ambiguous settings.
There was a problem hiding this comment.
so maybe something like this at the start of the function:
# Clean existing tags
for tag in ("LUSE_VDW", "BPARAM", "CPARAM", "IVDW", "VDW_S6", "VDW_S8", "VDW_A1", "VDW_A2", "IVDW_NL"):
if tag in incar:
val = incar.pop(k)
# possibly warn user of ambiguity too
| CONFIG = _load_yaml_config("MPRelaxSet") | ||
|
|
||
|
|
||
| def _config_updates( |
There was a problem hiding this comment.
Should probably call this function something like _set_dispersion_correction
|
Sorry I missed this will look today and give comments |
| "pbesol": {"S6": 1.0, "S8": 1.71885698, "A1": 0.47901421, "A2": 5.96771589}, | ||
| } | ||
| if xc not in d4_pars: | ||
| raise ValueError(f"D4 parameters for XC functional '{xc_functional}' are not defined yet.") |
There was a problem hiding this comment.
Can you expand this line to refer to the dftd4 code where all possible functional params are defined:
https://github.com/dftd4/dftd4/blob/30aa423f3ca90f21789c3a89a0ad4b464081290e/src/dftd4/param.f90#L353
This is also where I took the PBEsol params from
|
I will wait for @mkhorton and @esoteric-ephemera 's comments to be resolved first. Thanks. |
4 participants
This pull request updates the
MPScanRelaxSetclass insrc/pymatgen/io/vasp/sets.pyto improve support for the r2SCAN+rVV10 functional. It adds a new reference for r2SCAN+rVV10 and ensures that the appropriateBPARAMvalue is set in the INCAR when using this functional.Functional and documentation enhancements:
__post_init__method to automatically setBPARAM = 11.95in the INCAR whenvdwis set to"rvv10"andMETAGGAis"R2SCAN". If the user specifiesuser_incar_settings={"METAGGA": "SCAN"}, thenBPARAMkeeps the same value as defined in thevdW_parameters.yamlfile.