Modernize accessor naming: strip get_/set_, trailing-underscore members - #156
Open
susilehtola wants to merge 12 commits into
Open
Modernize accessor naming: strip get_/set_, trailing-underscore members#156susilehtola wants to merge 12 commits into
susilehtola wants to merge 12 commits into
Conversation
Apply the getter/setter naming convention to HirshfeldAtom and Hirshfeld: member variables gain a trailing underscore (dr_, rho_, atoms_, cen_); value-returning getters drop the get_ prefix (get_rho -> rho, get_range -> range, get_spacing -> spacing, get_density -> density, get_weight -> weight); the adjacent bare value accessors are normalized too (get -> density, compute_moment -> moment); and the assign-and-reconstruct Hirshfeld::set(cen,dr,rho) becomes the more descriptive set_atoms(). Pure rename -- no behavior change. Consumers (hirshfeldi, stockholder, dftgrid) updated accordingly; the identically named AngularGrid getters are a different class and left untouched. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012qGds7wJBAycBALfew1qeS
JDigestor::get_J, KDigestor::get_K and cxKDigestor::get_K are plain value accessors, so they drop the get_ prefix (-> J(), K()); the backing members gain a trailing underscore (J_, K_) to make room for the bare method names. Consumers in eriscreen and eritable updated. Pure rename, no behavior change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012qGds7wJBAycBALfew1qeS
get_core/set_core are a plain accessor pair, so they collapse to an overloaded core() getter/setter; the XRSSCF data members gain trailing underscores (spin_, nocca_, noccb_, coreorb_). main.cpp call sites updated. Pure rename, no behavior change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012qGds7wJBAycBALfew1qeS
ERItable value getters lose get_ (get_N -> N, get_range_separation -> range_separation); the multi-value configurator set_range_separation keeps its descriptive name. All data members gain trailing underscores (shpairs_, Q_, M_, Nbf_, ints_, shoff_, omega_, alpha_, beta_). ERItable call site in jkbuilder updated; the identically named ERIscreen accessors are untouched. Pure rename, no behavior change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012qGds7wJBAycBALfew1qeS
ERIscreen value getters lose get_ (get_N -> N, get_range_separation -> range_separation, get_screen_thresh/set_screen_thresh -> overloaded screen_thresh()); the pool-resizing configurator set_range_separation keeps its descriptive name. Data members underscored (shpairs_, Q_, M_, Nbf_, iidx_, omega_, alpha_, beta_). ERIscreen call sites in jkbuilder updated. Pure rename, no behavior change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012qGds7wJBAycBALfew1qeS
DensityFit value getters lose get_ (get_Naux -> Naux, get_Naux_indep -> Naux_indep, get_ab -> ab, get_range_separation -> range_separation); the multi-value configurator set_range_separation keeps its descriptive name. All ~25 data members gain trailing underscores. Call sites in jkbuilder, basistool and contrib/neo updated. Pure rename, no behavior change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012qGds7wJBAycBALfew1qeS
The inline is_cholesky() returned the pre-rename member cholesky_mode; it was uninstantiated in liberkale so the earlier build missed it, but casida instantiates it. Point it at cholesky_mode_. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012qGds7wJBAycBALfew1qeS
The member sweep wrongly rewrote s.Q/s.M/s.shpairs (reads from a ScreeningData value returned by compute_screening) to s.Q_/... The underscored names belong to the ERItable/ERIscreen members on the LHS, not to ScreeningData. Restore the struct field reads. (Missed earlier because the per-target build output was piped through grep, masking the failure.) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012qGds7wJBAycBALfew1qeS
spin/nocca/noccb/coreorb are referenced by bare name in the generated scf-fock/scf-solvers templates (compiled as XRSSCF methods in the XRS FULLHOLE/HALFHOLE variants), where the same file also declares local nocca/noccb. Underscoring them there is high-risk and low-value (they have no accessors), so keep them bare. The valuable get_core/set_core -> core() accessor rename stays. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012qGds7wJBAycBALfew1qeS
CintEnv value getters lose get_: get_Nsh/get_shell/get_Nsh_orb/get_Nbf/ get_first_ind/get_max_Nbf/get_fnorm/get_atm/get_natm/get_bas/get_nbas/ get_env/get_opt become Nsh/shell/Nsh_orb/Nbf/first_ind/max_Nbf/fnorm/atm/ natm/bas/nbas/env/opt. Idiomatic predicates (is_filled, lm_in_use, has_unit_norm) are left as-is. All data members gain trailing underscores; the nested OptSet's own opts vector is left alone. Call sites in eriworker, b_tensor, density_fitting, casida and the NEO contrib updated. The identically named BasisSet/GaussianShell getters (a different class) are untouched pending the basis commit. Pure rename, no behavior change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012qGds7wJBAycBALfew1qeS
…DFTGrid) All AngularGrid and DFTGrid data members gain trailing underscores. Value getters lose get_ (get_grid->grid, get_Npoints->Npoints, get_Nfuncs->Nfuncs, get_dens/pot/data, get_grad_tau_lapl, get_density->density); set_verbose-> verbose; set_basis->basis; the misleadingly named set_grid (which writes the angular-shell info, not the grid points) becomes set_shell; the void get_weights->compute_weights; multi-flag configurators set_grad_tau_lapl/ set_hess_lgrad keep their descriptive names. Renamed semantically via clangd; consumers (badergrid, casida_grid, pzstability) updated. Verified: full build green + 56/56 DFT (PBE/LDA) regression tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012qGds7wJBAycBALfew1qeS
…ty, FDHessian) All FDHessian and PZStability data members gain trailing underscores (including real_/imag_, which clangd renames safely without touching arma::real/imag). Value getters lose get_ (get_rsol/get_usol -> rsol/usol, get_E -> energy); matrix builders become make_ (get_H -> make_H, get_CO -> make_CO, get_CV -> make_CV); the reconfiguring setters set_method/set_params become configure_method/configure_dof; set(sol) -> set_reference. Renamed semantically via clangd; the set_method/set_params params that shadowed members were disambiguated (_v suffix) to avoid self-assignment. Full build green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012qGds7wJBAycBALfew1qeS
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
1 participant
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.
Sweep to modernize ERKALE's accessor naming, one class per commit. Convention:
get_foo()→foo(); single-valueset_foo(v)→foo(v);make_foo();set_*that validates/rebuilds → a descriptive verb;get(r)→density(r));is_*,has_*) left alone;GaussianShell::get_transwill becometransmat()(nottrans(), which shadowsarma::trans).Each commit is a pure rename — no behavior change — verified by a full
cmake --build(all targets) so inline-header and generated-template instantiations are covered.Done so far (partitioning + ERI/CD group)
hirshfeld— HirshfeldAtom/Hirshfelderi_digest— get_J/get_K → J()/K()xrs XRSSCF— get_core/set_core → core() (members kept bare: referenced in generated SCF templates)eritable,eriscreen,density_fitting,cintenv— get_ accessors stripped, members underscored,range_separation/Naux/ab/screen_threshetc.The identically named
BasisSet/GaussianShellgetters (get_shell/get_Nbf/get_first_ind) are deliberately untouched until the basis commit, since they live on a different class.Still to come
dftgrid,scf(generated-template entanglement),basis/basislibrary(58 getters, shared names),pzstability,unitary+localization(virtualsetW, single-letter members),emd. Contrib deprioritized.🤖 Generated with Claude Code
https://claude.ai/code/session_012qGds7wJBAycBALfew1qeS