-
Notifications
You must be signed in to change notification settings - Fork 89
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
Maybe plot non-converged bands differently #938
Comments
Thanks! Wouldn't it just make sense to only plot the converged bands? (we should possibly audit all uses of scfres for this, eg for the DOS plot also). I don't remember why we decided to keep them rather than have a set of extra bands, do you remember @mfherbst ? (This is graphene, right? This is particularly tricky because it looks like the band of interest is not bound at the gamma point. There are a bunch of parabolic bands that are free electron like in the z direction, and if you increase the size of the cell in the z direction they'll get denser.) |
(Yes this is graphene. It works well with a pz initial guess (added in #899) and with |
I don't know, actually. I guess the rational was to unify the data format during SCF and after to avoid conversion every time you checkpoint (external split format versus internal combined format) or continue an SCF from a returned scfres at a later point. Indeed we should check we do this consistently. In any case worth adding that the issue of @Technici4n is not the band plotting, since if you go via |
Actually also in many routines (e.g. DOS) it does not matter as the occupation is basically zero. |
I think it is, let me explain. 😄 I was using the default number of bands for the Wannier interface, but I set But I didn't see that DFTK.jl/src/postprocess/band_structure.jl Line 265 in e5fa7b6
In other words I thought I had enough bands because I saw all the bands I needed on the plot. |
Ah ok, but that could indeed be solved with a warning in the wannierisation as I suggest in #940. In general the SCF just computes as many bands as needed to converge the density (and thus obtain a meaningful self-consistent density). Post-processing routines might make different choices for user convenience. But I agree that the current (master) state is confusing and I can see how you were mislead. |
Hm. Maybe we should rethink that and have eg scfres return views into the full arrays with only the converged ones? But then the scfres would be inconsistent (ie scfres.rho is not the same as compute_density(scfres.psi))... We really should have an scfres struct where we could document this better... |
The warning in the wannierization doesn't hurt but it wouldn't have helped me.
Wannierization already behaves as expected IMO, the only confusing thing was the plot showing more bands than fully converged in the SCF. (I guess that makes sense because compute_bands can diagonalize the Hamiltonian with more eigenstates.) |
Hi, the bandstructure plots also contain bands that are not fully converged (if I understand
n_bands_converge
inAdaptiveBands
correctly). This can be misleading if you expect all plotted bands to be converged.To give some background:
I encountered the following issue with Wannierization, where the disentanglement step would fail to select the right band close to the Gamma point (see the red line). It turns out that I was only passing the 15 first bands to Wannier, but the expected band is the 16th. I assumed that the plot was only showing the first 15 bands because I set
n_bands_converge=15
.Would it be a good idea to plot non-converged bands a bit differently? (For example with a dotted line).
The text was updated successfully, but these errors were encountered: