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

Avoid using TriBITS in Kokkos #6164

Draft
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

masterleinad
Copy link
Contributor

This corresponds to trilinos/Trilinos#11779.

Currently, we are essentially using three build systems in Kokkos:

  • Makefile-based (only supported for using Kokkos inline, not standalone)
  • raw CMake
  • through Trilinos using TriBITS

We discussed that we want to try to lighten that burden by removing a full TriBITS option by only providing necessary additions to the raw CMake build system. This pull request explores doing this.

The approach taken is to simply delete all paths that mention KOKKOS_HAS_TRILINOS or TRIBITS directly which seems to work pretty well. The places where we still need to do something special (apart from adding additional aliases or making sure we can link against a Kokkos build directory) are:

  • Calling TRIBITS_PACKAGE(Kokkos) at the top of CMakeLists.txt
  • Dealing with Trilinos options in the new kokkos_configure_trilinos.cmake
  • Calling tribits_package_postprocess in kokkos_configure_trilinos.cmake
  • Dealing with Trilinos TPL's Kokkos provides/uses.

Note that we also remove all the special casing for running Kokkos tests in Trilinos.

@masterleinad
Copy link
Contributor Author

@bartlettroscoe I revived this after you asked for feedback wrt TriBITSPub/TriBITS#591 (comment).
It seems we neither need to call

  • TRIBITS_PACKAGE(Kokkos) at the top of CMakeLists.txt, nor
  • tribits_package_postprocess in kokkos_configure_trilinos.cmake

That leaves us basically with additional code for

  • dealing with Trilinos options in the new kokkos_configure_trilinos.cmake, and
  • dealing with Trilinos TPL's Kokkos provides/uses.

Also, Trilinos seems to have a problem with us exporting KokkosTargets.cmake twice, once to CMAKE_BINARY_DIR}/cmake_packages/Kokkos/KokkosTargets.cmake, and once to export(EXPORT KokkosTargets NAMESPACE Kokkos:: FILE ${Kokkos_BINARY_DIR}/KokkosTargets.cmake) (which is a more natural choice for supporting linking to the build tree; at least in a single repository build). It seems that including the former in the latter provides a workaround.

Apart from that, everything seems to be working here as described in the files you linked.

@bartlettroscoe
Copy link
Contributor

@masterleinad, one thing you might consider is to keep using the TriBITS function tribits_add_test() under your kokkos_add_test() function when building Kokkos under Trilinos. You can now use those functions apart from the rest of TriBITS as described in the new Howto section:

But in the case of building Kokkos under Trilinos, you don't need to include the module TribitsAddTest.cmake (the TriBITS framework already includes that before processing the package CMakeLists.txt files) or define the variable Kokkos_ENABLE_TESTS (TriBITS defines and sets that automatically). You just need to call the function tribits_add_test().

Using tribits_add_test() will allow the Trilinos framework team and other developers/users to continue to surgically disable Kokkos tests on particular configurations and platforms without having to touch anything under packages/kokkos/. Otherwise, it would not be very easy to maintain running the Kokkos test suite on all of the different Trilinos build configurations on all of the different platforms.

Just something to think about.

@masterleinad
Copy link
Contributor Author

Sure, we should discuss with the rest of the Kokkos team if we want to keep tribits_add_test.

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

Successfully merging this pull request may close these issues.

None yet

2 participants