aiidateam / aiida-quantumespresso

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

The schema needs to updated to support more disk_io settings #908

Closed unkcpz closed 1 year ago

unkcpz commented 1 year ago

When I used the nowf for pw calculation, which is now supported in pw.x https://www.quantum-espresso.org/Doc/INPUT_PW.html#idm128 I got:

Report: [357392|PwBaseWorkChain|run_process]: launching PwCalculation<357395> iteration #1
Error: 1 XML schema validation error(s) schema: /home/jyu/micromamba/envs/aiida-sssp-unstable/lib/python3.9/site-packages/aiida_quantumespresso/parsers/parse_xml/schemas/qes_220603.xsd:
Error: failed validating 'nowf' with XsdEnumerationFacets(['low', 'high']):

Reason: value must be one of ['low', 'high']

Schema:

  <enumeration xmlns="http://www.w3.org/2001/XMLSchema" value="low" />

Instance:

  <disk_io>nowf</disk_io>

Path: /{http://www.quantum-espresso.org/ns/qes/qes-1.0}espresso/input/control_variables/disk_io

It seems the options are not included in the latest schema at https://github.com/QEF/qeschemas/blob/master/PW_CPV/previous_schemas/qes_230310.xsd

The QE start support this from 6.6 https://gitlab.com/QEF/q-e/-/blob/qe-6.6/PW/Doc/INPUT_PW.html#L620

sphuber commented 1 year ago

Thanks @unkcpz . I opened a PR here: https://github.com/QEF/qeschemas/pull/17 I am still not fully sure how we should handle these bugs. I checked and all the schemas we provide only mention low and high. I am not sure if the oldest schema we provide was from QE 6.6 or later, but if so, we may just add the missing options ourselves to all schemas that we include with aiida-quantumespresso.

unkcpz commented 1 year ago

If we want to handle all such changes by ourselves, we can add just change the schema on our side in this repository (faster). But, it looks more standard if the schema in this repo is synchronized with the qeschemas, and changes should always happen in their official repo. I am not sure if the schema here is directly copied from qeschemas or if you already did some changes?

sphuber commented 1 year ago

If we want to handle all such changes by ourselves, we can add just change the schema on our side in this repository (faster). But, it looks more standard if the schema in this repo is synchronized with the qeschemas, and changes should always happen in their official repo. I am not sure if the schema here is directly copied from qeschemas or if you already did some changes?

That's the thing though. When a new version of QE is released, and so a new schema is released, we copy that schema into parsers/parse_xml/schemas and that is what is shipped with the plugin. When a job is parsed, the schema is taking from this directory. It is not dynamically fetched from some official QE repository. So even if the schema is fixed in the QEF/qeschemas repo, that should not affect our plugin. That is why we should also fix the schemas here, I don't see another solution. It is just that this feels "wrong", but for now there is no better solution I think. I am just afraid that in this way, there is some way we can get out of sync. But maybe there is no risk. The whole situation just feels very unclear and a bit fragile.

unkcpz commented 1 year ago

I agree to do the fix in this repo. At the same time, let's see how fast QE people will get that fixed and we try our best to keep things synchronized. I'll make the PR.

unkcpz commented 1 year ago

Hi @sphuber, I need some help with this. Where is the code defined which QE version uses which schema XML file? I assume I only need to change those xml files.

sphuber commented 1 year ago

Where is the code defined which QE version uses which schema XML file?

This isn't defined anywhere. The XML output file produced by running the code, contains a version number of the schema it should correspond to. The PwParser extracts this version number, and based on that loads the corresponding schema from parsers/parse_xml/schemas and then validates the output XML file to that schema.

So, indeed, you just have to correc the schema files in parsers/parse_xml/schemas as I did in the PR on the schema repo: https://github.com/QEF/qeschemas/pull/17

unkcpz commented 1 year ago

thanks! Here is it is: https://github.com/aiidateam/aiida-quantumespresso/pull/911

I made the same change as https://github.com/QEF/qeschemas/pull/17 for all XML, not sure this is necessary. I also not very satisfied the test where I simply change the disk of singe fixture file. Let's move the discussion to PR.