search

materialsproject / custodian

A simple, robust and flexible just-in-time job management framework in Python.
MIT License
140 stars 109 forks source link

Suggestion: disable algo_tet VASP handler by default #221

Closed utf closed 1 year ago

utf commented 2 years ago

180 and #202 introduced an error handler to catch calculations with ALGO = All and ISMEAR = -5 based on a warning in the OUTCAR file.

Currently, this error handler means it is a pain to run high quality hybrid density of states (you need to create a pre-converged WAVECAR) and then run ALGO=DAMPED + tetrahedron method.

I'm curious if this error handler was added due to experience or just based on the warning itself. In my experience running hybrid of states calculations (on 300+ systems) I've never seen this warning lead to a crash.

I would suggest disabling this error message by default. If we want to keep it enabled, the fix should only be applied if the calculation actually failed and not if the calculation completes successfully as is currently the case.

@arosen93 what are your thoughts?

Andrew-S-Rosen commented 2 years ago

Hi @utf. Thanks for bringing this up. I agree that the current implementation is quite burdensome for HSE06 DOS calculations using the tetrahedron method.

I must admit, I can't recall the exact scenarios that were leading to a crash originally. I tend to work on MOFs quite a bit, so it's possible that the crash was slightly material-specific. I think it's likely I was more concerned about the methodological approach simply being invalid for hybrids.

In any case, provided this warning doesn't frequently lead to a crash (per your experience), I think it's worth reconsidering as well. My only question is: is it "okay" to use ALGO = All, ISMEAR=-5, and a hybrid calculation? The warning message is somewhat vague about if this is something that can sometimes lead to a crash or is just downright wrong to do in many cases. I tried searching the forum, but the only information I got was here, which was written by users and not the admin.

Assuming it is valid to use ALGO = All, ISMEAR=-5, and a hybrid calculation, then my suggestion would be to retain the check but only apply it if it fails per your latter suggestion.

utf commented 2 years ago

Thanks for the quick response!

My only question is: is it "okay" to use ALGO = All, ISMEAR=-5, and a hybrid calculation?

Yes definitely. As long as the calculation converges there shouldn't be any issue (bearing in mind all the usual caveats on relaxing with tetrahedron method).

my suggestion would be to retain the check but only apply it if it fails per your latter suggestion.

I think this is the best way to proceed. I'll update the error handler.

Andrew-S-Rosen commented 2 years ago

I think this is the best way to proceed. I'll update the error handler.

Great, thanks! I also was left unsatisfied with the current approach and hybrids, so this makes me much happier that it's okay to do.