Discontinuous qpoints

[dgaines2]

Hello!

First of all, let me thank you for writing this fantastic package. It's been quite useful for me and seems very easily extensible and customizable.

Background:
For some crystal symmetries, some common softwares have conventions that construct a q-path that is discontinuous in the Brillouin zone.
E.g. BCC (Spacegroup #229) q-paths have X|R (SeekPath) or H|P (pymatgen)

Problem:
The current plotting joins phonon bands across this discontinuity, although it is mostly only visible if the (plot) linewdith of the band is thicker than the linewidth of the vlines at these q-points.

Solution:
These discontinuities can be found by checking for two adjacent x values in the q-path that have the same value. You can then fix this issue by splitting the x and frequency arrays across each discontinuity and making separate plotting calls for each "split". In the case where there are no q-points with discontinuities, this implementation results in the same plotting behavior as before.

Example:
BCC Iron (Fe) with spin polarization turned off displays imaginary modes. In Fe_229_nospin_original.pdf, note the band connection along H|P. In Fe_229_nospin_updated.pdf, the suggested fix is applied and the previous vertical line joining the bands is not present.
Fe_229_nospin_original.pdf
Fe_229_nospin_updated.pdf

Further comments:
I'd be very happy to address any feedback!

• I only updated add_dispersion and add_alt_dispersion, but this fix also addresses this issue automatically for add_multi. There might be cases where this still occurs for some of the other phonon plotting functions if linestyle is used with scatter.
• I'm not sure if I should have also changed ax.scatter() for add_alt_dispersion. If I'm completely honest, I'm not sure I understand the use cases for ax.scatter with linestyle vs ax.plot with markers.
• I also acknowledge that the way I implemented label_ni is probably not optimal, although it let me leave the tile_properties() calls alone.
• There's a difference in the ylimits (bottom) between these plots from another change I made, which is only relevant for cases where there are imaginary modes. I'm drafting another pull request for this.

[kbspooner]

Hi Dale,

Sorry for the slow response.

I'm glad you like ThermoParser! Thanks for raising, and especially fixing this issue. As you say, it fixes this issue without interfering with anything else so far as I can tell, so I'm happy to accept this as is (although in searching for problems in this fix I found another issue that predates this fix, which I've now fixed). For the record, it turns out this issue also occurs in some cases when the non-analytical correction (NAC) is applied, due to discontinuities in the frequency at gamma (and I think it can also change the order of the modes according to phonopy at the high symmetry points, but I'm not sure).

As for the add_alt_dispersion scatter vs line plot, I just found that different people prefer it different ways, and I prefer it different ways in different circumstances. They can get a bit messy is all, nothing particularly rigorous 🤷 .

This does mess with the tests slightly, but I'll leave those be for now in light of that other pull request.

Thanks,
Kieran

[dgaines2]

Hi Kieran,

Thanks for taking a look and merging! I'm glad to see it helped with NAC, even if I didn't originally think about that. Good catch on the possibility of changing the order of bands -- I did think about this briefly. As it's only changing the data that's plotted, I think an issue would only be visible if each band has a unique color. This difficulty in coloring might also pop up when dealing with degenerate bands / band crossings (and phonopy has its own way to determine band connections), but perhaps that's outside the scope of this PR. If an issue ever arises, I'd be happy to take a look.

Best regards,
Dale