materialsproject / atomate2

atomate2 is a library of computational materials science workflows
https://materialsproject.github.io/atomate2/
Other
167 stars 94 forks source link

BUG: validation error for RunStatistics #15

Closed Andrew-S-Rosen closed 2 years ago

Andrew-S-Rosen commented 2 years ago

Describe the bug Given the attached OUTCAR, I get

~/software/miniconda/envs/cms2/lib/python3.8/site-packages/atomate2/vasp/schemas/calculation.py in from_outcar(cls, outcar)
    208             "cores": "cores",
    209         }
--> 210         return cls(**{v: outcar.run_stats.get(k) or 0 for k, v in mapping.items()})
    211
    212

~/software/miniconda/envs/cms2/lib/python3.8/site-packages/pydantic/main.cpython-38-x86_64-linux-gnu.so in pydantic.main.BaseModel.__init__()

ValidationError: 1 validation error for RunStatistics
cores
  value is not a valid integer (type=type_error.integer)

The Atomate2 job then fails as a result. This is on the current version of Atomate2.

To Reproduce

from pymatgen.io.vasp import Outcar
from atomate2.vasp.schemas.calculation import RunStatistics
out = Outcar('OUTCAR.txt')
RunStatistics().from_outcar(out)

OUTCAR.txt

Solution

This is related to VASP 6.2.0+. First, as seen in the attached OUTCAR, sometimes VASP reports N/A for average memory used, and so Pymatgen converts this to None. The workaround solution in Atomate2 is to accept either float or None (if that's not being done so already). The second issue regarding the number of cores can be fixed upstream in Pymatgen. I opened a PR here: https://github.com/materialsproject/pymatgen/pull/2308

utf commented 2 years ago

Hi @arosen93. I feel a little guilty. I've fixed this bug and a few others in a branch I'm developing on: https://github.com/materialsproject/atomate2/tree/electron-phonon

Just so you know, there is another serious bug in the main branch, whereby changes to user_incar_settings sometimes don't get applied when the job runs.

Hopefully, this branch will stabilise within a day and then I'll merge into the main branch.

Sorry for the inconvenience!

Andrew-S-Rosen commented 2 years ago

Hi @utf, not a problem! I know that things are still being worked on. Good to know!

Edit: By the way, I noticed the user_incar_settings issue in the main branch. Haven't had an issue while testing out your electron-phonon branch! Feel free to close this issue when you get the branch merged.

utf commented 2 years ago

Fixed in main branch.