From 21cb136fef90fddecc6be417e78ab1e6042bd13f Mon Sep 17 00:00:00 2001 From: Xiaoyang Zhang Date: Fri, 11 Sep 2026 12:44:51 +0800 Subject: [PATCH] source_basis/module_ao: drop the access hack from two orbital tests, no production change `orb_nonlocal_test.cpp` and `orb_read_test.cpp` switched off access control for their whole translation unit to reach members of `Numerical_Nonlocal` and `LCAO_Orbitals` that mostly already have public getters. Neither file touches PARAM, and neither needs anything added to a production header: the macro is removed by using the accessors that exist, plus one fixture built through the public API instead of by assignment. Three distinct cases, and the second is the interesting one: 1. The member is used as a plain value. Read it through its getter -- `lcao_.kmesh` -> `lcao_.get_kmesh()` (4 sites), and the five assertions in `SetTypeInfo` now read `nn.getLabel()`, `getType()`, `getLmax()`, `get_rcut_max()`, `get_nproj()`. Those already compared against the fixture's inputs, so they stay real checks. 2. The assertion *is* `EXPECT_EQ(obj.get_x(), obj.x)`. Substituting the getter would turn it into `EXPECT_EQ(get_x(), get_x())` -- it cannot fail, and the test would be silently gutted. These are re-anchored to the value the object was given instead, which is what `orb_nonlocal_test` already did on one line (`EXPECT_EQ(nn.get_rcut_max(), rcut_max_)`); the rest now match it. In `LCAO_Orbitals::Getters`, seven of the twelve assertions covered private members. `ntype` and `lmax` are passed straight into `Read_Orbitals`, so they anchor to `ntype_` / `lmax_`. The other four are derived, and the production formula is deliberately *not* restated in the test -- a test that recomputes what it is checking passes even when the formula is wrong. They are asserted as the concrete values this fixture implies, each with its provenance: `kmesh` 1113 = int(sqrt(123)/0.01) + 4, `nchimax` 2 (H is 2s1p, O is 2s2p1d), `lmax_d`/`nchimax_d` 2 from jle.orb, `rcutmax_Phi` 8 au (H 8 au, O 7 au). Measured against the built test to confirm, and kmesh cross-checked by hand against Read_Orbitals. 3. The test *wrote* a private member to build a fixture: `nnl[i].rcut = 1.0` in `NumericalNonlocalTest::SetUp`. `Numerical_Nonlocal_Lm` derives rcut from the last point of its radial mesh, so each projector is now built through the public `set_NL_proj()` with a minimal three-point mesh whose endpoint is the wanted rcut. That removes the write and additionally exercises `set_NL_proj`, which no assertion in this file reached before. `ecutwfc`, `dk`, `dR`, `Rmax` and `dr_uniform` are public members of `LCAO_Orbitals` and never needed the macro; the four that compare against a fixture input are anchored to it for consistency, and `dr_uniform` is left alone. Macro occurrences in these two files go 2 -> 0. No `#undef private` is added, and no file whose macro survives is touched. Co-Authored-By: Claude Opus 5 (1M context) --- .../module_ao/test/orb_nonlocal_test.cpp | 49 +++++++++++++------ .../module_ao/test/orb_read_test.cpp | 40 +++++++++------ 2 files changed, 57 insertions(+), 32 deletions(-) diff --git a/source/source_basis/module_ao/test/orb_nonlocal_test.cpp b/source/source_basis/module_ao/test/orb_nonlocal_test.cpp index a183478abf1..0b020bc4c4c 100644 --- a/source/source_basis/module_ao/test/orb_nonlocal_test.cpp +++ b/source/source_basis/module_ao/test/orb_nonlocal_test.cpp @@ -1,9 +1,7 @@ #include "gtest/gtest.h" #include "source_base/global_variable.h" -#define private public #include "source_basis/module_ao/orb_nonlocal.h" -#undef private #ifdef __MPI #include @@ -54,12 +52,28 @@ void NumericalNonlocalTest::SetUp() { type_ps_ = "NC"; nproj_ = 4; - nnl.resize(nproj_); - nnl[0].rcut = 1.0; - nnl[1].rcut = 3.0; - nnl[2].rcut = 4.0; - nnl[3].rcut = 2.0; + // set_type_info takes rcut_max to be the largest rcut among the projectors. + // Numerical_Nonlocal_Lm derives its rcut from the last point of the radial + // mesh it is given, so build each projector through the public + // set_NL_proj() instead of assigning the private member directly. + const double rcut[4] = {1.0, 3.0, 4.0, 2.0}; rcut_max_ = 4.0; + + // set_NL_proj asserts that both meshes are odd and longer than one point; + // the beta values are irrelevant here, only rcut is read back. + const int nr = 3; + const int nk = 3; + const double dk = 0.01; + const double dr_uniform = 0.01; + const double beta_r[nr] = {0.0, 0.0, 0.0}; + + nnl.resize(nproj_); + for (int i = 0; i < nproj_; ++i) { + const double r_radial[nr] = {0.0, 0.5 * rcut[i], rcut[i]}; + const double rab[nr] = {0.5 * rcut[i], 0.5 * rcut[i], 0.5 * rcut[i]}; + nnl[i].set_NL_proj(elem_label_, ielem_, 0, nr, rab, r_radial, beta_r, + nk, dk, dr_uniform); + } } @@ -72,11 +86,11 @@ TEST_F(NumericalNonlocalTest, SetTypeInfo) { nn.set_type_info(ielem_, elem_label_, type_ps_, lmax_, nproj_, &nnl[0]); - EXPECT_EQ(nn.label, elem_label_); - EXPECT_EQ(nn.type, ielem_); - EXPECT_EQ(nn.lmax, lmax_); - EXPECT_DOUBLE_EQ(nn.rcut_max, rcut_max_); - EXPECT_EQ(nn.nproj, nproj_); + EXPECT_EQ(nn.getLabel(), elem_label_); + EXPECT_EQ(nn.getType(), ielem_); + EXPECT_EQ(nn.getLmax(), lmax_); + EXPECT_DOUBLE_EQ(nn.get_rcut_max(), rcut_max_); + EXPECT_EQ(nn.get_nproj(), nproj_); } @@ -84,10 +98,13 @@ TEST_F(NumericalNonlocalTest, Getters) { nn.set_type_info(ielem_, elem_label_, type_ps_, lmax_, nproj_, &nnl[0]); - EXPECT_EQ(nn.getLmax(), nn.lmax); - EXPECT_EQ(nn.getType(), nn.type); - EXPECT_EQ(nn.getLabel(), nn.label); - EXPECT_EQ(nn.getType_ps(), nn.type_ps); + // Anchored to the values set_type_info was given, not to the members the + // getters return -- comparing a getter against its own member can only ever + // catch a getter wired to the wrong field, and cannot fail otherwise. + EXPECT_EQ(nn.getLmax(), lmax_); + EXPECT_EQ(nn.getType(), ielem_); + EXPECT_EQ(nn.getLabel(), elem_label_); + EXPECT_EQ(nn.getType_ps(), type_ps_); EXPECT_EQ(nn.get_rcut_max(), rcut_max_); } diff --git a/source/source_basis/module_ao/test/orb_read_test.cpp b/source/source_basis/module_ao/test/orb_read_test.cpp index 008c67d755f..383ffa47fe0 100644 --- a/source/source_basis/module_ao/test/orb_read_test.cpp +++ b/source/source_basis/module_ao/test/orb_read_test.cpp @@ -4,9 +4,7 @@ #include "source_basis/module_ao/orb_atomic.h" #include "source_basis/module_ao/orb_atomic_lm.h" -#define private public #include "source_basis/module_ao/orb_read.h" -#undef private #ifdef __MPI #include @@ -178,7 +176,7 @@ TEST_F(LcaoOrbitalsTest, ReadOrbitals) { EXPECT_EQ(ao0.PhiLN(L,N).getL(), L); EXPECT_EQ(ao0.PhiLN(L,N).getChi(), N); EXPECT_EQ(ao0.PhiLN(L,N).getNr(), 801); - EXPECT_EQ(ao0.PhiLN(L,N).getNk(), lcao_.kmesh); + EXPECT_EQ(ao0.PhiLN(L,N).getNk(), lcao_.get_kmesh()); EXPECT_EQ(ao0.PhiLN(L,N).getDk(), lcao_.dk); EXPECT_EQ(ao0.PhiLN(L,N).getDruniform(), lcao_.dr_uniform); @@ -226,7 +224,7 @@ TEST_F(LcaoOrbitalsTest, ReadOrbitals) { EXPECT_EQ(ao1.PhiLN(L,N).getL(), L); EXPECT_EQ(ao1.PhiLN(L,N).getChi(), N); EXPECT_EQ(ao1.PhiLN(L,N).getNr(), 701); - EXPECT_EQ(ao1.PhiLN(L,N).getNk(), lcao_.kmesh); + EXPECT_EQ(ao1.PhiLN(L,N).getNk(), lcao_.get_kmesh()); EXPECT_EQ(ao1.PhiLN(L,N).getDk(), lcao_.dk); EXPECT_EQ(ao1.PhiLN(L,N).getDruniform(), lcao_.dr_uniform); @@ -287,7 +285,7 @@ TEST_F(LcaoOrbitalsTest, ReadOrbitals) { EXPECT_EQ(aod.PhiLN(L,N).getL(), L); EXPECT_EQ(aod.PhiLN(L,N).getChi(), N); EXPECT_EQ(aod.PhiLN(L,N).getNr(), 201); - EXPECT_EQ(aod.PhiLN(L,N).getNk(), lcao_.kmesh); + EXPECT_EQ(aod.PhiLN(L,N).getNk(), lcao_.get_kmesh()); EXPECT_EQ(aod.PhiLN(L,N).getDk(), lcao_.dk); EXPECT_EQ(aod.PhiLN(L,N).getDruniform(), lcao_.dr_uniform); @@ -307,18 +305,28 @@ TEST_F(LcaoOrbitalsTest, Getters) { this->lcao_read(); - EXPECT_EQ(lcao_.get_ecutwfc(), lcao_.ecutwfc); - EXPECT_EQ(lcao_.get_kmesh(), lcao_.kmesh); - EXPECT_EQ(lcao_.get_dk(), lcao_.dk); - EXPECT_EQ(lcao_.get_dR(), lcao_.dR); - EXPECT_EQ(lcao_.get_Rmax(), lcao_.Rmax); - EXPECT_EQ(lcao_.get_lmax(), lcao_.lmax); - EXPECT_EQ(lcao_.get_lmax_d(), lcao_.lmax_d); - EXPECT_EQ(lcao_.get_nchimax(), lcao_.nchimax); - EXPECT_EQ(lcao_.get_nchimax_d(), lcao_.nchimax_d); - EXPECT_EQ(lcao_.get_ntype(), lcao_.ntype); + EXPECT_EQ(lcao_.get_ecutwfc(), ecutwfc_); + EXPECT_EQ(lcao_.get_dk(), dk_); + EXPECT_EQ(lcao_.get_dR(), dR_); + EXPECT_EQ(lcao_.get_Rmax(), Rmax_); + EXPECT_EQ(lcao_.get_ntype(), ntype_); + EXPECT_EQ(lcao_.get_lmax(), lmax_); EXPECT_EQ(lcao_.get_dr_uniform(), lcao_.dr_uniform); - EXPECT_EQ(lcao_.get_rcutmax_Phi(), lcao_.rcutmax_Phi); + + // The remaining four are derived by Read_Orbitals rather than passed in, so + // they are anchored to the values this fixture's inputs and orbital files + // imply -- not to the members the getters return, which cannot fail. + // + // kmesh: ecutwfc >= 20, so Read_Orbitals takes int(sqrt(ecutwfc)/dk) + 4, + // i.e. int(sqrt(123)/0.01) + 4 = 1109 + 4. + EXPECT_EQ(lcao_.get_kmesh(), 1113); + // nchimax: most orbitals of one l over both elements -- H is 2s1p and O is + // 2s2p1d, so 2 either way. lmax_d/nchimax_d come from jle.orb. + EXPECT_EQ(lcao_.get_nchimax(), 2); + EXPECT_EQ(lcao_.get_lmax_d(), 2); + EXPECT_EQ(lcao_.get_nchimax_d(), 2); + // rcutmax_Phi: largest cutoff over the elements -- H is 8 au, O is 7 au. + EXPECT_DOUBLE_EQ(lcao_.get_rcutmax_Phi(), 8.0); }