test: refactor & add helpers for handling Views of arbitrary rank#221
Open
dssgabriel wants to merge 35 commits intokokkos:developfrom
Open
test: refactor & add helpers for handling Views of arbitrary rank#221dssgabriel wants to merge 35 commits intokokkos:developfrom
dssgabriel wants to merge 35 commits intokokkos:developfrom
Conversation
Simple wrapper of `int` with UDL for easy creation
Makes the code much easier to read for functions with attributes and long type/param/tparam names.
Signed-off-by: Gabriel Dos Santos <gabriel.dossantos@cea.fr>
Signed-off-by: Gabriel Dos Santos <gabriel.dossantos@cea.fr>
Signed-off-by: Gabriel Dos Santos <gabriel.dossantos@cea.fr>
Signed-off-by: Gabriel Dos Santos <gabriel.dossantos@cea.fr>
Signed-off-by: Gabriel Dos Santos <gabriel.dossantos@cea.fr>
Signed-off-by: Gabriel Dos Santos <gabriel.dossantos@cea.fr>
Signed-off-by: Gabriel Dos Santos <gabriel.dossantos@cea.fr>
`do_iteration` was taking its variadic arguments by value, causing the compiler to attempt a copy of any argument passed to it. This triggered a compile error when passing a `Communicator`, whose copy constructor is deleted by design. Fixed by taking `Args&&...` and forwarding with `std::forward<Args>(args)...`.
Correctly replace `exec_` with the passed in `exec` parameter.
Directly call the factory `split` instead of factory `duplicate`, since member functions can access their rank without having to recompute it.
For both MPI and NCCL specializations, covers: - Factory functions (`from_raw`, `duplicate`, `split`); - Accessors (`comm`, `size`, `rank`); - Move semantics.
Some drive-by updates: - Removed mentions of `barrier` in the core API - Fixed mentions of ~~`Req`~~ -> `Request`
This could lead to a compilation error if both MPI and NCCL backends were enabled at the same time, where `DefaultCommunicationSpace` would be defined twice. Now, we ensure it is only defined once. NCCL takes precedence over MPI if it has been enabled.
dssgabriel
added
C-enhancement
dssgabriel
force-pushed
the
test/view-utils
branch
from
da51abe to
46a5269
Compare
Update tests and docs. Remove `std::optional` from `from_raw` signature.
Add unit tests for exec() accessor, self-move assignment safety, duplicate independence chain, and sequential splits.
2 tasks
dssgabriel
force-pushed
the
test/view-utils
branch
from
46a5269 to
d3a8cf1
Compare
dssgabriel
changed the title
Turn the `ViewBuilder` struct into a `build_view` free function that can build Views of arbitrary rank (contiguous or not).
Also wrap in `test_utils` namespace`
dssgabriel
force-pushed
the
test/view-utils
branch
from
d3a8cf1 to
83107e5
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Refactored implementation of view builder helpers and added new ones, all now MD-capable:
ViewBuilderstruct into a free function (build_view) that allows building views of arbitrary type and dimension, contiguous or not.init_view) and error counting (count_error) helpers.These changes make it possible to write only the high-level test function and pass different kinds of Views to it. Such tests better mirror user code that Kokkos Comm is designed to handle correctly.
It is now trivial to test views of higher dimensions (>2), contiguous or not (provided we support them)*.
*Note (rant)
We currently don't support non-contiguous Views of rank higher than 2 with the DeepCopy packing strategy due to limitations stemming from a similar implementation approach to the one refactored in this PR.
In (a) follow-up PR(s), all tests should be refactored to use this new mechanism. Additionally, this will let us easily enhance existing tests to actually assert the expected behavior of all the functionalities Kokkos Comm is designed to provide.
Handleimpls #220Additional context
Motivation
Initializing views and counting errors were the two problematic pieces of code that forced us to have dedicated test functions for each view dimension we wanted to test. The new helpers abstract away the required MD logic, letting us write simpler tests that are much more representative of what users would write.
Changes
Details:
ViewBuilderstruct into a free function for building views of arbitrary type and dimension (contiguous or not)test_utils::namespaceview_builder.hppand consolidated logic intoview_utils.hppunit_tests/test_sendrecv.cppChecklist