Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use MPI_Init_thread if OpenMP is enabled #306

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Use MPI_Init_thread if OpenMP is enabled #306

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions src/MarDyn.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,20 @@ int run_unit_tests(const Values &options, const std::vector<std::string> &args)
*/
int main(int argc, char** argv) {
#ifdef ENABLE_MPI
MPI_Init(&argc, &argv);
#endif
#if defined(_OPENMP)
const int thread_support_level_requested = MPI_THREAD_FUNNELED; /* So far only the main thread makes MPI calls */
HomesGH marked this conversation as resolved.
Show resolved Hide resolved
#else
const int thread_support_level_requested = MPI_THREAD_SINGLE; /* No thread support required */
#endif /* _OPENMP */
cniethammer marked this conversation as resolved.
Show resolved Hide resolved
int thread_support_level_provided;
MPI_Init_thread(&argc, &argv, thread_support_level_requested, &thread_support_level_provided);
if (thread_support_level_provided < thread_support_level_requested) {
/* The logging infrastructure is not initalized yet, therfore perform manual output and exit. */
std::cerr << "ERROR: ls1 was build with OpenMP. However, the MPI implementation does not provide the necessary thread support level." << std::endl;
MPI_Abort(MPI_COMM_WORLD, EXIT_FAILURE);
::exit(EXIT_FAILURE);
Comment on lines +144 to +146
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
std::cerr << "ERROR: ls1 was build with OpenMP. However, the MPI implementation does not provide the necessary thread support level." << std::endl;
MPI_Abort(MPI_COMM_WORLD, EXIT_FAILURE);
::exit(EXIT_FAILURE);
MARDYN_EXIT("ERROR: ls1 was build with OpenMP. However, the MPI implementation does not provide the necessary thread support level.");

Please use the exit macro for consistent exit behavior.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We cannot use the mardyn_exit macro as it relies on the logger infrastructure, which is not initialized at this point in the program (and may also dependson the MPI part). However, IMHO, we are so early in the program, that we do not need the extensive logger.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

However, IMHO, we are so early in the program, that we do not need the extensive logger.

I respectfully disagree, especially when looking at your related PR #358. Adding exceptions to a pattern like this makes the code more confusing, prone to set bad precedent for new developers, and needs clear documentation.
I'll add a suggestion on how to tackle the issue in the mentioned PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, then I'm looking forward to your suggestion to address the Logger issues in a cleaner way

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-> here you go :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the inspiration!
I will update this PR when #359 is merged.

}
#endif /* ENABLE_MPI */

/* Initialize the global log file */
Log::global_log = new Log::Logger(Log::Info);
Expand Down
Loading