Include src/calc_bandreps.jl and add tests
#37
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #37 +/- ##
==========================================
+ Coverage 69.83% 75.25% +5.42%
==========================================
Files 19 20 +1
Lines 2314 2437 +123
==========================================
+ Hits 1616 1834 +218
+ Misses 698 603 -95
Continue to review full report at Codecov.
|
…ttps://doi.org/10.1103/PhysRevB.97.035139 - still WIP, because we still have 15 (down from 30) test failures in the "3D: Checking Wyckoff Position Sets" testset
… recrawl - there are some changes to the bandreps listings on Bilbao, mostly in the spinful irreps, but also a handful of changes to the spinless irrep multiplicities (e.g., space group 124); always the changes are swaps of what EBRs, so it does not affect earlier conclusions about topology/connectivity, only which EBR is assigned to which symmetry vector. - some of the EBR site symmetry labels have been abbreviated more correctly with this update (e.g., space group 68); there is still not full consistency on the abbreviations, however
…calculation & fix a missing `abs` in rotation axis normalization computation
…ut not returning only elementary EBRs to `calc_bandrep` docstring
- also explain the underlying issue in a note for posterity
… the point group that is "more natural" (i.e., is not merely isomorphic, but is just a similarity transform away) - also refactor some of the logic of this function into subfunctions and fast-paths that can be reused. - two "novelties" were added to the implementation: (1) we now sort operations not just by order, but also by rotation sense - the latter is not fully unique (one could do an inversion of the coordinate system to swap sense) but still often helps in finding a more natural match. (2) when possible, we now try to find a similarity transform between groups so we can check for equality of operations under this transform; that helps often (and e.g., let's us find -4m2 rather than -42m as the "more natural" parent point group) - add tests of `isomorphic_parent_pointgroup` by comparing to the site symmetry group labels from EBR listings on Bilbao
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi @AliGhorashiCMT,
I tried to rebase #22 but there were just too many changes since we last looked at it - and the rebase was looking very scary, so in the end I just created a new branch and pulled your original changes into that branch; hence, this new PR, superseding #22.
I did a bit of fiddling to make things work again - some changes broke a few minor things since last. As I recall, the main thing that held up merging back then was that tests were tricky due to the distinction between elementary band representations and "plain" band representations. The main problem being the existence of "exceptional" band representations (i.e. band reps that aren't elementary). We basically need to either come up with more holistic tests that don't care about the distinction, or we can add the tables of "exceptional" band reps and use that to skip the problematic cases. I'll think a little bit.
We should definitely get this merged; there's a lot of nice things in here...