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

replace check for potcar hash with check for potcar summary stats #966

Closed tsmathis closed 3 months ago

tsmathis commented 3 months ago

The function .get_potcar_hash() has been removed from the PotcarSingle class in pymatgen. As a stop gap while we are migrating to new validation practices, we need to still be able to verify that the potcars used for a calculation match the pymatgen input set.

The proposed change from @esoteric-ephemera is to now check the potcar summary stats.

tsmathis commented 3 months ago

@esoteric-ephemera , any further comments/details you have would be helpful

tsmathis commented 3 months ago

tests for emmet-core/tests/vasp/ need to be updated as well once the potcar checking is finalized

codecov-commenter commented 3 months ago

Codecov Report

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

Project coverage is 90.03%. Comparing base (183d74c) to head (960f19b). Report is 3 commits behind head on main.

Files Patch % Lines
emmet-core/emmet/core/vasp/validation.py 47.05% 9 Missing :warning:
emmet-builders/emmet/builders/utils.py 68.75% 5 Missing :warning:
...met-builders/emmet/builders/vasp/task_validator.py 71.42% 2 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #966 +/- ## ========================================== + Coverage 90.01% 90.03% +0.02% ========================================== Files 138 138 Lines 13164 13182 +18 ========================================== + Hits 11850 11869 +19 + Misses 1314 1313 -1 ```

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

esoteric-ephemera commented 3 months ago

We discussed this a bit and decided that for now, we'll maintain the legacy hash checking when the summary_stats kwarg isn't populated in a TaskDoc / TaskDocument. This field will be populated when parsing a VASP output directory, but is not currently populated in most (probably all) of the MP tasks.

Submitted a PR to this branch to add legacy checking as a fallback when the summary_stats kwarg isn't populated for any single potcar_spec.