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

STK: error compiling Trilinos is ArborX is visible #13555

Open
aprokop opened this issue Oct 29, 2024 · 5 comments
Open

STK: error compiling Trilinos is ArborX is visible #13555

aprokop opened this issue Oct 29, 2024 · 5 comments
Labels
pkg: STK type: bug The primary issue is a bug in Trilinos code or tests

Comments

@aprokop
Copy link
Contributor

aprokop commented Oct 29, 2024

@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 aprokop added pkg: STK type: bug The primary issue is a bug in Trilinos code or tests labels Oct 29, 2024
@aprokop
Copy link
Contributor Author

aprokop commented Oct 29, 2024

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
Copy link
Contributor

alanw0 commented Oct 29, 2024

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
Copy link
Contributor Author

aprokop commented Oct 29, 2024

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
Copy link
Contributor

alanw0 commented Oct 30, 2024

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
Copy link
Contributor

alanw0 commented Nov 8, 2024

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: STK type: bug The primary issue is a bug in Trilinos code or tests
Projects
None yet
Development

No branches or pull requests

2 participants