Skip to content

Commit

Permalink
Addressed review comments including renaming the new accessor to bloc…
Browse files Browse the repository at this point in the history 
…k_col_major

Co-authored-by: Thomas Grützmacher <thomas.gruetzmacher@kit.edu>
  • Loading branch information
@Slaedr
Slaedr and Thomas Grützmacher committed Mar 8, 2021
1 parent 3cb8b41 commit 79349ee
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 52 deletions.
26 changes: 13 additions & 13 deletions core/test/base/range_accessors.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -275,13 +275,13 @@ TEST_F(RowMajorAccessor3d, CanAssignSubranges)
}


class ColMajorAccessor3d : public ::testing::Test {
class BlockColMajorAccessor3d : public ::testing::Test {
protected:
using span = gko::span;
static constexpr gko::size_type dimensionality{3};

using col_major_range =
gko::range<gko::accessor::col_major<int, dimensionality>>;
using blk_col_major_range =
gko::range<gko::accessor::block_col_major<int, dimensionality>>;

// clang-format off
int data[2 * 3 * 4]{
Expand All @@ -308,14 +308,14 @@ class ColMajorAccessor3d : public ::testing::Test {
// clang-format on
const gko::dim<dimensionality> dim1{2, 3, 4};
const gko::dim<dimensionality> dim2{2, 2, 3};
col_major_range default_r{data, dim1};
col_major_range custom_r{
blk_col_major_range default_r{data, dim1};
blk_col_major_range custom_r{
data, dim2,
std::array<const gko::size_type, dimensionality - 1>{12, 3}};
};


TEST_F(ColMajorAccessor3d, ComputesCorrectStride)
TEST_F(BlockColMajorAccessor3d, ComputesCorrectStride)
{
auto range_stride = default_r.get_accessor().stride;
auto check_stride = std::array<const gko::size_type, 2>{12, 3};
Expand All @@ -324,7 +324,7 @@ TEST_F(ColMajorAccessor3d, ComputesCorrectStride)
}


TEST_F(ColMajorAccessor3d, CanAccessData)
TEST_F(BlockColMajorAccessor3d, CanAccessData)
{
EXPECT_EQ(default_r(0, 0, 0), 1);
EXPECT_EQ(custom_r(0, 0, 0), 1);
Expand All @@ -339,7 +339,7 @@ TEST_F(ColMajorAccessor3d, CanAccessData)
}


TEST_F(ColMajorAccessor3d, CanWriteData)
TEST_F(BlockColMajorAccessor3d, CanWriteData)
{
default_r(0, 0, 0) = 4;
custom_r(1, 1, 1) = 100;
Expand All @@ -351,7 +351,7 @@ TEST_F(ColMajorAccessor3d, CanWriteData)
}


TEST_F(ColMajorAccessor3d, CanCreateSubrange)
TEST_F(BlockColMajorAccessor3d, CanCreateSubrange)
{
auto subr = custom_r(span{0, 2}, span{1, 2}, span{1, 3});

Expand All @@ -362,7 +362,7 @@ TEST_F(ColMajorAccessor3d, CanCreateSubrange)
}


TEST_F(ColMajorAccessor3d, CanCreateRowVector)
TEST_F(BlockColMajorAccessor3d, CanCreateRowVector)
{
auto subr = default_r(1u, 2u, span{0, 2});

Expand All @@ -371,7 +371,7 @@ TEST_F(ColMajorAccessor3d, CanCreateRowVector)
}


TEST_F(ColMajorAccessor3d, CanCreateColumnVector)
TEST_F(BlockColMajorAccessor3d, CanCreateColumnVector)
{
auto subr = default_r(span{0u, 2u}, 1u, 3u);

Expand All @@ -380,15 +380,15 @@ TEST_F(ColMajorAccessor3d, CanCreateColumnVector)
}


TEST_F(ColMajorAccessor3d, CanAssignValues)
TEST_F(BlockColMajorAccessor3d, CanAssignValues)
{
default_r(1, 1, 1) = default_r(0, 0, 0);

EXPECT_EQ(data[16], 1);
}


TEST_F(ColMajorAccessor3d, CanAssignSubranges)
TEST_F(BlockColMajorAccessor3d, CanAssignSubranges)
{
default_r(1u, span{0, 2}, span{0, 3}) =
custom_r(0u, span{0, 2}, span{0, 3});
Expand Down
86 changes: 47 additions & 39 deletions include/ginkgo/core/base/range_accessors.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -468,6 +468,10 @@ template <typename ValueType>
class row_major<ValueType, 2> {
public:
friend class range<row_major>;

/**
* Number of dimensions of the accessor.
*/
static constexpr size_type dimensionality = 2;

/**
Expand All @@ -480,10 +484,6 @@ class row_major<ValueType, 2> {
*/
using data_type = value_type *;

/**
* Number of dimensions of the accessor.
*/

using const_accessor = row_major<const ValueType, dimensionality>;

protected:
Expand Down Expand Up @@ -631,7 +631,11 @@ class row_major<ValueType, 2> {
};


namespace detail_colmajor {
/**
* Namespace for helper functions and structs for
* the block column major accessor.
*/
namespace detail_block_col_major {


/**
Expand All @@ -658,9 +662,11 @@ struct index_helper_s {
FirstType first, Indices &&... idxs)
{
if (current_iter == total_dim - 1) {
return first +
index_helper_s<ValueType, total_dim, current_iter + 1>::
compute(size, stride, std::forward<Indices>(idxs)...);
return GKO_ASSERT(first < size[dim_idx]),
first +
index_helper_s<ValueType, total_dim, current_iter + 1>::
compute(size, stride,
std::forward<Indices>(idxs)...);
}

return GKO_ASSERT(first < size[dim_idx]),
Expand All @@ -676,12 +682,12 @@ struct index_helper_s<ValueType, total_dim, total_dim> {

static constexpr size_type dim_idx{total_dim - 1};

template <typename FirstType, typename... Indices>
template <typename FirstType>
static constexpr GKO_ATTRIBUTES ValueType
compute(const std::array<ValueType, total_dim> &size,
const std::array<ValueType, (total_dim > 1 ? total_dim - 1 : 0)>
&stride,
FirstType first, Indices &&...)
FirstType first)
{
return GKO_ASSERT(first < size[total_dim - 1]),
first * stride[dim_idx - 1];
Expand Down Expand Up @@ -733,15 +739,15 @@ constexpr GKO_ATTRIBUTES
}


} // namespace detail_colmajor
} // namespace detail_block_col_major


/**
* A bridge between a range and a column-major memory layout.
* A bridge between a range and a block-column-major memory layout.
*
* Note that this is not completely a 'layout right'. Only the innermost
* two dimensions are regarded as defining a column-major matrix, and
* the rest of the dimensions are treated identically to \ref row_major.
* Only the innermost two dimensions are regarded as defining
* a column-major matrix, and the rest of the dimensions are treated
* identically to \ref row_major.
*
* You should not try to explicitly create an instance of this accessor.
* Instead, supply it as a template parameter to a range, and pass the
Expand All @@ -752,14 +758,18 @@ constexpr GKO_ATTRIBUTES
* @tparam Dimensionality number of dimensions of this accessor
*/
template <typename ValueType, size_type Dimensionality>
class col_major {
class block_col_major {
public:
friend class range<col_major>;
friend class range<block_col_major>;

static_assert(Dimensionality != 0,
"This accessor does not support a dimensionality of 0!");
static_assert(Dimensionality != 1,
"Please use row_major accessor for 1D ranges.");

/**
* Number of dimensions of the accessor.
*/
static constexpr size_type dimensionality = Dimensionality;

/**
Expand All @@ -772,17 +782,13 @@ class col_major {
*/
using data_type = value_type *;

/**
* Number of dimensions of the accessor.
*/

using const_accessor = col_major<const ValueType, Dimensionality>;
using const_accessor = block_col_major<const ValueType, Dimensionality>;
using stride_type = std::array<const size_type, dimensionality - 1>;
using length_type = std::array<const size_type, dimensionality>;

protected:
/**
* Creates a col_major accessor.
* Creates a block_col_major accessor.
*
* @param data pointer to the block of memory containing the data
* @param lengths size / length of the accesses of each dimension
Expand All @@ -791,33 +797,34 @@ class col_major {
* `x_1 * stride_1 + x_2 * stride_2 * ... + x_(n-1) + x_n * stride_(n-1)`
* points to the element at (x_1, x_2, ..., x_n))
*/
constexpr GKO_ATTRIBUTES explicit col_major(data_type data,
dim<dimensionality> size,
stride_type stride)
constexpr GKO_ATTRIBUTES explicit block_col_major(data_type data,
dim<dimensionality> size,
stride_type stride)
: data{data},
lengths(detail::to_array<const size_type>(size)),
stride(stride)
{}

/**
* Creates a col_major accessor with a default stride (assumes no padding)
* Creates a block_col_major accessor with a default stride
* (assumes no padding)
*
* @param data pointer to the block of memory containing the data
* @param lengths size / length of the accesses of each dimension
*/
constexpr GKO_ATTRIBUTES explicit col_major(data_type data,
dim<dimensionality> size)
constexpr GKO_ATTRIBUTES explicit block_col_major(data_type data,
dim<dimensionality> size)
: data{data},
lengths(detail::to_array<const size_type>(size)),
stride(detail_colmajor::default_stride_array(lengths))
stride(detail_block_col_major::default_stride_array(lengths))
{}

public:
/**
* Creates a col_major range which contains a read-only version of the
* current accessor.
* Creates a block_col_major range which contains a read-only version of
* the current accessor.
*
* @returns a col major range which is read-only.
* @returns a block column major range which is read-only.
*/
constexpr GKO_ATTRIBUTES range<const_accessor> to_const() const
{
Expand All @@ -838,7 +845,7 @@ class col_major {
std::enable_if_t<are_all_integral<Indices...>::value, value_type &>
operator()(Indices &&... indices) const
{
return data[detail_colmajor::compute_index(
return data[detail_block_col_major::compute_index(
lengths, stride, std::forward<Indices>(indices)...)];
}

Expand All @@ -851,13 +858,14 @@ class col_major {
* @return sub-range spanning the given spans
*/
template <typename... SpanTypes>
constexpr GKO_ATTRIBUTES std::enable_if_t<
detail::are_span_compatible<SpanTypes...>::value, range<col_major>>
operator()(SpanTypes... spans) const
constexpr GKO_ATTRIBUTES
std::enable_if_t<detail::are_span_compatible<SpanTypes...>::value,
range<block_col_major>>
operator()(SpanTypes... spans) const
{
return detail::validate_spans(lengths, spans...),
range<col_major>{
data + detail_colmajor::compute_index(
range<block_col_major>{
data + detail_block_col_major::compute_index(
lengths, stride, (span{spans}.begin)...),
dim<dimensionality>{
(span{spans}.end - span{spans}.begin)...},
Expand Down

0 comments on commit 79349ee

Please sign in to comment.