Symmetrization M2: derive each 2D model's point group and report divergence from the declaration#369
Symmetrization M2: derive each 2D model's point group and report divergence from the declaration#369tylersax wants to merge 5 commits into
Conversation
|
Converting to draft until M1 #368 is resolved, since it looks like we will have some changes there that need to be incorporated here |
tylersax
changed the title
tylersax
changed the title
|
@PDoakORNL This PR is ready for review. @maierta and I discussed this morning and are aligned on the approach. |
Replace the per-geometry declared point-group files (2d_square/hexagonal/oblique/rectangular) with a single holohedries_2d.hpp defining the two 2D holohedries D4 and D6 and the candidate pool they generate. By the crystallographic restriction every 2D point group is a subgroup of one of these, so the lower-symmetry groups (rectangular D2, oblique C2, ...) are derived by the geometry filter rather than declared per model. - Correct D6 at the source: add the identity and use mirror denominator 12 so all six mirror lines survive (the legacy list omitted the identity and used denominator 6, collapsing to only three distinct mirrors). - Delete the now-unused subgroup structs and repoint all D4 consumers to holohedries_2d.hpp. - Migrate the triangular lattice test from C6 to D6 (its actual holohedry) and drop C6 - Add a completeness oracle (holohedries_2d_test) asserting that filtering the pool by each 2D lattice reproduces its full holohedry (8/12/4/2).
…rgence deriveAndComparePointGroup installs the 2D holohedry pool as the live group of a cluster family, gates the geometrically realized ops on H0 invariance, then restores (and verifies) the declared symmetry state. update_domains runs the check after each family's declared initializer and prints a rank-0 report only when the derived and declared groups differ; production symmetrization is unchanged. 3D models and legacy lattices without initializeH0 skip derivation.
Add point_group_symmetry_element::describe(): a short human-readable label decoded from the op matrix -- Schoenflies name (Cn^k, sigma) plus geometry (rotation angle, mirror-line angle). The derive-and-compare report now lists the labeled ops in both divergence directions (under-declared and unverified) instead of bare counts, so a report line like "sigma (mirror @ 45 deg)" pinpoints which symmetry a model's declaration is missing or overclaiming. Some minor comment cleanup as well
Hats off to Claude for catching this. Since set_symmetry_matrices records (-1, -1) when no band image is found (i.e. no symmetry gets accepted), there is a possibility that H0 gets indexed with -1 leading to an out-of-bounds read. (Identified statically, confirmed with AddressSanitizer). The fix is one more rejection branch on solveSignsForOp. Kagome is the model that reliably produces this state, so it gets added as a fixture.
PDoakORNL
left a comment
PDoakORNL
left a comment
There was a problem hiding this comment.
Outside of the CI failure which seems to be a simple header file update, the "turn" factor is all I see in this in PR I'd like to see updated.
| const double det = O[0] * O[3] - O[2] * O[1]; | ||
| if (det > 0.) { | ||
| const long raw = std::lround(std::atan2(O[1], O[0]) * 180. / M_PI); | ||
| const int deg = static_cast<int>((raw % 360 + 360) % 360); |
There was a problem hiding this comment.
Not that this doesn't seem to work, but to me it feels strange to be doing this in degrees. I think 12 being the lcm of the crystallographic rotation orders is the right factor to use here. Then I think this code will probably read in a way that aligns better with the group theory for 2 and 3D point groups.
There was a problem hiding this comment.
Fair point - calculating in degrees is a little goofy here. Updated to twelfths (say that word 10 times fast), but kept the degrees in the print statement since (I think) that's the natural way to read it. LMK if you think we should wipe that too and stick to fractions all the way.
There was a problem hiding this comment.
I don't expect to ever see any angle in physics or mathematics expressed in anything but radians but that might just be me @maierta?
|
Header issue should be fixed. Good to run CI again. |
2 participants
Note
Stacked on #368 — the first two commits here are M1, under review
We now derive a 2D model's point group: a universal holohedry pool, filtered by the lattice geometry (this PR), then gated on H0 invariance (M1's
verifiedSymmetryOps). In this milestone the derivation is diagnostic only — production symmetrization continues to use the declared group unchanged, and a report is printed when the two disagree.Commit 1 — holohedry pool (holohedries_2d.hpp). Replaces the per-geometry point-group headers (2d_square / 2d_hexagonal / 2d_rectangular / 2d_oblique) with a single header defining the two 2D holohedries (D4 and D6) and the candidate pool they generate. By the crystallographic restriction, every 2D point group is a subgroup of one of these — so the lower-symmetry groups (D2, C2, …) no longer need to be declared per model (the geometric filter derives them). This simpler representation of the candidate groups also addresses a suspected bug in D6: the legacy list omitted the identity (which I believe matters during imposition) and used mirror denominator 6 as opposed to the correct 12 (effect: collapses six mirror lines to three). A completeness oracle (holohedries_2d_test) asserts that filtering the pool by each 2D Bravais lattice reproduces each of the four 2D holohedries (8/12/4/2 ops).
Commit 2 — derive and report (derive_point_group.hpp). deriveAndComparePointGroup runs a three-stage derivation per cluster family: install the holohedry pool as the live candidate group → the existing geometric search keeps ops preserving the lattice and cluster → an H0-invariance gate (M1's verifiedSymmetryOps) keeps ops admitting a signed orbital permutation U_S with U_S H0(O·k) U_S† = H0(k). It then restores the declared symmetry state — verified, throwing on failure — and returns a report naming any divergence. update_domains calls it after each family's declared initializer and prints on rank 0 only when derived ≠ declared.
Commit 3 — name the divergent ops. Adds point_group_symmetry_element::describe(), a human-readable label decoded from the operation matrix: Schoenflies name plus geometry, e.g. C4^3 (rotation 270 deg), sigma (mirror @ 45 deg). The divergence report lists the labeled ops in both directions (under-declared and unverified) instead of bare counts, so a report pinpoints exactly which symmetry a model's declaration is missing or overclaiming.
Scope: 2D models only (no 3D pool yet); lattices without the initializeH0 interface skip derivation entirely (compile-time detection)
Mini-roadmap
Snapshot of where we are in the project (subject to change but this is what I'm thinking):
M0: Characterization harness