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

Deprecate Kokkos_ENABLE_PTHREAD in favor of Kokkos_ENABLE_THREADS #4619

Merged
merged 7 commits into from
Dec 16, 2021

Conversation

dalg24
Copy link
Member

@dalg24 dalg24 commented Dec 15, 2021

Follow up on #4081
It doesn't really make sense to keep enabling with -DKokkos_ENABLE_PTHREAD=ON.
Besides the departure between the exec space name Kokkos::Threads and the backend name was a pain.

@dalg24 dalg24 added the Refactor Tidying up: make code code, cleaner, simpler, understandable and robust label Dec 15, 2021
@dalg24
Copy link
Member Author

dalg24 commented Dec 15, 2021

Haven't dealt with Makefiles yet

@PhilMiller
Copy link
Contributor

Please fix the typo in the pr title

@dalg24 dalg24 changed the title Deprecate Kokkos_ENABLE_PTHREAD in favor of Kokkos_ENABLE_TRHEADS Deprecate Kokkos_ENABLE_PTHREAD in favor of Kokkos_ENABLE_THREADS Dec 15, 2021
IF(Kokkos_ENABLE_THREADS) # for backward compatibility
SET(Kokkos_ENABLE_PTHREAD ON)
LIST(APPEND Kokkos_DEVICES PTHREAD)
ENDIF()

IF(NOT Kokkos_FIND_QUIETLY)
MESSAGE(STATUS "Enabled Kokkos devices: ${Kokkos_DEVICES}")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we print Kokkos_THREADS and Kokkos_PTHREAD after this pull request? Is omitting to add PTHREAD to Kokkos_DEVICES feasible?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will append after printing. Unfortunately we have to append PTHREAD to Kokkos_DEVICES and set Kokkos_ENABLE_PTHREAD to ON or we may break code downstream.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now only appending if Kokkos_ENABLE_DEPRECATED_CODE_3 was ON

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

cmake/kokkos_enable_devices.cmake Outdated Show resolved Hide resolved
Copy link
Contributor

@nmm0 nmm0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

Btw -- I don't know if there is a good way in telling anyone consuming PTHREADS device or Kokkos_ENABLE_PTHREAD in their own projects that those are deprecated but we should consider how we eventually want to remove that.

cmake/kokkos_enable_devices.cmake Outdated Show resolved Hide resolved
gnu_generate_makefile.bash Show resolved Hide resolved
generate_makefile.bash Show resolved Hide resolved
Copy link
Contributor

@masterleinad masterleinad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks OK to me.

IF(Kokkos_ENABLE_THREADS) # for backward compatibility
SET(Kokkos_ENABLE_PTHREAD ON)
LIST(APPEND Kokkos_DEVICES PTHREAD)
ENDIF()

IF(NOT Kokkos_FIND_QUIETLY)
MESSAGE(STATUS "Enabled Kokkos devices: ${Kokkos_DEVICES}")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

@dalg24 dalg24 merged commit e47c706 into kokkos:develop Dec 16, 2021
@dalg24 dalg24 deleted the threads branch December 16, 2021 23:20
ndellingwood added a commit to ndellingwood/kokkos-kernels that referenced this pull request Dec 17, 2021
Compatibility change in accordance with kokkos/kokkos#4619
Resolves nightly build failures (due to undefined references at link stage)
when both Pthreads (not Threads) and Serial backends simultaneously enabled
ndellingwood added a commit to ndellingwood/kokkos-kernels that referenced this pull request Dec 17, 2021
Compatibility change in accordance with kokkos/kokkos#4619
Resolves nightly build failures (due to undefined references at link stage)
when both Pthreads (not Threads) and Serial backends simultaneously enabled
ndellingwood added a commit to trilinos/Trilinos that referenced this pull request Mar 2, 2022
Rename Kokkos_ENABLE_PTHREAD cache variable to Kokkos_ENABLE_THREADS
Coincides with kokkos/kokkos#4619 and kokkos/kokkos#4830
ndellingwood added a commit to trilinos/Trilinos that referenced this pull request Mar 14, 2022
Rename Kokkos_ENABLE_PTHREAD cache variable to Kokkos_ENABLE_THREADS
Coincides with kokkos/kokkos#4619 and kokkos/kokkos#4830
ndellingwood added a commit to trilinos/Trilinos that referenced this pull request Apr 7, 2022
Rename Kokkos_ENABLE_PTHREAD cache variable to Kokkos_ENABLE_THREADS
Coincides with kokkos/kokkos#4619 and kokkos/kokkos#4830
ndellingwood added a commit to trilinos/Trilinos that referenced this pull request Apr 8, 2022
Rename Kokkos_ENABLE_PTHREAD cache variable to Kokkos_ENABLE_THREADS
Coincides with kokkos/kokkos#4619 and kokkos/kokkos#4830
ndellingwood added a commit to trilinos/Trilinos that referenced this pull request Apr 18, 2022
Rename Kokkos_ENABLE_PTHREAD cache variable to Kokkos_ENABLE_THREADS
Coincides with kokkos/kokkos#4619 and kokkos/kokkos#4830
ndellingwood added a commit to trilinos/Trilinos that referenced this pull request Apr 18, 2022
Rename Kokkos_ENABLE_PTHREAD cache variable to Kokkos_ENABLE_THREADS
Coincides with kokkos/kokkos#4619 and kokkos/kokkos#4830
ndellingwood added a commit to trilinos/Trilinos that referenced this pull request Apr 20, 2022
Rename Kokkos_ENABLE_PTHREAD cache variable to Kokkos_ENABLE_THREADS
Coincides with kokkos/kokkos#4619 and kokkos/kokkos#4830
tcclevenger pushed a commit to trilinos/Trilinos that referenced this pull request May 26, 2022
Rename Kokkos_ENABLE_PTHREAD cache variable to Kokkos_ENABLE_THREADS
Coincides with kokkos/kokkos#4619 and kokkos/kokkos#4830
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Refactor Tidying up: make code code, cleaner, simpler, understandable and robust
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants