-
Notifications
You must be signed in to change notification settings - Fork 109
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix breaking change to ISMEAR=-5 handling for NKPT<4 #336
Conversation
@esoteric-ephemera Can you review this? Thanks. |
@esoteric-ephemera: You may prefer #337 instead since it fixes the handling for "tet" but doesn't alter your changes to "dentet". Let me know your thoughts. |
@shyuep will do - we're chatting about solutions and we'll post updates here later Just to get a few initial thoughts down here:
@Andrew-S-Rosen is right that our fix is insufficient. For both cases, the k-point density should be increased until it exceeds 4 k-points (fixes I would avoid changing the smearing method as a last resort, since the energies you get from tetrahedron (fully variational) will necessarily be more accurate than those of e.g., gaussian smearing (non-variational in the total energy, variational in the pseudo-free energy) |
There are some strong counterpoints to be made about modifying k-points, however. Consider one of the most common cases where you get 1x1x1 k-points: a huge structure. If you have some structure with hundreds of atoms per cell, you simply cannot afford to increase the k-points to more than 1x1x1. It would not be logical, both because you'd lose the benefits of the gamma-point only version of VASP and because the cell might be so large that increasing the number of k-points might be a huge computational burden. If we are going to play with modifying the total number of k-points, it needs to be done in such a way that computational cost is taken into account. For instance, we should be seeing how much the k-points per volume changes when having at least 4-kpoints; you don't want to do this if you're doing some huge 10x change or something. Another example where this comes up is with systems that have vacuum space introduced, like a surface slab. Regardless, it's clear the "tet" and "dentet" error handling needs to be split up so we can address those differently since only the former is strictly related to NKPT < 4. |
Sure, for the case of a single k-point in any direction it doesn't always make sense to bump up the k-point density. The safest assumption there is that the user is trying to simulate a lack of PBC in that direction and not adjust it In the case of KSPACING being too large (for either When using KPOINTS, we can increase the number of k-points on axes where the number of k-points > 1 until the number of k-points (pre-symmetry reduction) is > 4 In the previous two cases, changing smearing would be a fallback If all three axes have one k-point, then we just change smearing first |
Just for full clarity, this isn't generally true. Consider a MOF for example. It is a periodic crystal modeled under PBCs like usual, yet you rarely want or need more than 1x1x1 k-point. In any case, I think your logic seems reasonable on initial read. |
@esoteric-ephemera and I got on a call and have sorted things out. We are going to close this PR in favor of #337. Further review and any changes by @esoteric-ephemera will happen there, although the PR does most of what we ultimately need. |
Closes #335. See the tagged issue report for full details.
In short, this PR reverts the breaking changes made in #284 related to handling ISMEAR = -5 for NKPT<4. The change in #284 does not work and even if it did, it would not be the wisest course of action. I also split up the "tet" and "dentet" error handling because those are two separate errors that can, in principle, be handled differently.