source_relax: give BFGS_Basic and Ions_Move_BFGS an explicit test seam, removing four access hacks

[Critsium-xy]

What this does

bfgs_basic_test and ions_move_bfgs_test drove the BFGS machinery through
#define private public / #define protected public. Both now go through named
accessors on the two classes, so the access specifiers mean what they say. Four
macros removed; source_relax drops from 6 to 2.

BFGS_Basic

Eleven reference accessors — get_pos(), get_grad(), get_move(),
get_pos_p(), get_grad_p(), get_move_p(), get_save_flag(),
get_tr_min_hit(), get_wolfe_flag(), get_inv_hess(), get_bfgs_ndim() — and
seven wrappers for the protected/private steps the tests drive one stage at a
time: allocate_basic_for_testing(), new_step_for_testing(),
reset_hessian_for_testing(), save_bfgs_for_testing(),
update_inverse_hessian_for_testing(), check_wolfe_conditions_for_testing(),
compute_trust_radius_for_testing().

Ions_Move_BFGS

get_init_done(), get_first_step(), bfgs_routine_for_testing(),
restart_bfgs_for_testing().

Why references, and why these names

The accessors return non-const references because the tests both seed and
inspect this state. Measured across the two files: inv_hess alone is 34 reads
and 20 writes, and every member except pos_p is written somewhere, so
read-only getters would not have been enough.

T& get_x() matches the convention already used in roughly 83 places in the
tree (get_allocator(), get_bgrids(), get_nonlocal(), get_nproj(), …). The
method wrappers follow the single existing precedent for a test-only entry point,
set_density_rotations_for_testing() in symm_rotation.h, and both classes carry
a comment saying production code must keep calling the protected/private names
directly.

A tradeoff reviewers should weigh

Stating this plainly rather than burying it: this adds 21 public members that
only the tests use, and for the seven method wrappers a public
*_for_testing() forwarder is a weaker boundary than the alternative.

That alternative was measured and not taken: BFGS_Basic already keeps almost
all of its state protected (pos, grad, move, pos_p, grad_p, move_p,
save_flag, tr_min_hit, plus four of the methods), and its private: section
holds six declarations of the same kind — algorithm state and algorithm steps.
Moving those six into the protected section and letting the two fixtures derive
from the class would have changed no signature and added no public API, at the
cost of relaxing encapsulation toward subclasses. Of the 139 accesses in
bfgs_basic_test, exactly 68 are protected and 68 private, so deriving alone
would not have sufficed without that move.

Both routes were put to the maintainer and this one was chosen. If review prefers
the other, the change is small and mechanical to flip.

Verification

Remote Linux box, cmake -B build -G Ninja -DBUILD_TESTING=ON -DENABLE_LCAO=ON -DENABLE_MPI=ON -DENABLE_OPENMP=ON:

• configure exit 0; 3354/3354 targets, build exit 0, zero errors;
  ninja: no work to do on a follow-up full build
• all 15 MODULE_RELAX binaries link, including
  MODULE_RELAX_bfgs_basic_test, MODULE_RELAX_ions_move_bfgs_test and
  MODULE_RELAX_ions_move_methods_test (the last one still uses the macro and is
  unaffected), plus abacus_basic_para
• agent_governance_check.py --base upstream/develop --head HEAD: zero
  blockers, no access-hack ratchet finding (4 removed, 0 added), no
  PARAM/GlobalV/GlobalC change

No production logic changed. No assertion or expected value changed — the
tests call the same code with the same inputs through the new names. No INPUT
parameter or user-facing behaviour changed, so no documentation update is
required.

Not in this PR

ions_move_methods_test keeps source_relax's last two macros. It reaches
through Ions_Move_Methods' aggregated members — imm.bfgs.tr_min_hit,
imm.bfgs.save_flag, imm.bfgs.first_step, imm.bfgs.pos / pos_p,
imm.bfgs_trad.is_initialized — as well as its own converged_, update_iter_
and etot_info_. Note that a derived-fixture approach cannot work there at all:
C++ only lets a derived class reach a base's protected members through objects of
its own type, and Ions_Move_Methods does not derive from BFGS_Basic. It needs
accessors on Ions_Move_Methods too, and now has the Ions_Move_BFGS /
BFGS_Basic half of what it requires from this PR.

Continues #7921 / #7949 / #7952 / #7953 / #7963 / #7964 / #7965 / #7966 / #7967 /
#7980 / #7981 / #7982 / #7983.

🤖 Generated with Claude Code