🪛 [RFC] Returning index of vbm and cbm

[ezpzbz]

Is your feature request related to a problem? Please describe. Currently, eigenvalue_band_properties does not return the kpoint and eigenvalue index for vbm and cbm. The former one is already there and just needs to be returned while the eigenvalue index requires a little tweak to be captured and returned.

Use-case: I use vaspkit to visualize the real-part of wavefunction at specific eigenvalues (more specifically vbm and cbm). The above mentioned indexes makes the life much easier by quickly getting the kpoint and eigenvalue indexes and passing them to vaspkit.

Describe the solution you'd like I'm currently using the modified version of mentioned method to get this done:

def eigenvalue_band_properties(vrun, occu_tol):
    vbm = -float("inf")
    vbm_kpoint = None
    vbm_index = None
    cbm = float("inf")
    cbm_kpoint = None
    cbm_index = None
    for spin, d in vrun.eigenvalues.items():
        for k, val in enumerate(d):
            for index, (eigenval, occu) in enumerate(val):
                if occu > occu_tol and eigenval > vbm:
                    vbm = eigenval
                    vbm_kpoint = k
                    vbm_index = index
                elif occu <= occu_tol and eigenval < cbm:
                    cbm = eigenval
                    cbm_kpoint = k
                    cbm_index = index
    return max(cbm - vbm, 0), cbm, vbm, vbm_kpoint, cbm_kpoint, vbm_index, cbm_index

,where in the third for loop, I enumerate the val and record the index.

Describe alternatives you've considered I have not.

Additional context I have opened this issue to initiate discussion on this feature and request for comments. If this makes sense, I would be happy to open a PR to address this.

Cheers, Pezhman

[shyuep]

I have no objections to this really, but I would ask that you add a kwarg such as "include_kpoint_and_index=False". By default, the return will still be the same. If that kwarg is set to true, the kpoint and index will be returned as per your suggestion. That will ensure that the method itself retains backward compatibility with code already out there using it.

[ezpzbz]

I totally agree that this is a better approach @shyuep.