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 support for output format of pw.x v6.6 #552

Merged
merged 1 commit into from
Sep 14, 2020

Conversation

sphuber
Copy link
Contributor

@sphuber sphuber commented Sep 2, 2020

Fixes #551

Quantum ESPRESSO pw.x v6.6 not only comes with a new schema for the XML
output file, which is added here such that it can be parsed, it also has
changes to the standard output format. In a simple SCF calculation for a
silicon structure as a test, one change turned out to be problematic.
The line containing the Harris-Foulkes estimate that used to be printed
directly after the computed total energy has been removed. This line
itself was never parsed anyway, but the estimated SCF accuracy that is
printed afterwards was, and the parser relied on it being two lines
after the total energy. The parser is now adapted to, starting from the
total energy line, scout 5 lines ahead for the line containing the
marker of the estimate SCF accuracy. This once more shows how we should
not rely on parsing textual output.

Quantum ESPRESSO pw.x v6.6 not only comes with a new schema for the XML
output file, which is added here such that it can be parsed, it also has
changes to the standard output format. In a simple SCF calculation for a
silicon structure as a test, one change turned out to be problematic.
The line containing the Harris-Foulkes estimate that used to be printed
directly after the computed total energy has been removed. This line
itself was never parsed anyway, but the estimated SCF accuracy that is
printed afterwards was, and the parser relied on it being two lines
after the total energy. The parser is now adapted to, starting from the
total energy line, scout 5 lines ahead for the line containing the
marker of the estimate SCF accuracy. This once more shows how we should
not rely on parsing textual output.
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.

Thanks! Looks good to me.
@mbercx I don't remember if you wanted to do more tests here?
Otherwise for me it's ready to merge, could you merge it?

@mbercx
Copy link
Member

mbercx commented Sep 10, 2020

@mbercx I don't remember if you wanted to do more tests here?

I just wanted to do a little test run for qe v6.6 myself, but ran into some issues compiling the code on my laptop. I'll see if I can fix that and (squash and 😉) merge.

@mbercx
Copy link
Member

mbercx commented Sep 14, 2020

LGMT! 🚀

@mbercx mbercx merged commit fe75c80 into develop Sep 14, 2020
@mbercx mbercx deleted the fix/551/add-support-new-xml-format branch September 14, 2020 13:49
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.

Add support for new XML schema of QE v6.6
3 participants