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

support for bool via kabool wrapper #246

Merged
merged 41 commits into from
Jul 8, 2022
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
b2ca4d2
support for bool via kabool wrapper
niklas-uhl Apr 10, 2022
a90fe63
Fix formatting
niklas-uhl Apr 10, 2022
a59a4ad
Fix formatting
niklas-uhl Apr 11, 2022
185bd2b
Apply suggestions from code review
niklas-uhl Apr 11, 2022
ae1e452
Fix suggestions
niklas-uhl Apr 13, 2022
31e238c
Fix doc strings
niklas-uhl Apr 13, 2022
e985e66
Merge branch 'main' into features/bool-vectors
niklas-uhl Apr 13, 2022
0660262
Remove fixed_sized_uint because I realized it is unneccessary
niklas-uhl Apr 25, 2022
d856fa2
Allow single element bool buffers, but return a Collection of kabool
niklas-uhl Apr 25, 2022
e6cf548
Fix bug in for single element buffer of bool
niklas-uhl Apr 25, 2022
d40ea20
Add tests for bool and gather
niklas-uhl Apr 25, 2022
b1bffe7
Update include/kamping/mpi_datatype.hpp
niklas-uhl Apr 25, 2022
16adff9
Merge branch 'main' into features/bool-vectors
niklas-uhl May 19, 2022
404ace6
Fixes and additional tests
niklas-uhl May 19, 2022
98293a7
Apply clang-format changes
niklas-uhl May 19, 2022
20ce734
dispatch to kabool for all parameter factories
niklas-uhl May 19, 2022
91e6e89
Merge branch 'features/bool-vectors' of https://github.com/kamping-si…
niklas-uhl May 19, 2022
bc7fd2a
Fix vector<bool> bullshit
niklas-uhl May 19, 2022
d588c96
Clean up tests
niklas-uhl May 19, 2022
46ae1e4
Merge branch 'main' into features/bool-vectors
niklas-uhl Jul 1, 2022
be75c0a
Add static assertion for `std::vector<bool>`
niklas-uhl Jul 1, 2022
f01e639
Formatting
niklas-uhl Jul 1, 2022
4204c24
Formatting
niklas-uhl Jul 1, 2022
238ad3c
Formatting
niklas-uhl Jul 1, 2022
69bd45e
Formatting
niklas-uhl Jul 1, 2022
670d5e9
Remove test for has_data_member because it was moved to another PR
niklas-uhl Jul 6, 2022
defcb9b
Merge branch 'main' into features/bool-vectors
niklas-uhl Jul 6, 2022
f6c421a
Add explaining documentation to `is_specialization`
niklas-uhl Jul 6, 2022
a2fcf3d
comments for compilation failure tests
niklas-uhl Jul 6, 2022
ab0351b
OwnContainer does not use std::vector
niklas-uhl Jul 6, 2022
296893c
Add tests
niklas-uhl Jul 6, 2022
dac5903
Fix custom Allocator to work with vectors
niklas-uhl Jul 7, 2022
d3104ce
Add is_vector_bool check to DataBuffer
niklas-uhl Jul 7, 2022
10872df
Fix tests and test naming
niklas-uhl Jul 7, 2022
c03736b
Fix extraction assertion
niklas-uhl Jul 7, 2022
2032705
Formatting
niklas-uhl Jul 7, 2022
cff7ccd
Merge branch 'main' into features/bool-vectors
niklas-uhl Jul 7, 2022
dcf50d2
Formatting
niklas-uhl Jul 8, 2022
dcd07b9
Merge branch 'main' into features/bool-vectors
niklas-uhl Jul 8, 2022
6444b01
Make kabool constexpr
niklas-uhl Jul 8, 2022
6b88386
Merge branch 'main' into features/bool-vectors
niklas-uhl Jul 8, 2022
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
68 changes: 68 additions & 0 deletions include/kamping/mpi_datatype.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,66 @@
#include "kamping/error_handling.hpp"
#include "kamping/kassert.hpp"

namespace kamping::internal {
///@brief Helper for getting a unsigned integer of fixed size.
///@tparam Size in bytes.
template <size_t N>
struct fixed_sized_uint_t {
static_assert(N != 1 && N != 2 && N != 4 && N != 8, "Only 8, 16, 32 and 64 bit integers are supported");
};

///@brief Helper for getting a unsigned integer of fixed size.
///@tparam N the type size in bytes
template <>
struct fixed_sized_uint_t<1> {
using type = std::uint8_t; ///< the integer type
};

///@brief Helper for getting a unsigned integer of fixed size.
///@tparam N the type size in bytes
template <>
struct fixed_sized_uint_t<2> {
using type = std::uint16_t; ///< the integer type
};

///@brief Helper for getting a unsigned integer of fixed size.
///@tparam N the type size in bytes
template <>
struct fixed_sized_uint_t<4> {
using type = std::uint32_t; ///< the integer type
};

///@brief Helper for getting a unsigned integer of fixed size.
///@tparam N the type size in bytes
template <>
struct fixed_sized_uint_t<8> {
using type = std::uint64_t; ///< the integer type
};

///@brief Helper type definition for getting a unsigned integer of fixed size.
///@tparam N the type size in bytes
template <size_t N>
using fixed_sized_uint = typename fixed_sized_uint_t<N>::type;
} // namespace kamping::internal
niklas-uhl marked this conversation as resolved.
Show resolved Hide resolved

namespace kamping {
///@brief Wrapper around bool to allow handling containers of boolean values
class kabool {
public:
///@brief default constructor for a \c kabool with value \c false
kabool() noexcept : _value() {}
///@brief constructor to construct a \c kabool out of a \c bool
kabool(bool value) noexcept : _value(value) {}

///@brief implicit cast of \c kabool to \c bool
inline operator bool() const noexcept {
return static_cast<bool>(_value);
}

private:
// We need an unsigned integer with the correct size, because \c sizeof(bool) is implementation defined.
internal::fixed_sized_uint<sizeof(bool)> _value; ///< the wrapped boolean value
};

/// @addtogroup kamping_mpi_utility
/// @{
Expand Down Expand Up @@ -215,6 +274,15 @@ struct mpi_type_traits_impl<bool> : is_builtin_mpi_type_true {
}
static constexpr TypeCategory category = TypeCategory::logical;
};

template <>
struct mpi_type_traits_impl<kabool> : is_builtin_mpi_type_true {
static MPI_Datatype data_type() {
return MPI_CXX_BOOL;
}
static constexpr TypeCategory category = TypeCategory::logical;
};

template <>
struct mpi_type_traits_impl<std::complex<float>> : is_builtin_mpi_type_true {
static MPI_Datatype data_type() {
Expand Down
31 changes: 11 additions & 20 deletions include/kamping/parameter_factories.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,26 +24,6 @@ namespace kamping {
/// @addtogroup kamping_mpi_utility
/// @{

namespace internal {
/// @brief Boolean value helping to decide if data type has \c .data() method.
/// @return \c true if class has \c .data() method and \c false otherwise.
template <typename, typename = void>
constexpr bool has_data_member_v = false;

/// @brief Boolean value helping to decide if data type has \c .data() method.
/// @return \c true if class has \c .data() method and \c false otherwise.
template <typename T>
constexpr bool has_data_member_v<T, std::void_t<decltype(std::declval<T>().data())>> = true;

/// @brief Tag type for parameters that can be omitted on some PEs (e.g., root PE, or non-root PEs).
template <typename T>
struct ignore_t {};
} // namespace internal

/// @brief Tag for parameters that can be omitted on some PEs (e.g., root PE, or non-root PEs).
template <typename T>
constexpr internal::ignore_t<T> ignore{};
niklas-uhl marked this conversation as resolved.
Show resolved Hide resolved

/// @brief Generates a dummy send buf that wraps a \c nullptr.
///
/// This is useful for operations where a send_buf is required on some PEs, such as the root PE,
Expand All @@ -69,6 +49,9 @@ auto send_buf(internal::ignore_t<Data> ignore [[maybe_unused]]) {
/// @return Object referring to the storage containing the data elements to send.
template <typename Data>
auto send_buf(const Data& data) {
static_assert(
!internal::is_vector_bool_v<Data>,
"std::vector<bool> can not be used with MPI, please use a container of kamping::kabool instead.");
niklas-uhl marked this conversation as resolved.
Show resolved Hide resolved
if constexpr (internal::has_data_member_v<Data>) {
return internal::ContainerBasedConstBuffer<Data, internal::ParameterType::send_buf>(data);
} else {
Expand All @@ -94,6 +77,10 @@ auto send_buf(const Data& data) {
/// @return Object referring to the storage containing the data elements to send / the received elements.
template <typename Data>
auto send_recv_buf(Data& data) {
static_assert(
!internal::is_vector_bool_v<Data>,
"std::vector<bool> can not be used with MPI, please use a container of kamping::kabool instead.");

if constexpr (internal::has_data_member_v<Data>) {
return internal::UserAllocatedContainerBasedBuffer<Data, internal::ParameterType::send_recv_buf>(data);
} else {
Expand All @@ -115,6 +102,10 @@ auto send_recv_buf(Data& data) {
/// @return Object referring to the storage containing the data elements to send.
template <typename Data>
auto send_recv_buf(const Data& data) {
static_assert(
!internal::is_vector_bool_v<Data>,
"std::vector<bool> can not be used with MPI, please use a container of kamping::kabool instead.");

if constexpr (internal::has_data_member_v<Data>) {
return internal::ContainerBasedConstBuffer<Data, internal::ParameterType::send_recv_buf>(data);
} else {
Expand Down
75 changes: 75 additions & 0 deletions include/kamping/parameter_objects.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,69 @@ namespace kamping {
/// @addtogroup kamping_mpi_utility
/// @{

namespace internal {
/// @brief Boolean value helping to decide if data type has \c .data() method.
/// @return \c true if class has \c .data() method and \c false otherwise.
template <typename, typename = void>
constexpr bool has_data_member_v = false;

/// @brief Boolean value helping to decide if data type has \c .data() method.
/// @return \c true if class has \c .data() method and \c false otherwise.
template <typename T>
static constexpr bool has_data_member_v<T, std::void_t<decltype(std::declval<T>().data())>> = true;

/// @brief Boolean value helping to decide if type has a \c value_type member type.
/// @return \c true if class has \c value_type method and \c false otherwise.
template <typename, typename = void>
static constexpr bool has_value_type_v = false;

/// @brief Boolean value helping to decide if type has a \c value_type member type.
/// @return \c true if class has \c value_type method and \c false otherwise.
template <typename T>
static constexpr bool has_value_type_v<T, std::void_t<typename T::value_type>> = true;
niklas-uhl marked this conversation as resolved.
Show resolved Hide resolved

/// @brief Type trait to check if a type is an instance of a templated type.
/// @tparam T The concrete type.
/// @tparam Template The type template.
/// @return \c true if the type is an instance and \c false otherwise.
template <class T, template <class...> class Template>
struct is_specialization : std::false_type {};

/// @brief Type trait to check if a type is an instance of a templated type.
/// @tparam T The concrete type.
/// @tparam Template the type template
/// @return \c true if the type is an instance and \c false otherwise.
template <template <class...> class Template, class... Args>
struct is_specialization<Template<Args...>, Template> : std::true_type {};
niklas-uhl marked this conversation as resolved.
Show resolved Hide resolved
niklas-uhl marked this conversation as resolved.
Show resolved Hide resolved

/// @brief Boolean value helping to check if a type is an instance of \c std::vector<bool>.
/// @tparam T The type.
/// @return \c true if \c T is an template instance of \c std::vector<bool>, \c false otherwise.
template <typename T, typename = void>
static constexpr bool is_vector_bool_v = false;

/// @brief Boolean value helping to check if a type is an instance of \c std::vector<bool>.
/// @tparam T The type.
/// @return \c true if \T is an template instance of \c std::vector<bool>, \c false otherwise.
template <typename T>
static constexpr bool is_vector_bool_v<T, typename std::enable_if<!has_value_type_v<T>>::type> = false;
niklas-uhl marked this conversation as resolved.
Show resolved Hide resolved

/// @brief Boolean value helping to check if a type is an instance of \c std::vector<bool>.
/// @tparam T The type.
/// @return \c true if \T is an template instance of \c std::vector<bool>, \c false otherwise.
template <typename T>
static constexpr bool is_vector_bool_v<T, typename std::enable_if<has_value_type_v<T>>::type> =
is_specialization<T, std::vector>::value&& std::is_same_v<typename T::value_type, bool>;

/// @brief Tag type for parameters that can be omitted on some PEs (e.g., root PE, or non-root PEs).
template <typename T>
struct ignore_t {};
} // namespace internal

/// @brief Tag for parameters that can be omitted on some PEs (e.g., root PE, or non-root PEs).
template <typename T>
constexpr internal::ignore_t<T> ignore{};

/// @brief Type used for tag dispatching.
///
/// This types needs to be used to select internal::LibAllocContainerBasedBuffer as buffer type.
Expand Down Expand Up @@ -94,6 +157,10 @@ namespace internal {
/// @tparam ParameterType parameter type represented by this buffer.
template <typename Container, ParameterType type>
class ContainerBasedConstBuffer {
static_assert(
!internal::is_vector_bool_v<Container>,
"std::vector<bool> can not be used with MPI, please use a container of kamping::kabool instead.");

niklas-uhl marked this conversation as resolved.
Show resolved Hide resolved
public:
static constexpr ParameterType parameter_type = type; ///< The type of parameter this buffer represents.
static constexpr bool is_modifiable = false; ///< Indicates whether the underlying storage is modifiable.
Expand Down Expand Up @@ -276,6 +343,10 @@ struct BufferParameterType {
/// @tparam ParameterType parameter type represented by this buffer.
template <typename Container, ParameterType parameter_type>
class UserAllocatedContainerBasedBuffer : public BufferParameterType<parameter_type> {
static_assert(
!internal::is_vector_bool_v<Container>,
"std::vector<bool> can not be used with MPI, please use a container of kamping::kabool instead.");

public:
using value_type = typename Container::value_type; ///< Value type of the buffer.

Expand Down Expand Up @@ -344,6 +415,10 @@ class UserAllocatedContainerBasedBuffer : public BufferParameterType<parameter_t
/// @tparam ParameterType parameter type represented by this buffer.
template <typename Container, ParameterType type>
class LibAllocatedContainerBasedBuffer : public BufferParameterType<type> {
static_assert(
!internal::is_vector_bool_v<Container>,
"std::vector<bool> can not be used with MPI, please use a container of kamping::kabool instead.");

public:
using value_type = typename Container::value_type; ///< Value type of the buffer.

Expand Down
95 changes: 95 additions & 0 deletions tests/collectives/mpi_reduce_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@

#include "../helpers_for_testing.hpp"
#include "kamping/communicator.hpp"
#include "kamping/mpi_ops.hpp"
#include "kamping/parameter_factories.hpp"

using namespace ::kamping;
using namespace ::testing;
Expand Down Expand Up @@ -61,6 +63,99 @@ TEST(ReduceTest, reduce_no_receive_buffer) {
}
}

TEST(ReduceTest, reduce_no_receive_buffer_bool) {
niklas-uhl marked this conversation as resolved.
Show resolved Hide resolved
Communicator comm;

std::vector<kabool> input = {false, false};
if (comm.rank() == 1 % comm.size()) {
input[1] = true;
}

auto result = comm.reduce(send_buf(input), op(ops::logical_or<>{})).extract_recv_buffer();

if (comm.rank() == comm.root()) {
EXPECT_EQ(result.size(), 2);
niklas-uhl marked this conversation as resolved.
Show resolved Hide resolved
} else {
EXPECT_EQ(result.size(), 0);
}

std::vector<kabool> expected_result = {false, true};
if (comm.is_root()) {
EXPECT_EQ(result, expected_result);
}
niklas-uhl marked this conversation as resolved.
Show resolved Hide resolved
}

TEST(ReduceTest, reduce_no_receive_buffer_bool_custom_operation) {
niklas-uhl marked this conversation as resolved.
Show resolved Hide resolved
Communicator comm;

std::vector<kabool> input = {false, false};
if (comm.rank() == 1 % comm.size()) {
input[1] = true;
}

// test that we can use a operation defined on bool even though wrap them as kabool
auto my_or = [&](bool lhs, bool rhs) {
return lhs || rhs;
};
auto result = comm.reduce(send_buf(input), op(my_or, commutative)).extract_recv_buffer();

if (comm.rank() == comm.root()) {
EXPECT_EQ(result.size(), 2);
} else {
EXPECT_EQ(result.size(), 0);
}

std::vector<kabool> expected_result = {false, true};
if (comm.is_root()) {
EXPECT_EQ(result, expected_result);
}
niklas-uhl marked this conversation as resolved.
Show resolved Hide resolved
}

TEST(ReduceTest, reduce_single_element_no_receive_buffer_bool) {
niklas-uhl marked this conversation as resolved.
Show resolved Hide resolved
Communicator comm;

kabool input = false;
if (comm.rank() == 1 % comm.size()) {
input = true;
}

auto result = comm.reduce(send_buf(input), op(ops::logical_or<>{})).extract_recv_buffer();

if (comm.rank() == comm.root()) {
EXPECT_EQ(result.size(), 1);
} else {
EXPECT_EQ(result.size(), 0);
}

std::vector<kabool> expected_result = {true};
if (comm.is_root()) {
EXPECT_EQ(result, expected_result);
}
}
niklas-uhl marked this conversation as resolved.
Show resolved Hide resolved

TEST(ReduceTest, reduce_single_element_explicit_receive_buffer_bool) {
Communicator comm;

kabool input = false;
std::vector<kabool> result;
if (comm.rank() == 1 % comm.size()) {
input = true;
}

comm.reduce(send_buf(input), recv_buf(result), op(ops::logical_or<>{}));

if (comm.rank() == comm.root()) {
EXPECT_EQ(result.size(), 1);
} else {
EXPECT_EQ(result.size(), 0);
}

std::vector<kabool> expected_result = {true};
if (comm.is_root()) {
EXPECT_EQ(result, expected_result);
}
}

TEST(ReduceTest, reduce_with_receive_buffer) {
Communicator comm;

Expand Down
Loading