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

Multiple packages: Erroneous use of Kokkos_ENABLE_OpenMP, Kokkos_ENABLE_Pthread, Kokkos_ENABLE_Serial, Kokkos_ENABLE_Cuda* #11930

Closed
ndellingwood opened this issue May 31, 2023 · 11 comments
Labels
CLOSED_DUE_TO_INACTIVITY Issue or PR has been closed by the GitHub Actions bot due to inactivity. MARKED_FOR_CLOSURE Issue or PR is marked for auto-closure by the GitHub Actions bot. type: bug The primary issue is a bug in Trilinos code or tests

Comments

@ndellingwood
Copy link
Contributor

ndellingwood commented May 31, 2023

Bug Report

This issue is to report and coordinate updates to Trilinos packages of erroneous use of CMake options Kokkos_ENABLE_OpenMP, Kokkos_ENABLE_Pthread, Kokkos_ENABLE_Serial, Kokkos_ENABLE_Cuda*

The usages above were deprecated pre-3.0 release and should be updated as:
Kokkos_ENABLE_Serial -> Kokkos_ENABLE_SERIAL
Kokkos_ENABLE_OpenMP -> Kokkos_ENABLE_OPENMP
Kokkos_ENABLE_Pthread -> Kokkos_ENABLE_THREADS
Kokkos_ENABLE_Cuda* -> Kokkos_ENABLE_CUDA*, where all appended options to following CUDA should be capitalized as well.

It looks like the occurrences of these options in Trilinos/packages (via git grep) are in build scripts (old?) and documentation

Kokkos_ENABLE_Serial
RELEASE_NOTES: Kokkos_ENABLE_Serial is ON), Tpetra uses Serial by default.
RELEASE_NOTES: - Tpetra_INST_SERIAL (Kokkos_ENABLE_Serial must be ON)
panzer/maintenance/build_panzer_gcc_mac_s1002730.sh: -D Kokkos_ENABLE_Serial:BOOL=ON \

@trilinos/panzer

Kokkos_ENABLE_OpenMP
kokkos-kernels/BUILD.md: * Whether to pre instantiate kernels for the execution space Kokkos::OpenMP. Disabling this when Kokkos_ENABLE_OpenMP is enabled may increase build times.
kokkos-kernels/cmake/kokkoskernels_eti_devices.cmake: "Whether to pre instantiate kernels for the execution space Kokkos::Experimental::OpenMPTarget. Disabling this when Kokkos_ENABLE_OpenMPTarget is enabled may increase build times. Default: ON if Kokkos is OpenMPTarget-enabled, OFF otherwise."
kokkos-kernels/cmake/kokkoskernels_eti_devices.cmake: "Whether to pre instantiate kernels for the execution space Kokkos::OpenMP. Disabling this when Kokkos_ENABLE_OpenMP is enabled may increase build times. Default: ON if Kokkos is OpenMP-enabled, OFF otherwise."

@trilinos/kokkos-kernels

Kokkos_ENABLE_Pthread
phalanx/build_scripts/build_phalanx_amt_gcc.sh:-D Kokkos_ENABLE_Pthread:BOOL=ON
phalanx/build_scripts/build_phalanx_amt_gcc_gge.sh:-D Kokkos_ENABLE_Pthread:BOOL=ON
tempus/test_scripts/morgan/configure-tempus-morgan.sh:-D Kokkos_ENABLE_Pthread:BOOL=${USE_PTHREADS}
tempus/test_scripts/shiller/configure-tempus-shiller.sh:-D Kokkos_ENABLE_Pthread:BOOL=${USE_PTHREADS}
tpetra/core/guide/src/Obtaining/Building.rst:- The Pthreads (Kokkos::Threads) back-end is a special case; it does not get enabled by default. This avoids surprises, because Trilinos enables its Pthreads TPL by default as long as it can detect it. Users may set the CMake option Kokkos_ENABLE_Pthread:BOOL=ON to enable use of Pthreads in Tpetra, and to make it default.
tpetra/doc/FAQ.txt: Kokkos_ENABLE_Pthread:BOOL=OFF.
zoltan2/test/core/temp/XpetraEpetraMatrix.cpp:// -D Kokkos_ENABLE_Pthreadi:BOOL=ON

@trilinos/phalanx
@trilinos/tempus
@trilinos/tpetra
@trilinos/zoltan2

Kokkos_ENABLE_Cuda
phalanx/build_scripts/build_phalanx_cuda.sh:-D Kokkos_ENABLE_Cuda=ON
phalanx/build_scripts/build_phalanx_cuda.sh:-D Kokkos_ENABLE_Cuda_UVM=ON
phalanx/build_scripts/build_phalanx_gcc_gge.sh:-D Kokkos_ENABLE_Cuda:BOOL=OFF
phalanx/build_scripts/build_phalanx_gcc_mac_s1002730.sh:-D Kokkos_ENABLE_Cuda:BOOL=OFF
phalanx/src/design/DeviceDagNotes.txt:-D Kokkos_ENABLE_Cuda_Lambda:BOOL=ON
phalanx/src/design/DeviceDagNotes.txt:-D Kokkos_ENABLE_Cuda_Relocatable_Device_Code:BOOL=ON
phalanx/src/design/DeviceDagNotes.txt:-D Kokkos_ENABLE_Cuda_Lambda:BOOL=ON
phalanx/src/design/DeviceDagNotes.txt:-D Kokkos_ENABLE_Cuda_Relocatable_Device_Code:BOOL=ON
tempus/test_scripts/morgan/configure-tempus-morgan.sh:-D Kokkos_ENABLE_Cuda_UVM:BOOL=${USE_CUDA}
tempus/test_scripts/shiller/configure-tempus-shiller.sh:-D Kokkos_ENABLE_Cuda_UVM:BOOL=${USE_CUDA}
tpetra/core/example/BlockCrs/README.md: -D Kokkos_ENABLE_Cuda_UVM:BOOL=${USE_CUDA}
tpetra/core/example/BlockCrs/README.md: -D Kokkos_ENABLE_Cuda_Lambda:BOOL=${USE_CUDA}
tpetra/doc/FAQ.txt:-D Kokkos_ENABLE_Cuda_UVM:BOOL=ON
tpetra/doc/FAQ.txt:-D Kokkos_ENABLE_Cuda_Lambda:BOOL=ON

@trilinos/phalanx
@trilinos/tempus
@trilinos/tpetra


Other occurrences are within the RELEASE_NOTES and build scripts that I'm uncertain of their active/inactive state, hopefully @trilinos/framework can provide feedback

Kokkos_ENABLE_Serial
RELEASE_NOTES: Kokkos_ENABLE_Serial is ON), Tpetra uses Serial by default.
RELEASE_NOTES: - Tpetra_INST_SERIAL (Kokkos_ENABLE_Serial must be ON)
sampleScripts/checkin-test-mac-laptop-clang-3.5:-D Kokkos_ENABLE_Serial:BOOL=OFF
sampleScripts/checkin-test-using-sems-modules.sh:-D Kokkos_ENABLE_Serial:BOOL=OFF
sampleScripts/checkin-test-using-sems-modules.sh:-D Kokkos_ENABLE_Serial:BOOL=ON
sampleScripts/checkin-test-using-sems-modules.sh:-D Kokkos_ENABLE_Serial:BOOL=OFF

Kokkos_ENABLE_OpenMP
RELEASE_NOTES: - Tpetra_INST_OPENMP (Kokkos_ENABLE_OpenMP must be ON, and Trilinos
cmake/ctest/drivers/kokkos-dev/ctest_linux_nightly_serial_release_core_all_kokkos-dev.cmake: "-DKokkos_ENABLE_OpenMP:BOOL=ON"
cmake/ctest/drivers/parameterized/ctest_linux_nightly_generic_sierra.cmake: "-DKokkos_ENABLE_OpenMP:BOOL=$ENV{JENKINS_DO_OPENMP}"
sampleScripts/checkin-test-mac-laptop-clang-3.5:-D Kokkos_ENABLE_OpenMP:BOOL=OFF
sampleScripts/checkin-test-mac-laptop-clang-3.5:-D Kokkos_ENABLE_OpenMP:BOOL=OFF
sampleScripts/checkin-test-using-sems-modules.sh:-D Kokkos_ENABLE_OpenMP:BOOL=OFF
sampleScripts/checkin-test-using-sems-modules.sh:-D Kokkos_ENABLE_OpenMP:BOOL=OFF
sampleScripts/checkin-test-using-sems-modules.sh:-D Kokkos_ENABLE_OpenMP:BOOL=OFF

Kokkos_ENABLE_Pthread
RELEASE_NOTES: Kokkos_ENABLE_Pthread is ON), Tpetra uses Threads by default.
RELEASE_NOTES: - Tpetra_INST_PTHREAD (Kokkos_ENABLE_Pthread must be ON)
RELEASE_NOTES: -D Kokkos_ENABLE_Pthread:BOOL=OFF
cmake/ctest/drivers/apollo/ctest_linux_nightly_mpi_release_downstream_ETI_generic_perseus.cmake: "-DKokkos_ENABLE_Pthread:BOOL=$ENV{JENKINS_DO_PTHREAD}"
cmake/ctest/drivers/artemis/ctest_linux_nightly_mpi_release_downstream_ETI_generic_artemis.cmake: "-DKokkos_ENABLE_Pthread:BOOL=$ENV{JENKINS_DO_PTHREAD}"
cmake/ctest/drivers/kokkos-dev/ctest_linux_nightly_serial_release_core_all_kokkos-dev.cmake: "-DKokkos_ENABLE_Pthread:BOOL=ON"
cmake/ctest/drivers/parameterized/ctest_linux_nightly_generic_kokkos.cmake: "-DKokkos_ENABLE_Pthreadi:BOOL=$ENV{JENKINS_DO_PTHREAD}"
cmake/ctest/drivers/parameterized/ctest_linux_nightly_generic_sierra.cmake: "-DKokkos_ENABLE_Pthread:BOOL=$ENV{JENKINS_DO_PTHREAD}"
cmake/ctest/drivers/perseus/ctest_linux_nightly_mpi_release_downstream_ETI_generic_perseus.cmake: "-DKokkos_ENABLE_Pthread:BOOL=$ENV{JENKINS_DO_PTHREAD}"
cmake/ctest/drivers/ride/ctest_linux_nightly_mpi_release_downstream_ETI_generic_ride.cmake: "-DKokkos_ENABLE_Pthread:BOOL=$ENV{JENKINS_DO_PTHREAD}"
cmake/ctest/drivers/shiller/ctest_linux_nightly_mpi_release_downstream_ETI_generic_shiller.cmake: "-DKokkos_ENABLE_Pthread:BOOL=$ENV{JENKINS_DO_PTHREAD}"
sampleScripts/Sandia-SEMS/configure-all:-D Kokkos_ENABLE_Pthread=${PTHREAD}
sampleScripts/Sandia-SEMS/configure-sems-jenkins-all:-D Kokkos_ENABLE_Pthread=${PTHREAD}
sampleScripts/Sandia-SEMS/configure-testbeds-jenkins:-D Kokkos_ENABLE_Pthread=${PTHREAD}
sampleScripts/Sandia-SEMS/configure-testbeds-jenkins-all:-D Kokkos_ENABLE_Pthread=${PTHREAD}
sampleScripts/Sandia-SEMS/configure-testbeds-tpetra-jenkins:-D Kokkos_ENABLE_Pthread=${PTHREAD}
sampleScripts/checkin-test-mac-laptop-clang-3.5:-D Kokkos_ENABLE_Pthread:BOOL=ON
sampleScripts/checkin-test-mac-laptop-clang-3.5:-D Kokkos_ENABLE_Pthread:BOOL=OFF
sampleScripts/checkin-test-sems-intel-16.sh:# the Kokkos::Threads (Kokkos_ENABLE_Pthread) execution space enabled.
sampleScripts/checkin-test-using-sems-modules.sh:# the Kokkos::Threads (Kokkos_ENABLE_Pthread) execution space enabled.
sampleScripts/checkin-test-using-sems-modules.sh:-D Kokkos_ENABLE_Pthread:BOOL=ON
sampleScripts/checkin-test-using-sems-modules.sh:-D Kokkos_ENABLE_Pthread:BOOL=OFF
sampleScripts/checkin-test-using-sems-modules.sh:#-D Kokkos_ENABLE_Pthread:BOOL=OFF
sampleScripts/checkin-test-using-sems-modules.sh:-D Kokkos_ENABLE_Pthread:BOOL=ON

Kokkos_ENABLE_Cuda
RELEASE_NOTES: - Tpetra_INST_CUDA (Kokkos_ENABLE_Cuda must be ON, and Trilinos must
cmake/ctest/drivers/apollo/ctest_linux_nightly_mpi_release_downstream_ETI_generic_perseus.cmake: "-DKokkos_ENABLE_Cuda_UVM:BOOL=$ENV{JENKINS_DO_CUDA}"
cmake/ctest/drivers/apollo/ctest_linux_nightly_mpi_release_downstream_ETI_generic_perseus.cmake: "-DKokkos_ENABLE_Cuda_Lambda:BOOL=$ENV{JENKINS_DO_CUDA}"
cmake/ctest/drivers/artemis/ctest_linux_nightly_mpi_release_downstream_ETI_generic_artemis.cmake: "-DKokkos_ENABLE_Cuda_UVM:BOOL=$ENV{JENKINS_DO_CUDA}"
cmake/ctest/drivers/parameterized/ctest_linux_nightly_generic_kokkos.cmake: "-DKokkos_ENABLE_Cuda_UVM:BOOL=$ENV{JENKINS_DO_CUDA}"
cmake/ctest/drivers/parameterized/ctest_linux_nightly_generic_sierra.cmake: "-DKokkos_ENABLE_Cuda_UVM:BOOL=$ENV{JENKINS_DO_CUDA}"
cmake/ctest/drivers/parameterized/ctest_linux_nightly_generic_sierra.cmake: -"DKokkos_ENABLE_Cuda_Lambda:BOOL=$ENV{JENKINS_DO_COMPLEX}"
cmake/ctest/drivers/perseus/ctest_linux_nightly_mpi_release_downstream_ETI_generic_perseus.cmake: "-DKokkos_ENABLE_Cuda_UVM:BOOL=$ENV{JENKINS_DO_CUDA}"
cmake/ctest/drivers/ride/ctest_linux_nightly_mpi_release_downstream_ETI_generic_ride.cmake: "-DKokkos_ENABLE_Cuda_UVM:BOOL=$ENV{JENKINS_DO_CUDA}"
cmake/ctest/drivers/shiller/ctest_linux_nightly_mpi_release_downstream_ETI_generic_shiller.cmake: "-DKokkos_ENABLE_Cuda_UVM:BOOL=$ENV{JENKINS_DO_CUDA}"
cmake/std/atdm/README.md:Trilinos CMake cache var Kokkos_ENABLE_Cuda_Relocatable_Device_Code in CUDA
cmake/std/atdm/README.md:* rdc: Set Kokkos_ENABLE_Cuda_Relocatable_Device_Code=ON
cmake/std/atdm/README.md:* no-rdc: Set Kokkos_ENABLE_Cuda_Relocatable_Device_Code=OFF
sampleScripts/Sandia-SEMS/configure-all: -D Kokkos_ENABLE_Cuda_UVM:BOOL=ON
sampleScripts/Sandia-SEMS/configure-sems-jenkins-all: -D Kokkos_ENABLE_Cuda_UVM:BOOL=ON
sampleScripts/Sandia-SEMS/configure-testbeds-jenkins: -D Kokkos_ENABLE_Cuda_UVM:BOOL=ON
sampleScripts/Sandia-SEMS/configure-testbeds-jenkins-all:-D Kokkos_ENABLE_Cuda_UVM:BOOL=ON
sampleScripts/Sandia-SEMS/configure-testbeds-jenkins-all: -D Kokkos_ENABLE_Cuda_Lambda:BOOL=ON
sampleScripts/Sandia-SEMS/configure-testbeds-tpetra-jenkins: -D Kokkos_ENABLE_Cuda_UVM:BOOL=ON
sampleScripts/checkin-test-using-sems-modules.sh: -D Kokkos_ENABLE_Cuda_UVM:BOOL=${CUDA}


I can put in a PR using sed to update the naming, though any feedback on the testing script beforehand would be helpful as I don't want to disrupt anything actively used

Edit: Update to reflect the deprecation was pre-3.0 (previously I mistakenly stated this was pre-4.0)

@dalg24
Copy link
Contributor

dalg24 commented May 31, 2023

One clarification: this is not a Kokkos 4.0 change, these were already deprecated in v3.0

@ccober6
Copy link
Contributor

ccober6 commented May 31, 2023

@ndellingwood for the Tempus changes, please go ahead and make the replacement as these scripts are old and are likely not currently being used but may have some reference value. Thanks!

@bartlettroscoe
Copy link
Member

I am sure everyone would appreciate a PR that addresses all of these. (Finding these is 90% of the work.)

@jhux2
Copy link
Member

jhux2 commented May 31, 2023

Can we just delete some of the obselete scripts instead? Some of the script are no longer used, e.g., the check-in script. I'll also wager that some of the scripts in cmake/ctest/drivers are for machines that no longer exist, e.g., apollo.

@bartlettroscoe
Copy link
Member

Can we just delete some of the obselete scripts instead? Some of the script are no longer used, e.g., the check-in script. I'll also wager that some of the scripts in cmake/ctest/drivers are for machines that no longer exist, e.g., apollo.

+1. These are still in the Git history so people can find them that way if they want.

@jhux2
Copy link
Member

jhux2 commented May 31, 2023

Here are the driver subdirectories and the last commit that touched them:

I'll open a separate issue for the driver scripts.

@ndellingwood
Copy link
Contributor Author

I like the option of deleting the old scripts as well, I didn't want to jump the gun so I'll hold off on touching test script and for now I'll open a PR to update the naming convention in the various docs (test script updates / removal can be a separate PR)
Is there a way to label a PR indicating changes are to documentation only (and so the PR does not need to run through the full CI)?

@bartlettroscoe
Copy link
Member

Is there a way to label a PR indicating changes are to documentation only (and so the PR does not need to run through the full CI)?

@ndellingwood, not that I know of. That would create a backdoor way for developers to bypass the PR testing and get their PRs merged faster (which some people might use out of frustration).

There was discussion of allowing individual package's to label certain directories as not impacting the build or tests but that requires an extension to TriBITS (I can't seem to find that GitHub issue discussion).

The best strategy is to create one big PR with lots of changes and just accept that the PR builds and tests have to pass.

@ndellingwood
Copy link
Contributor Author

That would create a backdoor way for developers to bypass the PR testing and get their PRs merged faster

@bartlettroscoe ah, great point, thanks pointing that out!

ndellingwood added a commit to ndellingwood/kokkos-kernels that referenced this issue May 31, 2023
Kokkos_ENABLE_OpenMP* to Kokkos_ENALBE_OPENMP*

Related to trilinos/Trilinos#11930 and kokkos/kokkos#6138
ndellingwood added a commit to ndellingwood/Trilinos that referenced this issue May 31, 2023
Address documentation updates referred to in trilinos#11930
Copy link

github-actions bot commented Jun 1, 2024

This issue has had no activity for 365 days and is marked for closure. It will be closed after an additional 30 days of inactivity.
If you would like to keep this issue open please add a comment and/or remove the MARKED_FOR_CLOSURE label.
If this issue should be kept open even with no activity beyond the time limits you can add the label DO_NOT_AUTOCLOSE.
If it is ok for this issue to be closed, feel free to go ahead and close it. Please do not add any comments or change any labels or otherwise touch this issue unless your intention is to reset the inactivity counter for an additional year.

@github-actions github-actions bot added the MARKED_FOR_CLOSURE Issue or PR is marked for auto-closure by the GitHub Actions bot. label Jun 1, 2024
Copy link

github-actions bot commented Jul 3, 2024

This issue was closed due to inactivity for 395 days.

@github-actions github-actions bot added the CLOSED_DUE_TO_INACTIVITY Issue or PR has been closed by the GitHub Actions bot due to inactivity. label Jul 3, 2024
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jul 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLOSED_DUE_TO_INACTIVITY Issue or PR has been closed by the GitHub Actions bot due to inactivity. MARKED_FOR_CLOSURE Issue or PR is marked for auto-closure by the GitHub Actions bot. type: bug The primary issue is a bug in Trilinos code or tests
Projects
None yet
Development

No branches or pull requests

5 participants