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

Make ExecutionSpace::concurrency() a non-static member function #5655

Closed
dalg24 opened this issue Dec 1, 2022 · 2 comments
Closed

Make ExecutionSpace::concurrency() a non-static member function #5655

dalg24 opened this issue Dec 1, 2022 · 2 comments
Assignees
Labels
Enhancement Improve existing capability; will potentially require voting

Comments

@dalg24
Copy link
Member

dalg24 commented Dec 1, 2022

ExecutionSpace::concurrency() is currently declared as a static member function in all backends and this is an issue because the concurrency is really a property of an execution space instance, not from its type.

This is something we knew we would have to deal with for a while but we can't procrastinate much longer.
In particular, it is problematic as we add support for execution space instances to parallel host backends.
Apropos, #5105 attempts to work around that issue by providing OpenMP::concurrency(OpenMP const& = OpenMP()) -> int

The change from static to non-static member function is a breaking change but the fix on the user-side is rather trivial

int concurrency = Kokkos::DefaultExecutionSpace::concurrency();  // does not compile any more
// change to
int concurrency = Kokkos::DefaultExecutionSpace().concurrency();  // using default-constructed execution space
// or
int concurrency = execution_space.concurrency();  // calling member function on an instance

The intent of this issue is to announce our intent to make this change and to formulate a plan for the transition.

@ajpowelsnl ajpowelsnl added the Enhancement Improve existing capability; will potentially require voting label Dec 1, 2022
@PhilMiller
Copy link
Contributor

4.0 would be a really nice timeframe to make such a change. We didn't deprecate to provide a warning or anything, but I think users would survive without too much grumbling if we did it ASAP

@dalg24
Copy link
Member Author

dalg24 commented Dec 1, 2022

Unfortunately I can't think of any way to emit warnings because we would not be able to "overload" on the static member function.

ndellingwood added a commit to ndellingwood/Trilinos that referenced this issue Dec 5, 2022
ndellingwood added a commit to ndellingwood/Trilinos that referenced this issue Dec 5, 2022
@msimberg msimberg self-assigned this Feb 2, 2023
@dalg24 dalg24 closed this as completed Feb 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Improve existing capability; will potentially require voting
Projects
None yet
Development

No branches or pull requests

4 participants