search

dipc-cc / hubbard

Python tools for mean-field Hubbard models
https://dipc-cc.github.io/hubbard/
GNU Lesser General Public License v3.0
21 stars 8 forks source link

Fix DOS plot #115

Closed sofiasanz closed 2 years ago

sofiasanz commented 2 years ago

Previously we were expanding the PDOS on a grid as if it was a charge density (using sisl.DensityMatrix to expand the PDOS on the realspace grid) but this was incorrect. With this fix now sisl.Wavefunction is used to expand the wavefunction on the grid and then we square the grid to the power of 2 to obtain the PDOS resolved in realspace.

zerothi commented 2 years ago

Erhm. I think the notation here is somewhat misleading, or perhaps I am just too siesta like.

Generally people talk about PDOS as something projected onto orbitals etc. However, a wavefunction projection is more like a local DOS, LDOS. So I guess this depends on what you want?

tfrederiksen commented 2 years ago

Thanks for the fix!

Regarding the wording I agree with @zerothi that when we are on the realspace grid we should talk about LDOS. I am a little confused about mode="charge" vs mode="density". Maybe this could be made more concise?

sofiasanz commented 2 years ago

Regarding the wording I agree with @zerothi that when we are on the realspace grid we should talk about LDOS.

OK! Thanks for the tip :) I changed PDOS for LDOS.

I am a little confused about mode="charge" vs mode="density". Maybe this could be made more concise?

Any suggestions? I chose those words to match the charge plots and the density of states, but I'm open to other ideas :)

tfrederiksen commented 2 years ago

We should also revise the class LDOSmap which seems to suffer from the same issue.

sofiasanz commented 2 years ago

We should also revise the class LDOSmap which seems to suffer from the same issue.

The class LDOSmap doesn't use the routines from sisl to build the real-space grid.

tfrederiksen commented 2 years ago

We should also revise the class LDOSmap which seems to suffer from the same issue.

The class LDOSmap doesn't use the routines from sisl to build the real-space grid.

OK, you are right! But perhaps the function name is a bit misleading because it does not produce LDOS in the usual sense. Instead it shows the integrated probability density resolved along a given direction in space. Maybe it should be called something else?

sofiasanz commented 2 years ago

OK, you are right! But perhaps the function name is a bit misleading because it does not produce LDOS in the usual sense. Instead it shows the integrated probability density resolved along a given direction in space. Maybe it should be called something else?

Agreed. What about 1d2dPDOS (because the projection can be along the 1D direction or the 2D plane) or projDOS (although this may be confused with PDOS) or maybe PlanarAxialDOS? Or something similar?

tfrederiksen commented 2 years ago

Agreed. What about 1d2dPDOS (because the projection can be along the 1D direction or the 2D plane) or projDOS (although this may be confused with PDOS) or maybe PlanarAxialDOS? Or something similar?

Not sure what would be a good name. We are doing something "unusual" here compared with standard PDOS/LDOS visualizations. I am now also a bit unsure what we had in mind for the 1D mode. Wouldn't it not be more sensible in that case to compute the actual LDOS on a line (which would be a different calculation)?

More generally, we may want to reconsider the function names (and descriptions) in spectrum.py. For instance, we could also change class Spectrum(Plot) (not very evident what we have in mind) to something slightly more specific like Eigenvalue_spectrum(Plot) to better capture what it does?

sofiasanz commented 2 years ago

As we discussed @tfrederiksen, I finally changed the approach to having the grid created outside the plot class. Then we just create the grid and pass it to plot._realspace_ function to plot it.

I also did some cleaning here and there in the DOS plot classes (spectrum.py file) where I added some info and renamed some functions with the aim to clarify a bit what we do. Also now the function plot.LDOS allows to plot the LDOS with a broadening function at a certain desired energy E (committ ea3fc37).

sofiasanz commented 2 years ago

@tfrederiksen following your comments and our earlier discussion I made the corresponding changes in commits 7e82723, 98d2cad and 54cd86d. Let me know what you think!

sofiasanz commented 2 years ago

Sorry to jump in here. Just some minor remarks, and a possible bug?

Hi Nick! Your comments are always welcome of course! Thank you for the feedback.

sofiasanz commented 2 years ago

@zerothi what do you think of the changes in commit aade059? Another suitable option is to directly pass the SC to real_space_grid instead of its size and origin.

zerothi commented 2 years ago

@zerothi what do you think of the changes in commit aade059? Another suitable option is to directly pass the SC to real_space_grid instead of its size and origin.

Yes, I think this is more future proof. :)

sofiasanz commented 2 years ago

In commit 42cdcac now the SC object is directly passed instead of being created in real_space_grid, so now the user can pass the desired SC (if not the SC defaults to what we had before).