search

materialsproject / emmet

Be a master builder of databases of material properties. Avoid the Kragle.
https://materialsproject.github.io/emmet/
Other
49 stars 63 forks source link

Fix potcar checking #972

Closed esoteric-ephemera closed 3 months ago

esoteric-ephemera commented 3 months ago

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 commented 3 months ago

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 :warning:
Additional details and impacted files ```diff @@ 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 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

munrojm commented 3 months ago

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

tsmathis commented 3 months ago

Looks good on my end. Glad we’re getting the checks ironed out.

On Mon, Mar 25, 2024 at 1:06 PM Jason Munro @.***> wrote:

This looks great, thanks @esoteric-ephemera https://github.com/esoteric-ephemera ! Happy to merge unless @tsmathis https://github.com/tsmathis has anything to say.

— Reply to this email directly, view it on GitHub https://github.com/materialsproject/emmet/pull/972#issuecomment-2018817187, or unsubscribe https://github.com/notifications/unsubscribe-auth/AIPH7AC4CCZWKA723PTUNJ3Y2B7VBAVCNFSM6AAAAABFHQR3ZSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMJYHAYTOMJYG4 . You are receiving this because you were mentioned.Message ID: @.***>