Skip to content

Feature: point-counter-charge correction - #8075

Open
LKFEIYI wants to merge 19 commits into
deepmodeling:developfrom
LKFEIYI:codex/pcc
Open

LKFEIYI wants to merge 19 commits into
deepmodeling:developfrom
LKFEIYI:codex/pcc

Conversation

@LKFEIYI

@LKFEIYI LKFEIYI commented Oct 4, 2026

Copy link
Copy Markdown

What's changed?

Add assume_isolated pcc_0d for molecules in cubic cells and assume_isolated pcc_2d for periodic slabs. The correction energy is reported as E_pcc; pcc_2d_axis selects the open lattice vector (0/1/2, default 2).

This is the PCC-only part of #8052. Geometry and atom-data extraction are placed in source_cell, FFT-grid coordinates in source_basis/module_pw, and charge moments and PCC kernels in source_estate alongside Makov–Payne.

PCC0D requires a cubic cell. PCC2D requires the open vector perpendicular to the periodic plane, Gamma-only sampling along it, and symmetry operations that preserve the open direction. The wrapping boundary must lie in vacuum. PCC2D uses the open planar monopole coefficient -pi*L/(3*A).

pcc_0d_2d_convergence

Linked Issue

Follow-up to #8052, split according to the review feedback.

LKFEIYI added 12 commits October 4, 2026 09:47
Build: cmake --build build_pcc0d --target cell MODULE_CELL_cell_geometry MODULE_CELL_cell_tools --parallel 4 passed. CTest: both focused targets passed (10 tests), OMP_NUM_THREADS=1 outside sandbox. C++11 syntax checks passed for both changed production sources. Governance: no blockers; geometry header needs Vector3 and array for value members, string for validation diagnostics, and vector for input containers. No INPUT or user-visible behavior changes in this foundation step.
Place charge moments and PCC formulas beside makov_payne; geometry and atom extraction remain in source_cell. No INPUT behavior changes yet. Built both focused targets and passed 11 tests outside sandbox with OMP_NUM_THREADS=1, including energy/potential and ionic-force finite differences. C++11 syntax checks passed. Governance has no blockers; includes define the value moment type, diagnostic string and kernel interface.
Register standalone PCC for CPU KS-DFT PW/LCAO, add its energy once, and expose force and electrostatic output from the same component result. Generated INPUT YAML/Markdown from the new CLI. Full abacus_basic_para build and focused potential, energy, INPUT and grid tests passed. Neutral water, charged H3O+, LCAO and two-rank MPI SCF runs converged; old H3O+ energy/forces match at printed precision. C++11 syntax and staged governance checks passed without blockers. Remaining PARAM read is the existing PW registration dependency (net zero); header includes provide value member types, base class and standard containers/diagnostics.
Keep potential update orchestration distinct from geometry/atom preparation and distributed density integration. Rebuilt abacus_basic_para, MODULE_ESTATE_pot_pcc and MODULE_ESTATE_potentials_new; both CTest targets passed outside sandbox with OMP_NUM_THREADS=1. Quality score improves from 82 to 90. No INPUT behavior changes.
Register neutral water and charged H3O+ PW cases plus neutral water LCAO.
Collect E_pcc in eV alongside existing total-energy and force properties.
The new references use two MPI ranks; existing references are unchanged.

Verification:
- cmake --build build_pcc0d --target abacus_basic_para --parallel 2: passed.
- OMP_NUM_THREADS=1 OPENBLAS_NUM_THREADS=1 Autotest.sh, -n 2 -o 1:
  PW 220_PW_PCC0D_WATER / 221_PW_PCC0D_H3O and LCAO
  scf_pcc0d_water / existing scf_solvation passed default thresholds.
- Nine targeted CTest executables passed, including INPUT and PotPcc.
- Staged governance and git diff --cached --check: passed.
Drop the newly added 220_PW_PCC0D_WATER case and CASES_CPU entry.
This case took about 48 seconds and overlaps neutral LCAO plus analytic
and host coverage. Retain charged H3O+ PW and neutral water LCAO so both
solver paths remain covered. Existing tests and thresholds are unchanged.

Verification (OMP_NUM_THREADS=1, OPENBLAS_NUM_THREADS=1, outside sandbox):
- Autotest.sh -n 2 -o 1 -j 1 -r '^221_PW_PCC0D_H3O$': passed,
  10.826 s; energy, force and E_pcc checked at default thresholds.
- Autotest.sh -n 2 -o 1 -j 1 -r '^scf_pcc0d_water$': passed,
  9.858 s; energy, force and E_pcc checked at default thresholds.
- Nine targeted CTest executables: passed, 0.21 s total.
- Staged governance and git diff --cached --check: passed.
Only test files and registration changed; no executable rebuild required.
Build: abacus_basic_para, MODULE_CELL_cell_geometry and MODULE_ESTATE_pcc_2d passed. Both focused CTest targets and explicit C++11 syntax checks passed. This stage adds no INPUT behavior; user documentation follows in the integration stage. pcc_2d.h includes the charge-moment API used by its public kernel declarations; cell geometry is forward declared.
Keep dimension and open axis immutable at construction. Reuse charge moments,
pool reductions, energy, force and electrostatic output consumers. Require
Gamma-only open-direction sampling and symmetry that preserves the slab.
Add pcc_2d_axis INPUT metadata and regenerate both parameter documents.
Report the actual failure stage and reject missing density storage via the
standard ABACUS WARNING_QUIT path. INPUT errors identify conflicting settings.

Verification:
- Full executable and affected unit targets built successfully.
- Ten targeted CTest targets passed in 0.22 s (OMP_NUM_THREADS=1).
- Explicit C++11 syntax checks passed for geometry, kernel and PotPcc.
- PW charged and LCAO neutral 2D SCFs converged on two MPI ranks;
  comparison with the saved original executable agrees within 2.7e-10 eV
  energy and 2.5e-5 eV/Angstrom force components (old run used one rank).
- Metadata generation and pcc_2d_axis CLI help passed.
- Staged governance and diff checks passed without blocking findings.

Global references: added 2, removed 6, net -4. The existing potential factory
reads the axis from PARAM and passes it explicitly into PotPcc; PW setup now
shares one const INPUT reference for existing component selection. PotPcc and
numerical helpers add no globals. The new header include owns Pcc2dParameters
by value and thus needs the complete definition.
Add only two small vacuum PCC 2D integration cases: charged H3O+ PW,
open axis 2, and neutral water LCAO, open axis 1. Generate references
with two MPI ranks and one OpenMP thread. Keep all old references and
thresholds unchanged.

Verification with OMP_NUM_THREADS=1 OPENBLAS_NUM_THREADS=1:
- PW Autotest.sh -n 2 -o 1 -j 2 selecting both PCC0D/PCC2D H3O+: passed.
- LCAO Autotest.sh -n 2 -o 1 -j 2 selecting both PCC0D/PCC2D water: passed.
- New tests compare total energy, force magnitude and E_pcc.
- Staged governance and git diff --cached --check: passed.
- Two-rank runtime rejection of tilted open vector and non-Gamma open
  sampling passed; kpar=2 invalid sampling also exits 1 without hanging.
Only calculation inputs and registration changed; no rebuild needed.
Restore assume_isolated and pcc_2d_axis descriptions from abacus_sccs,
removing only solvent clauses. Regenerate parameter YAML and input-main.
Remove the PCC solvent-combination test, solvent wording in the added
energy test name/readmes, and explicit imp_sol=0 from four PCC cases.
Keep the INPUT mutual-exclusion check as requested; physics is unchanged.

Verification:
- Full executable, INPUT test and energy test rebuilt successfully.
- Both affected CTest targets passed in 0.18 s with OMP_NUM_THREADS=1.
- CLI parameter help and --check-input passed for all four PCC cases.
- Script comparison confirms exact original descriptions except the
  removed solvent clauses and confirms the mutual-exclusion check remains.
- Generated docs contain 567 parameters.
- Staged governance and git diff --cached --check passed.
@mohanchen

Copy link
Copy Markdown
Collaborator

Thanks! This is better, we will review it soon.

@mohanchen mohanchen closed this Oct 5, 2026
@mohanchen mohanchen reopened this Oct 5, 2026
@mohanchen mohanchen added Features Needed The features are indeed needed, and developers should have sophisticated knowledge Tests/Examples Issues/PR related to unit tests and integrate tests Refactor Refactor ABACUS codes Input&Output Suitable for coders without knowing too many DFT details labels Oct 5, 2026

@kluonju kluonju 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.

Overall I think the tests need some more thoughts to reflect close-to real scenarios.

namespace elecstate
{

/// Self-consistent vacuum PCC. Owns the result of its last density update.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It would be good to know the reference equations that is used for this implementation. A paper mentioned below? O. Andreussi and N. Marzari, Phys. Rev. B 90, 245101 (2014) or some ref to other codes.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Added. The implementation mainly follows ENVIRON. The one difference is the monopole constant for charged slabs. Andreussi and Marzari, Phys. Rev. B 90, 245101 (2014), and QE-Environ use −π/(3L), where L is the cell length along the open axis. We use −πL/(3A), with A the periodic area, following the reminder from Critsium-xy in #8052.
slab_vacuum_convergence_axes
It (Fig. c) shows that the coefficient -pi * L_y / (3 * A) works.


namespace elecstate
{
const PotPcc* Potential::pcc_component() const

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

adjust the header definition and consider to adapt these public functions as private member functions of class Potential. These lines should not be placed ahead of the constructor.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thanks for the suggestion. I moved the definitions after the constructor/destructor in potential_new.cpp, grouped the declarations with the other public getters, and moved pcc_component() to the private member functions.

Comment thread source/source_estate/module_pot/potential_new.h
bool XC_Functional::ked_flag = false;
namespace elecstate
{
double Potential::pcc_energy_rydberg() const

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This test seems unnecessary.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The remaining Potential::pcc_energy_rydberg() definition in this file is not a test but a link stub. It gives the linker a definition for a Potential member that ElecState::cal_energies references; here, the call is this->pot->pcc_energy_rydberg() in the pcc_0d/pcc_2d branch. It returns 0.0 and is never reached by the remaining tests, which all use assume_isolated = "none". Removing it would require either linking potential_new.cpp and its dependencies into this lightweight test or changing the cal_energies interface.

EXPECT_DOUBLE_EQ(elecstate->f_en.etot, 1.3);
}

TEST_F(ElecStateEnergyTest, AddsPccEnergyOnce)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Also this test is not needed. Consider to add more meaningful ones of an actual model or something else.

@LKFEIYI LKFEIYI Oct 6, 2026 •

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

removed. Four new tests were added.

- Document the PCC 0D/2D kernels, their references (Andreussi and
  Marzari 2014, Dabo et al. 2008, Rutter 2021 for the charged-slab
  constant) and the ENVIRON implementation they follow in pot_pcc.h;
  cite Rutter 2021 next to the 2D gauge in pcc_2d.h.
- Define the Potential PCC accessors after the constructor/destructor,
  declare them with the other public getters, and keep the
  pcc_component() helper with the private member functions.
- Drop the stubbed-energy PCC test from elecstate_energy_test.cpp.
- Add physical checks: periodic Hartree energy + E_pcc recovers the
  isolated energy of a charged Gaussian ion and of a polar Gaussian pair
  (0D), and of a charged layer and a polar neutral slab (2D),
  independently of the cell size.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0141raGGLbA6qqg3srvRcPnz
LKFEIYI and others added 5 commits October 6, 2026 17:53
The geometry, grid and moment helpers only receive values that ABACUS
has already built from STRU and INPUT, so their finite/size/storage
checks and bool+error returns could never fire.

- grid_positions and charge_moments return their result directly.
- weighted_center returns the center; make_pcc_2d_parameters returns
  the parameters.
- make_orthogonal_cell, make_slab_cell and make_pcc_0d_parameters keep a
  bool for the structure conditions a user can violate (perpendicular,
  cubic); PotPcc turns them into WARNING_QUIT messages.
- PotPcc no longer reduces replicated structure checks over the pool
  (every rank sees the same cell and stops together), and drops the
  storage, nonfinite-result and stale-result checks. validate_kpoints
  keeps its cross-pool reduction, since each pool holds its own k points.
- write_elecstat_pot drops the PCC potential size check.

Tests for the removed failure paths are removed; a death test covers
the non-cubic 0D and tilted 2D cells.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BKqk2SNWXoZn2PRRGRzGca
PotPcc takes the expected electron count (INPUT nelec) and writes a
warning when the PCC net charge differs from it by more than 1e-6 e,
e.g. after a very tight diagonalization. The correction keeps using the
grid charge, as before.
Drop the symmetry and origin details from assume_isolated pcc_0d and
pcc_2d and the LCAO load-balance note from pcc_2d_axis; the run still
stops with a clear message when the cell or symmetry is unsupported.
The exact monopole and dipole values are implied by the isolated-energy
tests, and the bilinear kernel is an internal step of pcc_0d_energy. The
finite-difference checks of each kernel, the orthogonal and slab geometry
checks, the PotPcc input rejections and the charge-moment checks now share
one test each. Every property stays checked; 31 tests become 18.

This branch has not been deployed

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

Labels

Features Needed The features are indeed needed, and developers should have sophisticated knowledge Input&Output Suitable for coders without knowing too many DFT details Refactor Refactor ABACUS codes Tests/Examples Issues/PR related to unit tests and integrate tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants