materialsproject / emmet

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

Potential issue with change in monty deserialization #879

Closed gpetretto closed 9 months ago

gpetretto commented 9 months ago

I realized that the recent change I made to the MontyDecoder https://github.com/materialsvirtuallab/monty/pull/580 may have a negative impact on the deserialization of the TaskDoc in some cases. The CustodianDoc contains the as_dict() of VaspJob, that is supposed to be just a dict: https://github.com/materialsproject/emmet/blob/9c755a3f8bc3c40b3a76eeece4d2bfb55ae544c8/emmet-core/emmet/core/tasks.py#L225 The change mentioned above causes two related issues: 1) the dictionary containing the VaspJob gets deserialized and thus the pydantic validation fails, as the type is wrong. 2) since the decoder tries to deserialize the dictionary, if the dictionary is incompatible with the current version of the objects (as in this test file: https://github.com/materialsproject/emmet/blob/main/test_files/vasp/Si_old_double_relax/custodian.json.gz), the entire deserialization will fail.

While the first point could be addressed by changing the type of job in CustodianDoc (would Any be an option?), I am not sure if there is a way to fix the second point without modifying (or reverting) the MontyDecoder. @munrojm, do you have any solution to this issue? Or do you think it would be better to address this in monty?

JaGeo commented 9 months ago

Hi @gpetretto, we partially fixed it in emmet already: https://github.com/materialsproject/emmet/pull/883

Currently, only the new release of atomate2 is missing.

gpetretto commented 9 months ago

Thanks!