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.

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

TriBits: using find_package(MPI) when MPI_USE_COMPILER_WRAPPERS is set to OFF #13839

Open
maartenarnst opened this issue Feb 26, 2025 · 2 comments

Comments

@maartenarnst
Copy link
Contributor

@trilinos/tribits
@trilinos/framework
@bartlettroscoe

We are exploring how to compile Trilinos with compilers that are not MPI compiler wrappers (option MPI_USE_COMPILER_WRAPPERS set to OFF).

A use case is compiling Trilinos as part of building a container image when the MPI inside that container image is GPU aware. We found that the Nvidia container runtime does not make certain libraries available during the build process, and this may cause problems with the OpenMPI compiler wrappers when they are linked with a GPU-aware hwloc.

In particular, we are exploring how to let Trilinos find MPI through find_package. Currently, to make this work, we modified cmake/tribits/core/std_tpls/FindTPLMPI.cmake by replacing tribits_tpl_find_include_dirs_and_libraries(MPI) with:

tribits_tpl_allow_pre_find_package(MPI MPI_ALLOW_PREFIND)
if(MPI_ALLOW_PREFIND)

  find_package(MPI)
  
  if(MPI_C_FOUND AND MPI_CXX_FOUND)
    tribits_extpkg_create_imported_all_libs_target_and_config_file(
      MPI
      INNER_FIND_PACKAGE_NAME MPI
      IMPORTED_TARGETS_FOR_ALL_LIBS MPI::MPI_C MPI::MPI_CXX)
  endif()
endif()

if(NOT TARGET MPI::all_libs)
  tribits_tpl_find_include_dirs_and_libraries(MPI)
endif()

@bartlettroscoe, would you have a moment to take a look? Is this a reasonable way of doing it? Or do you think of other functions from tribits that we should use? If such a change would be of interest to you, we would be happy to make a PR too. Thanks in advance!

Joint work with @romintomasetti.

@bartlettroscoe
Copy link
Member

@maartenarnst, @romintomasetti,

NOTE: Switching to using find_package(MPI) is one of the possible stories in the continuing TriBITS/Trilinos CMake modernization effort listed in:

So this change has been considered for a long time and is on the backlog of potential changes for TriBITS and Trilinos in this general CMake modernization effort.

@bartlettroscoe, would you have a moment to take a look? Is this a reasonable way of doing it? Or do you think of other functions from tribits that we should use? If such a change would be of interest to you, we would be happy to make a PR too. Thanks in advance!

The general approach looks reasonable (and using tribits_tpl_allow_pre_find_package() how you support a new way to find external TPLs without breaking backward compatibility for existing users and developers).

However, note that find_package(MPI) does more than just define the compilers, MPI include dirs, and MPI libraries. It also specifies show mpiexec (or whatever it is called) and defines various variables for how it gets called. You can see this by comparing:

and

So that may have to be addressed as well.

A variable missing from FindMPI.cmake is MPI_EXEC_PRE_NUMPROCS_FLAGS (which would be called something like MPIEXEC_PRE_NUMPROCS_FLAGS). That variable was added to TriBITS years ago because there are (or were) systems where some <mpiexec> flags have to be passed before the <numprocs> flag. Also, FindMPI.cmake defines MPIEXEC_POSTFLAGS which is not defined by TriBITS. So there are some incompatibilities there that will need to be addressed and will likely break backwards compatibility for some users.

@ccober6 and @sebrowne,

Where would providing support and/or switching to find_package(MPI) rank in priorities for Trilinos?

@maartenarnst, @romintomasetti,

But with the approach shown above using tribits_tpl_allow_pre_find_package(), this would not break backward compatibility for existing users and developers. So if this works for your uses cases, I don't see the harm in updating the FindTPLMPI.cmake module like shown above. And developers and users could start to experiment with using find_package(MPI) on their systems.

So I would say, go ahead and post a Trilinos PR for this change if you can get this to work for your use cases. That is my opinion.

@maartenarnst
Copy link
Contributor Author

@bartlettroscoe, thanks a lot for the detailed feedback! I have just made the PR:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants