Skip to content

Fix: refresh v_eff again after updating XC type - #7840

Merged
mohanchen merged 2 commits into
deepmodeling:developfrom
maki49:fix-refresh-exx
Aug 20, 2026
Merged

Fix: refresh v_eff again after updating XC type#7840
mohanchen merged 2 commits into
deepmodeling:developfrom
maki49:fix-refresh-exx

Conversation

@maki49

@maki49 maki49 commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

to prevent adding an extra PBE potential onto the hybrid Hamiltonian in the 1st iter of the EXX loop

Fix #7839

Copilot AI lite review requested due to automatic review settings August 20, 2026 03:34

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes Issue #7839 by ensuring v_eff is refreshed immediately after the XC functional switches from the first (PBE/GGA) loop to the hybrid/EXX loop, preventing the first EXX iteration from building a Hamiltonian with a stale GGA v_eff and then adding EXX on top (double-counting exchange).

Changes:

  • In the LRI EXX path, detect the two_level_step 0→1 transition after exx_after_converge() and force a potential refresh via ElecState::cal_converged().
  • In the LIP solver EXX path, apply the same refresh on the 0→1 transition in iter_finish() to avoid carrying stale v_eff into the first EXX iteration.
  • Add explanatory comments documenting why the refresh is required at the transition point.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
source/source_lcao/module_ri/exx_lri_interface.hpp Refreshes v_eff via cal_converged() on the 0→1 EXX two-level transition after switching XC type.
source/source_esolver/esolver_ks_lcaopw.cpp Applies the same v_eff refresh on the solver-side 0→1 transition for the LIP EXX workflow.
Suppressed comments (1)

source/source_esolver/esolver_ks_lcaopw.cpp:236

  • Rule: Add focused tests for key features, bug fixes, INPUT behavior changes, heterogeneous kernels, and core-module refactors.
    Severity: warning
    Location: source/source_esolver/esolver_ks_lcaopw.cpp:233-236
    Reason: This change fixes a two-level EXX/hybrid transition bug (Issue #7839) but the PR does not add a regression test, despite the repository having C++ test infrastructure (e.g. source/source_esolver/test/ and source/source_lcao/module_ri/test/).
    Suggested action: Add a focused regression test that exercises the 0->1 XC switch and verifies the first EXX-iteration Hamiltonian/eigenvalues are not polluted by a stale GGA v_eff (e.g. the reported H2 SZ HF case or a smaller unit-level check around the XC switch + potential refresh).
    Exception: human approval required
            if (!conv_esolver && two_level_step_before == 0 && this->two_level_step == 1)
            {
                this->pelec->cal_converged();
            }

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

Comment thread source/source_esolver/esolver_ks_lcaopw.cpp
Comment thread source/source_lcao/module_ri/exx_lri_interface.hpp
@mohanchen mohanchen added Refactor Refactor ABACUS codes Bugs Bugs that only solvable with sufficient knowledge of DFT EXX and lr-TDDFT Related to EXX or lr-TDDFT labels Aug 20, 2026
@mohanchen
mohanchen merged commit 815c370 into deepmodeling:develop Aug 20, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bugs Bugs that only solvable with sufficient knowledge of DFT EXX and lr-TDDFT Related to EXX or lr-TDDFT Refactor Refactor ABACUS codes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A extra PBE veff(xc) is added into 1st iter in EXX loop after the convergence of PBE loop

3 participants