source_estate: remove three #define private public in the charge/DM tests, redoing #7988 and #7990 on top of #7972 - #7998
Merged
mohanchen merged 1 commit intoSep 21, 2026
Conversation
Redo of the work in deepmodeling#7988 and deepmodeling#7990, both of which were closed while the charge density module was being restructured. That restructuring landed in deepmodeling#7972 and already did most of the decoupling those PRs proposed: allocate(), renormalize_rho() and sum_rho() now take their inputs explicitly, the mixing parameters are aggregated in a MixingConfig, and chg_mix.cpp / chg_drho.cpp / charge.cpp are free of global parameter reads. What was left was the test-side access. Production changes are additive only - no existing signature moves and no line is deleted from any production header: Charge::get_allocate_rho() - report whether allocate() has run Charge_Mixing::get_rho_mdata() - mirror the existing get_dmr_mdata() Charge_Mixing::get_tau_mdata() Charge_Mixing::set_mixing_config() - pair for the existing getter, for callers that must update the snapshot without rebuilding the mixing history XC_Functional::set_func_type() - pair for get_func_type() XC_Functional::set_ked_flag() - pair for get_ked_flag() Test changes: test_dm_r_init - two sites move to the already public get_DMR_save() test_charge - the global parameter scratchpad becomes fixture state (32 refs -> 0); PW_Basis setup goes through the public initgrids/initparameters/setuptransform sequence instead of the protected distribute_r()/distribute_g() test_chg_mix - the scratchpad becomes a fixture-owned MixingConfig (163 refs -> 0); the three blocks that hand-wired Charge::_space_* now take their buffers from the fixture, which owns them as vectors and points the public rho/rhog/kin_r views at them with the same stride No expected value or tolerance was changed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
mohanchen
approved these changes
Sep 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why this reopens closed work
@mohanchen closed #7988 ("The files will be refactored.") and #7990 ("I will refactor these files, so I will close this PR first. Reopen if needed.") because the charge density module was being restructured. That restructuring has landed as #7972, so this is the redo against the new tree.
#7972 already did most of what those PRs proposed, which makes this version much smaller.
Charge::allocate(),renormalize_rho()andsum_rho()now take their inputs explicitly, the mixing parameters are aggregated intoMixingConfig, andchg_mix.cpp/chg_drho.cpp/charge.cppno longer read the global parameter singleton at all. In particular theCharge::allocate()signature change and thenelecmember that #7990 proposed are obsolete and not included here - #7972 covers that ground.What was left was purely the test-side access.
Production changes: additive only
No existing signature moves, and no line is deleted from any production header - the three headers are add-only, which should keep this clear of any further work in the module.
Charge::get_allocate_rho()allocate()has runCharge_Mixing::get_rho_mdata()get_dmr_mdata()Charge_Mixing::get_tau_mdata()Charge_Mixing::set_mixing_config()get_mixing_config(), for callers that must update the snapshot without rebuilding the mixing historyXC_Functional::set_func_type()get_func_type()XC_Functional::set_ked_flag()get_ked_flag()Test changes
test_dm_r_init.cppget_DMR_save(); no production change neededtest_charge.cppPW_Basissetup uses the publicinitgrids/initparameters/setuptransform()instead of the protecteddistribute_r()/distribute_g()test_chg_mix.cppMixingConfig(163 refs -> 0); the three blocks that hand-wiredCharge::_space_*now take buffers from the fixture, which owns them as vectors and points the publicrho/rhog/kin_rviews at them with the same stride#define private publicin the tree: 17 files -> 14.test_chg_extra.cppis the remaining one in this module and is deliberately left for a follow-up; no file whose macro survives is touched.No expected value or tolerance was changed.
Verification
Full build with
-DBUILD_TESTING=ON -DENABLE_LCAO=ON -DENABLE_MPI=ON -DENABLE_OPENMP=ON: exit 0, zero errors, and all three changed test files confirmed compiled.MODULE_CHARGE+MODULE_ESTATE+MODULE_HAMILT: 41/41 pass, matching thedevelopbaseline built in the same tree.Running the tests rather than trusting the build mattered here - it caught a variable-shadowing bug where the two Kerker cases already had a local
MixingConfig cfg, so settingnspinon it instead of on the fixture under-allocated the arrays and read out of bounds. The locals are nowkcfg.Governance: zero blockers,
net_delta = -196(added 0, removed 196), and the access-hack ratchet sees 3 removed / 0 added.No INPUT parameter behaviour changed, so
docs/parameters.yamlandinput-main.mdneed no regeneration; the additions are internal accessors with no user-facing surface.🤖 Generated with Claude Code