Skip to content
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

Add fat bands to the BandPdosWidget. #624

Merged
merged 16 commits into from
May 2, 2024

Conversation

t-reents
Copy link
Contributor

Hi @unkcpz @AndresOrtegaGuerrero

As discussed, I worked on the support for the "fat bands" plots and I managed to merge this into the existing BandPdosWidget. While working on this, I also started to refactor some of the existing methods, see the commit message for more details, mainly to split things up in a more compact way.

I have some further ideas, but I already wanted to share this version, to get further input. One thing that would be useful to address in my opinion, are different linestyles for the spin-up and spin-down projected bands, as the underlying red and blue linecolour is not always easily visible.

Important to note, the Project bands checkbox is only used for this first draft. This would be changed to something more properly formatted in the final version.

One additional question @AndresOrtegaGuerrero: is it really necessary to have these nested loops to iterate each segment separately? At the moment, I don't really see the reason, please correct me if I'm wrong. As mentioned in the commit message, since this version relies on BandsData._get_bandplot_data, one could also plot the bands in a single trace (need to check how much this would improve performance).

for band in paths:
if not spin_type:
# Non-spin-polarized case
for bands in band["values"]:
bands_np = np.array(bands)

The current implementation works for non-spin-polarized and spin-polarized calculations. Moreover, the projected bands can be used with and without the PDOS (and all kind of groupings are supported as well).
image

The orbital/atom contributions in the legend are grouped if the PDOS is plotted as well, so that the chosen contributions can be hidden in both subplots at the same time.
image

This is the first commit to support `fat bands` in the `BandPdosWidget`, to visualize the contribution
of the orbitals directly on top of the bandstructure.
The commit contains the following main changes:

* `_add_projection_traces` is added to plot the projections on top of the bandstructure. It follows the logic of the existing methods.
* A checkbox is added to `BandPdosWidget` to enable the projected bands. To group the orbitals etc., the existing grouping features for the PDOS are used for the projected bands as well. In case PDOS and projected bands are plotted at the same time, the legend is grouped so that the data can be hidden in both subplots simultaneously.
* `get_bands_projections_data` is added to extract the projections from a `ProjwfcBandsWorkChain` (see `aiida-wannier90-workflows`). Moreover, the new version relies on `_get_bandplot_data` to extract the bandstructure. In addition to the previously used `_exportcontent` in `export_bands_data`, this method also provides the bands along the full k-path and not only the separate segments. This allows to plot all bands and projections (for each orbital group) in a single trace which should improve the performance. Important to note, the data of the previous method is still accessible.
* `_prepare_projections_to_plot` is used to convert the projections into a certain format (see the comments in the implementation for details). This involves some numpy operations to address the previous point of plotting the groups in a single trace.
* `_projections_curated_options` is refactored quite a bit. The method supports now the extraction and curation of the PDOS or the projections. Moreover, the different aspects are divided into separate functions to facilitate the maintenance
a band needs to be concatenated with its mirror image, first.
"""
projected_bands = []
for spin in [0, 1]:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe try to use the naming from aiidalab_qe , as collinear

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also try to see if this logic is consistent for SOC scenario ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure, if I fully get your point here. The 0 and 1 are the indicators for the different spin-channels that are used in the BandsData. _get_bandplot_data method. Therefore, I didn't use something like "up" and "down".

But good point, I'll check how these list would look like in case of SOC.

@AndresOrtegaGuerrero
Copy link
Member

@t-reents thank you ! nice work I had a few comments. First It would be nice to test the logic for all conditions , selecting atoms , orbitals ... and so on , also it is necessary to test a SOC simulation to determine if the logic is preserved. If the plot is only bands with projection then the widgets for displaying the projection should be displayed if there is a projection and hidden if not. Regarding the nested loops, I agree it would be the best to do one single trace , however the way the data in the dictionary was given by traces. Could you please suggest a solution where is only one trace, and double check that you still can select points from the bands (plot)

@@ -397,6 +431,7 @@ class BandPdosWidget(ipw.VBox):
Select the style of plotting the projected density of states.
</div>"""
)
projected_bands_width = 0.5

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add this to the SETTINGS dictionary

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense!

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one question any particular reason for this number ? Maybe try to test a system with a good amount of atoms and/or kinds , so you see the appearance onced you have plenty of bands

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was just to have a first starting point, to check that everything works.

Not sure, how you think about it (I'm also not a 100% sure, what I think about it, as the layout might get too complicated with too many options), but one could also add a dropdown/slider to choose the maximum linewidth, since the perfect value might differ for different applications?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@t-reents Since the idea is that the widget it has a general behaviour , is important you check with a medium and big system , so you can see how is the appearance, and if you need to maybe add this as a extra widget to control this width

@t-reents
Copy link
Contributor Author

Thanks for your comments @AndresOrtegaGuerrero. I tested all kind of selections and they seem to work. I was also thinking about a SOC calculation but don't have experience with those calculations and no data right now. Do you have a WorkChain to share as an archive, so that I can use it for the development?

Yes, I'll provide a "one trace" solution for the bands tomorrow, and I'll keep in mind that each band/k-point can still be selected.

If the plot is only bands with projection then the widgets for displaying the projection should be displayed if there is a projection and hidden if not

Exactly, I haven't done this yet, but the idea was that the grouping options are alway displayed in case PDOS or band projections are available

@AndresOrtegaGuerrero
Copy link
Member

For SOC , you can install this plugin to your aiidalab_qe (https://github.com/AndresOrtegaGuerrero/aiidalab-qe-soc/tree/main) , and run a calculation , however for the projections , we need to do a modification , you can check in (https://github.com/AndresOrtegaGuerrero/aiidalab-qe-soc/blob/main/aiidalab_qe_soc/socworkchain.py) , the special settings are in (https://github.com/AndresOrtegaGuerrero/aiidalab-qe-soc/blob/main/aiidalab_qe_soc/workchain.py) , one single thing is to replace the BandsWorkChain for the one you use for the projection

@t-reents
Copy link
Contributor Author

For SOC , you can install this plugin to your aiidalab_qe (https://github.com/AndresOrtegaGuerrero/aiidalab-qe-soc/tree/main) , and run a calculation , however for the projections , we need to do a modification , you can check in (https://github.com/AndresOrtegaGuerrero/aiidalab-qe-soc/blob/main/aiidalab_qe_soc/socworkchain.py) , the special settings are in (https://github.com/AndresOrtegaGuerrero/aiidalab-qe-soc/blob/main/aiidalab_qe_soc/workchain.py) , one single thing is to replace the BandsWorkChain for the one you use for the projection

Thanks, I'll give it a try. Probably I'll just do it via AiiDA-core as I'm used to it

@AndresOrtegaGuerrero
Copy link
Member

@t-reents Currently i am also working with a modification to the widget here #512 , the new feature is to allow the app to use tot_magnetization. When you use tot_magnetization , you now have two fermi energies in the BandsWorkChain and also in PdosWorkChain. I would suggest you run your workchain adding tot_magnetization , and verify your logic when the data doesnt have "fermi_energy".

Different methods are refactored in this commit:
* New functions to create the figures, add the different traces and to generate the final plot. Those replace the previous individual `_create..._plot` methods, to make things more compact
* The methods to generate the axes were also refactored to avoid code duplication.
* The bands are plotted in a single trace now, improving the performance of the widget
@unkcpz unkcpz mentioned this pull request Feb 29, 2024
2 tasks
t-reents and others added 3 commits April 30, 2024 16:11
If `tot_magnetization` is specified, the results contain a fermi energy
for each spin channel. The general support was added in aiidalab#512 and this
commit introduces the required changes to the widget.
(False, 0): self.SETTINGS["bands_linecolor"],
}
fermi_energy_mapping = {
(False, 0): self.fermi_energy.get("fermi_energy_up", None),

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what happens when there is only one fermi_energy?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The key would be (True, 0) or (True, 1) in that case but those are not present in the mapping. Therefore, the value for fermi_energy would be returned as expected:

fermi_energy = fermi_energy_mapping.get(
("fermi_energy" in self.fermi_energy, spin),
self.fermi_energy.get("fermi_energy"),
)

@t-reents t-reents marked this pull request as ready for review May 2, 2024 09:20
Copy link
Member

@AndresOrtegaGuerrero AndresOrtegaGuerrero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!, We did different tests and check the logic and test the qe app! It also improves the performance

@AndresOrtegaGuerrero AndresOrtegaGuerrero merged commit 4b03549 into aiidalab:main May 2, 2024
14 checks passed
@AndresOrtegaGuerrero
Copy link
Member

This PR solved #626

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants