QEF / qeschemas

4 stars 8 forks source link

fix of string parsing in qes_220601.xsd and current_master schemas #14

Closed pietrodelugas closed 1 year ago

pietrodelugas commented 2 years ago

fixes regular expressions in 220601 and qes_current_master (see issue #13)

pietrodelugas commented 2 years ago

Sebastiaan, @sphuber, could you review the changes? Also merge if you wish. Thanks!!

sphuber commented 2 years ago

Thanks a lot @pietrodelugas , I ran a very basic SCF calculation with the updated [qes_220601.xsd](https://github.com/QEF/qeschemas/pull/14/files#diff-aee5e76b1d0b4ead33245d26a74fc64490cd2f3eba27d9e98acf68dcaec8be07) schema and the parsing error is gone. Should we maybe also rename the file itself to qes_220603.xsd for consistency of the date?

pietrodelugas commented 2 years ago

Yes, we can do that. It was just that I like a lot that you parse the XML file to decide which schema to use. I am sorry to make you change that policy because of this mistake, even because the format prescribed by the two versions is the same; the correction is only in the syntax of the regular expressions that check for it.

sphuber commented 2 years ago

Just for my understanding: where was the mistake, in the schema definition or in the output written by QE v7.0? That is to say, is 220601 the correct date and QE is accidentally writing the wrong date 220603 or is it the other way around?

I ran two examples, one with 7.0 and one with 7.1. For 7.0 the XML contains:

<?xml version="1.0" encoding="UTF-8"?>
<qes:espresso xsi:schemaLocation="http://www.quantum-espresso.org/ns/qes/qes-1.0 http://www.quantum-espresso.org/ns/qes/qes_210716.xsd" Units="Hartree atomic units" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:qes="http://www.quantum-espresso.org/ns/qes/qes-1.0">
  <!--All quantities are in Hartree atomic units unless otherwise specified-->
  <general_info>
    <xml_format NAME="QEXSD" VERSION="21.11.01">QEXSD_21.11.01</xml_format>
    <creator NAME="PWSCF" VERSION="7.0">XML file generated by PWSCF</creator>

For 7.1 the XML contains:

<?xml version="1.0" encoding="UTF-8"?>
<qes:espresso xsi:schemaLocation="http://www.quantum-espresso.org/ns/qes/qes-1.0 http://www.quantum-espresso.org/ns/qes/qes_220603.xsd" Units="Hartree atomic units" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:qes="http://www.quantum-espresso.org/ns/qes/qes-1.0">
  <!--All quantities are in Hartree atomic units unless otherwise specified-->
  <general_info>
    <xml_format NAME="QEXSD" VERSION="21.11.01">QEXSD_21.11.01</xml_format>
    <creator NAME="PWSCF" VERSION="7.1">XML file generated by PWSCF</creator>
    <created DATE="25Nov2022" TIME="10:58:13">This run was terminated on:  10:58:13  25 Nov 2022</created>

There are two places where the schema version is defined:

In both versions they are inconsistent. We currently parse the version from the xsi:schemaLocation URL. But maybe we should be using the VERSION attribute?

I think for 7.0, the actual schema file that should be used is qes_211101.xsd. Is this correct? In any case, currently we have an exception for this version of QE to use qes_211101.xsd even though the schema location says qes_210716.xsd

For 7.1 we parse and therefore use qes_220603.xsd even though the VERSION says 21.11.01.

It would be great if we can decide on the right approach and try to enforce consistency in future versions of QE. Then I can update aiida-quantumespresso with the correct schema filenames and schemas.

pietrodelugas commented 1 year ago

Hi @sphuber yes it's correct, there were a couple of things that were overlooked. I will add some tests in the repository in order so to check that every-thing is in order. very sorry for answering this late here.
In any case this afternoon I will accept l request and then proceed with the testing and new version just after. It would be great if you ( or somebody else from the AiiDA group) could help me reviewing and checking the changes.