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_CUDA_UVM #5608

Merged
merged 6 commits into from
Nov 29, 2022
Merged

Conversation

JBludau
Copy link
Contributor

@JBludau JBludau commented Nov 1, 2022

As we have the new alias SharedSpace we can deprecate Kokkos_ENABLE_CUDA_UVM.

Copy link
Member

@dalg24 dalg24 left a comment

Choose a reason for hiding this comment

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

I am on board with the deprecation of Kokkos_ENABLE_CUDA_UVM in 4.0
We will need to elaborate on the why and what we want people to do instead in the documentation (we really need these deprecated code {2,3,4} pages)

I would have done this in two separate PRs as these are pretty much unrelated.
The deprecating/removing CudaUVMSpace::available() is uncontroversial as the function has become pointless (always returning true, not usable in constant expression, not provided by any other memory space, etc.)

Makefile.kokkos Outdated Show resolved Hide resolved
Makefile.kokkos Outdated Show resolved Hide resolved
Copy link
Contributor

@ldh4 ldh4 left a comment

Choose a reason for hiding this comment

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

I also think these should be two separate PRs. One has to do with a cmake option while the other has to do with an obsolete function that just happens to be distantly relevant.

@JBludau JBludau changed the title Deprecate Kokkos_ENABLE_CUDA_UVM and CudaUVMSpace::available() Deprecate Kokkos_ENABLE_CUDA_UVM Nov 4, 2022
@JBludau
Copy link
Contributor Author

JBludau commented Nov 4, 2022

Split into two prs. Still working on the documentation in kokkos/kokkos-core-wiki#201

Co-authored-by: Daniel Arndt <arndtd@ornl.gov>
@JBludau
Copy link
Contributor Author

JBludau commented Nov 5, 2022

Please take a look at the documentation before merging
kokkos/kokkos-core-wiki#201

@JBludau
Copy link
Contributor Author

JBludau commented Nov 6, 2022

ci seems unrelated (openmp.repeated_team_reduce)

cmake/kokkos_arch.cmake Outdated Show resolved Hide resolved
Co-authored-by: Phil Miller <unmobile+gh@gmail.com>
Copy link
Member

@dalg24 dalg24 left a comment

Choose a reason for hiding this comment

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

Need to coordinate with #5623
Same question here do we mean to define the option regardless of whether deprecated code 4 is enabled or not

(@JBludau I am not actually requesting any change at this time. I am just writing things down for bookkeeping. Maybe that is something we can harmonize as a follow up)

@dalg24 dalg24 merged commit 2420eb2 into kokkos:develop Nov 29, 2022
@JBludau
Copy link
Contributor Author

JBludau commented Nov 29, 2022

@dalg24 sure, let's harmonize it as soon as we have a verdict

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

6 participants