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

Add eddiag error handling and fix AMIN error handling in VaspErrorHandler #302

Merged
merged 12 commits into from
Nov 14, 2023

Conversation

esoteric-ephemera
Copy link
Contributor

@esoteric-ephemera esoteric-ephemera commented Nov 6, 2023

Key additions:

  • Add handling of new eddiag errors, which have a message that is more generic than the zheev error message (indicates that any of ZHEEV/ZHEEVX/DSYEV/DSYEVX failed)
  • Fix handling of AMIN errors (unit cells with at least one dimension greater than 50 Å) when calculation hasn't failed
  • Unit tests (x2) for both previous
  • Ensure that custodian does not induce algo_tet errors by switching to ALGO in [ALL, DAMPED] when ISMEAR < 0
  • Updated test files (unconverged/vasprun.xml*) to not use tetrahedron smearing (ISMEAR = -5) to allow for ALGO update checking

@esoteric-ephemera esoteric-ephemera marked this pull request as draft November 6, 2023 17:04
Copy link
Member

Choose a reason for hiding this comment

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

This looks like an accidental addition?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes it's Dropbox's way of resolving internet failures

@codecov-commenter
Copy link

codecov-commenter commented Nov 6, 2023

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (141209b) 65.35% compared to head (3ec4eb2) 65.56%.
Report is 1 commits behind head on master.

Files Patch % Lines
custodian/vasp/handlers.py 80.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #302      +/-   ##
==========================================
+ Coverage   65.35%   65.56%   +0.20%     
==========================================
  Files          54       54              
  Lines        5536     5546      +10     
==========================================
+ Hits         3618     3636      +18     
+ Misses       1918     1910       -8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@esoteric-ephemera esoteric-ephemera marked this pull request as ready for review November 6, 2023 19:02
@@ -548,6 +548,38 @@ def test_read_error(self):
assert dct["errors"] == ["read_error"]
assert dct["actions"] is None

def test_amin(self):
# Cell with at least one dimension >= 50 A, but AMIN > 0.01, and calculation not yet complete
shutil.copy("INCAR.amin", "INCAR")
Copy link
Member

Choose a reason for hiding this comment

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

This overwrites the existing INCAR that was copied to tmp_path by setUP which might cause the downstream test failure.

@janosh janosh merged commit 53b031c into materialsproject:master Nov 14, 2023
2 checks passed
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.

3 participants