msg-byu / kgridGen

Generator for Generalized Regular k-point grids
6 stars 6 forks source link

Confusing comment and line of code in generateKpoints.f90 #8

Closed glwhart closed 6 years ago

glwhart commented 6 years ago
          idx = findKptIndex(roKpt, InvK, L, D)
          ! Reshift the k-point before finding its index
          roKpt = roKpt + shift
          ! write(*,'("shKpt: ",3(f6.3,1x),i3)') roKpt
          ! idx = findKptIndex(roKpt, InvK, L, D)
          ! write(*,'("Op#",1x,i2,5x,"rkpt: ",3(f6.3,1x),5x,"idx: ",i4)') iOp,roKpt,idx
          ! Is this a new addition to the orbit?
          if (hashTable(idx)==0) then
             ! write(*,'(/,"Operator:",i3)') iOp 
             ! write(*,'(3(1x,f7.3))') (SymOps(i,:,iOp),i=1,3)
             ! write(*,'("roKpt: ",3(f6.3,1x),i3)') roKpt
             ! write(*,'("Op#",1x,i2,5x,"rkpt: ",3(f6.3,1x),5x,"idx: ",i4)') iOp,roKpt,idx

             ! print *,"point added"
             hashTable(idx) = cOrbit
             ! if so increase the number of members of this orbit by 1
             iWt(cOrbit)=iWt(cOrbit)+1
          endif
       enddo
       ! write(*,'(/,"iWt: ",i4)') iWt(cOrbit)
    enddo

In the current revision (0.5.4), the preceding lines (generateKpoints.f90) are confusing. The reshifted kpoint, roKpt, is never used (so why is it reshifted?). And the comment above that says ! Reshift the k-point before finding its index but the index is never calculated. Maybe this is just left over from the printout lines below that were used for checking/debugging?

glwhart commented 6 years ago

@wsmorgan Have you had time to think about my question?

wsmorgan commented 6 years ago

@glwhart I've looked over my commits that removed write statements and nothing that I did there touched this code sequence. However, given that there are write statements in the code block that would print out roKpt if they weren't commented out I think it may be a safe assumption that it's simply left over from the debugging period.

jerjorg commented 6 years ago

The way in which we index k-points doesn't work if the grid has been shifted away from the origin. We overcame this by removing the shift from the rotated k-point, calculating its index, and then adding the shift back to the k-point. The shift also needs to be removed for the check where we verify the rotated k-point belongs to the grid. It looks like we don't need to add the shift back to the k-point since all we are interested in is its index.

glwhart commented 6 years ago

but the variable we are re-shifting, roKpt, is never used after that in the routine...

There's no purpose in re-shifting it...

glwhart commented 6 years ago

And where did zz come from in that routine?

jerjorg commented 6 years ago

That's right. There's no purpose in adding back the shift to roKpt since all we are interested in is its index. zz is a left over variable from the debugging period.

glwhart commented 6 years ago

Can you clean up the code and close this issue and ask for a pull request? Thanks