Repository navigation
Feature: periodic self-consistent continuum solvation (SCCS) Step1: SCCS basic function - #8162
Conversation
|
@ABACUS-PR-BOT Could you review the PR? |
ABACUS-PR-BOT
left a comment
There was a problem hiding this comment.
Independently reviewed statically at 7cdc76650742af3d40120d8969465e46bce44172. The numerical finding is an analytic finite-grid limitation; molecular impact has not been measured. No builds, runtime/numerical jobs, or CLI/doc-generation checks were run; the required CI connector was unavailable. The trusted governance check exited 0 with warnings; git diff --check exited 2 for two added blank EOF lines.
Non-blocking (verification record): Please add the exact commands, configuration/executable identity and observed results for the SCCS unit/MPI/integration checks and any Environ comparison. The PR reminder is checked but the body has no verification section. Also record documentation-generation verification or why it was not run; those generated files are intentionally untracked.
|
The SCCS in the current PR is not fully ready, as it requires PCC for charged systems and a low-pass filter for cavity forces in the following PRs. |
- Drop RejectsNonfiniteReals, as already done on the upper layers; the finiteness check itself stays. - Keep only the resolved-grid half of the cavity-derivative refinement test: the coarse-grid half asserted the known aliasing mismatch rather than a requirement. - Shorten the continuum-derivative comments.
# Conflicts: # source/CMakeLists.txt
- Use ModuleBase::HARTREE_SI and ModuleBase::BOHR_RADIUS_SI from constants.h for the SCCS unit conversions instead of local constants. The preset surface tension and pressure change by about 6e-7 (relative); the SCCS integration energies move by about 2e-8 eV. - Keep the solvation interface out of Potential's vtable. Potential overrode the three PotBase solvation hooks, so every unit test that stubs Potential had to define them: the SDFT Hamiltonian test, the elecstate base/PW tests and the hsolver PW mock. Potential now aggregates its components through non-virtual members (component_solvation_energy, add_component_solvation_force, component_solvent_electrostatic_potential), and those tests are back to upstream. - Rename the PW test cases: 220_PW_SCCS_PERIODIC_WATER -> scf_sccs, 221_PW_PCC0D_H3O -> scf_pcc_0d, 222_PW_PCC2D_H3O -> scf_pcc_2d.
|
@ABACUS-PR-BOT do you have further comments? |
ABACUS-PR-BOT
left a comment
There was a problem hiding this comment.
Follow-up static review at 7367e0ac1c960e679f557aab38249596ec247200: no additional implementation defects found. The scope-help, LCAO regression/force-gate and component-hook updates are present in the current source. I left one non-blocking regression-test follow-up in the existing nonfinite-input thread; the earlier finite-grid derivative limitation and verification-record request remain open.
git diff --check and the pinned governance check exited 0 (governance warnings remain). No builds, runtime tests or CI verification were performed.
Follows mohanchen's review: rename the new SCCS functions and files whose names were longer than 15 characters. Continuation lines are re-aligned. - PotBase hooks: get_solvation_energy -> sol_energy, add_solvation_force -> add_sol_force, solvent_electrostatic_potential -> sol_elec_pot. - Potential aggregators: component_solvation_energy -> sum_sol_energy, add_component_solvation_force -> sum_sol_force, component_solvent_electrostatic_potential -> find_sol_pot, make_sccs_potential -> make_sccs_pot. - ModuleSccs: dyn_per_cm_to_hartree_per_bohr2 -> dyn_cm_to_au, gpa_to_hartree_per_bohr3 -> gpa_to_au, make_sccs_config -> preset_config, gaussian_ionic_density -> gauss_ion_rho, gaussian_ionic_force -> gauss_ion_force, solve_sccs_response -> solve_response, evaluate_functional -> eval_functional. - elecstate: make_sccs_config_from_input -> sccs_cfg_input, check_sccs_charge -> check_charge. - ModuleIO: parse_solvation_model -> parse_imp_sol, validate_sccs_input -> check_sccs_inp. - File-local helpers: apply_preconditioner -> apply_precond, sccs_context_error -> context_error, sccs_model_error -> model_error, sccs_nonfinite_error -> nonfinite_error, sccs_solver_error -> solver_error. - Files: sccs_ionic_charge -> sccs_ion_charge, sccs_ionic_force -> sccs_ion_force, with their tests.
|
LGTM. Thanks for your contributions. |
Reminder
AGENTS.mdanddocs/developers_guide/agent_governance.md.source/changes.Linked Issue
Second part of #8052, split according to the review feedback.
Follow-up PRs, each stacked on the previous one: PCC coupling → lowpass and full cavity → SCF integration → solvent-aware cavity.
What's changed?
imp_sol 2selects the self-consistent continuum solvation model (SCCS; Andreussi, Dabo and Marzari, J. Chem. Phys. 136, 064102 (2012)) for neutral, periodic cells, withcalculation scf, energies and forces, PW and LCAO,nspin1 and 2. The original ABACUS solvent model staysimp_sol 1, andtrue/falseare still read as 1/0.sccs_rho_minandsccs_rho_max.sccs_presetprovides Environ'svacuum,water-neutral,water-cationandwater-anionparameters;customusessccs_epsilon,sccs_rho_min/max,sccs_gammaandsccs_pressure.New code:
source/source_hamilt/module_sccs/(cavity, presets, periodic Coulomb operator, Gaussian ions and forces, sqrt-CG response, energy functional), thePotSccspotential component insource_estate/module_pot/, andread_inp_sccs.{h,cpp}for the INPUT keys.Deliberate differences from Environ:
-div(grad s / |grad s|_eta), instead of Environ's unregularized curvature formula. With a density saddle on a grid point (He2), QE-Environ's surface potential reaches 100-3000 times its median and 4 of 6 runs do not converge in 200 steps; ABACUS converges in 5. So this approach seems better.sccs_tol_rmsandsccs_tol_max. Environ stops on the unnormalizedsum(r^2), so its accuracy depends on the grid size.Governance Notes
imp_solbecomes an Integer (0/1/2);true/falseremain valid.sccs_preset,sccs_epsilon,sccs_rho_min,sccs_rho_max,sccs_gamma,sccs_pressure,sccs_maxiter,sccs_tol_rms,sccs_tol_max,sccs_surface_eta. They are listed right after the original solvent keys.PotBasegains three virtual hooks with empty defaults:get_solvation_energy,add_solvation_forceandsolvent_electrostatic_potential.Potentialforwards them to its components. No existing component overrides them, so other calculations are unchanged.Potential::pot_register/get_pot_typetakeconst Input_para&explicitly; the old one-argumentpot_registerforwardsPARAM.inp.get_pot_typenow readsgga_gradanddip_cor_flagfrom that argument instead ofPARAM.inp.ElecState::cal_energiestakesimp_solasint;imp_sol 2reads the energies from the potential.force_pw.cpp) and LCAO (force_stress_lcao.cpp) add the solvation force throughPotential::add_solvation_force;imp_sol 1keeps its own path.