Repository navigation
Fix divergent energies during EXX cell relaxation - #8099
Merged
Merged
Conversation
Refresh the initial hybrid Hamiltonian and EXX energy using the current geometry on later ionic steps. Retain Eexx alongside Hexxs until both are replaced, instead of resetting only the energy after convergence. Reinitialize convolved orbitals, integral tables and associated RI caches when the cell-dependent Coulomb settings change, including Spencer cutoff. Reuse the existing auxiliary basis and leave fixed-kernel settings unchanged. Add a two-atom diamond DDH cell-relax regression that compares the evaluated second-step structure with a fresh DDH single point using the same FFT grid. Both separate and simultaneous EXX loops pass a predefined 1e-5 eV tolerance: separate_loop=1: 3.7717291e-6 eV; separate_loop=0: 3.7752644e-6 eV. Both ionic SCFs and independent single points converge, and second-step initial EXX energies remain nonzero. Validation: cmake --build build -j6; Python regression for both EXX loops; py_compile; git diff --cached --check; staged governance check (exit 0). No INPUT names, public APIs, or default arguments change, so no user-facing documentation update is needed. Governance's default-parameter warning is an existing INPUT comparison, not a default argument; global migration budget is neutral. Existing diagnostic source edits are excluded. fix(exx): rebuild symmetry after cell-relax analysis Refresh LCAO EXX symmetry immediately after cell-relax reanalyzes the unit-cell symmetry. Clear lattice returns, sectors, Bloch rotations and angular-rotation caches before rebuilding, including invalidation of the RI tensor cache. Keep the extra refresh limited to cell-relax. Add a regression with unchanged point-group size and atoms crossing a cell boundary. The refreshed sectors, lattice returns and M matrices match a fresh object. Validation: - cmake --build build --target abacus_std_para -j2: passed. - cmake --build build_test --target MODULE_RI_EXX_SYMMETRY_rotation -j2: passed; OMP_NUM_THREADS=1 ctest --test-dir build_test -V -R '^MODULE_RI_EXX_SYMMETRY_rotation$': all 13 tests passed. - python3 tests/08_RI/test_exx_lri_interface.py with the built executable, SG15/Orb-v2 data and one MPI rank: both loops passed the 1e-5 eV tolerance (3.7717291e-6 and 3.7752644e-6 eV). - git diff --cached --check and staged agent governance check: passed; the documentation warning needs no parameter docs update because no INPUT option, default or intended user-facing semantics change. No new GlobalV/GlobalC/PARAM dependencies. Pre-existing unrelated edits are excluded. refactor(exx): separate Coulomb refresh from basis initialization Retain original orbitals, auxiliary basis and the shared Gaunt table when cell-dependent Coulomb parameters change. Rebuild only changed kernels' convolved orbitals, integral interpolations and point-value caches. Destroy old kernel objects first because Matrix_Orbs initialization inserts entries without clearing previous tables. Preserve unchanged kernel objects. The regression checks both EXX loop modes against independent singlepoints and requires one full init and two Coulomb refreshes across two ionic steps. No INPUT parameters or intended numerical behavior change; no documentation update is required. This extraction adds no global dependencies. Verification: - cmake --build build --target abacus_std_para -j2: passed. - python3 -m py_compile tests/08_RI/test_exx_lri_interface.py: passed. - OMP_NUM_THREADS=1 python3 tests/08_RI/test_exx_lri_interface.py --binary build/abacus_std_para --pseudo-dir /home/fortneu/work/pp_orbs/SG15_ONCV_v1.0_upf --orbital-dir /home/fortneu/work/pp_orbs/Orb-v2 --work-dir .diagnostics/ddh-cell-audit/coulomb-refactor-loop1-20261008 --mpi-ranks 1 --separate-loop 1: passed, delta 3.7717290979344398e-6 eV. - Same runtime command with work directory coulomb-refactor-loop0-20261008 and --separate-loop 0: passed, delta 3.77526436068365e-6 eV. - Both runtime modes: basis_initializations=1, coulomb_refreshes=2. - Executable version: ABACUS v3.11.0-beta10. - python3 tools/03_code_analysis/agent_governance_check.py --staged: no blockers; documentation warning explained above. - git diff --cached --check: passed. refactor(exx): refresh symmetry without adding ESolver hooks Remove the new ESolver_FP virtual hook and ESolver_KS_LCAO override. Refresh EXX symmetry in the existing LCAO before_scf immediately after its base call, which has completed symmetry analysis and k-point updates. Keep refresh limited to cell-relax with an updated cell and symmetry enabled, before Hamiltonian construction and EXX ionic-step preparation. Verification: git diff --cached --check passed; staged agent governance check passed without blockers. Test/documentation review warnings apply: per explicit user instruction, the build was stopped and regression was not run for this follow-up; the user will validate actual cases. Existing symmetry and EXX regression coverage is retained. No INPUT behavior or user-facing interface changes require documentation updates. refactor(exx): refresh symmetry inside ionic-step preparation Extract symmetry initialization into Exx_LRI_Interface::refresh_symmetry. Use it from exx_before_all_runners and at the beginning of exx_beforescf on subsequent cell-relax ionic steps with runtime symmetry enabled. Pass parallel orbital layout and the refresh condition explicitly. Update both ground-state and TDDFT callers and remove the ESolver refresh block and Exx_NAO refresh forwarding function. Runtime symm_flag is used because INPUT conversion can disable symmetry after parsing the symmetry setting. Verification: all call sites inspected; git diff --cached --check and staged agent governance check passed without blockers. No build or regression run, per the user's instruction to commit directly and validate actual cases later. Existing tests are retained. No INPUT options/defaults change, so parameter documentation updates are unnecessary. The existing symmetry flag read moves from ESolver to EXX setup with no net global budget increase.
Compute the integer return vector directly from r*g+t-r_mapped. Wrapping and tolerance-based snapping of fractional coordinates discarded integer shifts near cell boundaries, while RI integrals retained the original representatives. This corrupted Bloch phases and real-space EXX restoration, even for a fresh singlepoint when cell-relax enlarged symmetry_prec. Add tests for near-boundary atoms, changes of atom representatives, and translation representatives. Update the existing negative-coordinate test: z=-0.2 must remain -0.2, so its z return is zero rather than the old wrapped value -1. Also verify reconstruction of the original mapped coordinate. No INPUT options or defaults change; parameter documentation is unnecessary. Verification: - cmake --build build --target MODULE_RI_EXX_SYMMETRY_rotation -j2: passed. - OMP_NUM_THREADS=1 ctest --test-dir build -V -R '^MODULE_RI_EXX_SYMMETRY_rotation$': all 15 tests passed outside sandbox. - Initial test run exposed the obsolete wrapped-coordinate expectation; the intentional semantic update and coordinate-reconstruction check are described above. No tolerance was relaxed. - git diff --cached --check and staged agent governance: passed, no blockers; documentation warning explained above. - HPC isolated intervention on 72353ee, 4 MPI ranks x 48 threads, 8-atom diamond step-two geometry, symmetry_prec=3.2e-5: first EXX energy changed from -29976.5115 to -1239.36097 eV; next two iterations -1239.36111 eV. Native EXX relative non-Hermiticity at k=1 decreased from 0.467132 to 6.25019e-5. Base Hamiltonian remained Hermitian to about 2e-16. Diagnostic full build passed, executable version v3.11.0-beta10. Full hybrid SCF and cell-relax convergence were not verified.
Reindex existing global k-stars in set_after_vc using the newly analyzed symmetry operations, including time reversal. Preserve IBZ representatives, weights, spin and pool ownership so the previous ionic-step density matrix can be used with the refreshed EXX rotations. Pass symmetry explicitly from the existing ESolver_FP caller; no new ESolver functions or global dependencies. Verification: - cmake --build build --target abacus_std_para -j2: passed after removing a pre-existing zero-byte read_atoms_helper.cpp.o from the build cache. - OMP_NUM_THREADS=1 ctest --test-dir build -V -R '^MODULE_CELL_klist_test(_para[14])?$': all three targets passed (37 serial cases, 2 parallel cases with one and four ranks). - git diff --check and working-tree governance checks: no blockers. - No INPUT names, defaults or parsing behavior changed; no INPUT docs needed. - Existing global logger arguments retained with net reference change zero. - HPC cell-relax regression will follow deployment.
Refresh energy references for the current-geometry initial EXX Hamiltonian. Keep the quick tests' inputs, thresholds, outer-loop limits and timing references unchanged. Verified all three cases with Autotest.sh, four MPI ranks and one OpenMP thread; all six energy comparisons passed. No INPUT or interface changes require documentation updates.
mohanchen
added
EXX and lr-TDDFT
Collaborator
Is this HEX output introduced in this PR? If so, please fix this; if not, maybe there should be someone fixing this in another PR. |
Use std::scientific to replace the floating-point format rather than adding a scientific flag to an existing fixed flag. This avoids hexadecimal timing output after pressure logging. Verification: cmake --build build --target abacus_std_para -j2 passed. Autotest.sh with four MPI ranks and one OpenMP thread passed relax_hse_gamma and relax_cell_hse_multik (four energy checks). All eight EXX timings matched decimal scientific notation, including those after pressure output. No INPUT or interface changes require documentation updates.
Collaborator
Author
It's an existing problem. I've fixed it in 6e9e079 by the way. |
Flying-dragon-boxing
requested changes
Oct 9, 2026
Share periodic equivalence across initial IBZ reduction, rotation consistency checks, initial k-star construction and variable-cell k-star updates. Keep the epsilon-dependent normalization convention while allowing equivalent boundary representatives to match. Add regressions for reciprocal translations and tolerance changes at the k-point boundary. No INPUT defaults or tolerance semantics change; user-facing parameter documentation does not require an update.
Flying-dragon-boxing
approved these changes
Oct 10, 2026
Flying-dragon-boxing
left a comment
Flying-dragon-boxing
left a comment
Collaborator
There was a problem hiding this comment.
Verified the periodic k-point comparison fix at 11b7b3c with a fresh build and the previously failing automatic Gamma mesh (2 2 2 0.000002 0 0). The symmetry tolerance still increases from 1e-6 to 3.2e-5, and both SCF steps now complete without the k-star mismatch. The zero-offset control also passes. These were two-step cell-relax regression runs, not a check of final structural convergence; full hybrid/EXX runtime timing output was not rerun.
mohanchen
approved these changes
Oct 10, 2026
Critsium-xy
added a commit
to mohanchen/abacus-mc
that referenced
this pull request
Oct 10, 2026
…ng#8099 The previous merge of upstream/develop brought in deepmodeling#8099, whose code still used the pre-refactor API and broke every CI build: - symm_rotation_k.cpp: reset_symmetry() assigned Irreducible_Sector(), renamed to Irred_Sector on this branch. - symm_test_analysis.cpp and test_symm_rotation.cpp: replace ucell.st, find_irreducible_sector() and cal_Ms(kv, cell, ...) with the new explicit nat/ntype/iat2it/iat2ia/itia2iat signatures, and assign the index maps as std::vector instead of new int[]. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
mohanchen
added a commit
that referenced
this pull request
Oct 10, 2026
…ing reference members (#8080) * Refactor: remove the UnitCell Statistics layer and flattening reference members UnitCell stored its atom-count and index-map data in a nested `Statistics` struct and re-exported every field as a reference member (`int& nat = st.nat`, `int*& iat2it = st.iat2it`, ...). This created dual-view aliasing, an invisible raw-pointer ownership rule (fixtures had to `new[]` but never `delete[]`, and several unit tests double-freed as a result), and a latent shallow-copy double-free since the implicit copy constructor remained available while copy assignment was silently deleted by the reference members. Changes: - Repoint the only consumers of the `Statistics` type: module_symmetry and module_exx_symmetry interfaces now take `const UnitCell&` instead of `const Statistics&`; all call sites pass `ucell` (or `*ucell`). - Move the nine fields into UnitCell as direct members under the existing top-level names, so `ucell.nat`-style call sites are unchanged. Delete the `st` member, the reference members, and the `Statistics` struct. - Convert iat2it/iat2ia/iwt2iat/iwt2iw from raw int* to std::vector<int>; migrate the ~100 test/support allocation sites from `new int[]` to resize()/assign(). Production sites (unitcell.cpp, cal_wfc.cpp) and the two force_pw.cpp device-upload call sites use .data(). - Delete UnitCell copy constructor/assignment and default the move operations, so accidental copies fail at compile time. - Update AGENTS.md: replace the obsolete fixture note with the new ownership/vector rule. Verification: - Full build succeeds (no compile errors). - ctest -R "unitcell|CELL_SYMMETRY|symmetry|hcontainer|dftu|operator_lcao| module_dm|pdos|hsr|restart_exx|gint|relax|esolver": 33/33 passed. - agent_governance_check: no issues. No INPUT parameter or user-facing behavior change; docs/parameters.yaml unaffected. The Lattice lat reference-member block is intentionally out of scope and can follow the same recipe in a later PR. * Fix GPU build: use .data() for iat2it cudaMemcpy in gint_gpu_vars The Statistics refactor converted UnitCell::iat2it from int* to std::vector<int>. The CUDA-only translation unit gint_gpu_vars.cpp was not compiled by the local CPU build, so this call site was missed: cudaMemcpy expects a const void* source but received the vector object. Pass ucell.iat2it.data() instead, matching the force_pw.cpp device-upload sites. * Delete UnitCell move operations to avoid double free The defaulted move constructor shallow-copied the owning raw pointer 'atoms' together with 'set_atom_flag' without resetting the source, so both the moved-to and moved-from objects ran delete[] on the same Atom array. The defaulted move assignment was implicitly deleted because the reference aliases into 'lat' (Coordinate, a1, latvec, ...) cannot be re-bound, triggering -Wdefaulted-function-deleted. Explicitly delete both move operations so any accidental move fails at compile time; correct move semantics require an owning container for 'atoms' and removal of the reference aliases in later refactoring steps. * Add compile-time guard for UnitCell non-copyable/non-movable contract Add static_asserts on std::is_copy/move_constructible/assignable so that re-enabling either operation during future UnitCell refactoring fails the translation unit at compile time instead of reintroducing the double free and dangling 'lat' reference aliases. * update AGENTS.md * Decouple Symmetry::analy_sys from UnitCell Replace the const UnitCell& parameter of analy_sys (and its three private helpers is_all_movable, analyze_magnetic_group, analyze_magnetic_group_nspin4) with only the index data they actually read: nat, ntype, iat2it, iat2ia, itia2iat. This breaks the UnitCell <-> Symmetry type-level dependency cycle: symmetry.h no longer names UnitCell at all, and the three implementation files drop their unitcell.h includes. The two other classes in the module that still need UnitCell (Irreducible_Sector, Symmetry_rotation_k) carry their own forward declaration now. All production and test call sites pass the five data members explicitly; behavior is unchanged. * Decouple Irred_Sector and Symmetry_rotation_k from UnitCell Replace the const UnitCell& parameters of find_irred_sector, cal_return_lattice_all, gen_symm_bvk (Irred_Sector) and cal_Ms / contruct_2d_rot_mat_ao (Symmetry_rotation_k) with the data they actually read: nat, ntype, the iat2it/iat2ia maps, and explicit symm/atoms/lat/latvec/lmax arguments. This removes the possibility of passing mismatched atoms/lat alongside one cell's index maps and makes both headers free of any UnitCell name; the three implementation files drop their unitcell.h includes. Drop the unused get_aRb_direct helpers (no callers anywhere in the tree). Rename for brevity: Irreducible_Sector -> Irred_Sector, irreducible_sector.{h,cpp} -> irred_sec.{h,cpp} (matching the existing irred_sec_bvk.cpp and the irs_ member), find/get_irreducible_sector -> find/get_irred_sector, and gen_symmetry_BvK -> gen_symm_bvk. Comments, log strings and the output file name keep the standard "BvK" spelling. All three production call sites (DFTU x2, RDMFT) and the EXX info printer are updated; behavior is unchanged. * Decouple EXX Symmetry_rotation from UnitCell and drop dead test helpers The EXX/RPA/RDMFT rotation templates only consume the atom index maps, so pass iat2it/iat2ia explicitly instead of const UnitCell&, and remove the unitcell.h include from symm_rotation.h (forward-declare UnitCell for the non-template print_symrot_info_k). TUs that dereference UnitCell now include unitcell.h directly. Delete unreachable helpers: test_HR_rotation (both overloads), test_Cs_rotation, get_Rs_from_adjacent_list, get_Rs_from_BvK. Also fix the three missed .hpp call sites of the Irred_Sector rename (exx_lri_interface.hpp, exx_lri.hpp, rpa_lri.hpp). Verified: make -j 30 builds abacus_max_para with 0 warnings; MODULE_RI_EXX_SYMMETRY_rotation test target also builds. * Point integration tests to local build path Set the ABACUS executable in Autotest.sh and general_info to the local build_max_para_test/abacus_max_para binary for local test runs. * Fix Intel Makefile build after irred_sec rename Makefile.Objects still listed irreducible_sector.o, which no longer exists after renaming irreducible_sector.{h,cpp} to irred_sec.{h,cpp}; update it to irred_sec.o so the mpiicpx Makefile build links again. * Fix iat2it/iat2ia allocation in relax_sync unit test The upstream merge brought in a test that assigned new int[nat] to ucell.iat2it/iat2ia, but these are std::vector<int> members on this branch. Use resize() instead, matching the other fixtures in this file and removing the leaked allocation. Verified: MODULE_RELAX_relax_sync_test target builds with 0 errors. * Revert general_info EXEC to 'abacus' The local absolute build path broke CUDA CI jobs that do not have this directory; restore the upstream default of resolving abacus from PATH. * Revert Autotest.sh executable to 'abacus' Complete the partial revert in b6be72d: the hardcoded local path build_max_para_test/abacus_max_para was still set in Autotest.sh, so CUDA CI jobs (which lack that local CPU build) failed to launch every integration test case. Restore the upstream default and resolve the executable via PATH, ABACUS_EXE, or the -a option for local runs. * Fix stale Statistics/Irreducible_Sector uses after merging #8099 The previous merge of upstream/develop brought in #8099, whose code still used the pre-refactor API and broke every CI build: - symm_rotation_k.cpp: reset_symmetry() assigned Irreducible_Sector(), renamed to Irred_Sector on this branch. - symm_test_analysis.cpp and test_symm_rotation.cpp: replace ucell.st, find_irreducible_sector() and cal_Ms(kv, cell, ...) with the new explicit nat/ntype/iat2it/iat2ia/itia2iat signatures, and assign the index maps as std::vector instead of new int[]. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * Fix test_hsr_writer after merging #8068: iat2it is now std::vector #8068 added a test that assigned `new int[1]` to ucell.iat2it, which no longer compiles now that UnitCell owns iat2it as std::vector<int>. Use resize() as the other test in this file already does. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: abacus_fixer <mohanchen@pku.eud.cn> Co-authored-by: Critsium <tsfxwbbzxy@163.com> Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This file contains hidden or 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
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.
Fix divergent EXX energies during cell relaxation by:
symrot_) after the cell changes and the symmetry reanalyzes.Validation (Waiting for the run to complete)
Before fix:
After fix: