trilinos / Trilinos

Primary repository for the Trilinos Project
https://trilinos.org/
Other
1.22k stars 569 forks source link

STK: error compiling Trilinos is ArborX is visible #13555

Open aprokop opened 3 weeks ago

aprokop commented 3 weeks ago

@alanw0 @trilinos/stk

Compiling STK within Trilinos (latest develop) when ArborX is in the path leads to errors during configuration due to clashing Kokkos targets:

CMake Error at /Users/xap/local/opt/kokkos-4.4.0/lib/cmake/Kokkos/KokkosTargets.cmake:42 (message):
  Some (but not all) targets in this export set were already defined.

  Targets Defined: Kokkos::kokkoscore, Kokkos::kokkoscontainers,
  Kokkos::kokkosalgorithms, Kokkos::kokkossimd

  Targets not yet defined: Kokkos::LIBDL, Kokkos::kokkos

Call Stack (most recent call first):
  /Users/xap/local/opt/kokkos-4.4.0/lib/cmake/Kokkos/KokkosConfig.cmake:42 (INCLUDE)
  /opt/homebrew/Cellar/cmake/3.30.5/share/cmake/Modules/CMakeFindDependencyMacro.cmake:76 (find_package)
  /Users/xap/local/opt/arborx-mpi-devel-0975b50e/share/cmake/ArborX/ArborXConfig.cmake:35 (find_dependency)
  packages/stk/CMakeLists.txt:135 (find_package)

In stk_search/CMakeLists.txt, ArborX is linked only within the !HAVE_STK_TRILINOS clause.

Thus, I think similar thing should be done in the root CMake. So, I propose the following patch to STK:

--- a/packages/stk/CMakeLists.txt
+++ b/packages/stk/CMakeLists.txt
@@ -132,6 +132,7 @@ ELSE()
   ENDIF()
 ENDIF()

+IF (NOT HAVE_STK_Trilinos)
   find_package(ArborX QUIET)
   if(TARGET ArborX::ArborX)
     MESSAGE("Found ArborX, making it available within stk")
@@ -139,6 +140,7 @@ if(TARGET ArborX::ArborX)
   else()
     MESSAGE("Optional search library ArborX is not enabled.")
   endif()
+endif()

 if(NOT HAVE_STK_Trilinos)
   find_package(SEACAS)
aprokop commented 3 weeks ago

Alternatively, STK could use ArborX unconditionally whenever it is available. However, that would require ArborX and Trilinos to be built against the same external Kokkos. This is likely to be more involved.

alanw0 commented 3 weeks ago

Hi @aprokop , thanks for reporting this. We simply haven't yet put effort into how ArborX is enabled. In our 'sierra' repository, where stk is developed, ArborX is always on/enabled. In our cmake builds, with and without trilinos, we don't have great support yet but our intention is to enable users to link in ArborX optionally.

If someone is building stk within trilinos, I don't know how to handle the issue of different versions of Kokkos.

In general, we should probably try to detect whether ArborX is available (with find_package?) and if it is then use it. Perhaps your suggested patch above is a good first step, and then we will proceed to enable using ArborX whenever it is available.

aprokop commented 3 weeks ago

Perhaps your suggested patch above is a good first step, and then we will proceed to enable using ArborX whenever it is available.

Right now, STK within Trilinos does not build if ArborX is detected. As you don't use ArborX when building within Trilinos, this just patches the current hole.

If someone is building stk within trilinos, I don't know how to handle the issue of different versions of Kokkos.

I don't have a good idea. I think it may become common when Trilinos is built against an external Kokkos, and some of the Trilinos packages have dependencies that also depend on Kokkos. It may be just that it's user's responsibility at the moment to ensure the same version. Like, Kokkos and Kokkos-Kernels.

What I don't know how to handle at all is when building STK with Kokkos snapshotted in Trilinos, while also using ArborX.

alanw0 commented 3 weeks ago

ok thanks @aprokop . I'm applying your suggested patch to stk in sierra, it will come to trilinos/develop as soon as we can get another snapshot together.

alanw0 commented 2 weeks ago

@aprokop the stk snapshot PR got held up a little bit for unrelated testing issues, but I've got it trying again now.

I ended up modifying your proposed cmake patch to be if (STK_ENABLE_ARBORX OR (NOT HAVE_STK_Trilinos)) so that even though ArborX usage is off by default for a stk-in-trilinos build, it allows a user to turn it on if they want to, by adding -DSTK_ENABLE_ARBORX to the cmake line. Then of course it is on them to try to resolve kokkos consistency etc. But that way we aren't dis-allowing it.