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

Update concept for execution space instances #581

Merged
merged 8 commits into from
Nov 7, 2024

Conversation

masterleinad
Copy link
Contributor

@masterleinad masterleinad commented Sep 11, 2024

Related to kokkos/kokkos#7319.

  • in_parallel is deprecated
  • execution spaces are regular now
  • wake and sleep don't exist anymore
  • we missed concurrency

@@ -102,8 +102,7 @@ and ``OpenMPTarget``, the current state of the Kokkos |ExecutionSpaceTwo|_ conce

template <typename Ex>
concept ExecutionSpace =
CopyConstructible<Ex> &&
DefaultConstructible<Ex> &&
SemiRegular<Ex> &&
Copy link
Contributor Author

Choose a reason for hiding this comment

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

kokkos/kokkos#5398 (Kokkos 4.0.00) introduced the comparison operator. I'm not sure if that's worth mentioning.

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.

The "functionality" section is a a bit weird and somewhat redundant with what comes after the synopsis.
I am not expecting you to fix here abut I am curious if others agree with that observation.

docs/source/API/core/KokkosConcepts.rst Outdated Show resolved Hide resolved
{ ex.fence() };
{ ex.name() } -> const char*;
{ ex.concurrency() } -> int;
Copy link
Member

Choose a reason for hiding this comment

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

Remind me, did we promise anything about the return type?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's how its implemented and how we describe it in all documentation so far. I don't know if there would be any motivation to change it to size_type or something the like.

Copy link
Member

Choose a reason for hiding this comment

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

I meant more returns an integral type or something that is convertible to int.
Anyway that's fine by me if all backend do indeed return an int

docs/source/API/core/execution_spaces.rst Outdated Show resolved Hide resolved
@dalg24 dalg24 merged commit 2c67f41 into kokkos:main Nov 7, 2024
2 checks passed
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.

3 participants