[Bug]: `converged_ionic` does not account for stopped calculation

[R1j1t]

Email (Optional)

No response

Version

v2023.6.28

Which OS(es) are you using?

• [ ] MacOS
• [ ] Windows
• [X] Linux

What happened?

I have a long VASP calculation and I use STOPCAR to stop the calculation early before reaching wall time limit. Once the vasp finishes, I use the pymatgen.io.vasp.outputs.Vasprun to parse the vasprun.xml and check if the calculations converged based on that I make a decision to resubmit the calculation or not.

However, converged_ionic logic checks if the calculations reached the specified NSW steps and not respects the EDIFFG criteria.: https://github.com/materialsproject/pymatgen/blob/f4e605776b7afc6e74bd6e855f94280268aeb698/pymatgen/io/vasp/outputs.py#L619-L626

So, for my case because the calculation will be stopped early and ionic steps < specified NSW steps, the converged_ionic is always True. Is there something I am missing or is it the desired behavior?

Code snippet

vasprun = Vasprun(vasprun_path, parse_dos=False, parse_eigen=False)
if vasprun.converged:
    return # resubmission not required
else:
    # resubmit calculation

Log output

No response

Code of Conduct

• [X] I agree to follow this project's Code of Conduct

[janosh]

I agree this is misleading behavior. A PR that modifies converged_ionic to also check EDIFFG sounds like an improvement.

Though as @Andrew-S-Rosen pointed out, some calculations keep some atoms fixed in which case you may end up with large forces even though VASP is done. @shyuep @esoteric-ephemera curious to hear your thoughts. Is there some better heuristic we should use instead?

[Andrew-S-Rosen]

The fixed atoms isn't a big deal. We would compute the max force on the non-fixed atoms. Just something to be cognizant of.

In general, this seems like a change that is important but could easily break things if we aren't careful.

[R1j1t]

Similar logic is implemented in converged_electronic, it relies on NELM steps performed vs specified rather than EDIFF:

https://github.com/materialsproject/pymatgen/blob/46c71963b1c15c9bf7a7964a0908c602f7b67758/pymatgen/io/vasp/outputs.py#L509-L524

A calculation will stop at the next electronic step if STOPCAR has LABORT = .TRUE. (ref). I think the property should need to account for it.

[esoteric-ephemera]

Agree with @Andrew-S-Rosen, we should check and see how converged_ionic and converged_electronic are being used in, e.g., PMG, custodian, and atomate, to make sure we don't break expected behaviors. Custodian very likely uses this for the unconverged error handler

The "correct" check for electronic self-consistency would be to make sure that |E(IELM) - E(IELM - 1)| < EDIFF, where 1 <- IELM <= NELM is the electronic step. A calculation could converge on the NELM-th step, probably exceedingly rare though.

Similar for forces/EDIFFG, but with those caveats about full vs. partial relaxation