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

Fix OpenMPI check in cross-compiling mode #1446

Merged
merged 2 commits into from
Oct 31, 2023
Merged

Fix OpenMPI check in cross-compiling mode #1446

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2745299
replaces try-run with try-compile in OpenMPI check
MarcelKoch Oct 31, 2023
f135d1e
review updates:
MarcelKoch Oct 31, 2023
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
14 changes: 11 additions & 3 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,14 +242,22 @@ if(GINKGO_BUILD_MPI)
set(GINKGO_HAVE_GPU_AWARE_MPI OFF)
endif()

try_run(uses_openmpi gko_result_unused
# use try_compile instead of try_run to prevent cross-compiling issues
try_compile(uses_openmpi
${Ginkgo_BINARY_DIR}
${Ginkgo_SOURCE_DIR}/cmake/openmpi_test.cpp
COMPILE_DEFINITIONS -DCHECK_HAS_OPEN_MPI=1
LINK_LIBRARIES MPI::MPI_CXX
RUN_OUTPUT_VARIABLE openmpi_version
OUTPUT_VARIABLE output
)
if(uses_openmpi)
if(openmpi_version VERSION_LESS "4.1")
try_compile(valid_openmpi_version
${Ginkgo_BINARY_DIR}
${Ginkgo_SOURCE_DIR}/cmake/openmpi_test.cpp
COMPILE_DEFINITIONS -DCHECK_OPEN_MPI_VERSION=1
LINK_LIBRARIES MPI::MPI_CXX
)
if(NOT valid_openmpi_version)
Copy link
Member

Choose a reason for hiding this comment

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

By the way, how's the bug in OpenMPI v3.*?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not checked and I don't care. We only support openMPI >= 4. All older releases are officially retired.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think the more interesting question is about openMPI 5.0.0, which was released a few days ago. But I guess I will look into that after the release.

Copy link
Member

Choose a reason for hiding this comment

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

I see.

Copy link
Member

Choose a reason for hiding this comment

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

does OpenMPI support MPI 3.1 after OpenMPI v4?
Because our documentation claim we support MPI3.1+

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, v5 still only supports MPI3.1 fully. But some functionality of MPI4 is supported in v5, so I want to check that out.

message(WARNING
"OpenMPI v4.0.x has a bug that forces us to use blocking communication in our distributed "
"matrix class. To enable faster, non-blocking communication, consider updating your OpenMPI version or "
Expand Down
12 changes: 7 additions & 5 deletions cmake/openmpi_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,13 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

int main()
{
#if defined(OPEN_MPI) && OPEN_MPI
std::printf("%d.%d.%d", OMPI_MAJOR_VERSION, OMPI_MINOR_VERSION,
OMPI_RELEASE_VERSION);
return 1;
#if CHECK_HAS_OPEN_MPI && defined(OPEN_MPI) && OPEN_MPI
static_assert(true);
#elif CHECK_OPEN_MPI_VERSION
static_assert(OMPI_MAJOR_VERSION > 4 ||
(OMPI_MAJOR_VERSION == 4 && OMPI_MINOR_VERSION >= 1));
#else
return 0;
static_assert(false);
#endif
return 1;
}