Fix: add MPI_Finalize in QUIT function

[kirk0830]

Reminder

• Have you linked an issue with this pull request?
• Have you added adequate unit tests and/or case tests for your pull request?
• Have you noticed possible changes of behavior below or in the linked issue?
• Have you explained the changes of codes in core modules of ESolver, HSolver, ElecState, Hamilt, Operator or Psi? (ignore if not applicable)

Linked Issue

Fix #5486

Unit Tests and/or Case Tests for my changes

• A unit test is added for each new feature or bug fix.

What's changed?

• Example: My changes might affect the performance of the application under certain conditions, and I have tested the impact on various scenarios...

Any changes of core modules? (ignore if not applicable)

• Example: I have added a new virtual function in the esolver base class in order to ...

[kirk0830]

Those unittests failed in a weird way. I need more time to understand what happened.
For example the CMakeLists of unittest of Charge_Mixing removes the definition of macro __MPI, so the change made in function QUIT should have no effect at all, but this is not how the program works now.

PS. What I have done in function QUIT is adding lines:

#ifdef __MPI
    MPI_Finalize();
#endif

[caic99]

LGTM. Your comments in the codes would be more helpful if they are submitted as new issues.

[kirk0830]

@jinzx10 helps me find it is the math_libs will link with MPI. The line remove_definitions(-D__MPI) in CMakeLists.txt of unittest does not work as what the programmer expected.
The line I add, MPI_Finalize(), will still be called but without MPI_Init().
I need some time to solve this.

[kirk0830]

I met severe problems related to EXPECT_EXIT/EXPECT_DEATH. This kind of test works in the following way, according to the annotation of gtest-death-test.h:

. It is only when the test is run purely single-thread, will the test be thread-safe. However, Googletest will pop warning like there is more than 1 threads active, and the test will time out.

Due to the complexity of ABACUS unittest now, I cannot continue with this PR temporarily, so I close it for now.

[kirk0830]

here I paste a minimal work example:

#ifdef __MPI
#include <mpi.h>
#endif
#include <iostream>
#include <gtest/gtest.h>

void foo(const int signal)
{
    if (signal == 0)
    {
        std::cout << "Normal exit" << std::endl;
    }
    else
    {
        std::cout << "Abnormal exit with signal " << signal << std::endl;
        #ifdef __MPI // MPI_Finalize() is called in the case of abnormal exit?
        // MPI_Finalize();
        #endif
        exit(signal);
    }
}

TEST(hw, test1)
{
    // capture stdout
    testing::internal::CaptureStdout();
    foo(0);
    const std::string out = testing::internal::GetCapturedStdout();
    EXPECT_EQ(out, "Normal exit\n");

    int irank = 0;
    #ifdef __MPI
    MPI_Comm_rank(MPI_COMM_WORLD, &irank);
    #endif
    if (irank == 0)
    {
        std::cout << "Captured stdout: " << out << std::endl;
    }
}

TEST(hw, test2)
{
    testing::internal::CaptureStdout();
    EXPECT_EXIT(foo(1), ::testing::ExitedWithCode(1), "");
    const std::string out = testing::internal::GetCapturedStdout();
    EXPECT_EQ(out, "Abnormal exit with signal 1\n");

    int irank = 0;
    #ifdef __MPI
    MPI_Comm_rank(MPI_COMM_WORLD, &irank);
    #endif
    if (irank == 0)
    {
        std::cout << "Captured stdout: " << out << std::endl;
    }
}

int main(int argc, char **argv)
{
    #ifdef __MPI
    MPI_Init(&argc, &argv);
    #endif
    ::testing::InitGoogleTest(&argc, argv);
    // ::testing::FLAGS_gtest_death_test_style = "threadsafe";
    const int result = RUN_ALL_TESTS();
    #ifdef __MPI
    MPI_Finalize();
    #endif
    return result;
}

with following CMakeLists.txt

project(whatswrong) # this is merely soft-required, but meaningless pratically
cmake_minimum_required(VERSION 3.22) # this is merely soft-required, but meaningless pratically
add_executable(test hw_mpi.cpp)
target_link_libraries(test gtest gtest_main)
# find mpi
find_package(MPI REQUIRED)
include_directories(${MPI_INCLUDE_PATH})
add_test(NAME test COMMAND test)

# if the WITH_MPI flag is set, then define the __MPI macro
if(WITH_MPI)
    target_compile_definitions(test PRIVATE __MPI)
    target_link_libraries(test ${MPI_LIBRARIES})
endif()

[kirk0830]

the unittest of (for instance) the charge_mixing failed because:

1. The MPI macro definition is not removed absolutely, it is still in the state of being defined in those included libs such as math_libs and base. Therefore no matter whether there is remove_definition(-D__MPI) in CMakeLists.txt of the unittest, there will still be mpi related code linked.
2. However, because the MPI_Init() is never called in unittest (the unittest of charge_mixing even does not have a main function!), so the call of MPI_Finalize is illegal (because MPI_Init() and MPI_Finalize() calls are not well-paired).