Kokkos: Avoid using TriBITS in Kokkos #11779
Conversation
|
Status Flag 'Pre-Test Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging |
bartlettroscoe
added
the
AT: WIP
|
@masterleinad, what about defining and running tests in a Trilinos build with internal Kokkos? Would that be supported? I will submit a sister PR to remove the usage of subpackages from the Kokkos TriBITS build and the rest of the Trilinos packages at the end of this week. |
We can discuss if it's acceptable to temporarily disable all |
|
Agreed I think we start with just not having tests for Kokkos itself enabled in Trilinos and then go from there. |
From what I have seen a part of #11808 and kokkos/kokkos#6104 I don't see why you would have to disable Kokkos tests with the native Kokkos CMake build system when building Kokkos under Trilinos. It already responds to the options The more challenging part will be generating a KokkosConfig.cmake file for the build dir and the install dir and such issues. (And that has to be done or we break Trilinos for customers currently getting Trilinos from |
See kokkos/kokkos#6078. |
masterleinad
force-pushed
the
avoid_tribits_kokkos
branch
from
a0168b1 to
b8c45ed
Compare
| @@ -867,7 +867,7 @@ macro(tribits_package_postprocess) | |||
| tribits_package_finalize_dependency_vars() | |||
| tribits_package_postprocess_common() | |||
|
|
|||
| if (${PACKAGE_NAME}_SOURCE_DIR STREQUAL ${PROJECT_NAME}_SOURCE_DIR) | |||
| if (${PACKAGE_NAME}_SOURCE_DIR STREQUAL ${CMAKE_PROJECT_NAME}_SOURCE_DIR) | |||
There was a problem hiding this comment.
@bartlettroscoe What do you think about this change? This allows using project in Kokkos.
There was a problem hiding this comment.
Yes, the change from PROJECT_NAME to CMAKE_PROJECT_NAME needs to be made throughout TriBITS to address this issue and allow calls to project() from individual packages.
In addition, TriBITS will need to change ${PROJECT_SOURCE_DIR} and ${PROJECT_BINARY_DIR} to ${${CMAKE_PROJECT_NAME}_PROJECT_SOURCE_DIR} and ${{${CMAKE_PROJECT_NAME}_PROJECT_BINARY_DIR}, respectively, almost everywhere.
Would you mind creating a TriBITS GitHUb issue for this and point out the places where you had to make this change?
There was a problem hiding this comment.
| @@ -2177,7 +2177,7 @@ macro(tribits_configure_enabled_packages) | |||
| endif() | |||
| if (NOT ${PACKAGE_NAME}_TRIBITS_PACKAGE_POSTPROCESS) | |||
| tribits_report_invalid_tribits_usage( | |||
There was a problem hiding this comment.
I think I will be able to adjust the logic to avoid this check if the target ${PACKAGE_NAME}::all_libs is already defined.
I will be creating a PR that will duplicate this Kokkos use case (i.e. using a raw CMake build system for a TriBITS-compliant internal package that does not call any TrBITS functions). This is one of the issues I will hit with that.
There was a problem hiding this comment.
Yeah, that would be good. I think we would rather define the target then calling tribits_package_postprocess().
|
CC: @jwillenbring, @sebrowne, @rppawlo, @ccober6, @jhux2 FYI: If the native Kokkos CMake build system does not use So perhaps it is best to just disable Kokkos tests when building under Trilinos for the foreseeable future? |
masterleinad
force-pushed
the
avoid_tribits_kokkos
branch
from
2f4b430 to
0379a15
Compare
masterleinad
force-pushed
the
avoid_tribits_kokkos
branch
from
0379a15 to
0a7d48b
Compare
Yes, let me hardcode that. |
bartlettroscoe
added
AT: PRE-TEST INSPECTED
Not being able to run Kokkos tests when someone is building Trilinos on a new platform is a bit sketchy. Perhaps just disable the native Kokkos tests by default for all Trilinos builds but allow the user to enable them if they would like? Issues like #11940 (comment) are mostly a problem for automated testing that can't tolerate random failures.
I removed the label I will work on the TriBITS test and changes for this use case. |
trilinos-autotester
removed
the
AT: PRE-TEST INSPECTED
|
Status Flag 'Pre-Test Inspection' - SUCCESS: The last commit to this Pull Request has been INSPECTED by label AT: PRE-TEST INSPECTED! Autotester is Removing Label; This inspection will remain valid until a new commit to source branch is performed. |
|
Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects: Pull Request Auto Testing STARTING (click to expand)Build InformationTest Name: Trilinos_PR_gcc-8.3.0
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_gcc-8.3.0-serial
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_gcc-8.3.0-debug
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_clang-11.0.1
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_python3
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_cuda-11.4.2-uvm-off
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_intel-2021.3
Jenkins Parameters
Using Repos:
Pull Request Author: masterleinad |
|
Status Flag 'Pull Request AutoTester' - Jenkins Testing: 1 or more Jobs FAILED Note: Testing will normally be attempted again in approx. 2 Hrs 30 Mins. If a change to the PR source branch occurs, the testing will be attempted again on next available autotester run. Pull Request Auto Testing has FAILED (click to expand)Build InformationTest Name: Trilinos_PR_gcc-8.3.0
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_gcc-8.3.0-serial
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_gcc-8.3.0-debug
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_clang-11.0.1
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_python3
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_cuda-11.4.2-uvm-off
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_intel-2021.3
Jenkins Parameters
|
|
@masterleinad, since I suspect you can't see the PR testing results linked to on CDash above, all of the builds failed with the configure error: |
|
@bartlettroscoe Hmm... Can you give me an example configuration? Locally I can configure with ShyLU support and OpenMP |
@masterleinad, it can be hard to reproduce these PR configurations if you are not on an SNL machine. See: Any chance you have access to the SNL SRN as part of your Kokkos collaboration work? Otherwise, I will give this a try once I have a TriBITS example and test set up. |
|
@bartlettroscoe So it looks like the configuration does which should imply that the default for Kokkos_ENABLE_THREADS should be the same but this is not actually the place where the default is set but in
and we are missing something like
diff --git a/packages/kokkos/cmake/kokkos_enable_devices.cmake b/packages/kokkos/cmake/kokkos_enable_devices.cmake
index d4a7744eb59..0f7bad1e999 100644
--- a/packages/kokkos/cmake/kokkos_enable_devices.cmake
+++ b/packages/kokkos/cmake/kokkos_enable_devices.cmake
@@ -18,8 +18,11 @@ KOKKOS_CFG_DEPENDS(DEVICES NONE)
# Put a check in just in case people are using this option
KOKKOS_DEPRECATED_LIST(DEVICES ENABLE)
-
+IF(Trilinos_ENABLE_Kokkos AND TPL_ENABLE_Pthread)
+KOKKOS_DEVICE_OPTION(THREADS ON HOST "Whether to build C++ threads backend")
+ELSE()
KOKKOS_DEVICE_OPTION(THREADS OFF HOST "Whether to build C++ threads backend")
+ENDIF()with current |
|
This means the |
|
@masterleinad, technically speaking, the value of SIDENOTE: That reminds me, we need to update that in the Kokkos CMake function Trilinos/packages/kokkos/cmake/kokkos_tpls.cmake Lines 14 to 17 in c3b1a94 to change NOTE: The name We will work out these issues. |
|
Status Flag 'Pre-Test Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging |
|
Status Flag 'Pre-Test Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging |
|
Can this be closed in favor of #13423 ? |
Sure. |
@trilinos/kokkos @bartlettroscoe
Motivation
Currently, we are essentially using three build systems in Kokkos:
We discussed that we want to try to lighten that burden by removing a full TriBITS option by only providing only necessary additions to the raw CMake build system. This pull request explores doing this.
Related Issues
Testing
Compiling Trilinos locally
Additional Information
This is not fully working yet. I can compileTrilinosbut we don't exportKokkosin a way usable for exporting other libraries depending onKokkos.