Feature: Symmetry for DFT+U

[maki49]

Feature: DFT+U Symmetry

Fix #7950. The test results of the case provided by @liusss324 before and after this fix are as follows:

	IBZ-k	Amag	E/eV	steps to converge
sym-1	324	1.23	-2343.98580	31
sym1	37	1.19	-2344.17853	51
sym1-fixed	37	1.23	-2343.98447	32

@dyzheng The symmetrization is done by rotating DM(k) before the Fourier transformation instead of rotating DM(R), just for the easier implementation.

Refactor

• Move the EXX-irrelavant part of symmetry_rotation into module_symmetry and remove its LCAO+LibRI depencence (to make DFT+U not depend on module_ri), while EXX-relevant part remaining in module_ri.
• nspin-as-parameter: remove the PARAM dependence in symmetry_rotation

[Copilot]

🟡 Changes recommended

Unresolved build, k-point handling, cache, and symmetry-restoration issues block approval.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds crystal-symmetry support for LCAO DFT+U by restoring full-BZ density matrices and accumulating occupations over k-point stars.

Changes:

• Adds symmetry-based density and occupation-matrix restoration.
• Defers spin-expanded k-point allocation.
• Updates two DFT+U regression cases and references.

File summaries

File	Summary
tests/17_DS_DFTU/04_LCAO_DFTU_S4_XY/result.ref	Updates noncollinear symmetry results.
tests/17_DS_DFTU/04_LCAO_DFTU_S4_XY/INPUT	Enables symmetry for the test case.
tests/17_DS_DFTU/03_LCAO_DFTU_S2_Z/result.ref	Updates collinear symmetry results.
tests/17_DS_DFTU/03_LCAO_DFTU_S2_Z/INPUT	Enables symmetry for the test case.
source/source_lcao/module_dftu/dftu_nao_op.h	Adds k-vector and symmetry-rotation state.
source/source_lcao/module_dftu/dftu_nao_op.cpp	Restores full-BZ density matrices.
source/source_lcao/module_dftu/dftu_nao_occ.cpp	Accumulates occupations across k-point stars.
source/source_lcao/hamilt_lcao.cpp	Supplies k-point data to DFT+U operators.
source/source_cell/reciprocal_grid.cpp	Changes reciprocal-grid allocation.
source/source_cell/klist.cpp	Defers spin expansion and changes k-point resizing.

Review details

Suppressed comments (9)

source/source_cell/klist.cpp:453

• renew() also resizes ReciprocalGrid::kvec_c_full. At this point nkstot is the symmetry-reduced global count, while kvec_c_full must retain the original nkstot_nospin mesh; in MPI, using nkstot additionally allocates global-sized local containers on every rank. In a serial symmetry+LibRI run this truncates kvec_c_full, so later consumers such as Ewald_Vq iterate to nkstot_nospin past its end. Resize only the local spin-expanded arrays here and leave kvec_c_full untouched.

    this->renew(this->nkstot * this->spin_mult);

source/source_cell/klist.cpp:398

• Rule: Preserve the established contract of public k-point readers.\nSeverity: error\nLocation: source/source_cell/klist.cpp:398\nReason: The same regression affects line-interpolated input: KlistTest.ReadKpointsLineCartesian directly calls read_kpoints() with spin_mult=2 and expects 102 entries, but this path now allocates only the 51 input points because set_kup_and_kdw() is not called.\nSuggested action: Preserve the spin-sized allocation for this direct reader path, or update the API/callers and the existing regression test together with an explicit expansion step.\nException: not allowed

    this->renew(this->nkstot); // mohan fix bug 2009-09-01; spin_mult doubling done later by set_kup_and_kdw()

source/source_lcao/module_dftu/dftu_nao_occ.cpp:53

• Rule: Do not introduce new cross-layer control through GlobalV, GlobalC, or PARAM.
  Severity: error
  Location: source/source_lcao/module_dftu/dftu_nao_occ.cpp:53
  Reason: This adds a new PARAM.inp.nspin dependency inside the rotation helper even though cal_occ_mat_k already obtains the effective occupation-matrix spin count locally.
  Suggested action: Pass nspin (or an explicit nspin == 4 flag) into accumulate_occ_over_kstar and remove the new global read.
  Exception: not allowed

            if (PARAM.inp.nspin == 4)

source/source_lcao/module_dftu/dftu_nao_occ.cpp:159

• ik is the index local to the current k-point pool, so it does not identify the same IBZ point on every MPI rank. For a pool whose startk_pool is nonzero, this modulo maps a local point to the wrong kstars entry and rotates/accumulates the wrong density matrix. Use the existing local-to-global mapping kv.ik2iktot[ik] before reducing modulo the IBZ star count.

            const int ik_ibz = ik % static_cast<int>(kv.kstars.size());

source/source_lcao/module_dftu/dftu_nao_occ.cpp:104

• Checking only symm_flag == 1 does not guarantee that kstars exists: K_Vectors::set() skips IBZ reduction when berry_phase is enabled, so this branch can reach line 159 with an empty vector and divide by zero. Base the decision on actual k-star data (and apply the same guard in the operator path), or fall back to unsymmetrized accumulation when no IBZ expansion was built.

    const bool dftu_spacegroup_symmetry = (ModuleSymmetry::Symmetry::symm_flag == 1);

source/source_lcao/module_dftu/dftu_nao_occ.cpp:110

• The vector is pool-local when mpi_k completes; the later global resize only adds zero padding and does not populate nonlocal entries. cal_Ms iterates the global kv.kstars and reads kv.kvec_d[ik_ibz], so with KPAR > 1 this old-DFT+U path also constructs rotation matrices for nonlocal stars from zero k-vectors. Use a gathered/global k-vector view or a correct local-to-global mapping.

        dftu_occ_symrot.cal_Ms(kv, ucell, *pv);

source/source_lcao/module_dftu/dftu_nao_op.cpp:146

• The vector is pool-local when mpi_k completes; the new global resize at line 453 only adds zero padding and does not populate the nonlocal entries. cal_Ms iterates the global kv.kstars and reads kv.kvec_d[ik_ibz], so with KPAR > 1 it builds rotation matrices for nonlocal stars from zero k-vectors. Use a gathered/global k-vector view or a correct local-to-global mapping rather than relying on the padding.

            this->symrot_.cal_Ms(*this->kv_, *this->ucell, *pv);

tests/17_DS_DFTU/03_LCAO_DFTU_S2_Z/INPUT:18

• Rule: Document INPUT behavior changes in both user-facing parameter references.\nSeverity: warning\nLocation: tests/17_DS_DFTU/03_LCAO_DFTU_S2_Z/INPUT:18\nReason: This change makes existing symmetry=1 produce different DFT+U magnetic results, but the PR does not update docs/parameters.yaml or docs/advanced/input_files/input-main.md, nor explain why those synchronized documentation updates are unnecessary. Users need the documented support/limitations for this combination.\nSuggested action: Update both symmetry parameter references with the DFT+U magnetic behavior, or add a clear no-update rationale to the PR.\nException: human approval required

symmetry    1

tests/17_DS_DFTU/04_LCAO_DFTU_S4_XY/INPUT:19

• Rule: Focused tests for feature changes
  Severity: warning
  Location: tests/17_DS_DFTU/04_LCAO_DFTU_S4_XY/INPUT:19
  Reason: This nspin=4 regression case remains commented out in tests/17_DS_DFTU/CASES_CPU.txt, so the changed input/reference does not exercise the new symmetry path in CI; the enabled case covers only nspin=2.
  Suggested action: Add or enable a stable nspin=4 symmetry regression, or document a human-approved rationale and follow-up for the missing coverage.
  Exception: human approval required

symmetry    1

• Files reviewed: 10/10 changed files
• Comments generated: 7
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.