source_estate: give Occupy's stateless smearing helpers a real interface, removing two access hacks - #7983
Merged
mohanchen merged 1 commit intoSep 17, 2026
Conversation
…ace, removing two access hacks elecstate_occupy_test needed #define private public for two unrelated reasons, and both are fixed at the source rather than papered over. 1) Four private static members of Occupy -- wgauss(), w1gauss(), sumkg() and efermig() -- are free functions in a new occupy_smearing namespace. Each takes every input as an argument and none of them reads any of Occupy's state (use_gaussian_broadening, gaussian_type, gaussian_parameter, fixed_occupations), so they were never members in any meaningful sense; even the smearing width and type arrive as parameters. The six internal callers in gweights() and efermig() are updated, and the WARNING_QUIT tag and one comment now name where the code actually lives. 2) Occupy::iweights() read PARAM.inp.nspin twice -- once for the spin degeneracy and once to skip k points of the other spin -- which was the only reason the test had to write the private half of PARAM. It now takes nspin explicitly, following tweights() in the same class, which has always done so. Its three call sites are all in elecstate_tools.cpp, which already reads PARAM.inp for the surrounding arguments. The four extracted functions have no other callers anywhere in the tree, and occupy.cpp's remaining PARAM references (globalv.nbands_l, inp.bndpar) are in gweights()/sumkg() paths no test drives. No default arguments were added. No assertion or expected value changed; the tests call the same code with the same inputs through its new name. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
mohanchen
approved these changes
Sep 17, 2026
This was referenced Sep 18, 2026
Merged
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.
What this does
elecstate_occupy_testneeded#define private publicfor two unrelatedreasons. Both are fixed at the source rather than papered over, so the two
macros go away and
source_estatedrops from 12 to 10.1. Four stateless helpers become free functions
Occupy::wgauss(),w1gauss(),sumkg()andefermig()were privatestaticmembers. Every one of them takes all of its inputs as arguments, and none of
them reads any of
Occupy's state — notuse_gaussian_broadening, notgaussian_type, notgaussian_parameter, notfixed_occupations. Even thesmearing width and smearing type arrive as parameters (
smearing_sigma,ngauss). They were never members in any meaningful sense.They now live in a new
occupy_smearingnamespace in the same header, withdoc comments describing the smearing-type encoding (
-99Fermi-Dirac,-1cold,
0Gaussian,n > 0n-th order Methfessel-Paxton) that was previouslyonly discoverable from the function bodies.
The six internal callers in
gweights()andefermig()are updated. TheWARNING_QUITtag and one comment now name where the code actually livesinstead of a class it is no longer part of — nothing in
source/ortests/asserts on either string, which I checked before changing them.
These four have no other callers anywhere in the tree, so the namespace move
is contained.
2.
iweights()takesnspininstead of reading itOccupy::iweights()readPARAM.inp.nspintwice — once for the spin degeneracy(
nspin == 4halves it) and once to skip k points belonging to the other spin(
nspin == 2). That was the only reason the test had to write the private halfof
PARAM.It now takes
nspinexplicitly, followingtweights()in the same class, whosesignature has always been
tweights(const int nks, const int nspin, ...). Sothis is the class's own existing convention, not a new shape.
Its three call sites are all in
elecstate_tools.cpp, which already readsPARAM.inpfor the surrounding arguments (PARAM.inp.nelecon the same call).Governance
PARAM/GlobalV/GlobalCledger: added 4, removed 6, net_delta = −2.Zero blockers.
The three flagged additions are the
PARAM.inp.nspinarguments at theiweights()call sites inelecstate_tools.cpp. This is the intended direction:the global read moves out of the leaf routine and into the caller that was
already reading
PARAM.inpon the same statement, exactly as in #7921. Two readsare removed from
occupy.cppand one write from the test, hence the netreduction. (The fourth addition is the same argument on the second of the two
two-Fermi calls.)
occupy.cpp's remainingPARAMreferences —globalv.nbands_landinp.bndpar— are ingweights()/sumkg()paths that no test drives, soinjecting them would only cost budget without removing a macro.
No default arguments were added. No assertion or expected value changed: the
tests call the same code with the same inputs through its new name. No INPUT
parameter or user-facing behaviour changed, so no documentation update is
required.
Verification
Remote Linux box,
cmake -B build -G Ninja -DBUILD_TESTING=ON -DENABLE_LCAO=ON -DENABLE_MPI=ON -DENABLE_OPENMP=ON:ninja: no work to doon a follow-up full buildMODULE_ESTATE_elecstate_occupyandMODULE_ESTATE_elecstate_baseboth link,plus
abacus_basic_para, which is what exercises the changediweights()callsites
occupy.cppinto 7 targets,elecstate_tools.cppinto 4)agent_governance_check.py --base upstream/develop --head HEAD: zero blockers,no access-hack ratchet finding (2 removed, 0 added)
Continues #7921 / #7949 / #7952 / #7953 / #7963 / #7964 / #7965 / #7966 / #7967 /
#7980 / #7981 / #7982, and keeps their scope rule: a file is either taken fully
off the macro or left alone with a stated reason.
🤖 Generated with Claude Code