search

materialsproject / pymatgen

Python Materials Genomics (pymatgen) is a robust materials analysis code that defines classes for structures and molecules with support for many electronic structure codes. It powers the Materials Project.
https://pymatgen.org
Other
1.45k stars 842 forks source link

MPXANESSet: COREHOLE setting inconsistent with literature #2624

Open rkingsbury opened 1 year ago

rkingsbury commented 1 year ago

Both studies (cited below) that describe the development of high-throughput XANES workflows state that the 'RPA' method was chosen for screening of the core-hole potential. However MPXANESSet.yaml uses the default 'FSR' setting.

The ML paper (Zhenget al. 2018) seems to indicate that RPA gave more accurate results, and according to the user guide this is the preferred method if using reciprocal-space calculations.

I personally do not have a strong opinion one way or another, but if MPXANESSet is going to deviate from what is in the publications, I think it would be good to document that somewhere. The other settings in the yaml file appear to be generally consistent with the benchmarking work described in the studies.

I propose that we either revise the .yaml to use the RPA method, or add something to the docstrings to explain when FSR is preferable.

https://www.nature.com/articles/sdata2018151#citeas

COREHOLE: The COREHOLE card is used for specifying how the core is treated during XANES calculation. The default choice in FEFF treats the core-hole interaction based on the Final State Rule (FSR), which could overestimate the strength of the core-hole and excludes the core-hole mixing effect24. To overcome these deficiencies and avoid possible break down of FSR for the L-shell metals25, the random phase approximation (RPA) is used to approximate the core-hole interactions in our high-throughput K-edge XANES calculations.

https://www.nature.com/articles/s41524-018-0067-x#citeas (in Supporting Information Fig 3B)

We use RPA as the default core-hole treatment setting.

The FEFF10 user guide states

image

rkingsbury commented 1 year ago

Ah, I have just noticed that there is logic in FEFFDictSet that changes COREHOLE to RPA when a reciprocal space calculation is invoked.

https://github.com/materialsproject/pymatgen/blob/2a88940b8a4a1eb84e0077dce934765caada9395/pymatgen/io/feff/sets.py#L202

In my opinion, there is still value in considering a change to the .yaml value to minimize confusion for other users. I know that I (and I would suspect many others) look at the .yaml files as a clear, quick synopsis of what calculation settings are used by different input sets, so if it's likely that COREHOLE would be overwritten in many cases of practical interest, I'd advocate changing the .yaml to reflect the more likely scenario.

We could always reverse the logic in FEFFDictSet if it is desired to default to FSR for real-space calculations (although from what I can tell it's not clear that FSR is superior to RPA in this case; rather that RPA is definitely preferred for k-space calcs).

matthewcarbone commented 1 year ago

@rkingsbury I agree with everything you're saying here. I think though what's most important is that the default settings and behavior be clearly exposed in the documentation. They're currently not, as they're buried in the MPXANESSet.yaml files and related. As I mentioned in #2611 I think this really needs to change. I am happy to implement those PR's if I get approval, then it will be much easier to specify the default behaviors and clarify these confusions.