emmet icon indicating copy to clipboard operation
emmet copied to clipboard

[WIP] TaskDoc refactor

Open esoteric-ephemera opened this issue 2 years ago • 3 comments

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:

  • The fields of TaskDocument are deserialized, thus TaskDocument.calcs_reversed[:].input will be a dict whereas TaskDocument.calcs_reversed[:].input is a CalculationInput object. Calling .model_dump() doesn't help since this isn't recursive (e.g., the structure in CalculationInput will still be serialized)
  • Added a BaseCalculationModel to emmet.core.calculation which just wraps the pydantic BaseModel with a .get method to emulate dict key access (needed in the ValidationDoc)
  • Added a few fields to add missing attrs from TaskDocument: calc_type, run_type, and a post init update for task_type
  • Because the TaskDoc accepts extra model input, and the way that pydantic looks for non-field attrs in the class, there are new private fields defined on TaskDoc. pydantic first looks for attrs in the model extra field, but then raises an AttributeError if it can't find it, without looking to see if hasattr(class,attr). There's a longer explanation in a comment near _calc_type

To Do:

  • Remove atomate dependence from emmet-cli: VaspCalcDb --> generic maggma store
  • Improve credential handling in emmet-cli settings

esoteric-ephemera avatar Mar 20 '24 18:03 esoteric-ephemera

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
@@            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.

codecov-commenter avatar Mar 20 '24 19:03 codecov-commenter

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

mkhorton avatar Apr 16 '24 03:04 mkhorton

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

esoteric-ephemera avatar Apr 16 '24 15:04 esoteric-ephemera

merging as rc

tsmathis avatar May 21 '24 23:05 tsmathis