XAS: Fix Conflict Between tot_charge Override and CH Protocol

[PNOGillespie]

Addresses #807.

Fixes a conflict between tot_charge being set explicitly in overrides by default and the XAS plugin assuming that tot_charge isn't specified in the overrides. This change should restore the previous intended behaviour. For calculations of XAS or XPS with charged systems, it is recommended to instead warn the user that such calculations should be either avoided or carried out with care.

[PNOGillespie]

@superstar54: No action needed here to keep XPS calculations working, though I would recommend adding a warning for users (either in documentation or in UI) to not do XPS calculations with charged cells.

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 3 lines in your changes missing coverage. Please review.

Project coverage is 67.09%. Comparing base (924ae5f) to head (7e57056).
Report is 112 commits behind head on main.

Files with missing lines	Patch %	Lines
src/aiidalab_qe/plugins/xas/workchain.py	0.00%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #809      +/-   ##
==========================================
- Coverage   67.10%   67.09%   -0.02%     
==========================================
  Files          51       51              
  Lines        4712     4713       +1     
==========================================
  Hits         3162     3162              
- Misses       1550     1551       +1     

Flag	Coverage Δ
python-3.11	?
python-3.9	67.09% <0.00%> (-0.05%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[superstar54]

I think the logic should be: if using FCH,

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

[PNOGillespie]

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).

[PNOGillespie]

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

[superstar54]

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

[PNOGillespie]

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.

[PNOGillespie]

Hi @superstar54. I've tried to implement something using @traitlets.observe(), but as far as I can see there is either no traitlet for the advanced settings or it's not at all clear which one corresponds to the "Advanced Settings" tab of the UI. I've tried configuration_settings and input_settings, but neither work interactively.

I see this code block in aiidalab_qe.app.configuration (lines 68-70):

        # list of trailets to link
        # if new trailets are added to the settings, they need to be added here
        trailets_list = ["input_structure", "protocol", "electronic_type", "spin_type"]

Do we need to add the advanced settings dictionary to that list as well?

[superstar54]

Hi Peter, yes, we can not link the tot_charge to the XAS/XPS setting. It needs to refactor the code to allow linking the whole advance setting (the dictionary).

You can add a trailet tot_charge in the XAS setting, and then add tot_charge to the trailets_list.

For the moment, please only update the documentation part.

[PNOGillespie]

Hi @superstar54. Do you want to finish this PR and handle the documentation in a separate one?

[PNOGillespie]

@superstar54, I've made the changes to the documentation - I decided to just add an extra section to the documentation page since this should only matter to more advanced users. Let me know if anything needs changing.

I will update the issue page after we're done here, since we still would need to add a UI element at some point (once @edan-bainglass has made progress with the back-end changes).

[superstar54]

LGTM!