Implement web JSON export #318
Conversation
Seems to be basically working! The main limitation is that the seekpath band structures typically include some non-contiguous sections, and the phonon website doesn't handle those well. We might need to manipulate the distances a bit.
|
I think the mass-weighting may also need tweaking, the acoustic modes don't quite look right! |
This significantly improves appearance, but there are still some oddities.
This is looking better but - there are still some odd gaps in the band structure, especially at (repeated) Gamma points - Some high-symmetry labels are put in the wrong places; usually these correspond to discontinuities
An off-by-one error was creating havoc with the label positions!
|
Mass weighting looks good now. The only remaining oddity is related to line-breaking; we get a gap in the spectrum even though the line break is supposed to happen at coincident points. e.g. but in the JSON and in the |
- The last value of line_break range seems to not be included? By - setting max index "too high" we get expected behaviour at discontinuities. - Gamma-point LO-TO split still looks wrong, fix that separately?
|
It looks like the last point of each segment is dropped: by "touching" segments we get correct-looking discontinuities except at the LO-TO-split Gamma
|
This looks a lot more reasonable overall; was just some inconsistency in which side of each break was being counted!
|
Just a bit of cleanup needed to put things in sensible places with good names. |
This is a fair chunk of code with a specific duty, and while it _can_ be used purely with QpointPhononModes as input is better characterised as a function that uses both modes and a corresponding set of tick labels. Move to a new "writers" module. Use the existing CLI --title option to set title when using euphonic-dispersion to generate JSON Introduce a named type for XTickLabels; this helps legibility. There is now some inconsistency between use of generic list/tuple and classes from Typing: don't worry about it for now, this can be tidied up afterwards.
euphonic/cli/utils.py
Outdated
| def _get_title(filename: str, title: str = '') -> str: | ||
| """Get a plot title: either user-provided string, or from filename""" | ||
| if title: | ||
| return title | ||
| return pathlib.Path(filename).stem |
There was a problem hiding this comment.
Warrant using a ternary if?
Would it be better to use None? Is there a reason a user might want to have an empty title?
There was a problem hiding this comment.
I guess the root of the problem is inconsistency 🙈
- Currently various plotters take '' to mean "no title" and use it as default so that only one type is allowed
- Here I want to have a non-empty dynamically-generated default.
I guess the answer is to modify some of the other code to allow None as well, then it can be used as a default here and '' remains available as a way to specify "no seriously I don't want a title"
There was a problem hiding this comment.
Ok, I would say that's a different issue and if this is the behaviour elsewhere that's fine. May want to make an issue even if it just sits there.
There was a problem hiding this comment.
The fix required fewer changes than I expected, let's give it a try
Suggested by @oerc0122
It turns out that the band structure will render ok with coincident q-points, now that the other indexing issues have been resolved :)
This allows empty input '' to be distinguished from default None
|
Not sure what's going on with the test failure; I can reproduce it locally but only within |
This was causing some "works on my machine" issues with mysterious tox failures
|
... got there in the end. |
Co-authored-by: Jacob Wilkins <46597752+oerc0122@users.noreply.github.com>
|
It still needs a couple of proper tests! |
Closes #299
Current status: it basically works but there is some funny business on the x-axis. The phonon website doesn't directly support breaks in the x-axis, so we should clean them up:
["X", "Y"]becomes["X|Y"]["X", "X"]becomes["X"]