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

Fix potcar checking #972

Merged
merged 5 commits into from
Mar 25, 2024
Merged

Conversation

esoteric-ephemera
Copy link
Collaborator

In a previous PR, I set the "titel" field of the POTCAR spec to pymatgen.io.vasp.inputs.PotcarSingle.TITEL. However, there is an almost-always identical header attr on PotcarSingle which should be used in the spec instead

On the builders side, this is needed for emmet.builders.utils.get_potcar_stats method, which is used to validate calculations. The header attr of PotcarSingle is now used in this function. I've also added the option to load a dict of pre-defined POTCAR specs with this function, and added such a dict for MP calcs

Added tests (two that don't rely on a POTCAR library being established) for get_potcar_stats

For backwards compatibility reasons, I don't think we can change the titel attr of emmet.core.vasp.calculation.PotcarSingle to be header, but maybe we can simply alias it for now and eventually deprecate it. This notation is confusing / not correct

@codecov-commenter
Copy link

codecov-commenter commented Mar 25, 2024

Codecov Report

Attention: Patch coverage is 0% with 10 lines in your changes are missing coverage. Please review.

Project coverage is 88.61%. Comparing base (98b75bf) to head (49b3dc3).
Report is 2 commits behind head on main.

Files Patch % Lines
emmet-core/emmet/core/vasp/validation.py 0.00% 10 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #972      +/-   ##
==========================================
- Coverage   90.05%   88.61%   -1.44%     
==========================================
  Files         138      109      -29     
  Lines       13204    10130    -3074     
==========================================
- Hits        11891     8977    -2914     
+ Misses       1313     1153     -160     

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

@munrojm
Copy link
Member

munrojm commented Mar 25, 2024

This looks great, thanks @esoteric-ephemera ! Happy to merge unless @tsmathis has anything to say.

@tsmathis
Copy link
Collaborator

tsmathis commented Mar 25, 2024 via email

@munrojm munrojm merged commit 93353a9 into materialsproject:main Mar 25, 2024
10 checks passed
@esoteric-ephemera esoteric-ephemera deleted the potcar branch May 21, 2024 20:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants