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.

Sign up for GitHub

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

XAS: Fix Conflict Between tot_charge Override and CH Protocol #809

Merged
merged 5 commits into from
Oct 31, 2024
Merged

XAS: Fix Conflict Between tot_charge Override and CH Protocol #809

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
04d2e90
XAS: Fix Conflict Between `tot_charge` Override and CH Protocol
PNOGillespie Sep 24, 2024
1e698c3
Merge branch 'main' into fix/xas/tot_charge
PNOGillespie Oct 25, 2024
04e6ff4
XAS: Add Documentation Warning for Charged Systems
PNOGillespie Oct 25, 2024
86f6a6a
Merge branch 'main' into fix/xas/tot_charge
superstar54 Oct 28, 2024
7e57056
Merge branch 'main' into fix/xas/tot_charge
superstar54 Oct 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions docs/source/howto/xas.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,19 @@ Finally, click on the "Download CSV" button to the upper left of the plot area t
* The normalised & weighted spectrum. (with respect to ratio of site multiplicity to total multiplicity)
* The normalised & un-weighted spectrum.

Additional Note on Charged Systems
----------------------------------
Computing XANES spectra for systems where a charge has been applied (in this case using the `Total charge` advanced setting) is possible using the tools
available in the QE App, however such workflows should always be tested by the user against experimental data if possible.

When running XAS workflows for systems where a total charge has been applied, it is suggested to use the following settings for the total charge to ensure the corresponding
core-hole treatment is applied correctly:

* "xch_fixed" or "xch_smear": Set the total charge as required for the system's charged ground-state.
* "full": **Increase** the total charge by 1 *relative* to the system's charged ground-state (e.g. set total charge = 2 in the advanced settings tab if the charge is normally 1).

Note that for neutral systems (total charge = 0), the QE App will handle these settings automatically.

Summary
-------

Expand Down
7 changes: 6 additions & 1 deletion src/aiidalab_qe/plugins/xas/workchain.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ def update_resources(builder, codes):
def get_builder(codes, structure, parameters, **kwargs):
from copy import deepcopy

adv_parameters = deepcopy(parameters["advanced"])
# Setting `tot_charge = 0` will cause FCH calculations to fail due to
# inputs being incorrect, thus we pop this from the overrides
if adv_parameters["pw"]["parameters"]["SYSTEM"].get("tot_charge") == 0:
adv_parameters["pw"]["parameters"]["SYSTEM"].pop("tot_charge")
Comment on lines +29 to +30
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the logic should be: if using FCH,

adv_parameters["pw"]["parameters"]["SYSTEM"]["tot_charge"] += 1

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this was my thinking originally, on the basis that (nominally) FCH screens the core-hole by adding a +1 charge to the system, so it would be logical to simply do tot_charge += 1. However, there are two things to bear in mind:

  1. There aren't any cases in the literature for XAS calculations performed on already-charged systems. Everything else has been done on a system which starts out neutral (tot_charge = 0).
  2. It's safe to assume that if the user is working with the advanced options, then they should be able to play around with the options without the program interfering in a way which they cant see happening.

As said in the top comment: for charged systems, it would be better instead to let the user set things as they wish and advise them via the documentation (which I will update later when I get the time).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to add: It's safe (IMO) to let users run XAS with FCH alongside other properties for neutral systems, since we've tested that extensively by now, but we should instead advise users to run XAS separately for charged systems and test the behaviour if they're using FCH. For XCH, it's probably fine, but should ideally be tested anyway

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In addition to the documentation, if the tot_charge is not zero, we need to raise a warning in the XAS/XPS GUI.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, I will work on adding a warning to the GUI for XAS. In principle this should only apply to FCH XAS and molecule XPS calculations. Anything to do with XCH (XAS or XPS) should not require a serious warning, but adding a section to the documentation for XAS and XPS and pointing to the section would be the ideal solution IMO.

protocol = parameters["workchain"]["protocol"]
xas_parameters = parameters["xas"]
core_hole_treatments = xas_parameters["core_hole_treatments"]
Expand Down Expand Up @@ -61,7 +66,7 @@ def get_builder(codes, structure, parameters, **kwargs):
xs_code = codes["xspectra"]["code"]
overrides = {
"core": {
"scf": deepcopy(parameters["advanced"]),
"scf": adv_parameters,
# PG: Here, we set a "variable" broadening scheme, which actually defines a constant broadening
# The reason for this is that in "gamma_mode = constant", the Lorenzian broadening parameter
# is defined by "xgamma" (in "PLOT"), but this parameter *also* controls the broadening value
Expand Down
Loading