Repository navigation
Add constant-potential electron control to the i-PI socket driver - #8136
zhangchenyu-USTC wants to merge 14 commits into
Conversation
|
@ABACUS-PR-BOT please review this PR |
ABACUS-PR-BOT
left a comment
There was a problem hiding this comment.
Static review independently cross-checked at 8bfcec692120952fdfa27fd6dec7d2f8518b73ec. Trusted governance and whitespace checks passed (0 governance errors, 8 warnings reviewed). No builds or runtime tests were run; CI diagnostics were unavailable.
| if (inp.esolver_type != "ksdft" || (inp.basis_type != "pw" && inp.basis_type != "lcao") | ||
| || inp.device != "cpu" || inp.precision != "double" || parameters.globalv.use_uspp | ||
| || inp.sc_mag_switch || inp.deepks_scf || inp.rpa || inp.rdmft || inp.dft_plus_dmft) |
There was a problem hiding this comment.
[P2] Reject energy-only Makov–Payne correction for electron control
With a charged cubic cell and assume_isolated=makov-payne, this gate accepts the session, but cal_energies() adds the charge-dependent correction to the returned energy while electronic_observables() returns the uncorrected band Fermi level. Even the leading alpha*e2*Q^2/(2L) term contributes -alpha*e2*Q/L to the electron-number derivative, so those outputs describe different energy functionals. Please reject this combination here unless consistent potential/derivative support is provided, and add a focused rejection test.
There was a problem hiding this comment.
Added the Makov–Payne check at the start of ESolver_FP::apply_electronic_control(), before any electronic state is changed. Real PW/LCAO tests cover makov-payne, mp, and m-p with 1 and 4 MPI ranks, and ordinary Makov–Payne SCF/socket comparisons still pass. Tests.
| const double coordinate_A = double(indices[axis]) / grid[axis] * height_A; | ||
| if (!std::isfinite(potential[ir])) { reduction[2] += 1.0; } | ||
| if (coordinate_A >= minimum_A && coordinate_A < maximum_A) | ||
| { reduction[0] += potential[ir]; reduction[1] += 1.0; } |
There was a problem hiding this comment.
Non-blocking: The linked triclinic cube comparisons cover all three axes, but a small in-tree test of this helper would make its window and partition rules easier to preserve. A hand-computed grid can check points exactly on the half-open window edges, an empty window, and an MPI rank with no local planes against the serial average. This can be a follow-up.
There was a problem hiding this comment.
Added test_electrostatic_potential.cpp, calling the production planar_potential_average() with hand-calculated expectations. It covers x/y/z, half-open window boundaries, empty windows, empty MPI slabs, unequal partitions, and triclinic cells. Both MPI 1/4 CTest targets pass. The averaging implementation is unchanged.
There was a problem hiding this comment.
I would suggest moving this file to source_estate
There was a problem hiding this comment.
Moved the electronic request, window, and result types to source_estate/electronic_control.h under the elecstate namespace. Updated the users and build wiring in 8539ebb63a7a1ad3bdfaaa51a7e1602fb0bb99de.
There was a problem hiding this comment.
I would suggest moving this file to source_estate
There was a problem hiding this comment.
Moved the occupation sum, MPI reduction, and electron-count check to source_estate/electronic_occupations.cpp/.h, with the tests in source_estate/unittests. MPI1/4 checks pass. See 8539ebb63a7a1ad3bdfaaa51a7e1602fb0bb99de.
There was a problem hiding this comment.
make this a free function
There was a problem hiding this comment.
The check is a free function, elecstate::checked_occupation_electron_count(), with its occupation and distribution inputs passed explicitly. It remains independent of the solver class in 8539ebb63a7a1ad3bdfaaa51a7e1602fb0bb99de.
There was a problem hiding this comment.
I would suggest moving this to source_relax
There was a problem hiding this comment.
Removed esolver_fp_electronic.cpp and moved request checks and evaluation scheduling into the existing source_relax/socket_constant_potential.cpp. Electronic-state updates and physical observables are handled by source_estate. ESolver retains two short bridges, apply_electronic_control() and electronic_observables(), to the state and potentials it owns; their names and interface convention are available for your review. No production parameter.h include or result file is added. See 8539ebb63a7a1ad3bdfaaa51a7e1602fb0bb99de.
The updated branch is ready for review. GitHub currently reports viewerCanReopen=false for my account, and both API and CLI reopening attempts failed. Could you reopen this PR?
|
Thanks for adding electron-number control to the socket interface. For related work, ATST-Tools 2.3.2 provides experimental constant-potential single-point/scan, fixed-cell relaxation, NEB and ASE MD workflows through the independent abacuslite backend. The MD approaches appear complementary: ATST converges the electron number to a prescribed chemical-potential tolerance before each nuclear force evaluation, whereas the linked i-PI implementation propagates it as an extended dynamical variable with a fictitious mass and momentum. ATST uses an explicit compensated-gate boundary, so comparing targets requires aligning the electrostatic reference and energy convention. We have performed short LCAO/cusolver V100 checks with ABACUS LTS 3.10.1, including force/grand-potential finite differences and split-restart comparisons (validation scope). These exercise ATST’s file-based route, not this socket implementation. Is the CPU/double restriction here an initial validation boundary, or are there specific device-dependent state updates that still need implementation? A shared fixed-geometry, fixed-electron-number comparison could help establish matching energy, force and potential conventions before comparing the two MD schemes. A future abacuslite socket adapter could also let ASE workflows reuse this interface. |
|
Since the PR hasn't been updated for a few weeks, I will close it for convenience. Please feel free to reopen it if you have new updates. |
|
Thanks for the comparison with ATST. CPU/double currently defines the supported and validated scope of this socket implementation; it is not a requirement of the extended-variable method. We have not identified a specific missing GPU electron-number update. Before extending support, we need to validate occupations, host/device state synchronization, and repeated socket evaluations. LCAO/cuSOLVER should also be assessed separately from the general Your distinction between the MD schemes is correct. i-PI propagates the electron number with a fictitious mass, momentum, and thermostat, while ABACUS performs SCF at the requested electron number. There is no inner electron-number optimization to a chemical-potential tolerance. I agree that a fixed-geometry, fixed-electron-number comparison is the appropriate first step. We can align the ABACUS revision, basis, pseudopotentials, smearing, gate settings, and electrostatic reference, then compare free energies, forces, occupations, Fermi levels, and reference potentials. The socket returns ABACUS’s free energy; i-PI’s grand-potential output uses the neutral electron number as its reference. This convention needs to be aligned when comparing results. A direct ATST comparison has not yet been performed. |
ABACUS-PR-BOT
left a comment
There was a problem hiding this comment.
Two non-blocking maintenance notes remain below. Static review of 99e89e74; no builds or runtime tests were run. The reported numerical results concern an earlier revision and were not independently verified for this head.
| void relax_driver(ModuleESolver::ESolver* p_esolver, | ||
| UnitCell& ucell, | ||
| const Input_para& inp, | ||
| Parameter& parameters, |
There was a problem hiding this comment.
Non-blocking: python/pyabacus/src/ModuleDriver/py_driver.cpp:454 still passes PARAM.inp to this new Parameter& parameter. Please update the argument to PARAM. The unconditional CMake return currently excludes that binding, so this affects the retained source when re-enabled.
There was a problem hiding this comment.
Updated the retained Python driver call to pass PARAM, matching Relax_Driver::relax_driver(..., Parameter&, ...), in c36ae05b9. A compile-only check using the production headers and the extracted call passes; the original PARAM.inp argument fails with the expected type mismatch. The C++ ModuleDriver binding remains disabled, so this is not a claim that the full binding builds. No include or additional global access was introduced.
There was a problem hiding this comment.
Confirmed in c36ae05b9: the retained Python driver now passes PARAM to the Parameter& interface. Static inspection only; the full ModuleDriver binding remains excluded by CMake.
| { | ||
| const bool unfinished = context.cp.enabled | ||
| && (context.state == DriverState::HasData || context.charge_pending); | ||
| throw_if_any_rank_failed(unfinished ? 1 : 0, "EXIT with an unfinished request/result"); |
There was a problem hiding this comment.
Non-blocking: AGENTS.md rule 14 asks for conditional arguments to be computed before the call. Please assign const int local_failed = unfinished ? 1 : 0; and pass local_failed here.
There was a problem hiding this comment.
Added const int local_failed = unfinished ? 1 : 0; before the call in c36ae05b9. The exit condition, MPI reduction, and error message are unchanged. The full CPU PW/LCAO build, 13 related CTest targets, and 10 MPI4 real-client tests passed with no failures or skips. These include pending-charge/pending-result EXIT and EOF rejection, ordinary unread-result exits, zero-frame exits, and successful electron-count/position sequences. The PR verification section now distinguishes these current-head checks from the earlier full numerical matrix.
There was a problem hiding this comment.
Confirmed in c36ae05b9: local_failed is named before the call, with the same unfinished-request predicate and rank reduction.
Keep electronic-state and socket test registrations under the upstream CMake format. Verified full CPU build, 13 CTest targets, and eight real-client smoke tests.
Add constant-potential electron control to the i-PI socket driver
What's changed?
Extend the native i-PI socket driver to receive a total electron count before each structure evaluation. PW and LCAO update occupations and charge-dependent potentials, run SCF, and return energy, forces, the actual occupation sum, the Fermi level, and an optional work function. Each response represents one completed frame.
The work-function reference shares the electrostatic-potential construction used for cube output, including PCC and the current SCCS reaction potential, and averages the requested window in memory. Constant-potential evaluations check SCF convergence and the electron count with an absolute tolerance of
1e-8. The PW ACE path receives the current occupations before constructing exchange.Use the existing
socket_driverINPUT option; electron-control settings belong to i-PI. Socket calculations default tosymmetry=0unless explicitly set, and position updates use nearest-image displacements for charge extrapolation. INPUT registration includes the interface help; documentation is generated by CI. Constant-potential control rejects the energy-only Makov–Payne correction before SCF.The socket workflow remains in
source_relax; electronic types, occupation checks, and state updates belong tosource_estate. The dedicatedesolver_fp_electronic.cppis removed. ESolver retains two short bridges,apply_electronic_control()andelectronic_observables(), to the electronic state and potentials it owns. Their naming and interface convention remain available for maintainer review. The implementation reuses the existing socket control module and adds no result files; request/response summaries use the existing running log.The current supported and validated configuration is CPU/double PW/LCAO KSDFT. i-PI propagates the electron number as an extended dynamical variable; ABACUS evaluates each requested electron number without an internal potential-control iteration.
The matching i-PI server, PW/LCAO examples, and real-client tests are published together. This PR changes seven focused C++ test sources and one existing mock header, covering protocol parsing, complete socket sessions, occupation reduction, explicit spin updates, INPUT behavior, electrostatic-potential averaging, and relax/socket finalization. Physical regressions stay in i-PI;
tests/integrate/validation_toolsis unchanged. NoOUT.*or calculated reference files are included.Linked Issue
This extends the existing i-PI socket interface; there is no separate tracking issue.
Unit Tests and/or Case Tests for my changes
Current revision c36ae05b9 includes the CMake merge with upstream
c9079539e69a9b65f1bf7d0ca6fcd7bb2197ec67and the two maintenance fixes: the retained Python driver passesPARAMtoParameter&, and EXIT validation computeslocal_failedbefore the call.For this revision, the full CPU PW/LCAO program and affected socket/relax test targets were rebuilt. All 13 CTest targets selected below and 10 MPI4 real-client tests passed, with no failures or skips. The real-client selection was:
A compile-only probe using the production headers and the extracted Python-driver call passes for
PARAM; the oldPARAM.inpcall fails with the expected argument-type error. The C++ ModuleDriver binding remains disabled by upstream CMake, and was not enabled or built as a complete binding.The broader numerical results below were obtained on the earlier pinned revision; that full matrix and long trajectories were not repeated for these maintenance changes.
Built the candidate and unmodified reference with the same GNU/OpenMPI/LibXC/LibRI configuration and JSON enabled, against pinned upstream
d20090cc137fdc3b3baddd9217f792bb7af922d7. The tested source is published in 8539ebb63a7a1ad3bdfaaa51a7e1602fb0bb99de. PW-only MPI, PW-only non-MPI, and clean patch-reconstruction builds also passed.The implementation follows upstream
scf_loop()and SCCS delayed activation. Each frame resets and records its own convergence status.Relax_Driverowns finalization: a zero-frame socket exit performs no solver finalization, and a session with completed frames finalizes once.All 13 nonempty CTest targets (205 test cases) and 46 core real-client tests passed, with no failures or skips. They cover electron-count and position changes, static-reference comparisons, cube potentials, ordinary SCF/MD/relax, ASE, trajectories, restart, and exits. Occupation and potential tests run with 1 and 4 MPI ranks. Complete socket tests cover two different electron counts and rejection of POSDATA without CHGDATA; relax tests check zero-frame exits and exactly-once finalization.
Six DFT+U/EXX tests and eight additional MPI1 Makov–Payne tests also passed. PW/LCAO gate MD checks completed three steps with four evaluations each and normal client exits. Four PCC and five SCCS upstream cases matched the independent reference in cube potentials, energies, and forces. Four immediate/delayed SCCS response tests passed the original numerical thresholds with
scf_thr=1e-12in both candidate and reference. Independent occupation checks covered 24 cases and 96 frames at MPI1/4; the largest occupation-sum error against the request was2.931e-14, below the1e-8tolerance.Governance Notes
The local check against
c9079539reports 0 errors and 10 warnings. Five concern existing wrapper/top-level global accesses, relocation of the shared potential construction, and the retained Python driver call; global references decrease by eight across the PR (9 added, 17 removed). ChangingPARAM.inptoPARAMpreserves the existing global-access count at that call site. Four concern headers needed for standard containers and stored value members. One requests the INPUT/documentation explanation given above. No productionparameter.hinclude is added. Configuration and spin parameters are passed explicitly through the socket and electronic-state helpers; native parameter writeback uses the existingParamUpdaterentry point.Reminder
AGENTS.mdanddocs/developers_guide/agent_governance.md.source/changes.