Enable direct reading of UPF version >= 2 PP files

electronic-structure / SIRIUS

Domain specific library for electronic structure calculations

BSD 3-Clause "New" or "Revised" License

115 stars 40 forks source link

Enable direct reading of UPF version >= 2 PP files #980

Closed abussy closed 2 months ago

abussy commented 3 months ago

This PR enables the direct reading of standard pseudopotential UPF files with version >= 2, without the need for the upf_to_json tool. The pugixml library is used for this purpose.

All functions reading information from JSON PP files, namely Atom_type::read_pseudo_uspp and Atom_type::read_pseudo_paw have been overloaded with function reading from xml files (stepwise identical). The relevant function is called depending on the file termination in the SIRIUS input file.

This PR is fully backward compatible. At this stage, the dependency on the pugixml library was made a hard requirement. This would imply a change in the spack recipe for the next release.

Edit: at this stage, it is an open question whether pugixml should be a hard requirement or an option.

abussy commented 3 months ago

The pugixml dependency was made optional with the addition of a spack variant +pugixml. Whether pugixml is enabled or not, json potential files can be parsed. This PR is fully backward compatible.

Note: for use of pgixmp with CP2K, the CP2K spack recipe (and probably the CP2K toolchain) will need to be adapted. Moreover, the CP2K code will need a slight refactorization, as it currently assumes that SIRIUS only reads JSON files, and it falls back to its own (incomplete) UPF v2 parser otherwise: https://github.com/cp2k/cp2k/blob/2ed94b8b96b2a4590f5340063d84c5d639a6fefe/src/atom_upf.F#L120-L127.

simonpintarelli commented 3 months ago

Just as a comment: SSSP currently still contains many UPFv1 pseudos (GRBV), but in the future it will be all UPFv2 🎉 . The remaining GRBV UPFv1 will be converted to v2 using https://github.com/azadoks/PseudoPotentialIO.jl.

simonpintarelli commented 3 months ago

Edit: at this stage, it is an open question whether pugixml should be a hard requirement or an option.

Imho as long as QE parses the UPFs, pugixml should be a variant. The question is if we want to rely on QE to parse UPFs in q-e-sirius or not. At least until a future SSSP is released only containing UPFv2, we should keep it as an optional dependency.

simonpintarelli commented 3 months ago

Adding a test under verification would be useful too. E.g. first generate an output_ref.json using the native json format and then run the same input using UPF files directly.

simonpintarelli commented 3 months ago

cscs-ci run default