Skip to content

Fix Vasprun not interpreting float overflow as nan #3452

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

Merged
merged 4 commits into from
Nov 8, 2023
Merged

Fix Vasprun not interpreting float overflow as nan #3452

merged 4 commits into from
Nov 8, 2023

Conversation

tawe141
Copy link
Contributor

@tawe141 tawe141 commented Nov 3, 2023

VASP will sometimes print "***********" when there's a float overflow. There's already a function in here to interpret these as np.nan, but it looks like it wasn't applied everywhere. I found one place where it wasn't used (ie just using Python's float function) that helped me process a large batch of vasprun.xmls correctly, but maybe the same function can be used elsewhere.

@Andrew-S-Rosen
Copy link
Member

Good to see you around these parts, @tawe141!!!

Copy link
Member

@janosh janosh left a comment

Choose a reason for hiding this comment

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

Thanks @tawe141! This should go along with a new test that applies _parse_calculation to a file containing a VASP overflow.

@janosh janosh added needs testing PRs that are not ready to merge due to lacking tests io Input/output functionality fix Bug fix PRs vasp Vienna Ab initio Simulation Package labels Nov 5, 2023
@tawe141
Copy link
Contributor Author

tawe141 commented Nov 8, 2023

Added test and confirmed it passes on my end!

pre-commit-ci bot and others added 2 commits November 8, 2023 17:06
introduce artificial overflow in <varray name="forces"> in vasprun.xml.sc_overflow
rm tests/files/vasprun.xml.force_overflow
@janosh janosh removed the needs testing PRs that are not ready to merge due to lacking tests label Nov 8, 2023
Copy link
Member

@janosh janosh left a comment

Choose a reason for hiding this comment

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

Thanks a lot for fixing @tawe141! 👍

@janosh janosh enabled auto-merge (squash) November 8, 2023 17:22
@janosh janosh merged commit 52d6b8c into materialsproject:master Nov 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix Bug fix PRs io Input/output functionality vasp Vienna Ab initio Simulation Package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants