Skip to content

Feat: add PPCG solver for PW diagonalization (reopened) - #8042

Closed
cheerly-pku wants to merge 206 commits into
deepmodeling:developfrom
cheerly-pku:ppcg-v2-reopen
Closed

cheerly-pku wants to merge 206 commits into
deepmodeling:developfrom
cheerly-pku:ppcg-v2-reopen

Conversation

@cheerly-pku

@cheerly-pku cheerly-pku commented Sep 28, 2026 •

Copy link
Copy Markdown

Summary

Adds ks_solver=ppcg for plane-wave Kohn–Sham diagonalization, continuing #7848. The solver updates blocks in [X,W,P], removes dependent local-subspace directions, recovers competing low roots discarded by local blocks, and validates the final residuals with freshly applied operators. CPU is the main implementation; GPU execution uses device H/S operators with host-side PPCG linear algebra and explicit transfers.

Current revision: 61d4b7ba3, checked 2026-10-29. All 18 GitHub check-runs and both Read the Docs projects pass. Merge conflicts with develop are resolved. Five review discussions remain open, listed below.

Implementation and INPUT changes

  • pw_diag_ndim selects PPCG's local block size. pw_diag_rr_step controls the full Rayleigh–Ritz interval, defaults to 1, and must be positive. Intermediate steps maintain S-orthogonality. Initialization, startup correction, convergence verification, failed orthogonalization, the last update, and conditional recovery of competing discarded roots can add Ritz solves. H/S image refresh is independent: every 16 updates, after severe projection cancellation, and near the numerical precision limit.
  • Stopping thresholds retain solver-specific meanings. CG/Davidson generally compare eigenvalue changes; PPCG checks ||Hψ − εSψ|| against max(ethr_band[i], ethr, 1e-14). The direct residual criterion is retained after a square-root criterion failed force/stress validation. The INPUT help and solver guide describe this distinction.
  • pw_diag_thr_max=0 preserves adaptive thresholds. A positive value caps the supplied threshold from the first electronic iteration, including each fixed-cell relaxation step. It requires PW KSDFT, calculation=scf or relax, and diago_smooth_ethr=false; single precision requires at least 5e-5. Cell relaxation, MD and NSCF reject a positive cap. Each solver keeps its own stopping rule and numerical floor.
  • PPCG configuration is passed explicitly through HSolverPW. Invalid sizes and inputs are rejected collectively before solver collectives, application-boundary exceptions receive ABACUS diagnostics, and unconverged k points produce one summary per solve/electronic step. Stochastic sdft + ppcg is rejected, including zero-stochastic-band configurations.

The PR also contains shared fixes exposed by cross-solver regressions: CG projection and stable energy-decrease evaluation; Davidson orthogonalization, dependent-direction rejection and restart handling; dense-grid charge/kinetic-density mixing; and the local Libxc meta-GGA term in the uniform-gradient limit. Empty and uneven MPI ownership remains supported. Shared NSCF setup now reads the current pw_diag_nmax for CG, Davidson, Davidson-subspace and PPCG; BPCG retains its separate update budget.

PW text coefficients use 17 significant digits. Complete wavefunction and Wannier-bundle comparisons, their corruption tests, and independently calibrated fixtures are included. The shared NSCF change and Wannier comparator are still in this PR; their separation into other PRs remains under review.

Validation: current revision and expanded replacement scan

Verification Revision Observed result
GitHub Actions, including CPU and CUDA tests and build configurations 61d4b7ba3 18/18 check-runs pass; CPU, CUDA
Read the Docs 61d4b7ba3 Both projects pass; abacus-develop, abacus-rtd
Focused local CTest: solvers/MPI, INPUT, charge mixing, estate output and stochastic linkage 61d4b7ba3 60 enabled tests pass; existing comparison benchmark disabled
Local integration, original solver selections 61d4b7ba3 137/137 PW + 27/27 RI cases pass, comprising 1063 reference-property checks
Expanded CPU scan, PPCG-side reference checks 5ae5435a0 135 cases: 83 pass, 52 warnings, 0 fatal outcomes
Actual solver replacements within that expanded scan 5ae5435a0 126 cases: 74 pass, 52 warnings; nine native PPCG/BPCG controls pass
Inner PPCG convergence diagnostics in the same scan 5ae5435a0 113 warning summaries across 27 cases, counted separately from reference warnings

The current 164-case integration run uses each case's original solver selection. The expanded scan replaces CG/Davidson/Davidson-subspace with PPCG and belongs to the earlier fixture revision 5ae5435a0; it has not been rerun on 61d4b7ba3. Thus 52 is the latest measured replacement-warning count. Eight independently calibrated fixtures reduced that count from 60 to 52, with no other case-status regression. All 270 baseline/PPCG calculations in that paired scan exited normally.

Current local verification used GNU 12.3, OpenMPI 5.0.10, Release, four MPI ranks for integration, and one OpenMP/BLAS thread. The final CPU binary SHA256 is ba7a329ce12350558eaf927c2ef3ccae8d7acc614c572c26ec01add36a09fbdb. The preceding merge aaaf7f530 also passed the 60 CTests with both OpenBLAS 0.3.20 and 0.3.33; final-head CTests used the toolchain OpenBLAS. LDA/PBE LR-force tests loaded matching Libxc 7.0.0 with KXC enabled. No INPUT, reference, threshold or checker was altered during the isolated current-head PW and RI runs.

Reference and collector calibration

Fixture READMEs record the accuracy settings, independent references, solver/MPI cross-checks and retained diagnostics. Existing scalar acceptance tolerances were retained.

  • Wavefunctions: scf_out_band_dos_wf checks all complex coefficients at both k points, metadata, G-vectors, band energies, normalization and orthogonality. It fits one phase per nondegenerate band, with 1e-7 coefficient/Gram limits and the existing 1e-8 eV energy limit. Missing, malformed, incomplete or non-finite requested output fails.
  • Wannier output: 101_PW_W90 uses one unitary gauge per k point jointly for the full AMN matrix, every MMN link and the energy operator. Every labelled EIG value retains its 1e-8 eV limit; the energy-operator limit is also 1e-8 eV, and AMN/MMN entries use 1e-8. There is no additional degeneracy window or cluster-average allowance. Independent CG references follow the CG accuracy fixes and were cross-checked with Davidson-family methods and PPCG. CompareFile.py is identical to the current upstream base.
  • LR forces: three Davidson fixtures previously used lr_thr=1e-2 with force comparisons at 1e-6 eV/Angstrom. An independent build of the then-upstream c82d87d9e and the PR build produced identical printed dense-LAPACK results for all 108 signed force components. The fixtures now request lr_thr=1e-16 and use those upstream dense references, retaining Davidson, Z-CG, the retry policy and the original force limits. Twelve controls across OpenBLAS 0.3.20/0.3.33 and MPI 1/4 passed; maximum component/sum differences were 7.64e-8/1.43e-7 eV/Angstrom. Details are in the LDA fixture README.
  • PW warning reduction: cases 011/076/077/202/204 use independently tighter Davidson-subspace references checked against CG/PPCG. Lattice cases 808/809/812 retain their original references. Forty focused CPU/GPU controls passed 220 property checks; the eight corrected cases had no inner PPCG warnings in those controls or the paired MPI-4 sweep.

Open review discussions and validation limits

Fourteen of 19 inline review threads are resolved. The following five remain open for the original reviewer:

  1. Residual stopping semantics and accuracy/cost tradeoff.
  2. Ritz cadence and conditional root recovery.
  3. Reason for retaining the base-layer MPI rank query.
  4. Scope of the shared NSCF iteration-budget change.
  5. Scope of the Wannier comparison changes.

The 52 expanded-scan reference warnings and the inner convergence diagnostics remain open numerical validation work. That scan also retains two incomplete baseline reference records (036_PW_AF, 061_PW_RE_NEW). Deliberately short SCF/relax tests and LR-window diagnostics are listed separately in the audit; reference-check passes do not certify full physical convergence of those fixtures.

An additional MPI-2 GPU exploration retained a scf_Al100_kspace3 force-reference warning (0.001466 difference against a 1e-3 limit) and encountered k-point-pool restrictions in 39 cases. CI runs the PW GPU list at MPI-1. These extra MPI-2 findings remain open.

PPCG's GPU path uses host-side linear algebra. Native distributed GPU PPCG, multi-GPU, Intel and the complete LCAO integration matrix remain outside the local validation, as does ROCm; no general speedup or peak-memory advantage is claimed. Workspace accounting includes projected matrices, active columns, LAPACK temporaries and transfer buffers in addition to the eight persistent CPU arrays. Implementation size and complexity remain cleanup work.

The CI audit also identified an unchanged upstream VDW death-test assertion whose driver returns zero. It is recorded separately and excluded from the focused 60-test evidence above.

Governance

PR-relative governance and whitespace checks pass: seven reviewed warnings, no blockers. diago_ppcg.h requires complete types for owned vector/function members, complex/scalar traits and lightweight device declarations; it introduces no .hpp implementation dependency. The DeltaSpin call sites consolidate existing PARAM reads and pass configuration explicitly; the global-reference budget is 2 additions/20 removals, net -18. PPCG numerical code does not read globals.

C++ Input_Item registrations contain the INPUT metadata; generated YAML/Markdown are build artifacts. No governance exception is requested. This continues #7848; local report, thesis and presentation files are excluded.

Verification commands and evidence locations

The focused CTest selection used the configured CPU build:

export OMP_NUM_THREADS=1 OPENBLAS_NUM_THREADS=1 MKL_NUM_THREADS=1
ctest --test-dir "$BUILD_DIR" -V --output-on-failure --timeout 1700 \
  -R 'MODULE_HSOLVER|MODULE_ESTATE_elecstate_print|MODULE_CHARGE_mix|MODULE_IO_read_item_serial|MODULE_IO_input_domain_reader|MODULE_IO_input_error_mpi|MODULE_PW_Sto_HSolver_UTs'

Each isolated tests/01_PW and tests/08_RI snapshot ran the original harness:

timeout --kill-after=30s 7200s bash ../integrate/Autotest.sh \
  -a "$VALIDATION_BINARY" -n 4 -o 1 -j 4 -f CASES_CPU.txt

The expanded paired scan used separate original/PPCG trees and changed only the selected ks_solver values between them. Its original invocation was:

timeout --kill-after=30s 7200s bash ../integrate/Autotest.sh \
  -a "$VALIDATION_BINARY" -n4 -o1 -j4 -f "$ACTIVE_CASE_LIST"

Other recorded checks include 92 Python comparator/collector tests, 14 CLI/INPUT boundary controls, MPI PPCG at 2/3/6 ranks and two pools, and empty/uneven ownership controls. The Python command, run from tests/integrate at the earlier validated source revision, was:

python3 -m unittest -v validation_tools/test_compare_pw_wfc.py validation_tools/test_compare_wannier_bundle.py

Documentation verification used the normal parameter-export workflow with only the documentation build flags changed:

cmake -Bbuild -GNinja -DCMAKE_INSTALL_PREFIX=install \
  -DENABLE_MPI=OFF -DENABLE_LCAO=OFF -DENABLE_NATIVE_OPTIMIZATION=OFF \
  -DCMAKE_BUILD_TYPE=Debug '-DCMAKE_CXX_FLAGS_DEBUG=-O0 -g0'
cmake --build build --target install -j4
install/bin/abacus --generate-parameters-yaml > docs/parameters.yaml
python3 docs/generate_input_main.py docs/parameters.yaml
sphinx-build -E -b html docs/ html --fail-on-warning
sphinx-build -E -b latex docs/ latex --fail-on-warning

Local artifacts retain full configure commands, library identities, file manifests and raw logs. These filesystem archives are local records; the CI links and committed fixture/test sources above provide the public review entry points.

Local archive Scope
/mnt/data/Cheerly/ppcg-reconcile-20261029 Final merge build, 60 CTests, original 164-case regression, documentation comparison and review audit
/mnt/data/Cheerly/ppcg-warning-reduction-20261029 Paired expanded scan, eight-case calibration and independent raw-log audit
/mnt/data/Cheerly/ppcg-warning-reduction-20261029/lr-ci Independent upstream/dense LR controls and force calibration
/mnt/data/Cheerly/ppcg-ci-fix-20261029 Earlier HSolver, SCAN and CPU/GPU fixture controls
/mnt/data/Cheerly/ppcg-push-20261029 Earlier matrix, MPI, comparator and CLI/INPUT checks
Resolved CI failures and earlier validation history
  • HSolver/OpenBLAS: the dav_subspace_restart_spd fixture stopped at an eigenvalue-change threshold of 1e-14 with residual 8.135e-7 on OpenBLAS 0.3.20, exceeding its 2e-7 limit. Requesting 1e-15 reduced residuals to 2.028e-11 and 4.346e-10 on OpenBLAS 0.3.20/0.3.33. Production stopping rules and the acceptance limit stayed unchanged.
  • SCAN LCAO: the uniform-gradient Libxc correction required recalibrating scf_out_chg_tau. Its 1e-10 SCF setting and 12-digit cube output were checked against independent 1e-12 CPU references. CPU/GPU, Libxc 5.1.7/7.0.0 and MPI controls agreed across all 3375 grid points with original limits. Only builds with matching Libxc headers/libraries count as evidence; an initial mismatched build was excluded.
  • PW 051: pw_diag_thr=1e-12 and pw_diag_nmax=100 resolve the band-output boundary and remove its inner warning. The original references and 1e-8 eV limit remain. Eight CPU/GPU, CG/PPCG, MPI 1/4 controls passed; a budget-200 control produced the same result.
  • LR fixtures: the three failures in CPU run 40844739478 were repaired in f151acf90 using the independently verified dense references described above. At that revision, the 27-case RI suite and subsequent 28-case DeePKS/4-case other suites passed locally, followed by successful CPU and CUDA CI.
  • RTD timeout: the metadata-export build exceeded 900 seconds with default -O3 -g. The documentation-only -O0 -g0 build completed locally in 129.315 seconds; all 605 parameter records, generated Markdown and 73 HTML files were byte-identical to Release output. Strict HTML/LaTeX generation passed; PDF compilation was not part of that local check. The repaired remote build passed in 305 seconds, including 205 seconds of compilation.
  • Earlier GPU coverage: at 5cf8b634a, the matching Libxc 5.1.7 CUDA build passed 186 cases/1249 checks across the original GPU lists: PW at MPI-1; NAO, RT-TDDFT and SDFT at MPI-2. The latest remote CUDA result is linked in the current validation table.
  • Replacement-scan history: the earlier a7e7bd0f7 scan recorded 75 passes/60 reference warnings/0 fatal outcomes and 120 inner summaries across 30 cases. The later 5ae5435a0 scan supersedes those figures with 83/52/0 and 113 summaries across 27 cases. Concurrent runs support correctness comparisons; they are not controlled efficiency measurements.

cheerly-pku and others added 30 commits March 28, 2026 13:19
Consider the previous contributions made by classmates, I'm only capable to make small difference without disrupting the entire program ---- like such a small "static".
…w_Small-Changes

2025PKUCourseHW5: Case: 1 - Change rank_seed_offset to static const
…ent)

Add PPCG iterative diagonalization with two strategies:
- CONJUGATE_GRADIENT: band-by-band Polak-Ribiere CG (verified working)
- BLOCK_SUBSPACE: block subspace diagonalization

Includes potrf retry fix: save/restore original matrix before applying
diagonal shift, preventing accumulated shifts from corrupting the matrix.

Test: 1D particle-in-a-box (n_dim=10), CG strategy matches exact
eigenvalues with error 4.3e-12.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ormalization

Three fixes for numerical stability:

1. potrf: save/restore original matrix before diagonal shift retries,
   preventing accumulated shifts from corrupting the Cholesky factor.

2. sygvd/syevd: skip workspace query (lwork=-1) and allocate directly.
   The LAPACK replacement ignores workspace queries, causing the second
   call to operate on already-transformed data, corrupting eigenvalues.

3. Block subspace: add chol_qr + hpsi/spi recomputation after
   update_one_block and every rayleigh_ritz, keeping wavefunctions
   S-orthonormal and preventing numerical drift of H|psi> and S|psi>.

Results (1D particle-in-a-box, S=I):
- CG nband=1: error 4.3e-12 (unchanged, already working)
- BLOCK_SUBSPACE nband=1: no longer NaN, converges (to wrong eigenvalue
  due to algorithmic limitation with S=I)
… RR steps

1. solve_small_generalized: save/restore M matrix before retry with shifts
   (prevents accumulation of shifts on sygvd-corrupted M)

2. BLOCK_SUBSPACE: add Krylov fallback for near-collinear p/w vectors
   When p is nearly parallel to w (cos^2 > 0.99), replace p with H·w
   to keep the 3-vector subspace [psi, w, p] full rank.
   This fixes NaN eigenvalues for nband>1 with S=I.

3. LAPACK: use standard workspace query (lwork=-1) pattern for syevd/sygvd
   More robust with real LAPACK implementations.

4. CG: add periodic Rayleigh-Ritz subspace rotation every rr_step iterations
   Corrects band ordering and eigenvalue estimates after band-by-band
   line minimization. Resets PR state after rotation.
The gamma_dot function returns only the real part of inner products,
which is correct for Hermitian forms like <psi|H|psi> but wrong for
projection coefficients where the imaginary part matters.

In orth_gradient and project_against, the projection coefficient
<psi_i | v> must use the full complex inner product to correctly
remove the overlap. Using only Re(<psi_i | v>) leaves an imaginary
component that corrupts the search direction, causing excited-state
bands to converge to wrong eigenvalues.

This fixes the CG strategy bands 1 and 2 converging to the highest
eigenvalue (3.919) instead of the first excited states.
…PACE

The chol_qr_active call after update_one_block re-orthonormalized psi
but left the p vector in the old basis, creating an inconsistency.
The p vector is constructed in update_one_block using the same subspace
rotation as psi, so they start consistent. Adding chol_qr_active before
the p vector is updated breaks this consistency.
This change was unrelated to the PPCG integration and should not
have been included.
H and S are real symmetric operators whose eigenvectors are real.
The previous complex random initialization produced complex
off-diagonal elements in the H-gram matrix (max |Im| ~ 0.5 for
nband=3), causing Re(<psi_i|H|psi_j>) != <psi_i|H|psi_j>.  The
gamma_dot function only returns the real part, so all subspace
Gram matrices (built via gram()) computed wrong off-diagonals,
leading to incorrect eigenvalues from sygvd.

With real-only psi all inner products are real and gamma_dot is
exact, so both BLOCK_SUBSPACE and CONJUGATE_GRADIENT strategies
should now converge to the correct eigenvalues.
…tioning

The 3-block subspace method builds a generalized eigenvalue problem
with basis V = [psi, w, p] where p is constructed from the previous
subspace eigenvectors (p_new += w_l * cw in update_one_block).  This
makes p a linear combination of the w vectors, causing the [w, p]
block of the S-gram matrix M to become nearly rank-deficient.

With nband=3 and sbsize=4 the 9x9 M matrix has condition number
large enough that dsygvd produces negative eigenvalues for the
positive-definite problem (observed: -0.26 at iter=2), and the
eigenvalues diverge exponentially thereafter.

Setting use_p=false reduces the subspace to [psi, w] (2-block),
which is a preconditioned Davidson-like method.  It converges
robustly: the BLOCK_SUBSPACE test now passes in 57 ms with all
3 eigenvalues within 1e-8 of the exact values.

The 3-block code path is preserved for future re-enablement once
a more robust p-vector construction is implemented.
With rr_step=4, the non-RR iterations use Cholesky orthonormalization
which mixes bands through the upper-triangular U^{-1}, causing high-energy
bands to contaminate low-energy ones.  This drives CG eigenvalues to the
spectrum maximum [3.31, 3.68, 3.92] instead of the correct lowest values
[0.081, 0.317, 0.690].

Using rr_step=1 forces Rayleigh-Ritz every iteration, which correctly
diagonalizes the subspace and preserves band ordering.
The orth_cholesky call before rayleigh_ritz mixes bands through the
upper-triangular U^{-1} factor, contaminating low-energy bands with
high-energy components.  This drives CG eigenvalues to the spectrum
maximum instead of the minimum.

rayleigh_ritz solves the generalized eigenvalue problem K v = λ M v
via dsygvd, which correctly handles non-S-orthogonal bases.  The
orth_cholesky is not needed and is actively harmful.

This makes the CG RR path consistent with BLOCK_SUBSPACE, which
calls rayleigh_ritz without prior orth_cholesky.
BLOCK_SUBSPACE starts with rayleigh_ritz (line 1085) which finds correct
eigenvalues and rotates psi before the iteration loop.  CG was using
diagonal Rayleigh quotients instead — these are poor approximations for
random initial guesses, producing wrong gradients that drive the band-by-band
line_minimize toward high-energy eigenstates.

With rr_step=1 (every-iteration RR), the CG loop itself is now correct,
but without an initial RR the first line_minimize step already pushes
psi in the wrong direction, and subsequent RR steps cannot fully recover.
Backup preserved at diago_ppcg_test.cpp.bak
The linear approximation α = -C/B drops the α² term from the Rayleigh
quotient derivative dR/dα = 0.  This picks one of the two stationary
points (minimum or maximum) arbitrarily.  For bands far from convergence
it can select the MAXIMUM, driving ψ toward high-energy states instead
of the desired lowest eigenvalues.

Solve the full quadratic Aα² + Bα + C = 0, evaluate R(α) for both
roots (and the linear guess), and pick the one with the lowest R.

Also restore the CG unit test (rr_step=1, initial rayleigh_ritz).
…e use_p

Three changes to make both PPCG strategies correctly converge with rr_step=4:

1. CG non-RR path: After orth_cholesky, solve the nband x nband subspace
   generalized eigenvalue problem instead of using diagonal Rayleigh quotients.
   The upper-triangular U^{-1} from Cholesky mixes high-energy components into
   low-energy bands, making diagonal RQs overestimate the eigenvalues.  The
   subspace solve gives correct Ritz values without rotating the states,
   preserving Polak-Ribiere conjugate-direction accumulators.

2. BLOCK_SUBSPACE: Re-enable use_p=true (3-block [psi, w, p] subspace).
   The Krylov fallback (replace p with H·w when p ~ w) was already in place
   but dead because use_p was hardcoded to false.  Now it activates on the
   first iteration (p is zero-initialized) and whenever p becomes collinear
   with w after update_one_block.

3. CG test: Change rr_step from 1 back to 4 so the non-RR Cholesky path
   is exercised, validating the true Polak-Ribiere CG mechanism.
The 3-block [psi, w, p] subspace generalized eigenproblem becomes
ill-conditioned when residuals are small (near convergence).  The
[w, p] Gram block shrinks, the M matrix approaches singularity, and
dsygvd produces garbage eigenvectors that drive eigenvalues to
catastrophic values (e.g., -137775 instead of 0.081).

The p-bad H·w Krylov fallback fixes p~w collinearity but does not
address the small-residual ill-conditioning, which is fundamental to
the 3-block construction.  Keep use_p=false for robust convergence.
The [w,p] block of the Gram matrix M shrinks as residuals converge,
making M nearly singular and causing sygvd to produce garbage
eigenvectors.  Scaling w and p to unit S-norm keeps M well-conditioned
(diagonal ~1) without changing the subspace — Ritz values are identical
and Ritz vector coefficients cancel in update_one_block.

This enables the full 3-block [psi,w,p] subspace (use_p=true) by
addressing the fundamental ill-conditioning that the p-bad Krylov
fallback alone could not handle.
The Krylov fallback (replace p with Hw when p~w) was flawed:
when w is approximately an eigenvector (Hw ≈ λw), the replacement
does not fix collinearity.  After S-norm scaling, p ≈ w still,
M_wp ≈ [1,1;1,1] is rank-1, and dsygvd fails.

Instead, simply skip p for this iteration (use_p_now=false).
update_one_block still produces a valid p for the next iteration
from the w Ritz-vector contribution.
Add tests for:
- 2x2 matrix (smallest non-trivial case)
- Degenerate eigenvalues (H = I + J, multiplicity-3 degeneracy)
- Larger 20x20 tridiagonal with 5 bands
- Dense 8x8 matrix via Givens rotations (addresses full-matrix coverage)

All use CONJUGATE_GRADIENT strategy which has sygvd fallback.
BLOCK_SUBSPACE tests deferred due to dsygvd instability with some LAPACK builds.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Covers diagonal, tridiagonal, dense, pentadiagonal, degenerate,
Neumann, S≠I, gamma_g0, single-band, all-band, many-band,
bad preconditioner, tight threshold, scaled, gapped spectrum,
rr_step=1, 1x1, and eigenvector quality checks.

Adds QuickBenchmark (CI-friendly) and DISABLED_FullBenchmark.
Resolve the stochastic HSolver test CMake conflict by keeping develop's formatting and the PPCG source registration. Preserve the original PR history and numerical implementation.

@ABACUS-PR-BOT ABACUS-PR-BOT left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Static review of 8613b18477a71b0da091305ad7acfa9614497c4f against merge base 3ab6d32e7f45e626e98fdcf0121c3d6aa3e6b2ba, with independent A/B review and compatibility inspection of current develop at 75debc23d977ec932870bd4ed8f1178a3e628251.

git diff --check and the trusted governance check passed (eight reviewed warnings). No build, runtime, or CI verification was performed; the previously reported numerical validation limits remain open. The current target integration has a content conflict in source/source_hsolver/test/CMakeLists.txt, confirmed by static merge-tree inspection and GitHub.

Eight scoped inline points follow; the remaining cadence-guide correction is tracked in the existing Ritz discussion. This review does not establish an ordinary-size CPU mathematical defect.

Comment thread tests/11_PW_GPU/scf_ppcg/threshold Resolved
Comment thread source/source_hsolver/diago_ppcg.cpp Resolved
Retain PPCG tests in the configure-time fixture setup, register ppcg in the unified solver domain, and pass the MPI test executable explicitly. Add INPUT round-trip, metadata and basis-compatibility checks. Seven focused CTests pass; full build and native-case validation are recorded separately.
Reject the unsupported sdft/ppcg combination during unified INPUT validation, including zero deterministic bands, and cover both rejection and supported CG controls. Document this restriction and correct the guide to distinguish full Ritz cadence from independent H/S image refresh.
Guard the optional mallinfo2 API for supported glibc versions and report an unavailable metric elsewhere. Label post-diag retained heap separately from peak memory, and describe each solver's stopping and restart behavior accurately.
Share checked int-index and LAPACK workspace limits across CPU and GPU entry paths, reject rank-local invalid shapes collectively, and avoid overflow in block-count arithmetic. Add bounded serial and asymmetric MPI regressions. Strengthen the generalized fixture to check lowest analytic roots, fresh residuals and S-orthogonality, and split grouped test declarations.
Replace the all-one smoke thresholds with measured per-property limits: 1e-6 eV energy, 0.02 eV/Angstrom force aggregate and 0.5 kbar stress aggregate. Preserve existing inputs and references. Record RTX3090 CPU/GPU paired agreement and observed reference offsets from repeated two-rank and four-rank runs.
@cheerly-pku

Copy link
Copy Markdown
Author

Replying specifically to your October 22 review summary of 8613b184.

The conflict in source/source_hsolver/test/CMakeLists.txt was resolved in 985a22332, preserving the PPCG test registration. I have just checked the current head, ac9f106a56d538ef75fd495e97957573dcd5c7b3: GitHub reports no merge conflict, and all 19 check contexts are successful.

The wider numerical-validation limitations you mention remain open. The expanded solver-substitution failures are still under investigation on a separate, unpublished local tree. Its changes and measurements are recorded separately from the validation of this pushed head.

…curacy

Preserve accurate iterative eigensolver updates and basis recovery, add the
optional pw_diag_thr_max bound across SCF and fixed-cell relaxation cycles,
and cover numerical and INPUT boundaries with focused tests. Retain the
validated noncollinear density/Libxc fixes and full-precision PW output.

Compare complete wavefunction and Wannier data with phase/gauge-aware checks.
Record independent reference calibration and explicit accuracy settings in
the affected case READMEs without relaxing acceptance thresholds.

Local validation of this snapshot (GNU 12.3, OpenMPI 5.0.10, OMP=1):
- Autotest.sh -n4 -o1 -j4 over 130 baseline and 130 PPCG-side calculations:
  all runtime exits zero; 121 substitutions: 64 pass, 57 warning, 0 fatal.
- Nine unchanged/native controls pass. Two existing incomplete baseline
  reference records and remaining SCF/inner warnings are preserved.
- Final relax_dftu_spin4 cross-checks: CG, Davidson-subspace and PPCG;
  MPI1/MPI4 reference checks pass at original tolerances.
- HSolver/INPUT cap tests: 33/33. Final binary CLI controls: 7/7.
- C++11 checks for cap changes and staged governance pass.

INPUT help is generated from updated Input_Item registrations; generated
documentation is not committed. Case READMEs describe reference provenance.
The local report, thesis, presentation, and experiment archives are excluded.
Integrate origin/ppcg-v2-reopen without rewriting either history. Preserve its workspace guards, stochastic INPUT rejection, GPU calibration, benchmark portability, and empty-rank MPI support alongside the locally validated solver and reference changes.

Retain both Davidson stability and ownership test harnesses. Apply empty-rank BLAS dimensions and zero projection contributions to the new orthogonalization paths. Keep complete shared-gauge Wannier comparisons and both INPUT descriptions. The merged tree is being rebuilt and retested before publication.
Set nscf_hse_same pw_diag_thr to 1e-12 with the default 50-iteration budget. The former threshold leaves a 1.05e-8 eV CG band error after the numerical update and crosses the stored 1e-8 eV comparison boundary.

CG, Davidson-subspace and PPCG reproduce the existing band reference with the tighter input and pass the original Autotest energy/band checks at four MPI ranks. Four tighter controls independently agree within 2.4e-13 eV. Preserve references, comparison tolerances and the PPCG residual warning at the default budget. No production source changes.
The restart_spd fixture stops on eigenvalue changes but independently checks a residual. Its 1e-14 request stopped one expansion too early with OpenBLAS 0.3.20, yielding 8.14e-7 against the unchanged 2e-7 residual limit.

Request 1e-15 for this fixture without changing production stopping rules, the 200-iteration budget, or any acceptance threshold. The exact test passes all 18 stability scenarios on OpenBLAS 0.3.20 and 0.3.33 (36 runs), plus all 47 focused CTests. The original CI failure is reproduced with 0.3.20; the corrected residual is 2.03e-11.
The shared Libxc uniform-gradient correction also changes this LCAO case. Its old charge cube failed CPU and GPU checks at the Fermi-energy header. Tighten SCF to 1e-10 and print 12-digit cube fields; generate references independently at 1e-12 while retaining all acceptance limits and the original 100-step budget.

Validate all 3375 grid points and scalar checks with Libxc 5.1.7 and 7.0.0, explicit CPU/GPU runs and one/two MPI ranks. Eight original Autotest runs pass, plus a Libxc 5.1.7 CPU four-rank control. The 65-case GPU LCAO multik suite passes with the CI Libxc version.
Set pw_diag_thr=1e-12 to keep CG bands away from the original 1e-8 eV printed comparison boundary. Set pw_diag_nmax=100 so PPCG substitutions converge at the tighter residual request without the warnings seen at 50. Retain every reference and comparison tolerance; the 200-step control adds no improvement.

The exact fixture passes all eight CPU/GPU, CG/PPCG, MPI 1/4 runs and 32 original property checks with zero inner residual warnings. Maximum band differences against the tighter Davidson-subspace control are 1.954e-11 eV for CG and 1.901e-13 eV for PPCG.
Tighten SCF and diagonalization settings for eight PW fixtures. Keep the original references for the three lattice cases; calibrate the five remaining scalar references with independent high-accuracy Davidson-subspace results cross-checked against CG and PPCG. Keep all acceptance tolerances, physical inputs, and the comparison pipeline unchanged.

Fresh paired runs of all 135 active cases reduce PPCG reference warnings from 60 to 52, with 83 passes and zero fatal failures. All 270 calculations exit normally and no prior reference pass regresses. Forty focused CPU/GPU runs pass 220 property checks with no inner residual warning. Production source is unchanged. Full-sweep residual warnings and the two intentionally short SCF cases remain separately recorded.
The three LR force fixtures requested lr_thr=1e-2 while checking signed forces and their absolute-value sum to 1e-6. The shared Davidson changes alter this loose stopping path. An independent build of upstream c82d87d reproduces the old references with loose Davidson, while upstream and current LAPACK give identical printed signed force components.

Request lr_thr=1e-16 and calibrate the scalar/signed references from upstream dense LAPACK. Retain Davidson, the existing retry policy, Z-CG coverage, ground-state inputs and all acceptance tolerances. Twelve final controls across OpenBLAS 0.3.20/0.3.33 and MPI1/4 pass; signed-force and sum differences stay below 2e-7. The original 27-case 08_RI suite passes 143 property checks, with the unrelated existing HF-ULR SCF nonconvergence recorded separately. Subsequent 09_DeePKS and 10_others suites also pass locally.
Merge 8a6df3b while retaining the PPCG, Davidson stability and CG accuracy checks. Follow the upstream test-to-unittests migration, keep the upstream CG tests and the existing accuracy harness in separate targets, and use the relocated linear-algebra tests. Preserve both sides of the PW case list.

Validation: all selected targets rebuilt; 60 enabled CTests pass with each of OpenBLAS 0.3.20 and 0.3.33. The comparison benchmark remains disabled. Full CPU integration verification follows before publication.
The Read the Docs build for f151acf hit its 900-second limit while compiling ABACUS, before generating parameters or running Sphinx. The default untyped CMake build adds -O3 -g even though this executable is used only to export INPUT metadata.

Set Debug with -O0 -g0 and disable native optimization for this documentation-only build. Keep assertions enabled and preserve the regular build defaults.

Validation: the complete serial/no-LCAO install finished locally in 129.315 seconds with four build jobs. All 605 documented parameters, generated Markdown, and 73 HTML files match the merged Release build byte for byte. Strict HTML and LaTeX generation pass with warnings treated as errors. Remote RTD timing requires the next build.

@ABACUS-PR-BOT ABACUS-PR-BOT left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The new PPCG recovery path exceeds the existing workspace bounds; the follow-up belongs in the existing workspace-bounds discussion. Two maintenance suggestions follow inline.

Static source review of 61d4b7ba31696259d5dbee0c9f94604631d6b646 against 14c224d04dfcc5d74271c56e20145f4b5c40e563, including the incremental changes since ac9f106a56d538ef75fd495e97957573dcd5c7b3. Source, call paths, tests and CI registration were inspected; no build, test, runtime or CI result was independently executed or verified. The regenerated physical references and wider solver-substitution validation remain unverified here. The existing NSCF and Wannier separation decisions remain open.

Comment on lines +492 to +495
const double residual2 = ModuleBase::dot_real_op<T, Device>()(dim, candidate, candidate);
if (!std::isfinite(residual2) || residual2 < 0.0)
{
throw std::runtime_error("DiagoDavid: non-finite residual before preconditioning");

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[P3] Scope Davidson scratch across the new failure exits

Non-blocking: the new nonfinite-residual throw exits cal_grad after vc_ev_vector and lagrange are allocated, before their frees at lines 543-544; the revised SchmidtOrth can exit through the same path. A public/Python caller that catches the exception therefore retains these allocations. Please give both buffers device-aware scoped ownership or a cleanup guard covering these exits.

self.assertEqual((directory/'result.out').read_text(),'Max_wfc_1 0.5000\nMax_wfc_2 0.5000\nMax_wfc_3 0.5000\n')


if __name__=='__main__':unittest.main()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[P3] Run the comparator negative controls in PR checks

Non-blocking: neither this suite nor test_compare_wannier_bundle.py is registered in CTest or invoked by the PR workflows. The integration collectors exercise the comparators on ordinary outputs, so they do not run these corruption, malformed-input and missing-reference controls. Please register and select both suites in a normal PR check, with their Python/NumPy/Bash dependencies.

@ABACUS-PR-BOT ABACUS-PR-BOT left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The remaining SCAN calibrations still assume the withdrawn Libxc correction; the reference-consistency issue is detailed inline.

Non-blocking: please refresh the PR description after the scope removals. It still advertises dense-grid mixing, the Libxc correction and calibrated LR fixtures, and labels 61d4b7ba current. Preserve the tested SHA for each historical validation claim.

The three restored LR force fixtures still call the modified Davidson solver, so their current compatibility remains unverified. A merge-tree check against current develop (0ae4d926) also reports conflicts in test_diago_david.cpp, test_hsolver.cpp and test_hsolver_pw.cpp; recheck the affected fixtures after resolving these.

Static incremental review of 493060c5 → 7726f2bf against API/merge base 14c224d0, with bounded inspection of actual target 0ae4d926. No builds or tests were run; numerical results and CI were not independently verified.

Comment on lines +1 to +2
etotref -4.47823118100424
etotperatomref -2.2391155905

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[P2] Keep the retained SCAN references coupled to the XC implementation

This reference and the fixtures for 201_PW_UPF201_Ce_f, 214_PW_SCAN_NLCC and 820_PW_PPCG_Cutoff remain calibrated with finite-density, zero-gradient local XC terms, while 7726f2bf restores the mask that suppresses those terms. Their READMEs explicitly identify the removed correction, so the recorded calibration describes different XC behavior from this head. Please move the remaining correction-dependent fixture changes with the Libxc fix, or independently recalibrate matching references—including the PPCG-specific 820 case—with CG/Davidson controls and the existing comparison limits.

@mohanchen

Copy link
Copy Markdown
Collaborator

I think the PR is not ready yet. Some codes unrelated to PPCG have also been included. Please clean up the codes and maybe submit the new codes in multiple smaller PRs.

@cheerly-pku

cheerly-pku commented Nov 2, 2026 •

Copy link
Copy Markdown
Author

I reviewed the closed PR against the current develop branch and rebuilt the PPCG work as a focused successor.

The closure reason was the scope problem identified in the maintainer comment: #8042 combined PPCG with unrelated solver, XC/Libxc, charge-mixing, WFC/Wannier comparator, LR, output, and ordinary 01_PW reference changes. The bot review also left three concrete items open: the Davidson scratch-buffer lifetime issue, unregistered comparator tests, and SCAN references calibrated against the withdrawn Libxc behavior.

I did not reopen #8042 or carry those changes forward. I created PR #8264 from current develop (initial base 036b6414d, synchronized with 597deb7bf). The new PR keeps only the PPCG implementation and PW dispatch, the required build/source registration, focused double/single/MPI/workspace tests, the MPI parallel harness, and CPU fixtures 817–819 plus the GPU bridge smoke fixture. The SCAN cutoff fixture 820 is deliberately excluded until its reference is regenerated independently on current develop with the current XC implementation.

The audit also fixed the PPCG-specific issues found during review: root-recovery workspace validation now covers the possible 3*nband expanded subspace, the parallel harness ignores GoogleTest flags instead of parsing them as a pool count, test traces use testing::TempDir(), the new M_PI macros were removed, and MPI world-root detection is routed through the existing parallel wrapper. The production path keeps the conservative full Rayleigh–Ritz cadence; the unvalidated user-facing cadence parameter was removed from this focused PR.

Fresh local verification on the new branch:

  • CMake Release CPU build with MPI, no LCAO, and testing enabled: passed.
  • PPCG, single-precision PPCG, HSolverPW, MPI, and parallel-pool tests: 7/7 passed.
  • Input-domain and solver-label tests: 2/2 passed.
  • abacus_pw_para with two MPI processes and one OpenMP/BLAS thread: fixtures 817, 818, and 819 each passed all 7 reference checks.
  • Governance and pre-commit checks: passed.

All required GitHub checks on #8264 now pass: GNU and Intel builds, Makefile/Intel, CUDA build and CUDA test, extra components, main Test, abacuslite, PyTest, documentation, governance, and pre-commit. The PR is mergeable with no conflict; GitHub currently marks it REVIEW_REQUIRED until a maintainer review.

The excluded Davidson/comparator/SCAN calibration work is not silently discarded; it needs separate PRs with their own current-develop evidence. I will keep those changes out of the PPCG successor unless a focused, independently reproducible patch is prepared.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Diago Issues related to diagonalizaiton methods Features Needed The features are indeed needed, and developers should have sophisticated knowledge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants