Fix PP upload widget

[edan-bainglass]

Motivated by #1299

This PR aims to make the pseudopotential uploaders compatible with the various changes that came after its initial implementation.

Logic

• Rejected uploads (accompanied by a dedicated notification for each)
  • Cases
    • Invalid UPF
    • Wrong element
    • Valid upload that shares a filename with an existing one (in the DB)
  • Behavior
    • No change to the UI/model
• Accepted uploads (again, with clear notifications)
  • Cases
    • Identical md5 (replaces upload with existing entry from DB)
    • Valid upload
  • Behavior
    • Set functional/library/family to None
    • Show permanent warning regarding the custom state, the need for functional consistency, and how to reset to defaults

Comments

• In all cases, cutoffs remain consistent (i.e., max across all pseudos, regardless if custom or default)
• For custom pseudos that do not belong to a family, we use upf-tools to parse the UPF file and extract the relevant info
• As long as uploading was successful, we go into a custom state (even if the upload defaulted to a built-in pseudo - rare!)

Included changes to magnetization settings

The magnetization panel was dependent on the pseudo family to extract Z values for magnetization-to-moment conversion. To support custom pseudos (family set to None), the dependency was replaced by one on the pseudos dictionary. In the conversion process, Z values are now extracted from the UpfData node associated with the UUID in the dictionary, which supports successfully uploaded custom pseudos. Pinging @t-reents due to #1252 to particularly have a look at this part 🙏

Regarding functional consistency

Ideally, we would do better than just inform the user of this need. Unfortunately, functional information is not consistently provided. For example, the functional entry in the built-in `` pseudo is Functional: SLA PW PSX PSC, which is the template, i.e., the pseudopotential author did not bother to change it. For this reason, there is no consistent information we can rely on to determine a given pseudo's functional. Suggestions welcomed 🙏 Pinging @giovannipizzi

Note that we would ideally want this to prevent calculations from crashing due to the inconsistency. This is also true across plugins, e.g., core-hole potentials for XPS/XAS. We have a system of blockers in place, just no reliable information to act on 🥲 Pinging @superstar54

Dependencies

• upf-tools for handling custom pseudos
• aiida-quantumespresso plugin - to support custom pseudos (no family), the plugin can no longer rely on the presence of a pseudo family. This is done in this PR. Once merged and a new release is made, this PR will be updated with the dependency version bump.

[AndresOrtegaGuerrero]

Does the summary display that is a different pseudo ?

[edan-bainglass]

Does the summary display that is a different pseudo ?

Are you asking if the summary displays an uploaded pseudo?

[AndresOrtegaGuerrero]

Does the summary display that is a different pseudo ?

Are you asking if the summary displays an uploaded pseudo?

Yes, or how this logic will be handle

[edan-bainglass]

Does the summary display that is a different pseudo ?

See #1353

[codecov]

Codecov Report

❌ Patch coverage is 86.23188% with 19 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@20039d0). Learn more about missing BASE report.
⚠️ Report is 29 commits behind head on main.

Files with missing lines	Patch %	Lines
...b_qe/app/configuration/advanced/pseudos/pseudos.py	89.32%	11 Missing ⚠️
...rc/aiidalab_qe/app/configuration/advanced/model.py	42.85%	4 Missing ⚠️
...aiidalab_qe/app/result/components/summary/model.py	69.23%	4 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1348   +/-   ##
=======================================
  Coverage        ?   72.28%           
=======================================
  Files           ?       99           
  Lines           ?     7026           
  Branches        ?        0           
=======================================
  Hits            ?     5079           
  Misses          ?     1947           
  Partials        ?        0           

Flag	Coverage Δ
python-3.11	72.28% <86.23%> (?)
python-3.9	72.30% <86.23%> (?)

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.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[AndresOrtegaGuerrero]

Another test to consider , is what happens is the user sets a simulations w/o SOC , and upload a Full Relativistic pseudo , or the opposite , a SOC calculation and a non FR pseudo is uploaded, this should trigger a warning that they should match otherwise the workflow will fail

[edan-bainglass]

Another test to consider , is what happens is the user sets a simulations w/o SOC , and upload a Full Relativistic pseudo , or the opposite , a SOC calculation and a non FR pseudo is uploaded, this should trigger a warning that they should match otherwise the workflow will fail

Good idea! We can capture this relatively (😉) easily.

Regarding your error above, you won't be able to submit with custom pseudos until the plugin PR is merged.

[AndresOrtegaGuerrero]

Another test to consider , is what happens is the user sets a simulations w/o SOC , and upload a Full Relativistic pseudo , or the opposite , a SOC calculation and a non FR pseudo is uploaded, this should trigger a warning that they should match otherwise the workflow will fail

Good idea! We can capture this relatively (😉) easily.

Regarding your error above, you won't be able to submit with custom pseudos until the plugin PR is merged.
Ok , so until we also get a new aiida-qe release as well

[AndresOrtegaGuerrero]

@edan-bainglass what would occur if the user for some reason upload LDA pseudos ? is this consider or a warning of LDA not supported ?

[edan-bainglass]

@edan-bainglass what would occur if the user for some reason upload LDA pseudos ? is this consider or a warning of LDA not supported ?

The problem is that we have no consistent way of detecting the functional of a given UPF file. Though the format provides a functional field, some ignore it, leaving the template value behind. So we can't reliably extract the functional.

In general, I support adding warnings/blockers, as long as we can build them on reliable information.

[edan-bainglass]

@edan-bainglass what would occur if the user for some reason upload LDA pseudos ? is this consider or a warning of LDA not supported ?

The problem is that we have no consistent way of detecting the functional of a given UPF file. Though the format provides a functional field, some ignore it, leaving the template value behind. So we can't reliably extract the functional.

In general, I support adding warnings/blockers, as long as we can build them on reliable information.

I think for now, we can at least add a comment regarding unsupported functionals, but it would be on the user to verify since we can't auto detect.

@AndresOrtegaGuerrero mind drafting a clear warning to the user regarding functional consistency, relativistic treatment, etc.? I'll then split it accordingly, depending on what the user selects in the app (e.g., SOC).

[AndresOrtegaGuerrero]

what is the logic behind the suggested cut-offs ?

[AndresOrtegaGuerrero]

@edan-bainglass what would occur if the user for some reason upload LDA pseudos ? is this consider or a warning of LDA not supported ?

The problem is that we have no consistent way of detecting the functional of a given UPF file. Though the format provides a functional field, some ignore it, leaving the template value behind. So we can't reliably extract the functional.
In general, I support adding warnings/blockers, as long as we can build them on reliable information.

I think for now, we can at least add a comment regarding unsupported functionals, but it would be on the user to verify since we can't auto detect.

@AndresOrtegaGuerrero mind drafting a clear warning to the user regarding functional consistency, relativistic treatment, etc.? I'll then split it accordingly, depending on what the user selects in the app (e.g., SOC).

When uploading a pseudopotential, please ensure that you select the appropriate exchange-correlation functional supported by the application. For spin–orbit coupling (SOC) calculations, make sure to upload a fully relativistic pseudopotential. Additionally, remember to adjust the plane-wave cutoff energy to ensure convergence.

[edan-bainglass]

what is the logic behind the suggested cut-offs ?

Hasn't changed. Still the max across all pseudos. Per pseudo, I try to parse the cutoffs. Each gets 0 if I can't parse it. Then the suggested final cutoffs are the max of each "column"

[AndresOrtegaGuerrero]

I also verified that uploading the same pseudopotential again triggers the warning. Additionally, I tested uploading a pseudopotential with a mismatched element, and the corresponding warning also displayed.

[edan-bainglass]

When uploading a pseudopotential, please ensure that you select the appropriate exchange-correlation functional supported by the application. For spin–orbit coupling (SOC) calculations, make sure to upload a fully relativistic pseudopotential. Additionally, remember to adjust the plane-wave cutoff energy to ensure convergence.

This is great! I would just maybe not say "select the appropriate exchange-correlation functional", as they may think we mean select PBE or PBEsol in the app. Instead, maybe we can say "upload a pseudopotential with an exchange-correlation functional that is supported by the application (e.g., PBE, PBEsol)"

[AndresOrtegaGuerrero]

LGTM! Nice work

[giovannipizzi]

Thanks a lot @edan-bainglass!!

Just one thing. QE is able to check consistency of the functionals, so the information must be there.
Also, I double checked and the string Functional: SLA PW PSX PSC is not a template, but the fully expanded definition of PBEsol (that is just a commonly used shortcut). Check here.

For the functionals we need for the app, the two to check are:

  !              "pbe"   = "sla+pw+pbx+pbc"    = PBE
  !              "pbesol"= "sla+pw+psx+psc"    = PBEsol

Note that there are many alternatives in the pseudos, both depending on the UPF version, and on how it's written.
In the v1 (I think), I always find (for SSSP PBEsol), in the <PP_HEADER> block, the string

SLA  PW   PSX  PSC     PBEsol  Exchange-Correlation functional

(5th line, I think, and I think the part from PBEsol onwards is just a comment).

In what I think is v2 I find instead functional=" SLA PW PSX PSC" (with different spacing) but sometimes also functional="PBESOL". So both need to be checked.
Similarly for PBE.

Hope this is helpful (for the exact logic, if we don't want to reverse engineer, we would need to check the Fortran logic, but I think this is already good for most pseudos and we can fine-tune later.

[edan-bainglass]

Thanks @giovannipizzi. Yeah, this might be enough for now. Let me see what I can do.