Skip to content

Commit e1ca0ae

Browse files
committed
Merge branch 'kinit' into 'main'
Fixes in the LibHPG RAII class to initialize/finalize HPG/Kokoks once in a process. See merge request ardg/libra!141
2 parents 993d6a5 + 958bb97 commit e1ca0ae

File tree

9 files changed

+70
-438
lines changed

9 files changed

+70
-438
lines changed

.gitlab-ci.yml

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -151,8 +151,9 @@ test-job:
151151
# --- C++ GTest section ---
152152
cd ${base_path}/apps/src/tests/
153153
# Run C++ gtest binaries if they exist
154-
test_binaries=("LibRATests" "test_roadrunner_robustMinus2" "test_roadrunner_robustPlus2" "test_roadrunner_snrpsf" "test_roadrunner_weight")
155-
154+
#test_binaries=("LibRATests" "test_roadrunner_robustMinus2" "test_roadrunner_robustPlus2" "test_roadrunner_snrpsf" "test_roadrunner_weight")
155+
test_binaries=("LibRATests")
156+
156157
for test_binary in "${test_binaries[@]}"; do
157158
if [ -x "${base_path}/install/bin/tests/${test_binary}" ]; then
158159
echo "Running ${test_binary}..."

apps/src/RoadRunner/roadrunner.cc

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -339,13 +339,24 @@ auto Roadrunner(//bool& restartUI, int& argc, char** argv,
339339
bool& conjBeams, float& pbLimit, vector<float>& posigdev,
340340
bool& doSPWDataIter) -> RRReturnType
341341
{
342-
LogFilter filter(LogMessage::NORMAL);
343-
LogSink::globalSink().filter(filter);
342+
// LogFilter filter(LogMessage::NORMAL);
343+
// LogSink::globalSink().filter(filter);
344344
LogIO log_l(LogOrigin("roadrunner","Roadrunner_func"));
345345
RRReturnType rrr;
346346

347347
try
348348
{
349+
// set the default of rmode to be "norm"
350+
if (rmode =="")
351+
rmode = "norm";
352+
353+
// put the guard here so in the UI() users don't need to correct `rmode` if they don't know how to
354+
if (weighting == "briggs" && rmode !="norm")
355+
{
356+
log_l << "'rmode' parameter set to 'norm' for Briggs weighting to function correctly"
357+
<< LogIO::WARN << LogIO::POST;
358+
rmode = "norm";
359+
}
349360
// Set safe defaults...
350361
casa::refim::FTMachine::Type dataCol_l=casa::refim::FTMachine::CORRECTED;
351362
if (imagingMode=="predict") dataCol_l=casa::refim::FTMachine::MODEL;
@@ -368,7 +379,7 @@ auto Roadrunner(//bool& restartUI, int& argc, char** argv,
368379
// A RAII class instance that manages the HPG initialize/finalize scope.
369380
// And hpg::finalize() is called when this instance goes out of
370381
// scope.
371-
LibHPG libhpg(ftmName=="awphpg");
382+
LibHPG libhpg(ftmName=="awphpg",&std::cerr);
372383
// std::atexit(tpl_finalize);
373384

374385
bool const doSow = sowImageExt != "";
@@ -443,19 +454,6 @@ auto Roadrunner(//bool& restartUI, int& argc, char** argv,
443454
stokes, refFreqStr, mode);
444455
PagedImage<Float> skyImage(cgrid.shape(),cgrid.coordinates(), imageName);
445456

446-
// set the default of rmode to be "norm"
447-
if (rmode =="")
448-
rmode = "norm";
449-
// put the guard here so in the UI() users don't need to correct `rmode` if they don't know how to
450-
if (rmode != "norm")
451-
{
452-
log_l << "'rmode' parameter is not set to 'norm'. Please verify this is intentional." << LogIO::WARN;
453-
log_l << "For Briggs weighting, `rmode` is automatically set to `norm` to ensure the robust parameter functions correctly." << LogIO::WARN;
454-
}
455-
if (weighting == "briggs" && rmode !="norm")
456-
rmode = "norm";
457-
458-
459457
// cgrid.table().markForDelete();
460458

461459
// Setup the weighting scheme in the supplied VI2

apps/src/RoadRunner/roadrunner.h

Lines changed: 46 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,8 @@ class LibHPG
115115
* @brief Constructor for the LibHPG class.
116116
* @param usingHPG A boolean indicating whether to use HPG. Default is true.
117117
*/
118-
LibHPG(const bool usingHPG=true): init_hpg(usingHPG)
118+
LibHPG(const bool usingHPG=true, std::ostream* os=nullptr):
119+
init_hpg(usingHPG),hpg_initialized(false),os_p(os)
119120
{
120121
initialize();
121122
}
@@ -125,43 +126,65 @@ class LibHPG
125126
*/
126127
bool initialize()
127128
{
128-
bool ret = true;
129129
startTime = std::chrono::steady_clock::now();
130-
#ifdef ROADRUNNER_USE_HPG
131-
if (init_hpg) ret = hpg::initialize();
132-
#endif
133-
if (!ret)
134-
throw(AipsError("LibHPG::initialize() failed"));
135-
return ret;
136-
};
137-
/**
138-
* @brief Finalizes the HPG library.
139-
*/
140-
void finalize()
141-
{
130+
131+
hpg_initialized=true;
142132
#ifdef ROADRUNNER_USE_HPG
143133
if (init_hpg)
144134
{
145-
cerr << endl << "CALLING hpg::finalize()" << endl;
146-
hpg::finalize();
147-
}
135+
static bool once = [&]
136+
{
137+
assert(!hpg::is_finalized());
138+
if (!hpg::is_initialized())
139+
{
140+
hpg_initialized=hpg::initialize();
141+
// cerr << "LibHPG:: hpg::initialize() called" << endl;
142+
// Kokkos::push_finalize_hook([]() {
143+
// cerr << "Kokkos::finalize() called" << endl;
144+
// });
145+
std::atexit(hpg::finalize);
146+
}
147+
return hpg_initialized;
148+
}();
149+
}
150+
assert(hpg::is_intialized);
148151
#endif
152+
return hpg_initialized;
149153
};
154+
155+
//
156+
// @brief Return the internal state variable to indicate if the
157+
// hpg::initialize() method was called.
158+
//
159+
bool hpg_intialize_called() {return hpg_initialized;}
160+
161+
//
162+
// @brief Return the total time elasped since the class constructor was called.
163+
//
164+
double runtime()
165+
{
166+
auto endTime = std::chrono::steady_clock::now();
167+
std::chrono::duration<double> totalRuntime = endTime - startTime;
168+
return totalRuntime.count();
169+
}
150170
/**
151171
* @brief Destructor for the LibHPG class.
172+
*
173+
* If an output stream is give, this also prints the total runtime
174+
* on that stream.
152175
*/
153176
~LibHPG()
154177
{
155-
finalize();
156-
157-
auto endTime = std::chrono::steady_clock::now();
158-
std::chrono::duration<double> runtime = endTime - startTime;
159-
std::cerr << "roadrunner runtime: " << runtime.count()
160-
<< " sec" << std::endl;
178+
if (os_p != nullptr)
179+
(*os_p) << "roadrunner runtime: "
180+
<< std::fixed << std::setprecision(2)
181+
<< runtime() << " sec" << std::endl;
161182
};
162183

163184
std::chrono::time_point<std::chrono::steady_clock> startTime;
164185
bool init_hpg;
186+
bool hpg_initialized;
187+
std::ostream *os_p;
165188
};
166189
//
167190
//-------------------------------------------------------------------------
@@ -226,8 +249,6 @@ void CFServer(libracore::ThreadCoordinator& thcoord,
226249
casacore::Vector<double>& spwRefFreqList,
227250
int& nDataPol);
228251

229-
230-
231252
//--------------------------------------------------------------------------------------------
232253
// Place-holder functions to build the appropriate Mueller index
233254
// matrix. For production this should be generalized and perhaps
@@ -337,4 +358,3 @@ void UI(Bool restart, int argc, char **argv, bool interactive,
337358

338359

339360
#endif
340-

apps/src/tests/CMakeLists.txt

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -33,24 +33,11 @@ add_executable(LibRATests ${TEST_SOURCES})
3333
# Link with Google Test
3434
set(INTERFACE_LIBRARIES libra libracore roadrunner_cl_interface coyote_cl_interface hummbee_cl_interface mssplit_cl_interface subms_cl_interface tableinfo_cl_interface chip_cl_interface dale_cl_interface casacore_asp)
3535

36-
#target_link_libraries(LibRATests GTest::gtest_main stdc++fs ${APP_LINK_LIBRARIES} ${INTERFACE_LIBRARIES})
36+
target_link_libraries(LibRATests GTest::gtest_main stdc++fs ${APP_LINK_LIBRARIES} ${INTERFACE_LIBRARIES})
3737

38-
add_executable(test_roadrunner_robustMinus2 test_roadrunner_robustMinus2.cc)
39-
add_executable(test_roadrunner_robustPlus2 test_roadrunner_robustPlus2.cc)
40-
add_executable(test_roadrunner_snrpsf test_roadrunner_snrpsf.cc)
41-
add_executable(test_roadrunner_weight test_roadrunner_weight.cc)
42-
43-
# Link all with required libraries
44-
foreach(test_target LibRATests test_roadrunner_robustMinus2 test_roadrunner_robustPlus2 test_roadrunner_snrpsf test_roadrunner_weight)
45-
target_link_libraries(${test_target} GTest::gtest_main stdc++fs ${APP_LINK_LIBRARIES} ${INTERFACE_LIBRARIES})
46-
endforeach()
4738

4839
# Add the test executable as a test target
4940
add_test(NAME LibRATests COMMAND LibRATests)
50-
add_test(NAME test_roadrunner_robustMinus2 COMMAND test_roadrunner_robustMinus2)
51-
add_test(NAME test_roadrunner_robustPlus2 COMMAND test_roadrunner_robustPlus2)
52-
add_test(NAME test_roadrunner_snrpsf COMMAND test_roadrunner_snrpsf)
53-
add_test(NAME test_roadrunner_weight COMMAND test_roadrunner_weight)
5441

5542
if(Apps_BUILD_TESTS_PYTHON)
5643
find_program(PYTHON_EXECUTABLE python)
@@ -61,6 +48,6 @@ if(Apps_BUILD_TESTS_PYTHON)
6148
endif()
6249

6350
# Install the test executable
64-
install(TARGETS LibRATests test_roadrunner_robustMinus2 test_roadrunner_robustPlus2 test_roadrunner_snrpsf test_roadrunner_weight
51+
install(TARGETS LibRATests
6552
RUNTIME DESTINATION "${CMAKE_INSTALL_PREFIX}/bin/tests"
6653
)

apps/src/tests/test_roadrunner.cc

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ namespace test{
88
const path goldDir = current_path() / "gold_standard";
99

1010

11-
/*TEST(RoadrunnerTest, InitializeTest) {
11+
TEST(RoadrunnerTest, InitializeTest) {
1212
// Create a LibHPG object
1313
LibHPG lib_hpg;
1414

@@ -17,10 +17,8 @@ namespace test{
1717

1818
// Check that the return value is true
1919
EXPECT_TRUE(ret);
20-
}*/
20+
}
2121

22-
// uncomment the following when the multiple hpg initialization error is fixed.
23-
/*
2422
TEST(RoadrunnerTest, AppLevelSNRPSF_performance) {
2523
// Get the test name
2624
string testName = ::testing::UnitTest::GetInstance()->current_test_info()->name();
@@ -185,8 +183,8 @@ TEST(RoadrunnerTest, AppLevelWeight) {
185183

186184
PagedImage<Float> weight("htclean_gpu_newpsf.weight");
187185
float tol = 0.1;
188-
float goldValLoc0 = -277710.1875;
189-
float goldValLoc1 = 470668.9375;
186+
float goldValLoc0 = 1255553.875;
187+
float goldValLoc1 = 580468.3125;
190188
EXPECT_NEAR(weight(IPosition(4,2000,2053,0,0)), goldValLoc0, tol);
191189
EXPECT_NEAR(weight(IPosition(4,1379,1969,0,0)), goldValLoc1, tol);
192190

@@ -348,7 +346,7 @@ TEST(RoadrunnerTest, Interface_rmode_minus2) {
348346
std::filesystem::current_path(testDir.parent_path());
349347

350348
remove_all(testDir);
351-
}*/
349+
}
352350

353351
TEST(RoadrunnerTest, UIFactory) {
354352
// The Factory Settings.

apps/src/tests/test_roadrunner_robustMinus2.cc

Lines changed: 0 additions & 86 deletions
This file was deleted.

0 commit comments

Comments
 (0)