janosh / pymatviz

A toolkit for visualizations in materials informatics.
https://janosh.github.io/pymatviz
MIT License
176 stars 16 forks source link

Add `brillouin_zone_3d` plot function #251

Closed janosh closed 2 days ago

janosh commented 1 week ago

See pymatviz/brillouin.py.

brillouin_zone_3d(cubic_struct) brillouin_zone_3d(hexagonal_struct)
brillouin-cubic-mp-10018 brillouin-hexagonal-mp-862690
brillouin_zone_3d(monoclinic_struct) brillouin_zone_3d(orthorhombic_struct)
brillouin-monoclinic-mp-1183089 brillouin-orthorhombic-mp-1183085
DanielYang59 commented 1 week ago

Thanks for asking me, this one looks gorgeous (both the code and figures) and surely would be super helpful for visualizing Brillouin zones. I just did a quick search and it seems one package provides some functionality but only limited crystal systems (I cannot install this package with pip), so we might be confident that we're not reinventing the wheels.

Regarding the API, I personally seldom plot on a given fig (I prefer to plot individual figure/subplots on a separate canvas for stability) so I might prefer taking fig and subplot_idx as optional keyword args (instead of positional). Apart from this everything looks good and clean to me (one kwargs to control one "element").

One minor certain though, point_kwargs/path_kwargs take both False (for turning off that element) and None (as default empty dict), having two falsy values might be a bit confusing, would using Literal["off"] | None be more self-descriptive?

I believe you have caught the appearance issues like missing arrow, basis vectors flying out of the canvas, random white lines cutting the figure (wrong trace order I guess?) and perhaps the edges at the back should be dotted?

janosh commented 1 week ago

I just did a quick search and it seems one package provides some functionality but only limited crystal systems (I cannot install this package with pip)

thanks for checking. i also looked around before starting the implementation but didn't find arpes. the ones i saw were

neither had an interactive (drag and zoomable) 3D version implemented with plotly which is what i need.

having two falsy values might be a bit confusing, would using Literal["off"] | None be more self-descriptive?

yeah, need to think about that

so I might prefer taking fig and subplot_idx as optional keyword args (instead of positional)

good point. that was hastily done to make the example figure above but i was also considering removing the fig arg entirely. the reason it might make sense to keep it as optional arg is that one common use case is to plot the BZ into a (phonon or electronic) bandstructure plot to visualize the path through k-spce being sampled by the bands. in which case this function might frequently be used to make small overlayed subfigures.

and perhaps the edges at the back should be dotted?

that would be cool! but i think it's quite hard since you can rotate the figure, meaning the line style needs to change from solid to dotted dynamically

DanielYang59 commented 1 week ago

thanks for checking. i also looked around before starting the implementation but didn't find arpes. the ones i saw were

Yes I found these two as well, I guess there's no existing package for this.

that would be cool! but i think it's quite hard since you can rotate the figure, meaning the line style needs to change from solid to dotted dynamically

Always easier said (me) than done. I never touched plotly before but it looks good, I would have a try on it as well :)

janosh commented 3 days ago

Always easier said (me) than done. I never touched plotly before but it looks good, I would have a try on it as well :)

definitely worth trying. the interactivity is helpful. and the fact that you can print to PDF and export as HTML for websites makes it versatile. i find the API is better designed and less confusing than that of matplotlib

DanielYang59 commented 2 days ago

This is truly amazing quality plot, thanks so much for the work :)

janosh commented 1 day ago

don't have time right now but next step is to add a brillouin_zone: dict | False keyword to the phonon_bands plot that allows overlaying or plotting next to the bands the Brillouin zone to show what path in reciprocal space the bands correspond to. a bit like this one:

Screenshot 2024-11-30 at 6 46 09 AM