Fix 1571 (CMake variable & detection of MPI CUDA awareness) #1602
Conversation
@trilinos/tpetra If building with CUDA, explicitly enforce that CUDA_VERSION be at least 7.5. See #1278 for details. Also, state CUDA_VERSION requirement explicitly in Tpetra's release notes (packages/tpetra/ReleaseNotes.txt).
@trilinos/tpetra Add a CMake option Tpetra_ASSUME_CUDA_AWARE_MPI, with associated macro TPETRA_ASSUME_CUDA_AWARE_MPI defined in TpetraCore_config.h. If the CMake option is ON, Tpetra may assume that the MPI implementation it uses is CUDA aware. See #1571 for discussion, and #1088 for an application. The option currently defaults to OFF. #1571 requires that we actually have a test for this option, at least for OpenMPI, so we haven't finished the issue yet.
@trilinos/tpetra Tpetra's CMake logic now attempts to detect whether
the MPI implementation is CUDA aware. If automatic detection does not
succeed, Tpetra just makes the safe assumption that MPI is not CUDA
aware.
Currently, automatic detection requires OpenMPI. If not using
OpenMPI, Tpetra conservatively assumes lack of CUDA awareness. It
would be wise for us to extend detection to support other MPI
implementations, but for now, this covers a common use case for
Trilinos testing.
Automatic detection depends on running an executable. This is
relevant for cross compilation, so I have added two measures to
protect against misleading results in that case:
1. If CMAKE_CROSSCOMPILING is ON, Tpetra skips detection and prints
a configure-time message telling users that they may set
Tpetra_ASSUME_CUDA_AWARE_MPI explicitly.
2. If users set Tpetra_ASSUME_CUDA_AWARE_MPI explicitly, Tpetra
skips detection and assumes the user's value as the default.
mhoemmen
added
Framework tasks
mhoemmen
added
system: gpu
stage: in review
packages/tpetra/CMakeLists.txt
Outdated
| # The user set Tpetra_ASSUME_CUDA_AWARE_MPI explicitly. | ||
| # Just use that value as the default. | ||
|
|
||
| IF (Tpetra_ASSUME_CUDA_AWARE_MPI) |
There was a problem hiding this comment.
This seems a bit backwards... if the user explicitly set the value, why does this go back up and set the default?
There was a problem hiding this comment.
Before we set up the option, either the variable is undefined, or users set it explicitly. I think you're right, though, that it's not necessary to set a default value if the user has already set it. I would like some status messages regardless, though.
packages/tpetra/CMakeLists.txt
Outdated
| # The user did not set Tpetra_ASSUME_CUDA_AWARE_MPI explicitly. | ||
|
|
||
| # This is annoying, but necessary, because expressions like "DEFINED | ||
| # X AND (NOT X)" may not be valid CMake syntax. |
There was a problem hiding this comment.
What is the evidence that such statements may not be valid syntax?
There was a problem hiding this comment.
@ibaned Past experience ;-) I can try it, though!
There was a problem hiding this comment.
@ibaned I tried changing this to a one-level IF statement, as follows:
SET (Tpetra_CROSSCOMPILING OFF)
IF (DEFINED CMAKE_CROSSCOMPILING AND CMAKE_CROSSCOMPILING)
SET (Tpetra_CROSSCOMPILING ON)
ENDIF ()
This worked when I ran CMake with CMAKE_CROSSCOMPILING=OFF set explicitly (I checked that CMake took that branch, by printing out a status message). With CMAKE_CROSSCOMPILING not explicitly set, CMake correctly deduced that I was not cross compiling, and everything worked as expected. Thanks for making me test this :-) .
packages/tpetra/CMakeLists.txt
Outdated
| # This only makes sense if MPI_USE_COMPILER_WRAPPERS is ON. | ||
|
|
||
| IF (DEFINED MPI_USE_COMPILER_WRAPPERS) | ||
| IF (MPI_USE_COMPILER_WRAPPERS AND DEFINED MPI_CXX_COMPILER AND (NOT Tpetra_FOUND_OMPI_INFO_EXECUTABLE)) |
There was a problem hiding this comment.
how could Tpetra_FOUND_OMPI_INFO_EXECUTABLE be true at this point?
There was a problem hiding this comment.
It can't be, but I'm being explicit just to imitate the pattern of the cases below. I should probably break out each check into a CMake function, but I'm not sure if it would save many lines of code.
packages/tpetra/CMakeLists.txt
Outdated
| MESSAGE (STATUS " - While I found the \"ompi_info\" executable, it would not run. Thus, I will make the sane assumption that your MPI implementation is NOT CUDA aware.") | ||
| ENDIF () | ||
| ELSE () | ||
| MESSAGE (STATUS " - Tpetra did not find the \"ompi_info\" executable. This may not be bad; for example, if your MPI implementation is not OpenMPI, then you won't have this executable. Tpetra will conservatively assume that your MPI implementation is NOT CUDA aware. If you would like to change this, please set this CMake variable Tpetra_ASSUME_CUDA_AWARE_MPI:BOOL=ON explicitly at configure time.") |
There was a problem hiding this comment.
"please set this" -> "please set the"
There was a problem hiding this comment.
Thanks! I'll fix that.
packages/tpetra/CMakeLists.txt
Outdated
| MESSAGE (STATUS " - Tpetra did not find the \"ompi_info\" executable. This may not be bad; for example, if your MPI implementation is not OpenMPI, then you won't have this executable. Tpetra will conservatively assume that your MPI implementation is NOT CUDA aware. If you would like to change this, please set this CMake variable Tpetra_ASSUME_CUDA_AWARE_MPI:BOOL=ON explicitly at configure time.") | ||
| ENDIF () | ||
| ENDIF () | ||
| ENDIF () |
There was a problem hiding this comment.
Could you give the arguments to these ENDIFs? I believe the outermost if statement is quite long
There was a problem hiding this comment.
Good point -- I'll add those, or at least some comments.
packages/tpetra/CMakeLists.txt
Outdated
| TRIBITS_ADD_OPTION_AND_DEFINE( | ||
| Tpetra_ASSUME_CUDA_AWARE_MPI | ||
| TPETRA_ASSUME_CUDA_AWARE_MPI | ||
| "Assume that the MPI implementation that Tpetra uses is CUDA aware. \"CUDA aware\" means that MPI can use CUDA device allocations (the result of cudaMalloc) as send and receive buffers. If MPI is CUDA aware, then Tpetra can give CUDA device allocations directly to MPI sends and receives. MPI implementations might not support all MPI communication operations. For example, different versions of OpenMPI support different subsets of MPI operations (see https://www.open-mpi.org/faq/?category=runcuda ). Tpetra developers may safely assume that MPI_*[Ss]end and MPI*_[Rr]ecv work. Anything more than that should be considered specific to the MPI implementation." |
There was a problem hiding this comment.
Thanks for the link to supported APIs... is the * in the right place in MPI*_[Rr]ecv ?
There was a problem hiding this comment.
Actually not -- it should be MPI_*[Rr]ecv. Thanks!
@trilinos/tpetra Tpetra's CMake logic now attempts to detect whether
the MPI implementation is CUDA aware. If automatic detection does not
succeed, Tpetra just makes the safe assumption that MPI is not CUDA
aware.
Currently, automatic detection requires OpenMPI. If not using
OpenMPI, Tpetra conservatively assumes lack of CUDA awareness. It
would be wise for us to extend detection to support other MPI
implementations, but for now, this covers a common use case for
Trilinos testing.
Automatic detection depends on running an executable. This is
relevant for cross compilation, so I have added two measures to
protect against misleading results in that case:
1. If CMAKE_CROSSCOMPILING is ON, Tpetra skips detection and prints
a configure-time message telling users that they may set
Tpetra_ASSUME_CUDA_AWARE_MPI explicitly if they want Tpetra to
assume that MPI is CUDA aware.
2. If users set Tpetra_ASSUME_CUDA_AWARE_MPI explicitly, Tpetra
skips detection and assumes the user's value as the default.
Thanks to @ibaned for the review and suggestions! I incorporated
these into the original pull request trilinos#1602.
|
I changed the commit using @ibaned 's kind suggestions, and let the remote check-in test script test and push my changes. Before that, I tested with CUDA (MPI both CUDA aware and not CUDA aware) and non-CUDA builds. Thanks! |
mhoemmen
removed
the
stage: in review
@trilinos/tpetra Tpetra's CMake logic now attempts to detect whether
the MPI implementation is CUDA aware. If automatic detection does not
succeed, Tpetra just makes the safe assumption that MPI is not CUDA
aware.
Currently, automatic detection requires OpenMPI. If not using
OpenMPI, Tpetra conservatively assumes lack of CUDA awareness. It
would be wise for us to extend detection to support other MPI
implementations, but for now, this covers a common use case for
Trilinos testing.
Automatic detection depends on running an executable. This is
relevant for cross compilation, so I have added two measures to
protect against misleading results in that case:
1. If CMAKE_CROSSCOMPILING is ON, Tpetra skips detection and prints
a configure-time message telling users that they may set
Tpetra_ASSUME_CUDA_AWARE_MPI explicitly if they want Tpetra to
assume that MPI is CUDA aware.
2. If users set Tpetra_ASSUME_CUDA_AWARE_MPI explicitly, Tpetra
skips detection and assumes the user's value as the default.
Thanks to @ibaned for the review and suggestions! I incorporated
these into the original pull request trilinos#1602.
|
Thanks @bartlettroscoe ! :-D I was careful to have a fall-back, with useful messages, if users are cross-compiling. The environment variable can always override configure-time settings in that case. |
@trilinos/tpetra Hi all! I wrote a PR that does the following:
Tpetra_ASSUME_CUDA_AWARE_MPI, for whether Tpetra may assume that the MPI implementation is CUDA aware.CUDA_VERSION>= 7.5.I took particular care to avoid breaking the cross-compilation case. The PR explicitly checks
CMAKE_CROSSCOMPILING; ifON, Tpetra does not attempt to run executables. This is relevant because I know users who do cross-compilation with CUDA builds right now.I welcome feedback! My only concern is that the PR forces Tpetra to decide whether MPI is CUDA aware at configure time. Some MPI implementations, like MVAPICH (see http://mvapich.cse.ohio-state.edu/userguide/gdr/2.2/ ), let users control this at run time, by setting an environment variable. This means that Tpetra may also need run-time environment variable control. However, we can always add that feature later. The CMake option is still useful, because it could determine the environment variable's default value. Thus, I think the PR is fine as it stands.