-
Notifications
You must be signed in to change notification settings - Fork 565
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
Kokkos: remove TriBITs usage - DO NOT MERGE! #13423
base: develop
Are you sure you want to change the base?
Conversation
So far all the AT2 checks fail the PullRequestLinuxDriver.py check, with these changes |
@trilinos/framework @sebrowne @achauphan is there a way to view the |
@ndellingwood yes there is. Currently, you will have to sift through the PullRequestLinuxDriverTest.py section of the GitHub Actions log for the CDash URLs which are located near the bottom of that section (we're working on improving where CDash links appear). From that CDash URL, there will be the normal uploaded artifacts button by each build name. |
@achauphan thank you! |
FWIW a local build with the Threads bcakend works fine. |
Status Flag 'Pre-Test Inspection' - Auto Inspected - Inspection is Not Necessary for this Pull Request. |
Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects: Pull Request Auto Testing STARTING (click to expand)Build InformationTest Name: PR_gcc-openmpi-openmp
Jenkins Parameters
Build InformationTest Name: PR_gcc
Jenkins Parameters
Build InformationTest Name: PR_gcc-openmpi_debug
Jenkins Parameters
Build InformationTest Name: PR_clang
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_python3
Jenkins Parameters
Build InformationTest Name: PR_cuda
Jenkins Parameters
Build InformationTest Name: PR_intel
Jenkins Parameters
Build InformationTest Name: PR_cuda-uvm
Jenkins Parameters
Using Repos:
Pull Request Author: ndellingwood |
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: PR_gcc-openmpi-openmp
Jenkins Parameters
Build InformationTest Name: PR_gcc
Jenkins Parameters
Build InformationTest Name: PR_gcc-openmpi_debug
Jenkins Parameters
Build InformationTest Name: PR_clang
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_python3
Jenkins Parameters
Build InformationTest Name: PR_cuda
Jenkins Parameters
Build InformationTest Name: PR_intel
Jenkins Parameters
Build InformationTest Name: PR_cuda-uvm
Jenkins Parameters
|
@masterleinad I think the issue is that something is triggering the Threads backend to be enabled implicitly, which is causing configuration issues with other packages enabled in the build that do not expect Threads to be enabled |
Every job failed during configuration
Looking at the generatedPRFragment.cmake , Kokkos_ENABLE_THREADS is not explicitly set, something is implicitly triggering the backend to be enabled, my suspicion is |
These are missing the |
@dalg24 right. The Tacho config error is because it detected Kokkos_ENABLE_THREADS, which is being triggered in the other configurations as well (e.g. Cuda+Serial) I marked as WIP for now to prevent a rerun through the AT until more updates are made to the branch |
What does the configuration line look like? |
Is that for the same configuration? |
I can confirm the configuration failure for
that results in
which worked before (by not enabling |
TPL_ENABLE_Pthread=ON does not imply Kokkos_ENABLE_THREADS=ON but the latter need to be set explicitly. I changed that accordingly in the respective Kokkos pull request, see kokkos/kokkos@4a23d90.
|
@masterleinad here is a reproducer configuration for the errors related to the unit test naming mishap (missing Kokkos_ prefix):
If I drop |
Fixed in kokkos/kokkos@3dc8e02. Can you check that that resolves the issue for you as well? |
@masterleinad yes, the two fixes you pushed resolved the configuration issues I was seeing (the missing Kokkos_ prefix on some tests, and TPL_ENABLE_Pthread triggering Kokkos_ENABLE_THREADS). I'll update the snapshot or 6164 to this PR and relaunch tests so we can get updated status |
The AT2 builds are failing with e.g.
That is hitting a change I submitted in #13395 , after the reorg of the view-related headers into View in kokkos/kokkos#7256 The changes from kokkos/kokkos#7256 are not contained in kokkos/kokkos#6164 , which is bumping into the incompatibility, will need a merge of develop back into the PR before retest |
I merged upstream/develop into it now. |
7446834
to
deb7872
Compare
@masterleinad thanks! I updated the branch for retest |
@trilinos/framework @sebrowne the clang-format check has been failing with output
I think kokkos ran into something similar that was resolved by updating the |
It should be working the same in both cases. There are copies of I don't see an include directive (command-line) for any gtest header directories in the command for one of those failures though. It seems like that could cause the error. EDIT: Found a likely trail: This is in the AT2 build I checked: While the AT1 build shows: Seems to me like this could/should have been a configure-time error |
kokkos/kokkos#4563 might be related. It would be awesome to have a reproducer outside the testing framework so I could understand better what's going on. |
@ndellingwood Would you mind trying if forcing an internal gtest via diff --git a/CMakeLists.txt b/CMakeLists.txt
index 12e452c6a..1e8c42b48 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -183,7 +183,7 @@ IF(NOT MSVC)
ENDIF()
IF(Kokkos_ENABLE_TESTS)
- find_package(GTest QUIET)
+ #find_package(GTest QUIET)
ENDIF()
# Include a set of Kokkos-specific wrapper functions that fixes it? If so, I will find a way to do this when configuring with |
|
@masterleinad I pushed ade0414 to test that change |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello all, looking at this CDash query, it appears that all of the Kokkos tests got removed. Is that really want you want? So when someone installs Kokkos along with Trilinos, if there is some problem with Kokkos they will be forced to debug this through downstream Trilinos packages? That does not seem ideal.
@bartlettroscoe interesting, thanks for posting that. There is a discrepancy between the AT1 and AT2 handling of the tests (with the changes in this PR) that is unclear. I just retested with sha ade0414 , this was my cmake line
Then compile and the tests are present
Also, looking at one of the previous AT2 runs (I pushed a change ade0414 earlier that retriggered the tests and they are in progress), for example here https://github.com/trilinos/Trilinos/actions/runs/10797615169/job/29949260224 , the tests are present there and ran, here is a snip of the first test's result from the raw logs
This is strange... |
Status Flag 'Pre-Test Inspection' - Auto Inspected - Inspection is Not Necessary for this Pull Request. |
Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects: Pull Request Auto Testing STARTING (click to expand)Build InformationTest Name: PR_gcc-openmpi-openmp
Jenkins Parameters
Build InformationTest Name: PR_gcc
Jenkins Parameters
Build InformationTest Name: PR_gcc-openmpi_debug
Jenkins Parameters
Build InformationTest Name: PR_clang
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_python3
Jenkins Parameters
Build InformationTest Name: PR_cuda
Jenkins Parameters
Build InformationTest Name: PR_intel
Jenkins Parameters
Build InformationTest Name: PR_cuda-uvm
Jenkins Parameters
Using Repos:
Pull Request Author: ndellingwood |
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: PR_gcc-openmpi-openmp
Jenkins Parameters
Build InformationTest Name: PR_gcc
Jenkins Parameters
Build InformationTest Name: PR_gcc-openmpi_debug
Jenkins Parameters
Build InformationTest Name: PR_clang
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_python3
Jenkins Parameters
Build InformationTest Name: PR_cuda
Jenkins Parameters
Build InformationTest Name: PR_intel
Jenkins Parameters
Build InformationTest Name: PR_cuda-uvm
Jenkins Parameters
|
Yes, that's the direction I wanted to do in but I'm not sure if that would cause problems with |
@ndellingwood What do the failures for |
We should make sure to enable Kokkos tests when Trilinos wants to enable tests. |
... and that seems to work well AFAICT. |
@masterleinad the test results seem to be mismatched with the project, but this is the summary after commenting out the The
However, those errors show up on CDash listed under Gtest and Zoltan2 subprojects rather than kokkos The
Testing with Ross's cdash query still shows 0 tests run for Kokkos as a subproject, but the results seem to be showing up under other subprojects; for example @bartlettroscoe do you have any guidance on how to make sense of this misplacement of test results under different packages in the CDash results? |
I assume that this works then and we just have to figure out how the tests are reported to CDash. I pushed a commit that should disable external |
@ndellingwood, yes, you have to add the package name (i.e., ctest/cdash subproject name) Also note that
These features have been used for many years to allow users, developers (and devops teams) to deal with challenging tests, without having to touch CMakeLists.txt files (which is not possible to do with raw |
@bartlettroscoe thanks for the fast response, that is helpful. SET_RUN_SERIAL and DISABLED properties were added in the kokkos/kokkos#6164 PR and work as expected, but use of LABEL still needs to be addressed. @masterleinad can using the |
@trilinos/framework adding a reference here to a couple failures in cdash unrelated to changes in this PR, showing in the
|
@masterleinad, the minimal requirements for that are spelled out in: And the easiest way to do that is to just call Let me know if you have any questions about that.
@ndellingwood, okay, I see that logic in fake_tribits.cmake. But note that there is also FYI: To get my attention, please put in @bartlettroscoe so that I will be sure and see the comment. |
I added a corresponding commit to kokkos/kokkos#6164. |
@masterleinad I updated the snapshot to include your most recent changes for retest |
Status Flag 'Pre-Test Inspection' - Auto Inspected - Inspection is Not Necessary for this Pull Request. |
Status Flag 'Pull Request AutoTester' - GitHub reports Mergeable status = False |
…426d8a From repository at git@github.com:kokkos/kokkos.git At commit: commit 3907e607ca83e54909c29959a09e9a54fe426d8a Merge: a72909fdd 5e49ad1ad Author: Daniel Arndt <arndtd@ornl.gov> Date: Mon Sep 16 10:57:00 2024 -0400 Merge remote-tracking branch 'upstream/develop' into avoid_tribits_in_trilinos
04cecbb
to
8b6afb1
Compare
Status Flag 'Pre-Test Inspection' - Auto Inspected - Inspection is Not Necessary for this Pull Request. |
Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects: Pull Request Auto Testing STARTING (click to expand)Build InformationTest Name: PR_gcc-openmpi-openmp
Jenkins Parameters
Build InformationTest Name: PR_gcc
Jenkins Parameters
Build InformationTest Name: PR_gcc-openmpi_debug
Jenkins Parameters
Build InformationTest Name: PR_clang
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_python3
Jenkins Parameters
Build InformationTest Name: PR_cuda
Jenkins Parameters
Build InformationTest Name: PR_intel
Jenkins Parameters
Build InformationTest Name: PR_cuda-uvm
Jenkins Parameters
Using Repos:
Pull Request Author: ndellingwood |
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: PR_gcc-openmpi-openmp
Jenkins Parameters
Build InformationTest Name: PR_gcc
Jenkins Parameters
Build InformationTest Name: PR_gcc-openmpi_debug
Jenkins Parameters
Build InformationTest Name: PR_clang
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_python3
Jenkins Parameters
Build InformationTest Name: PR_cuda
Jenkins Parameters
Build InformationTest Name: PR_intel
Jenkins Parameters
Build InformationTest Name: PR_cuda-uvm
Jenkins Parameters
|
Snapshot of kokkos.git from commit 242859b5d0f2be41892ac0eb0f4bead8ae113beb
From repository at git@github.com:kokkos/kokkos.git
At commit:
commit 242859b5d0f2be41892ac0eb0f4bead8ae113beb
Merge: ca654abb5 3fddbfb6b
Author: Daniel Arndt arndtd@ornl.gov
Date: Wed Aug 28 15:24:43 2024 -0400
@trilinos/kokkos
Motivation
Early AT testing feedback of Kokkos with kokkos/kokkos#6164 , which removes much usage of TriBITS
Related Issues
put-issue-number-here
Stakeholder Feedback
@crtrott @dalg24 @masterleinad @lucbv @bartlettroscoe
Testing
AT results will provide feedback