Skip to content

Adding support for MPI serial library#696

Merged
jayeshkrishna merged 3 commits into
masterfrom
jayeshkrishna/avoid_comm_split_type_for_mpiserial
May 22, 2026
Merged

Adding support for MPI serial library#696
jayeshkrishna merged 3 commits into
masterfrom
jayeshkrishna/avoid_comm_split_type_for_mpiserial

Conversation

@jayeshkrishna

Copy link
Copy Markdown
Contributor

MPI serial library does not support the following functions,

  • MPI_Comm_split_type()
  • MPI_Type_dup()

The library also requires including the header as a
"C header file".

Since MPI_Comm_split_type() is not supported by the MPI serial
library switch to MPI_Comm_dup() when using the MPI serial library
@jayeshkrishna jayeshkrishna self-assigned this May 15, 2026
@jayeshkrishna jayeshkrishna added the Next Release Enhancements slated for the upcoming (next) release label May 15, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds compatibility for the MPI serial library by avoiding unsupported MPI
functions and ensuring MPI headers are included with C linkage when needed.

Changes:

  • Wrap/adjust MPI header inclusion to support MPI serial builds in C++.
  • Add MPI_SERIAL fallbacks for unsupported MPI calls (MPI_Comm_split_type,
    MPI_Type_dup) in core code and tests.
  • Remove redundant direct MPI include from a unit test in favor of pio.h.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
util/argparser.cxx Wraps mpi.h include with extern "C" under MPI_SERIAL and uses <mpi.h>.
tests/cunit/test_spio_rearr_utils_gather.cpp Uses MPI_Type_contiguous when MPI_Type_dup is unavailable in MPI serial.
tests/cunit/test_req_block_wait.cpp Drops direct mpi.h include (relies on pio.h).
src/clib/core/util/spio_decomp_logger.hpp Falls back to MPI_Comm_dup when MPI_Comm_split_type is unavailable in MPI serial.
src/clib/core/progress_engine/spio_async_tcomm.hpp Replaces direct MPI include with pio.h (which handles MPI serial include semantics).
src/clib/core/pioc.cpp Falls back to MPI_Comm_dup for node_comm when MPI_Comm_split_type is unavailable in MPI serial.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 225 to +229
ret = MPI_Type_dup(sendtype, &(recvtypes[i]));
#else
/* MPI serial does not support MPI_Type_dup() */
ret = MPI_Type_contiguous(LOCAL_SZ, MPI_DOUBLE, &(recvtypes[i]));
#endif

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed and rebased branch

Since MPI serial lib does not support MPI_Type_dup() avoid duping
the type in test
@jayeshkrishna jayeshkrishna force-pushed the jayeshkrishna/avoid_comm_split_type_for_mpiserial branch from c6658b9 to bbe81d2 Compare May 15, 2026 19:00
@jayeshkrishna jayeshkrishna requested a review from Copilot May 15, 2026 19:00

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Comment thread util/argparser.cxx Outdated
Comment on lines +1 to +9
#ifdef MPI_SERIAL
extern "C" {
#endif

#include <mpi.h>

#ifdef MPI_SERIAL
}
#endif

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed and rebased branch

Comment on lines +8 to +9
#include "pio_config.h"
#include "pio.h"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed and rebased branch

Including MPI header from the MPI serial library from a C++
source requires decorating the include with "extern C" blocks.

So either add these decorations explicitly or use the includes
in pio.h/argparser.h
@jayeshkrishna jayeshkrishna force-pushed the jayeshkrishna/avoid_comm_split_type_for_mpiserial branch from bbe81d2 to 6edbb07 Compare May 15, 2026 19:34
@jayeshkrishna jayeshkrishna requested a review from Copilot May 15, 2026 19:35

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Comment thread src/clib/core/pioc.cpp
Comment on lines +1550 to +1554
#ifndef MPI_SERIAL
mpierr = MPI_Comm_split_type(ios->comp_comm, MPI_COMM_TYPE_SHARED, 0,
ios->info, &(ios->node_comm));
#else
/* MPI serial library does not support MPI_Comm_split_type() */

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think we can skip adding MPI serial support for ADIOS (IMO, we are never going to need that)

Comment on lines 230 to 235
if(ret == MPI_SUCCESS){
ret = MPI_Type_commit(&(recvtypes[i]));
}
if(ret != MPI_SUCCESS){
LOG_RANK0(wrank, "ERROR: Unable to create MPI dup of send type to recv doubles\n");
LOG_RANK0(wrank, "ERROR: Unable to create recv type (same as send type) to recv doubles\n");
return PIO_EINTERNAL;

@jayeshkrishna jayeshkrishna May 15, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is probably a bigger change than what can fit in this PR (There are multiple cases in tests, across several tests, where we don't handle freeing of resources on fails). IMO, we can fix this (free resources when tests fail) in a later PR.

jayeshkrishna added a commit that referenced this pull request May 21, 2026
… develop (PR #696)

MPI serial library does not support the following functions,
* MPI_Comm_split_type()
* MPI_Type_dup()

The library also requires including the header as a
"C header file".

* jayeshkrishna/avoid_comm_split_type_for_mpiserial:
  Include MPI header compatible with MPI serial lib
  Avoid MPI_Type_dup() for mpi serial
  Skip MPI_Comm_split_type() for mpi serial
@jayeshkrishna jayeshkrishna merged commit cd33bb5 into master May 22, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Next Release Enhancements slated for the upcoming (next) release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants