Fix: add DeePKS V_delta commutator -i[V_delta, r] to rt-TDDFT velocity-gauge current#7486
Open
xuan112358 wants to merge 2 commits into
Open
Fix: add DeePKS V_delta commutator -i[V_delta, r] to rt-TDDFT velocity-gauge current#7486xuan112358 wants to merge 2 commits into
xuan112358 wants to merge 2 commits into
Conversation
…y-gauge current
In velocity gauge (td_stype=1), out_current=1 builds the current operator
(current_term) only from the kinetic and nonlocal-pseudopotential terms, so the
DeePKS non-local potential V_delta contribution -i[V_delta, r] was missing. Since
V_delta couples different atoms, this commutator is nonzero, and its omission
produced a spurious DC component in the reported DeePKS current.
- Add TDDeePKS operator (td_deepks_lcao.{h,cpp}): a lcao_tddft_periodic sub-chain
node inserted between TDEkinetic and TDNonlocal that accumulates -i[V_delta, r]
into current_term, mirroring the nonlocal term's phase-free commutator. It only
writes the current term and forwards hR_tmp; it does not modify hR.
- Add get_psi_r_alpha / init_alpha to cal_r_overlap_R for the r-weighted projection
<phi|r|alpha> between orbitals and DeePKS projectors, reusing the existing
Center2_Orb machinery. Add an optional lmax_extra argument to initialize_orb_table
(default 0 keeps existing callers byte-identical) so the tables cover the higher
descriptor Lmax.
- Register the operator in hamilt_lcao.cpp and add the new source to the build lists.
Currently supports nspin=1. Validated: <phi|alpha> matches phialpha to ~2e-5;
disabling the term reproduces the pre-fix current to ~1e-17; the DC fraction of the
DeePKS current drops from +0.5 to ~0 (PBE/HSE level).
There was a problem hiding this comment.
Pull request overview
This PR fixes rt-TDDFT velocity-gauge DeePKS current reporting by adding the missing DeePKS nonlocal potential commutator contribution into the velocity-operator current term, and extends the two-center integral machinery to support the needed r-weighted orbital–projector projections.
Changes:
- Introduces a new LCAO TD operator (
TDDeePKS) that accumulates the DeePKS commutator contribution intoTD_info::td_vel_op->current_term, placed betweenTDEkineticandTDNonlocal. - Extends
cal_r_overlap_Rwithinit_alpha()andget_psi_r_alpha()plus aninitialize_orb_table(..., lmax_extra)option to cover higher-L DeePKS descriptor projectors. - Registers/builds the new operator by wiring it into
hamilt_lcao.cppand adding sources to CMake/Makefile object lists.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| source/source_lcao/module_operator_lcao/td_deepks_lcao.h | Declares the new TD DeePKS operator that contributes the missing commutator term to the velocity-gauge current. |
| source/source_lcao/module_operator_lcao/td_deepks_lcao.cpp | Implements commutator accumulation logic and forwards the shared TD real-space buffer along the TD operator sub-chain. |
| source/source_lcao/module_operator_lcao/CMakeLists.txt | Adds td_deepks_lcao.cpp to the operator library build. |
| source/source_lcao/hamilt_lcao.cpp | Registers/inserts the new TD DeePKS operator between TDEkinetic and TDNonlocal when DeePKS is enabled. |
| source/source_lcao/CMakeLists.txt | Adds td_deepks_lcao.cpp to the broader LCAO build list. |
| source/source_io/module_hs/cal_r_overlap_R.h | Adds alpha/projector initialization + `<phi |
| source/source_io/module_hs/cal_r_overlap_R.cpp | Implements alpha/projector tables and get_psi_r_alpha(), and updates orb table sizing logic. |
| source/Makefile.Objects | Adds td_deepks_lcao.o to object build lists. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+126
to
+130
| ModuleBase::timer::start("TDDeePKS", "calculate_current"); | ||
|
|
||
| const Parallel_Orbitals* paraV = TD_info::td_vel_op->get_current_term_pointer(0)->get_atom_pair(0).get_paraV(); | ||
| const int npol = this->ucell->get_npol(); | ||
|
|
Comment on lines
+22
to
+29
| /// The TDDeePKS class template inherits from class T. | ||
| /// It supplies the missing commutator contribution -i[V_delta, r] of the DeePKS | ||
| /// non-local potential to the rt-TDDFT velocity-gauge current (out_current=1, | ||
| /// td_stype=1). V_delta is a cross-atom projected operator | ||
| /// (V_delta_{mu,nu} = sum_iat0 <phi_mu|alpha> gedm <alpha|phi_nu>), so [V_delta,r] | ||
| /// is non-zero and contributes to the current operator. This term is otherwise | ||
| /// absent from current_term (only filled by TDEkinetic and TDNonlocal), which | ||
| /// produces a spurious DC offset in the DeePKS current/spectrum. |
Comment on lines
+58
to
+64
| /** | ||
| * @brief contributeHR() adds -i[V_delta, r] to TD_info::td_vel_op->current_term. | ||
| * This operator does NOT modify hR (V_delta itself is added by the DeePKS | ||
| * operator); it only forwards the shared hR_tmp pointer to the next TD | ||
| * sub-operator (TDNonlocal) and writes the current term. | ||
| */ | ||
| virtual void contributeHR() override; |
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.
In velocity gauge (td_stype=1), out_current=1 builds the current operator
(current_term) only from the kinetic and nonlocal-pseudopotential terms, so the
DeePKS non-local potential V_delta contribution -i[V_delta, r] was missing. Since
V_delta couples different atoms, this commutator is nonzero, and its omission
produced a spurious DC component in the reported DeePKS current.
node inserted between TDEkinetic and TDNonlocal that accumulates -i[V_delta, r]
into current_term, mirroring the nonlocal term's phase-free commutator. It only
writes the current term and forwards hR_tmp; it does not modify hR.
<phi|r|alpha> between orbitals and DeePKS projectors, reusing the existing
Center2_Orb machinery. Add an optional lmax_extra argument to initialize_orb_table
(default 0 keeps existing callers byte-identical) so the tables cover the higher
descriptor Lmax.
Currently supports nspin=1. Validated: <phi|alpha> matches phialpha to ~2e-5;
disabling the term reproduces the pre-fix current to ~1e-17; the DC fraction of the
DeePKS current drops from +0.5 to ~0 (PBE/HSE level).