materialsproject / emmet

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

TaskDoc refactor #971

Closed esoteric-ephemera closed 4 months ago

esoteric-ephemera commented 6 months ago

Summary

The main goal is removing atomate dependencies from emmet, and transitioning from using emmet.core.vasp.task_valid.TaskDocumement to emmet.core.tasks.TaskDoc. This will allow for eventual deprecation / aliasing of TaskDocument

Completed:

Changes to TaskDoc and its fields are required since it is not directly compatible with TaskDocument:

Will be part of a separate PR: ~## To Do:~ ~- Remove atomate dependence from emmet-cli: VaspCalcDb --> generic maggma store~ ~- Improve credential handling in emmet-cli settings~

codecov-commenter commented 6 months ago

Codecov Report

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

Project coverage is 77.11%. Comparing base (d9d7d21) to head (7ab775f).

Files Patch % Lines
emmet-core/emmet/core/vasp/validation.py 6.66% 14 Missing :warning:
emmet-core/emmet/core/utils.py 50.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #971 +/- ## ========================================== - Coverage 77.24% 77.11% -0.14% ========================================== Files 75 75 Lines 4329 4339 +10 ========================================== + Hits 3344 3346 +2 - Misses 985 993 +8 ```

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

mkhorton commented 5 months ago

If there are breaking changes to TaskDoc, eg for current users of atomate2, could a migration script be provided?

esoteric-ephemera commented 5 months ago

Hi @mkhorton, there shouldn't be breaking changes to TaskDoc, I've just had to add fields/methods to it for improved compatibility with both atomate2 and TaskDocument

A good example is the TaskDoc.task_type field, which actually corresponds to TaskDocument.calc_type in the atomate2 schema. The calc_type field is the union of TaskDocument.task_type and TaskDocument.run_type (a CalcType)

For compatibility with atomate2, TaskDoc.task_type still accepts CalcType as input, but for backwards compatibility with TaskDocument, there are new calc_type and run_type fields on TaskDoc

I presume that the TaskDoc.task_type field was intended to reduce some of the redundancy of these three fields, but some glue code was needed for full backwards compatibility. These added fields won't be used in atomate2

Apologies in advance for the long-winded answer

tsmathis commented 4 months ago

merging as rc