Skip to content

Use time reversal symmetry in kpoint reduction #1317

Open
antoine-levitt wants to merge 26 commits into
JuliaMolSim:masterfrom
antoine-levitt:trs-plan
Open

Use time reversal symmetry in kpoint reduction #1317
antoine-levitt wants to merge 26 commits into
JuliaMolSim:masterfrom
antoine-levitt:trs-plan

Conversation

@antoine-levitt

Copy link
Copy Markdown
Member

This is fully written by Claude, prodded by me. Looks reasonable and tests pass, but still needs careful review.

antoine-levitt and others added 18 commits May 8, 2026 09:17
- Extend SymOp with θ field (±1 for unitary/antiunitary)
- Add breaks_TRS predicate for Magnetic and Anyonic terms
- Stop discarding spin_flips==-1 from spglib in collinear case
- Synthesize θ=-1 partners for :none/:spinless when TRS not broken
- Flip is_time_reversal flag in bzmesh.jl based on symmetry group
- Update symmetries_preserving_kgrid, unfold_kcoords to use θ·S·k
- Fix symmetrize_ρ for collinear spin swap under antiunitary ops
- Fix apply_symop for θ=-1 (conj + G-flip)

Result: Si 4×4×4 goes from ~28 to 8 irreducible k-points.
Energy matches symmetries=false run to machine precision.

Next: transfer.jl audit (find_equivalent_kpt + transfer_blochwave)
so that forces/stresses work on TRS-reduced meshes. See progress.md.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- plan.md: clarify transfer.jl strategy — forces (q=0) need no change;
  phonons (q≠0) should use apply_symop via symmetry search, not
  extend find_equivalent_kpt
- progress.md: record steps 1-4 complete with verified results
- test_trs.jl: replace ad-hoc Si test with two GaAs tests:
  (1) equilibrium GaAs (Td+TRS, forces~0, checks E/f/ρ)
  (2) rattled GaAs (TRS-only, non-zero forces, checks E/f/ρ)
  All quantities agree to machine precision vs no-symmetry runs.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- current.jl: update assertion from `length(symmetries)==1` to
  `!use_symmetries_for_kpoint_reduction` — clearer intent, and correctly
  allows spatial-only symmetrized bases while blocking TRS-reduced ones
- symmetry.jl (symmetrize_hubbard_n): restrict to unitary (θ=+1) symops;
  antiunitary partners require conj(WigD) + spin swap which is not yet
  implemented. For n_spin=1 the result is unchanged (real n makes θ=−1
  identical to θ=+1); for n_spin=2 this prevents silently wrong output.
- Phonon step 4b: dropped — phonon tests always use symmetries=false
  (full BZ) and require TRS implicitly; no code change needed.
- plan.md / progress.md: updated accordingly.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
For collinear (n_spin==2), basis.kweights are duplicated per spin and sum
to n_spin_components, not 1.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- test/symmetry_issues.jl: expect 96 (=48 spatial × 2 TRS) for the CuO2 group
- test/bzmesh.jl: reconstruct kcoords using θ·S·k now that the symmetry
  group includes antiunitary partners
- src/transfer.jl: replace the implicit nothing-arithmetic crash in
  find_equivalent_kpt with an explicit error pointing users to unfold_bz
  or use_symmetries_for_kpoint_reduction=false

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
symmetrize_hubbard_n now incorporates antiunitary symops: the contribution
becomes WigD' · conj(n_src) · WigD, with n_src drawn from the opposite spin
channel for collinear (σ_src = 3 - σ when n_spin == 2). No more silently-
restricted-to-θ=+1 path.

irreducible_kcoords now distinguishes the two TRS augmentation paths. For the
synthesised :none/:spinless case (θ=-1 partners share W with θ=+1) it keeps
the previous behaviour: pass θ=+1 rotations + is_time_reversal=true. For
collinear AFM (θ=-1 W's differ — they come from spglib's spin_flips==-1
rows), is_time_reversal stays false: spglib would otherwise treat W and W·T
as both being magnetic-group symmetries and over-reduce the BZ. The
antiunitary part is still exercised at the symmetrise / unfold layer.

docs/src/developer/symmetries.md: the theory section now describes the
antiunitary action explicitly and uses θ·S·k as the Brillouin-zone action.

progress.md: rewrites the Hubbard / docs / AFM-orbit items.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Replace the old per-case testitems with a unified coverage matrix that
exercises Si non-magnetic, GaAs equilibrium, rattled GaAs, AFM Si, and FM Si
across a battery of kgrid/kshift combinations. Checks T1–T4 and T6 (SCF
equivalence, unfold round-trip, kweight normalisation, group closure,
k-count halving) plus a dedicated Hubbard+AFM testitem for θ=-1 symmetrisation.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@antoine-levitt

antoine-levitt commented May 8, 2026

Copy link
Copy Markdown
Member Author

This is my first substantial PR for AI code intended to get into dftk so if it's of interest to anybody, here's my workflow. I started by brainstorming with opus, telling it to write a plan.md file. Then I switched to a fresh sonnet session and told it to keep notes in a progress.md file in case the session got aborted. Running tests went fine once I made sure julia setup was OK (julia --project). I used remote control to control it from my phone. Whole thing took less than a day split over a few sessions. The most annoying bit was the stop and go part: approving commands, waiting for it to think or tests to run, etc. I should use auto approve, julia mcp with revise and notifications.

Things it's great all : coding style, git stuff, tests (designing, running and debugging), getting classical formulas by itself, knowing about physics . Nonzero chance it's so good because it knows quantum espresso source code by heart...

Things it's bad at : getting the right fix as opposed to the quickest fix, inserting itself in existing code rather than adding new hacks, limiting code size (it loves copy paste), sometimes getting stuck (eg its commits sometimes hung, I don't know why). Also an annoying tendency to prioritize what it's doing atm as opposed to the broader context (eg adding a 10 lines comment related to trs inside a nontrivial function doing something else). All this possibly fixable with more explicit instructions.

@antoine-levitt

Copy link
Copy Markdown
Member Author

Oh and all in all it used up about a third of my weekly quota (probably mainly because I switched back to opus at the end)

@antoine-levitt

Copy link
Copy Markdown
Member Author

There's a subtlety with spin which this PR somewhat conflates. The actual physical TRS (the one that reverses the sign of p times sigma) is (0 1;-1 0) times the conjugation. This symmetry is broken in ferromagnetic states, but is preserved in one type of systems: anti-ferromagnets (TRS plus translation). In ferromagnetic states however, the symmetry of just conjugation (not acting on spins) is preserved. So the most general SymOp would allow for both full TRS and also just conjugation; in fact, with noncollinear spin it would allow general SU(2) matrices. I think we should go with just conjugation (so not really TRS) for now and leave spin alone, which would simplify this PR a bit. Longer term when we have noncollinear spins we should allow full SU(2).

antoine-levitt and others added 7 commits May 9, 2026 14:04
…_reversal default

- symmetry_operations: change time_reversal default to false so direct callers
  don't accidentally augment magnetic systems with conjugation partners
- lowpass_for_symmetry!: use θ·S instead of S so antiunitary ops (θ=-1) correctly
  check membership of -S·G in the grid (benign on symmetric grids but now explicit)
- accumulate_over_symmetries!: explain why θ is not used (ρ real ⟹ ρ̂(S⁻¹G)=ρ̂(-S⁻¹G))
- symmetrize_ρ: document the isempty guard and why fast-path applies to all n_spin
- terms.jl: note why ExternalFromReal/Fourier don't need breaks_conjugation

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ymop θ=-1 density test

- FM GaAs [+1,+1]: collinear system without inversion; expect_halving=true
  exercises the new collinear conjugation path and confirms k-point reduction
- apply_symop density θ=-1: verifies that for real ρ, antiunitary and unitary
  versions of the same spatial op give identical results (minimal tag)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…AFM spin-swap)

Per PR review: implement conjugation (k→-k, conj per spin channel) rather than full
physical TRS (which would need off-diagonal spin mixing for AFM). Conjugation is the
symmetry that is actually preserved for both non-magnetic and collinear systems.

Changes:
- SymOp: rename "time-reversal" → "conjugation" in comments and θ field description
- Model.jl: extend conjugation to :collinear (was :none/:spinless only); rename
  breaks_TRS → breaks_conjugation
- symmetry.jl: collinear case reverts to spin_flips==1 only (no AFM detection);
  antiunitary augmentation now applies to all eligible spin types; remove spin-swap
  (σ_src = 3-σ) from accumulate_over_symmetries!, unfold_mapping, symmetrize_hubbard_n;
  fast-path in symmetrize_ρ extended to all n_spin
- bzmesh.jl: simplify is_time_reversal — antiunitary ops now always mirror θ=+1 set
- docs: update symmetries.md accordingly

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…atrix (T5)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… test

symmetry_operations now defaults to time_reversal=false; callers that want
conjugation augmentation must opt in explicitly.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…helpers

Replace all inline θ·S·k expressions with transform_kpoint_coordinate(op, k)
and θ==-1 conj branches with maybe_conjugate(op, x) throughout symmetry.jl,
bzmesh.jl, and tests. Export transform_kpoint_coordinate as public API.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Comment thread src/symmetry.jl
Comment thread src/SymOp.jl
Comment thread docs/src/developer/symmetries.md
- symmetry.jl: add TODO for AFM spin-flipping symops
- SymOp.jl: note that a SU(2) spin matrix should eventually be added for noncollinear
- symmetries.md: same note about future SU(2) / AFM extension

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@antoine-levitt antoine-levitt marked this pull request as ready for review May 27, 2026 08:24
@antoine-levitt

Copy link
Copy Markdown
Member Author

Ready for review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant