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.

Sign up for GitHub

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
Merged

Update concept for execution space instances #581

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
40a1726
wake and sleep don't exist anymore
masterleinad Sep 11, 2024
5e0d0ba
Remove in_parallel
masterleinad Sep 11, 2024
891c8f3
Use SemiRegular
masterleinad Sep 11, 2024
3ef74f0
Document concurrency
masterleinad Sep 11, 2024
b2e84bf
Add fence without argument
masterleinad Sep 11, 2024
0103497
Update docs/source/API/core/execution_spaces.rst
masterleinad Sep 11, 2024
6cd5bc7
SemiRegular -> Regular
masterleinad Sep 11, 2024
cfb0c31
Apply suggestions from code review
masterleinad Nov 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 4 additions & 29 deletions docs/source/API/core/KokkosConcepts.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,7 @@ and ``OpenMPTarget``, the current state of the Kokkos |ExecutionSpaceTwo|_ conce

template <typename Ex>
concept ExecutionSpace =
CopyConstructible<Ex> &&
DefaultConstructible<Ex> &&
Regular<Ex> &&
Destructible<Ex> &&
// Member type requirements:
requires() {
Expand All @@ -120,10 +119,11 @@ and ``OpenMPTarget``, the current state of the Kokkos |ExecutionSpaceTwo|_ conce
typename Ex::device_type;
} &&
// Required methods:
requires(Ex ex, std::ostream& ostr, bool detail) {
{ ex.in_parallel() } -> bool;
requires(Ex ex, std::string str, std::ostream& ostr, bool detail) {
{ ex.fence(str) };
{ 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

{ ex.print_configuration(ostr) };
{ ex.print_configuration(ostr, detail) };
} &&
Expand Down Expand Up @@ -207,23 +207,6 @@ Support for ``UniqueToken`` adds the following requirements:
&& CopyConstructible<Experimental::UniqueToken<Ex, Experimental::UniqueTokenScope::Global>>
&& DefaultConstructible<Experimental::UniqueToken<Ex, Experimental::UniqueTokenScope::Global>>;

An Additional Concept for ``DeviceExecutionSpace``?
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

All the device execution spaces, in their current state, have two extra member functions,
``sleep()`` and ``wake()``. It's unclear whether this is intended to be general,
but if it is, there is an additional concept in the hierarchy:

.. code-block:: cpp

template <typename Ex>
concept DeviceExecutionSpace =
ExecutionSpace<Ex> &&
requires(Ex ex) {
{ ex.sleep() };
{ ex.wake() };
}

Some *de facto* Requirements
~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Expand All @@ -232,14 +215,6 @@ such as ``Impl::TeamPolicyInternal``. These also qualify as "requirements" on an
just like ``Impl::ParallelFor<...>``. In many of these cases, it would be nice if we could refactor
some things to use a less "all-or-nothing" approach to customization than partial class template specialization.

Design Thoughts
~~~~~~~~~~~~~~~

The first thing that comes to mind is that ``CopyConstructible<T> && DefaultConstructible<T> && Destructible<T>``
is very close to ``SemiRegular<T>``; all we need to do is add ``operator==()``.

TODO more here

The ``MemorySpace`` Concept
---------------------------

Expand Down
17 changes: 4 additions & 13 deletions docs/source/API/core/execution_spaces.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,16 +210,9 @@ and an instance of that type ``ex``, Kokkos guarantees the following expressions

.. code-block:: cpp

ex.in_parallel();
ex.fence(str);

*Returns:* a value convertible to ``bool`` indicating whether or not the caller is executing as part of a Kokkos parallel pattern.
*Note:* as currently implemented, there is no guarantee that ``true`` means the caller is necessarily executing as part of a pattern on the particular instance ``ex``; just *some* instance of ``Ex``. This may be strengthened in the future.

.. code-block:: cpp

ex.fence();

*Effects:* Upon return, all parallel patterns executed on the instance ``ex`` are guaranteed to have completed, and their effects are guaranteed visible to the calling thread.
*Effects:* Upon return, all parallel patterns executed on the instance ``ex`` are guaranteed to have completed, and their effects are guaranteed visible to the calling thread. The optiopnal ``str`` argument allows customizing the event reported to Kokkos Tools.
*Returns:* Nothing.
*Note:* This *cannot* be called from within a parallel pattern. Doing so will lead to unspecified effects (i.e., it might work, but only for some execution spaces, so be extra careful not to do it).

Expand Down Expand Up @@ -280,9 +273,9 @@ Synopsis
void print_configuration(std::ostream ostr&) const;
void print_configuration(std::ostream ostr&, bool details) const;

bool in_parallel() const;
int concurrency() const;

void fence(const std::string&) const;
void fence() const;

friend bool operator==(const execution_space& lhs, const execution_space& rhs);
Expand Down Expand Up @@ -325,11 +318,9 @@ Functions

* ``const char* name() const;``: *Returns* the label of the execution space instance.

* ``bool in_parallel() const;``: *Returns* a value convertible to ``bool`` indicating whether the caller is executing as part of a Kokkos parallel pattern. *Note:* as currently implemented, there is no guarantee that ``true`` means the caller is necessarily executing as part of a pattern on the particular instance |ExecutionSpaceConcept|_; just *some* instance of |ExecutionSpaceConcept|_. This may be strengthened in the future.

* ``int concurrency() const;`` *Returns* the maximum amount of concurrently executing work items in a parallel setting, i.e. the maximum number of threads utilized by an execution space instance.

* ``void fence() const;`` *Effects:* Upon return, all parallel patterns executed on the instance |ExecutionSpaceConcept|_ are guaranteed to have completed, and their effects are guaranteed visible to the calling thread. *Note:* This *cannot* be called from within a parallel pattern. Doing so will lead to unspecified effects (i.e., it might work, but only for some execution spaces, so be extra careful not to do it).
* ``void fence(const std::string& label = unspecified-default-value) const;`` *Effects:* Upon return, all parallel patterns executed on the instance |ExecutionSpaceConcept|_ are guaranteed to have completed, and their effects are guaranteed visible to the calling thread. *Note:* This *cannot* be called from within a parallel pattern. Doing so will lead to unspecified effects (i.e., it might work, but only for some execution spaces, so be extra careful not to do it). The optional ``label`` argument allows customizing the event reported to Kokkos Tools.

* ``void print_configuration(std::ostream ostr) const;``: *Effects:* Outputs the configuration of ``ex`` to the given ``std::ostream``. *Note:* This *cannot* be called from within a parallel pattern.

Expand Down
Loading