-
Notifications
You must be signed in to change notification settings - Fork 68
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
Combine atomate2 vasp calc types #764
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #764 +/- ##
==========================================
- Coverage 91.22% 90.95% -0.27%
==========================================
Files 151 136 -15
Lines 11948 11723 -225
==========================================
- Hits 10899 10663 -236
- Misses 1049 1060 +11
☔ View full report in Codecov by Sentry. |
Anyone knows these more please comment and we can then move forward. |
I can't remember what this is doing. But I would be reluctant to remove it to avoid any breaking changes. |
Ready for review! |
Just checking if there has been any progress on this? Ideally we can get this merged to progress to a new atomate2 release. |
Sorry for the delay, I will take a look today and test it myself today. |
Thanks @munrojm! |
|
||
AM05 = "AM05" | ||
GGA = "GGA" | ||
PBE = "PBE" | ||
PBESol = "PBESol" | ||
PBEsol = "PBEsol" |
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.
@mjwen Why was this changed from PBEsol to PBESol? This (very minorly) breaks some of the validation code I'm working on. Also, it isn't consistent with the original PBEsol paper's capitalization (https://arxiv.org/abs/0711.0156)
Sorry that this causes trouble for you! The change was introduced when merging this and the atomate2 enums.py, where PBESol. I am OK to change it back, but we need to ensure this will not introduce any problem for atomate2. @utf Any idea on this? Will atomate2 get into trouble if we use |
Related, we also did |
This PR addresses #735. The below has been performed:
Molecular Dynamics
task typeTODO:
PB0->PBE0
and removingPBEsol
(now usingPBESol
?).elif len([x for x in kpts.get("labels") or [] if x is not None]) > 0:
inutils.py
is doing and possibly rewrite it. (@utf can give a quick answer).