Skip to content

Commit 9a33f39

Browse files
author
abacus_fixer
committed
fix(test_rhog_io): keep __MPI per-target so the MPI test initializes MPI
The test aborted with "MPI_Comm_rank() called before MPI_INIT" because the directory-level abacus_disable_feature_definitions(__MPI) in source_estate/test stripped __MPI from every target there. With __MPI gone, the test's main() never called MPI_Init, while the linked base/planewave libraries still issue real MPI calls. Add a target-level escape hatch: a new ABACUS_KEPT_FEATURE_DEFINITIONS target property (exposed as AddTest's KEEP_FEATURE_DEFINITIONS) lets a single target keep definitions its directory disables. Use it for this test, link planewave instead of planewave_serial, and copy the support/ data at configure time (install() is skipped by plain make+ctest). Also fix test bugs hidden while __MPI was stripped: drop const from kpar (passed by reference to divide_pools), call setup_pw_basis() in the three tests that need a valid grid, and set npwtot=1000 in InconsistentGammaOnly to trigger the expected "planewaves not used" warning. Verified: make MODULE_ESTATE_test_rhog_io && ctest -R MODULE_ESTATE_test_rhog_io -> 10/10 passed.
1 parent 3b6ad5e commit 9a33f39

4 files changed

Lines changed: 38 additions & 3 deletions

File tree

cmake/Testing.cmake

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,17 @@ endif()
3232

3333
function(AddTest) # function for UT
3434
cmake_parse_arguments(UT "DYN" "TARGET"
35-
"LIBS;DYN_LIBS;STATIC_LIBS;SOURCES;DEPENDS" ${ARGN})
35+
"LIBS;DYN_LIBS;STATIC_LIBS;SOURCES;DEPENDS;KEEP_FEATURE_DEFINITIONS" ${ARGN})
3636
add_executable(${UT_TARGET} ${UT_SOURCES})
3737

38+
# Let this target keep feature definitions (e.g. __MPI) that its source
39+
# directory disables via abacus_disable_feature_definitions(). Needed by
40+
# tests that genuinely exercise the feature.
41+
if(UT_KEEP_FEATURE_DEFINITIONS)
42+
set_property(TARGET ${UT_TARGET} PROPERTY
43+
ABACUS_KEPT_FEATURE_DEFINITIONS ${UT_KEEP_FEATURE_DEFINITIONS})
44+
endif()
45+
3846
if(ENABLE_COVERAGE)
3947
add_coverage(${UT_TARGET})
4048
endif()

source/CMakeLists.txt

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -417,6 +417,15 @@ define_property(
417417
BRIEF_DOCS "Additional ABACUS feature definitions for targets in this directory"
418418
FULL_DOCS "Additional feature definitions for targets created in this directory.")
419419

420+
define_property(
421+
TARGET
422+
PROPERTY ABACUS_KEPT_FEATURE_DEFINITIONS
423+
BRIEF_DOCS "Feature definitions this target keeps despite a directory-level disable"
424+
FULL_DOCS "Feature definitions that must not be stripped from this target even "
425+
"when its source directory disables them via "
426+
"abacus_disable_feature_definitions(). Used by tests that genuinely need a "
427+
"feature (e.g. __MPI) inside a directory that otherwise disables it.")
428+
420429
function(abacus_disable_feature_definitions)
421430
abacus_normalize_definitions(_defs ${ARGN})
422431
set_property(DIRECTORY APPEND PROPERTY
@@ -448,7 +457,12 @@ function(abacus_apply_build_options target)
448457
set(_defs "${_abacus_feature_definitions}")
449458
get_property(_disabled DIRECTORY "${_source_dir}" PROPERTY ABACUS_DISABLED_FEATURE_DEFINITIONS)
450459
get_property(_local DIRECTORY "${_source_dir}" PROPERTY ABACUS_LOCAL_FEATURE_DEFINITIONS)
460+
get_target_property(_kept "${target}" ABACUS_KEPT_FEATURE_DEFINITIONS)
451461

462+
if(_kept)
463+
# A target may opt back into definitions its directory disables.
464+
list(REMOVE_ITEM _disabled ${_kept})
465+
endif()
452466
if(_disabled)
453467
# Filter after conditional definitions have been evaluated.
454468
string(JOIN "|" _disabled_regex ${_disabled})

source/source_estate/test/CMakeLists.txt

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@ abacus_disable_feature_definitions(_OPENMP)
99

1010
if (ENABLE_MPI)
1111

12+
# Copy at configure time so a plain `make` + `ctest` run finds the data.
13+
# install() only runs during `cmake --install`, which local test runs skip.
14+
file(COPY support DESTINATION ${CMAKE_CURRENT_BINARY_DIR})
1215
install(DIRECTORY support DESTINATION ${CMAKE_CURRENT_BINARY_DIR})
1316

1417
AddTest(
@@ -115,8 +118,12 @@ AddTest(
115118

116119
AddTest(
117120
TARGET MODULE_ESTATE_test_rhog_io
118-
LIBS parameter base device planewave_serial
121+
LIBS parameter base device planewave
119122
SOURCES test_rhog_io.cpp ../rhog_io.cpp ../../source_basis/module_pw/test/test_tool.cpp
123+
# This test drives PW_Basis::initmpi and read/write_rhog's MPI collectives,
124+
# so it must keep __MPI even though this directory disables it. Its main()
125+
# calls MPI_Init via test_tool.cpp's setupmpi().
126+
KEEP_FEATURE_DEFINITIONS __MPI
120127
)
121128

122129
endif()

source/source_estate/test/test_rhog_io.cpp

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ TEST_F(ReadRhogTest, ReadRhog)
9191
// Test the read_rhog function when the file is not found
9292
TEST_F(ReadRhogTest, NotFoundFile)
9393
{
94+
setup_pw_basis();
9495
std::string filename = "notfound.txt";
9596

9697
open_warning("test_read_rhog.txt");
@@ -106,8 +107,12 @@ TEST_F(ReadRhogTest, NotFoundFile)
106107
// Test the read_rhog function when gamma_only is inconsistent
107108
TEST_F(ReadRhogTest, InconsistentGammaOnly)
108109
{
110+
setup_pw_basis();
109111
std::string filename = "./support/charge-density.dat";
110112
rhopw.gamma_only = true;
113+
// Fewer planewaves than the file holds (1471) triggers the
114+
// "some planewaves in file are not used" warning.
115+
rhopw.npwtot = 1000;
111116

112117
open_warning("test_read_rhog.txt");
113118
bool result = elecstate::read_rhog(filename, &rhopw, 2, rhog.data(), pw_world, &warning_stream);
@@ -126,6 +131,7 @@ TEST_F(ReadRhogTest, InconsistentGammaOnly)
126131
// Test the read_rhog function when some planewaves in file are missing
127132
TEST_F(ReadRhogTest, SomePWMissing)
128133
{
134+
setup_pw_basis();
129135
std::string filename = "./support/charge-density.dat";
130136
rhopw.npwtot = 2000;
131137

@@ -383,7 +389,7 @@ int main(int argc, char** argv)
383389
int nproc = 1;
384390
int myrank = 0;
385391
int nproc_in_pool = 1;
386-
const int kpar = 1;
392+
int kpar = 1;
387393
int mypool = 0;
388394
int rank_in_pool = 0;
389395
setupmpi(argc, argv, nproc, myrank);

0 commit comments

Comments
 (0)