-
Notifications
You must be signed in to change notification settings - Fork 88
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
Fix prev_dir
behavior in input set generator of MPGGAStaticMaker
#996
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
glad you caught this @Andrew-S-Rosen. we absolutely need a regression test. this is as a big a gotcha as they come.
maybe we want to pass inherit_incar=False
here as well to be explicit. it already defaults to False
but that's reliant on a default value two parent classes away in a different repo. better be safe and repeat
atomate2/src/atomate2/vasp/jobs/mp.py
Line 58 in 9600cef
default_factory=lambda: MPRelaxSet(force_gamma=True, auto_metal_kpoints=True) |
@janosh: I was also wondering about that --- I had to dig through classes to see what value Definitely agree about the regression test. I wish I could be more help on this. If nobody is able to chip in, I'll see if I can carve out some time in the next week or two. |
@janosh: Under-promise and over-delivering over here --- I added a regression test for the However, what is concerning is that there does not appear to be any pre-existing tests for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
excellent, thanks a lot @Andrew-S-Rosen! 👍
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #996 +/- ##
==========================================
+ Coverage 4.38% 76.01% +71.62%
==========================================
Files 174 174
Lines 12671 12690 +19
Branches 1890 1892 +2
==========================================
+ Hits 556 9646 +9090
+ Misses 12081 2506 -9575
- Partials 34 538 +504
|
Would it be possible to get a bugfix release after this PR? I want to reduce potential issues with users trying to reproduce MP settings and getting conflicting results, especially given how hard it is to track down... |
v0.0.17 should be released shortly @Andrew-S-Rosen. Thanks for reporting this. |
Closes #995.