-
Notifications
You must be signed in to change notification settings - Fork 24
Description
This class has grown too convoluted and with contradictory standards in indexing. It should be streamlined, better commented and made more consistent.
Check Riccardo's comments in issue yambo-code/yambopy-devel#43:
Hi, I add a deeper look into this, and I was able to reproduce the problem.
I have few comments on the routines related to this plot in the YamboQPDB class.
In this file, the band index counting start from 1 instead of 0 and this is creating quite few inconsistencies. First, we should set a convention if we want in the tutorial file n_top_vb to be 4 (case a) or 3 (case b). If I understood correctly, in this example we have 4 valence bands and 4 conduction bands. Therefore, it makes sense to set n_top_vb=4. Note that self.min_band=1 and self.min_band=8 (instead of 0 and 7 as usual in Python counting).
get_direct_gaps : this function should indeed give the direct gaps, and it does not currently work for case b) due to this counting issue. A solution could be to replace the line shifted_valence = valence-self.min_band+1 with shifted_valence = valence-self.min_band+2.
plot_scissor_ax: this function has been changed. Before we were plotting the points as given from the linear relationship. y = mx +q. Now instead, we are plotting the "qp" energies against the dft ones. However, I don't think it makes sense to plot the difference between the 'qp' value and the '0' one, as in ax.plot(v0, vqp-v0). A possible solution would be to plot ax.scatter(ce0,cqp-cintercept) or ax.scatter(ce0,cqp). Regarding case a) and b), in this function we have to be particularly careful in the calls for get_filtered_qps. Depending on the convention that we choose, the calls to get_filtered_qps would have to change accordingly.
Please, give me your comments and thoughts so that we can proceed accordingly to the set convention.
Anyway, I forked my branch and can fix the error, but it is not clear to me how we should handle bug-fixes in the GPL release. Should I clone it and create merge requests?
Originally posted by @rreho in https://github.com/yambo-code/yambopy-devel/issues/43#issuecomment-1628494250