aiidateam / aiida-quantumespresso

The official AiiDA plugin for Quantum ESPRESSO
https://aiida-quantumespresso.readthedocs.io
Other
53 stars 78 forks source link

`XspectraCrystalWorkChain`: Enable Symmetry Data Inputs #1028

Closed PNOGillespie closed 2 months ago

PNOGillespie commented 4 months ago

Overview

In this PR we add an input namespace for the XspectraCrystalWorkChain which allows the user to define the spacegroup and equivalent sites data for the incoming structure, thus instructing the WorkChain to generate structures and run calculations for only the sites specified. The new symmetry_data input requires entries for the system's spacegroup number (Int) and equivalent sites data (Dict). The former must simply be a valid spacegroup number (1-230) while the latter must contain an entry for each site (in the format site_<index>_<element>), where the minimal data required are the kind_name, element, site_index, and multiplicity.

In order to not be too restrictive on the user and make it easier to use external packages to build input symmetry data, we limit input validation to simple checks which ensure that the required data are present and that the data provided won't cause an obvious crash. Thus beyond the minimum required information, any extra data provided in the entries of the equivalent_sites_data node will be kept in case they may be needed by the user outside of the WorkChain (e.g. the list of symmetrically equivalent sites, which the WorkChain doesn't need).

Changes

PNOGillespie commented 4 months ago

Hi @superstar54, not sure what the issue with the docs is. If you could take a look at it (or ask someone who would know more about it), that would be much appreciated.

As always, I'm available to make changes as requested.

PNOGillespie commented 4 months ago

Hi @superstar54, thanks for the review. I've committed your suggestions, as well as a couple of other things which I noticed on taking a second look at the input validation steps.

let me know if you need anything else.

PNOGillespie commented 4 months ago

Hi @superstar54, let me know if there are any other changes that you think are necessary for this PR.

PNOGillespie commented 3 months ago

Hi @PNOGillespie , please fix the failed test, and then we can merge.

Hi @superstar54. Looking at the output from the failed test, it seems to be a more general issue with the GitHub workflow - I see exactly the same thing in the recent PR by @yakutovicha (#1029).

Unfortunately, I don't know what the problem is. When I compile the docs in my docker container, everything runs fine, but here it returns a few errors:

Do we know anyone who might be able to diagnose and fix the problem?

superstar54 commented 3 months ago

Hi @mbercx , Could you please help check why the docs failed?

PNOGillespie commented 2 months ago

Hi @superstar54 and @sphuber, thanks to both of you for the feedback. I've added @sphuber's proposed changes (as well as fixing a little formatting whoopsy that I spotted in the help string that you were pointing to).

Let me know if any other changes are needed.

sphuber commented 2 months ago

Thanks @PNOGillespie