Skip to content

Commit 9958006

Browse files
Critsium-xyclaude
andauthored
source_basis/module_ao: drop the access hack from two orbital tests, no production change (#7952)
`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) <noreply@anthropic.com>
1 parent 9cbcc0b commit 9958006

2 files changed

Lines changed: 57 additions & 32 deletions

File tree

source/source_basis/module_ao/test/orb_nonlocal_test.cpp

Lines changed: 33 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,7 @@
11
#include "gtest/gtest.h"
22
#include "source_base/global_variable.h"
33

4-
#define private public
54
#include "source_basis/module_ao/orb_nonlocal.h"
6-
#undef private
75

86
#ifdef __MPI
97
#include <mpi.h>
@@ -54,12 +52,28 @@ void NumericalNonlocalTest::SetUp() {
5452
type_ps_ = "NC";
5553
nproj_ = 4;
5654

57-
nnl.resize(nproj_);
58-
nnl[0].rcut = 1.0;
59-
nnl[1].rcut = 3.0;
60-
nnl[2].rcut = 4.0;
61-
nnl[3].rcut = 2.0;
55+
// set_type_info takes rcut_max to be the largest rcut among the projectors.
56+
// Numerical_Nonlocal_Lm derives its rcut from the last point of the radial
57+
// mesh it is given, so build each projector through the public
58+
// set_NL_proj() instead of assigning the private member directly.
59+
const double rcut[4] = {1.0, 3.0, 4.0, 2.0};
6260
rcut_max_ = 4.0;
61+
62+
// set_NL_proj asserts that both meshes are odd and longer than one point;
63+
// the beta values are irrelevant here, only rcut is read back.
64+
const int nr = 3;
65+
const int nk = 3;
66+
const double dk = 0.01;
67+
const double dr_uniform = 0.01;
68+
const double beta_r[nr] = {0.0, 0.0, 0.0};
69+
70+
nnl.resize(nproj_);
71+
for (int i = 0; i < nproj_; ++i) {
72+
const double r_radial[nr] = {0.0, 0.5 * rcut[i], rcut[i]};
73+
const double rab[nr] = {0.5 * rcut[i], 0.5 * rcut[i], 0.5 * rcut[i]};
74+
nnl[i].set_NL_proj(elem_label_, ielem_, 0, nr, rab, r_radial, beta_r,
75+
nk, dk, dr_uniform);
76+
}
6377
}
6478

6579

@@ -72,22 +86,25 @@ TEST_F(NumericalNonlocalTest, SetTypeInfo) {
7286

7387
nn.set_type_info(ielem_, elem_label_, type_ps_, lmax_, nproj_, &nnl[0]);
7488

75-
EXPECT_EQ(nn.label, elem_label_);
76-
EXPECT_EQ(nn.type, ielem_);
77-
EXPECT_EQ(nn.lmax, lmax_);
78-
EXPECT_DOUBLE_EQ(nn.rcut_max, rcut_max_);
79-
EXPECT_EQ(nn.nproj, nproj_);
89+
EXPECT_EQ(nn.getLabel(), elem_label_);
90+
EXPECT_EQ(nn.getType(), ielem_);
91+
EXPECT_EQ(nn.getLmax(), lmax_);
92+
EXPECT_DOUBLE_EQ(nn.get_rcut_max(), rcut_max_);
93+
EXPECT_EQ(nn.get_nproj(), nproj_);
8094
}
8195

8296

8397
TEST_F(NumericalNonlocalTest, Getters) {
8498

8599
nn.set_type_info(ielem_, elem_label_, type_ps_, lmax_, nproj_, &nnl[0]);
86100

87-
EXPECT_EQ(nn.getLmax(), nn.lmax);
88-
EXPECT_EQ(nn.getType(), nn.type);
89-
EXPECT_EQ(nn.getLabel(), nn.label);
90-
EXPECT_EQ(nn.getType_ps(), nn.type_ps);
101+
// Anchored to the values set_type_info was given, not to the members the
102+
// getters return -- comparing a getter against its own member can only ever
103+
// catch a getter wired to the wrong field, and cannot fail otherwise.
104+
EXPECT_EQ(nn.getLmax(), lmax_);
105+
EXPECT_EQ(nn.getType(), ielem_);
106+
EXPECT_EQ(nn.getLabel(), elem_label_);
107+
EXPECT_EQ(nn.getType_ps(), type_ps_);
91108
EXPECT_EQ(nn.get_rcut_max(), rcut_max_);
92109
}
93110

source/source_basis/module_ao/test/orb_read_test.cpp

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,7 @@
44
#include "source_basis/module_ao/orb_atomic.h"
55
#include "source_basis/module_ao/orb_atomic_lm.h"
66

7-
#define private public
87
#include "source_basis/module_ao/orb_read.h"
9-
#undef private
108

119
#ifdef __MPI
1210
#include <mpi.h>
@@ -178,7 +176,7 @@ TEST_F(LcaoOrbitalsTest, ReadOrbitals) {
178176
EXPECT_EQ(ao0.PhiLN(L,N).getL(), L);
179177
EXPECT_EQ(ao0.PhiLN(L,N).getChi(), N);
180178
EXPECT_EQ(ao0.PhiLN(L,N).getNr(), 801);
181-
EXPECT_EQ(ao0.PhiLN(L,N).getNk(), lcao_.kmesh);
179+
EXPECT_EQ(ao0.PhiLN(L,N).getNk(), lcao_.get_kmesh());
182180
EXPECT_EQ(ao0.PhiLN(L,N).getDk(), lcao_.dk);
183181
EXPECT_EQ(ao0.PhiLN(L,N).getDruniform(), lcao_.dr_uniform);
184182

@@ -226,7 +224,7 @@ TEST_F(LcaoOrbitalsTest, ReadOrbitals) {
226224
EXPECT_EQ(ao1.PhiLN(L,N).getL(), L);
227225
EXPECT_EQ(ao1.PhiLN(L,N).getChi(), N);
228226
EXPECT_EQ(ao1.PhiLN(L,N).getNr(), 701);
229-
EXPECT_EQ(ao1.PhiLN(L,N).getNk(), lcao_.kmesh);
227+
EXPECT_EQ(ao1.PhiLN(L,N).getNk(), lcao_.get_kmesh());
230228
EXPECT_EQ(ao1.PhiLN(L,N).getDk(), lcao_.dk);
231229
EXPECT_EQ(ao1.PhiLN(L,N).getDruniform(), lcao_.dr_uniform);
232230

@@ -287,7 +285,7 @@ TEST_F(LcaoOrbitalsTest, ReadOrbitals) {
287285
EXPECT_EQ(aod.PhiLN(L,N).getL(), L);
288286
EXPECT_EQ(aod.PhiLN(L,N).getChi(), N);
289287
EXPECT_EQ(aod.PhiLN(L,N).getNr(), 201);
290-
EXPECT_EQ(aod.PhiLN(L,N).getNk(), lcao_.kmesh);
288+
EXPECT_EQ(aod.PhiLN(L,N).getNk(), lcao_.get_kmesh());
291289
EXPECT_EQ(aod.PhiLN(L,N).getDk(), lcao_.dk);
292290
EXPECT_EQ(aod.PhiLN(L,N).getDruniform(), lcao_.dr_uniform);
293291

@@ -307,18 +305,28 @@ TEST_F(LcaoOrbitalsTest, Getters) {
307305

308306
this->lcao_read();
309307

310-
EXPECT_EQ(lcao_.get_ecutwfc(), lcao_.ecutwfc);
311-
EXPECT_EQ(lcao_.get_kmesh(), lcao_.kmesh);
312-
EXPECT_EQ(lcao_.get_dk(), lcao_.dk);
313-
EXPECT_EQ(lcao_.get_dR(), lcao_.dR);
314-
EXPECT_EQ(lcao_.get_Rmax(), lcao_.Rmax);
315-
EXPECT_EQ(lcao_.get_lmax(), lcao_.lmax);
316-
EXPECT_EQ(lcao_.get_lmax_d(), lcao_.lmax_d);
317-
EXPECT_EQ(lcao_.get_nchimax(), lcao_.nchimax);
318-
EXPECT_EQ(lcao_.get_nchimax_d(), lcao_.nchimax_d);
319-
EXPECT_EQ(lcao_.get_ntype(), lcao_.ntype);
308+
EXPECT_EQ(lcao_.get_ecutwfc(), ecutwfc_);
309+
EXPECT_EQ(lcao_.get_dk(), dk_);
310+
EXPECT_EQ(lcao_.get_dR(), dR_);
311+
EXPECT_EQ(lcao_.get_Rmax(), Rmax_);
312+
EXPECT_EQ(lcao_.get_ntype(), ntype_);
313+
EXPECT_EQ(lcao_.get_lmax(), lmax_);
320314
EXPECT_EQ(lcao_.get_dr_uniform(), lcao_.dr_uniform);
321-
EXPECT_EQ(lcao_.get_rcutmax_Phi(), lcao_.rcutmax_Phi);
315+
316+
// The remaining four are derived by Read_Orbitals rather than passed in, so
317+
// they are anchored to the values this fixture's inputs and orbital files
318+
// imply -- not to the members the getters return, which cannot fail.
319+
//
320+
// kmesh: ecutwfc >= 20, so Read_Orbitals takes int(sqrt(ecutwfc)/dk) + 4,
321+
// i.e. int(sqrt(123)/0.01) + 4 = 1109 + 4.
322+
EXPECT_EQ(lcao_.get_kmesh(), 1113);
323+
// nchimax: most orbitals of one l over both elements -- H is 2s1p and O is
324+
// 2s2p1d, so 2 either way. lmax_d/nchimax_d come from jle.orb.
325+
EXPECT_EQ(lcao_.get_nchimax(), 2);
326+
EXPECT_EQ(lcao_.get_lmax_d(), 2);
327+
EXPECT_EQ(lcao_.get_nchimax_d(), 2);
328+
// rcutmax_Phi: largest cutoff over the elements -- H is 8 au, O is 7 au.
329+
EXPECT_DOUBLE_EQ(lcao_.get_rcutmax_Phi(), 8.0);
322330
}
323331

324332

0 commit comments

Comments
 (0)