Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PwParser: add smearing_type and smearing from new XML output #445

Merged
merged 1 commit into from
Apr 1, 2020

Conversation

sphuber
Copy link
Contributor

@sphuber sphuber commented Nov 16, 2019

Fixes #415

These keys were not yet being parsed from the output file with the new
XML schema. In addition, if an exception is thrown by the raw XML or
stdout parser, it is now caught and added as a critical warning. Finally
the test input generator for PwParser is simplified and only a single
generator is used for both scf and relax tests.

@sphuber sphuber force-pushed the fix_415_pw_parser_smearing_type branch 4 times, most recently from 87fb1c6 to ef43055 Compare November 19, 2019 21:02
Comment on lines 317 to 328
xml_data['degauss'] = smearing_xml['@degauss']
xml_data['smearing_type'] = smearing_xml['$']
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this is one of the first cases where we are really changing the behaviour w.r.t. the old parser, right?

  • Can we add already now a list of things that changed between version 2 and 3 (or even more precisely, that depend on how QE was compiled)? Possibly the list is already there, we should then just add also this

  • I don't see any unit conversion - normally, in the parsed output, we try to use consistently eV for energy

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have started a list in issue #384 . The units are not converted because it is not clear what they are. I sent an email for this to Delugas and put you in CC. The docs and file itself say it should be Hartree, but it is clearly Rydberg.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, let's add this to the issue, and let's wait for the units (or add conversion from Rydberg since that's what they are and if they fix this, this will be only for future versions)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I performed a calculation for 6.4, 6.4.1 and 6.5 and only 6.5 has the correct units. So for now I have just added the correct unit conversion in the parser. Everything 6.5 and above is assumed Hartree, for all version below I assume it is Rydberg. So @giovannipizzi this should be ready for review

@sphuber sphuber force-pushed the fix_415_pw_parser_smearing_type branch 6 times, most recently from 477d079 to cb3bf01 Compare March 26, 2020 15:07
@sphuber sphuber force-pushed the fix_415_pw_parser_smearing_type branch from cb3bf01 to dac35e4 Compare March 27, 2020 11:28
These keys were not yet being parsed from the output file with the new
XML schema. In addition, if an exception is thrown by the raw XML or
stdout parser, it is now caught and added as a critical warning. Finally
the test input generator for `PwParser` is simplified and only a single
generator is used for both scf and relax tests.

Note that the units of the `degauss` depends on the version of the XML
output. It should always be in Hartree, but up until 6.5, it was
erroneously being written in Rydberg.
@sphuber sphuber force-pushed the fix_415_pw_parser_smearing_type branch from dac35e4 to 0012a1f Compare March 27, 2020 12:27
Copy link
Member

@giovannipizzi giovannipizzi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just please add in the commit message the QE commit ID where the units change has been done a9698a6971e4ad975ce89a15bc9b74294799ef3f

@sphuber sphuber merged commit 1c8e240 into develop Apr 1, 2020
@sphuber sphuber deleted the fix_415_pw_parser_smearing_type branch April 1, 2020 11:12
@greschd greschd mentioned this pull request Apr 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Smearing type not always present in XML output
2 participants