Repository navigation
Conversation
ABACUS-PR-BOT
left a comment
There was a problem hiding this comment.
Reviewed statically at 5477d6c4b16b22604fe33ff7902356fcf02c6cf4: the local phase sign, units and conversion-before-history/source-time logic are consistent with the inspected conventions. No confirmed new implementation defect; the two comments are non-blocking. Pinned governance, whitespace and a static merge check against 27bbd6cfa6ec40ef4c5699b0c46f2e7412b840b7 passed; build/runtime validation was not run and CI diagnostics were unavailable. This review does not establish restart-replay, atom-wrap or complete finite-field cell-stress support.
| const auto& cell = block.first.second; | ||
| const ModuleBase::Vector3<double> cell_index(cell[0], cell[1], cell[2]); | ||
| const ModuleBase::Vector3<double> translation = (cell_index * lattice) * lat0; | ||
| const std::complex<double> phase = hybrid_input_phase(source_A, translation); | ||
| block.second = block.second * phase; |
There was a problem hiding this comment.
Non-blocking: Even for a Gamma-centered mesh, extending the transformed density across a BvK translation B gives D0(R+B)=exp(-i A_source.B)D0(R), while the configured LibRI fallback folds those images onto one representative. Please document that finite-mesh boundary and retain a noncommensurate-field k-mesh/cell-convergence validation; the local phase tests and one mesh do not quantify its effect. A documented follow-up after merge is acceptable; no measured trajectory failure is asserted.
| const double left_value = 0.1; | ||
| const double right_value = 0.2 * iteration; | ||
| const double source_value = time == RI_2D_Comm::HybridDensityTime::Left ? left_value : right_value; | ||
| const ModuleBase::Vector3<double> source_A(source_value, 0.0, 0.0); |
There was a problem hiding this comment.
Non-blocking: These tests construct source_A themselves, so swapping A_left_ha()/A_right_ha() in Exx_LRI_Interface::exx_eachiterinit would leave their inputs unchanged. Please add a narrow regression through the production adapter with distinct endpoints, a later electronic step and an in-step mixer restart, checking that an accessor swap fails. The existing helper/Pulay/Broyden coverage is useful; this can follow after merge.
What's changed?
For LCAO RT-TDDFT with
td_stype=2, RI exact exchange currently receives density blocks in the hybrid gauge representation, although the RI integrals are evaluated in the ordinary NAO basis. Convert the input density to that same ordinary basis before the RI contraction. The existing phase applied to the assembled Fock matrix does not correct a contraction made with an inconsistent input representation.With ABACUS's conjugate-first DMK storage, RI transpose and
exp(-ik.R)Fourier conventions, the required transformation isHere HG denotes hybrid gauge.$D^0$ and $D^{\mathrm{HG}}$ represent the same density operator in the ordinary NAO and hybrid gauge representations, respectively. $\mathbf A_{\mathrm{source}}$ is the vector potential associated with the source density. The Cartesian lattice translation includes
lat0; the vector potential is already in Hartree atomic units.Convert each incoming density with its own source vector potential before storing or mixing history. Mixing unconverted densities from different source frames and then multiplying by one current phase is not equivalent. The first iteration of a later electronic step uses the left density and
A_left_ha; later iterations and post-convergence EXX updates use the right density andA_right_ha. A mixer reset within an electronic step does not change the density's source time.The conversion, layout checks and common-frame mixing live in
module_ri; ESolver only passes the existing field context. Ordinary SCF and other gauges retain their existing k-space mixing path. Complex RI tensors are required for this hybrid gauge phase, soexx_real_number=1is rejected during EXX initialization for LCAO hybrid gauge RT calculations, before RI setup.Linked Issue
No separate issue: the defect, representation convention and regression coverage are described here.
Unit Tests and/or Case Tests for my changes
Verification on commit
5477d6c4b16b22604fe33ff7902356fcf02c6cf4, based onf264b4022fd477734de0f3ace723f6da9e306943:BUILD_TESTING=ON.cmake --build /tmp/upstream-hse-pr-20261014 --target esolver ri xc_ io_input MODULE_RI_hybrid_gauge_phase_test MODULE_RI_exx_info_test -j4: passed.cmake --build /tmp/upstream-hse-pr-20261014 --target abacus_std_para -j4: full executable compiled and linked;--versionreports beta10 and-h exx_real_numberdisplays the compatibility requirement.OMP_NUM_THREADS=1 OPENBLAS_NUM_THREADS=1 ctest --test-dir /tmp/upstream-hse-pr-20261014 -V -R 'MODULE_RI_(hybrid_gauge_phase|exx_info)_test': 2/2 CTests, 12/12 GTests passed. Coverage includes phase sign, source-time selection, independent tensor storage, common-frame plain/Pulay/Broyden mixing, in-step history reset, topology/nonfinite guards and early real-RI rejection with compatible-input controls.git diff --checkand the governance checker passed without blocking findings. A read-only merge check against develop27bbd6cfa6ec40ef4c5699b0c46f2e7412b840b7was clean; the merged tree has not been rebuilt.Additional runtime validation was performed on a separate validation build containing this density-phase correction. A fixed-ion MoS₂ HSE calculation in hybrid gauge (
td_stype=2), using a DZP basis and a Γ-centered 12×12×1 k-point mesh, completed 14 fs of propagation under an applied electric field, with all electronic steps converged. The results were qualitatively consistent with expectations.Governance Notes
exx_real_numberInput_Item description documents the complex-RI requirement; the initialization check gives an early diagnostic for the incompatible setting. No additional developer-note file or generated documentation is needed for this localized fix.PARAM.inp.nspinread moves into the preparation call; global-reference budget is unchanged (added=removed=1).Reminder