From e8bdd151447fb5efb017ad54e8d8d4b7d5e48bde Mon Sep 17 00:00:00 2001 From: Mikael Simberg Date: Mon, 10 Jul 2023 17:42:30 +0200 Subject: [PATCH 01/18] Add constructor to create sub-distribution from a distribution --- include/dlaf/matrix/distribution.h | 11 +++++ src/matrix/distribution.cpp | 15 +++++++ test/unit/matrix/test_distribution.cpp | 58 ++++++++++++++++++++++++++ 3 files changed, 84 insertions(+) diff --git a/include/dlaf/matrix/distribution.h b/include/dlaf/matrix/distribution.h index 10cb35b914..588c41f2c1 100644 --- a/include/dlaf/matrix/distribution.h +++ b/include/dlaf/matrix/distribution.h @@ -119,6 +119,17 @@ class Distribution { Distribution& operator=(Distribution&& rhs) noexcept; + /// Constructs a sub-distribution based on the given distribution @p dist with + /// an @p offset and @p size. + /// + /// @param[in] dist is the input distribution, + /// @param[in] offset is the offset of the new distribution relative to the input distribution, + /// @param[in] size is the size of the new distribution relative to the offset, + /// @pre origin.isValid() + /// @pre size.isValid() + /// @pre origin + size <= dist.size() + Distribution(Distribution dist, const GlobalElementIndex& offset, const GlobalElementSize& size); + bool operator==(const Distribution& rhs) const noexcept { return size_ == rhs.size_ && local_size_ == rhs.local_size_ && tile_size_ == rhs.tile_size_ && block_size_ == rhs.block_size_ && global_nr_tiles_ == rhs.global_nr_tiles_ && diff --git a/src/matrix/distribution.cpp b/src/matrix/distribution.cpp index b25af8909d..8b2cbe03cf 100644 --- a/src/matrix/distribution.cpp +++ b/src/matrix/distribution.cpp @@ -88,6 +88,21 @@ Distribution& Distribution::operator=(Distribution&& rhs) noexcept { return *this; } +Distribution::Distribution(Distribution rhs, const GlobalElementIndex& origin, + const GlobalElementSize& size) + : Distribution(std::move(rhs)) { + DLAF_ASSERT(origin.isValid(), origin); + DLAF_ASSERT(size.isValid(), size); + DLAF_ASSERT(origin.row() + size.rows() <= size_.rows(), origin, size_); + DLAF_ASSERT(origin.col() + size.cols() <= size_.cols(), origin, size_); + + offset_ = GlobalElementIndex(offset_.get() + origin.get(), + offset_.get() + origin.get()); + size_ = size; + + computeGlobalAndLocalNrTilesAndLocalSize(); +} + void Distribution::computeGlobalSizeForNonDistr() noexcept { size_ = GlobalElementSize(local_size_.rows(), local_size_.cols()); } diff --git a/test/unit/matrix/test_distribution.cpp b/test/unit/matrix/test_distribution.cpp index c99043605d..0f0e7b3256 100644 --- a/test/unit/matrix/test_distribution.cpp +++ b/test/unit/matrix/test_distribution.cpp @@ -633,3 +633,61 @@ TEST(DistributionTest, LocalElementDistanceFromGlobalTile) { obj.localElementDistanceFromGlobalTile(test.global_tile_begin, test.global_tile_end)); } } + +struct ParametersSubDistribution { + // Distribution settings + GlobalElementSize size; + TileElementSize block_size; + comm::Index2D rank; + comm::Size2D grid_size; + comm::Index2D src_rank; + GlobalElementIndex offset; + // Sub-distribution settings + GlobalElementIndex suboffset; + GlobalElementSize subsize; + // Valid indices + GlobalElementIndex global_element; + GlobalTileIndex global_tile; + comm::Index2D rank_tile; + std::array local_tile; // can be an invalid LocalTileIndex +}; + +const std::vector tests_sub_distribution = { + // {size, block_size, rank, grid_size, src_rank, offset, suboffset, subsize, + // global_element, global_tile, rank_tile, local_tile} + + // TODO: More tests + + // Sub-distribution == distribution + {{3, 4}, {2, 2}, {0, 0}, {1, 1}, {0, 0}, {0, 0}, {0, 0}, {3, 4}, {1, 3}, {0, 1}, {0, 0}, {0, 1}}, + {{5, 9}, {3, 2}, {1, 1}, {2, 4}, {0, 2}, {1, 1}, {0, 0}, {5, 9}, {1, 3}, {0, 2}, {0, 0}, {-1, -1}}, +}; + +TEST(DistributionTest, SubDistribution) { + for (const auto& test : tests_sub_distribution) { + Distribution dist(test.size, test.block_size, test.grid_size, test.rank, test.src_rank, test.offset); + Distribution sub_dist(dist, test.suboffset, test.subsize); + + EXPECT_EQ(sub_dist.size(), test.subsize); + + EXPECT_EQ(sub_dist.blockSize(), dist.blockSize()); + EXPECT_EQ(sub_dist.baseTileSize(), dist.baseTileSize()); + EXPECT_EQ(sub_dist.rankIndex(), dist.rankIndex()); + EXPECT_EQ(sub_dist.commGridSize(), dist.commGridSize()); + + EXPECT_LE(sub_dist.localSize().rows(), dist.localSize().rows()); + EXPECT_LE(sub_dist.localSize().cols(), dist.localSize().cols()); + EXPECT_LE(sub_dist.localNrTiles().rows(), dist.localNrTiles().rows()); + EXPECT_LE(sub_dist.localNrTiles().cols(), dist.localNrTiles().cols()); + EXPECT_LE(sub_dist.nrTiles().rows(), dist.nrTiles().rows()); + EXPECT_LE(sub_dist.nrTiles().cols(), dist.nrTiles().cols()); + + EXPECT_EQ(sub_dist.globalTileIndex(test.global_element), test.global_tile); + EXPECT_EQ(sub_dist.rankGlobalTile(sub_dist.globalTileIndex(test.global_element)), test.rank_tile); + + EXPECT_EQ(sub_dist.localTileFromGlobalElement(test.global_element.get()), + test.local_tile[0]); + EXPECT_EQ(sub_dist.localTileFromGlobalElement(test.global_element.get()), + test.local_tile[1]); + } +} From c1c06feec5f77d0b43bf698bb53b11977d3de976 Mon Sep 17 00:00:00 2001 From: Mikael Simberg Date: Tue, 11 Jul 2023 14:24:08 +0200 Subject: [PATCH 02/18] Add MatrixRef class to create a reference to a sub-matrix --- include/dlaf/matrix/matrix_ref.h | 193 +++++++++++++++++++++++++++ test/unit/matrix/CMakeLists.txt | 8 ++ test/unit/matrix/test_matrix_ref.cpp | 120 +++++++++++++++++ 3 files changed, 321 insertions(+) create mode 100644 include/dlaf/matrix/matrix_ref.h create mode 100644 test/unit/matrix/test_matrix_ref.cpp diff --git a/include/dlaf/matrix/matrix_ref.h b/include/dlaf/matrix/matrix_ref.h new file mode 100644 index 0000000000..906345c411 --- /dev/null +++ b/include/dlaf/matrix/matrix_ref.h @@ -0,0 +1,193 @@ +// +// Distributed Linear Algebra with Future (DLAF) +// +// Copyright (c) 2018-2023, ETH Zurich +// All rights reserved. +// +// Please, refer to the LICENSE file in the root directory. +// SPDX-License-Identifier: BSD-3-Clause +// + +#pragma once + +/// @file + +#include +#include +#include +#include +#include + +namespace dlaf::matrix { +/// A @c MatrixRef represents a sub-matrix of a @c Matrix. +/// +/// The class has reference semantics, meaning accesses to a @c MatrixRef and +/// it's corresponding @c Matrix are interleaved if calls to read/readwrite are +/// interleaved. Access to a @c MatrixRef and its corresponding @c Matrix is not +/// thread-safe. A @c MatrixRef must outlive its corresponding @c Matrix. +template +class MatrixRef; + +template +class MatrixRef : public internal::MatrixBase { +public: + static constexpr Device device = D; + + using ElementType = T; + using TileType = Tile; + using ConstTileType = Tile; + using TileDataType = internal::TileData; + using ReadOnlySenderType = ReadOnlyTileSender; + + /// Create a sub-matrix of @p mat with an @p offset and @p size. + /// + /// @param[in] mat is the input matrix, + /// @param[in] offset is the offset of the new matrix relative to the input matrix, + /// @param[in] size is the size of the new matrix relative to the offset, + /// @pre origin.isValid() + /// @pre size.isValid() + /// @pre origin + size <= mat.size() + MatrixRef(Matrix& mat, const GlobalElementIndex& offset, const GlobalElementSize& size) + : internal::MatrixBase(Distribution(mat.distribution(), offset, size)), mat_const_(mat), + offset_(offset), tile_offset_(mat.distribution().globalTileIndex(offset)) {} + + // TODO: default, copy, move construction? + // - default: no, don't want empty MatrixRef + // - copy: implementable, still refer to the original matrix + // - move: implement as copy, i.e. still refer to original matrix? + MatrixRef() = delete; + + /// Returns a read-only sender of the Tile with local index @p index. + /// + /// @pre index.isIn(distribution().localNrTiles()). + ReadOnlySenderType read(const LocalTileIndex& index) noexcept { + DLAF_ASSERT(index.isIn(distribution().localNrTiles()), index, distribution().localNrTiles()); + + // Note: the overload with GlobalTileIndex handles taking a subtile if needed + const GlobalTileIndex global_index(distribution().globalTileFromLocalTile(index.row()), + distribution().globalTileFromLocalTile(index.col())); + return read(global_index); + } + + /// Returns a read-only sender of the Tile with global index @p index. + /// + /// @pre the global tile is stored in the current process, + /// @pre index.isIn(globalNrTiles()). + ReadOnlySenderType read(const GlobalTileIndex& index) { + DLAF_ASSERT(index.isIn(distribution().nrTiles()), index, distribution().nrTiles()); + + const GlobalTileIndex parent_index(tile_offset_.row() + index.row(), + tile_offset_.col() + index.col()); + auto tile_sender = mat_const_.read(parent_index); + + const auto parent_dist = mat_const_.distribution(); + const auto parent_tile_size = parent_dist.tileSize(parent_index); + const auto tile_size = tileSize(index); + + // If the corresponding tile in the parent distribution is exactly the same + // size as the tile in the sub-distribution, we don't need to take a subtile + // and can return the tile sender directly. + if (parent_tile_size == tile_size) { + return tile_sender; + } + + // Otherwise we have to extract a subtile from the tile in the parent + // distribution. + const TileElementIndex ij_tile{ + index.row() == 0 ? parent_dist.template tileElementFromGlobalElement(offset_.row()) + : 0, + index.col() == 0 ? parent_dist.template tileElementFromGlobalElement(offset_.col()) + : 0, + }; + + return splitTile(std::move(tile_sender), SubTileSpec{ij_tile, tile_size}); + } + +private: + Matrix& mat_const_; + +protected: + GlobalElementIndex offset_; + GlobalTileIndex tile_offset_; +}; + +template +class MatrixRef : public MatrixRef { +public: + static constexpr Device device = D; + + using ElementType = T; + using TileType = Tile; + using ConstTileType = Tile; + using TileDataType = internal::TileData; + using ReadWriteSenderType = ReadWriteTileSender; + + /// Create a sub-matrix of @p mat with an @p offset and @p size. + /// + /// @param[in] mat is the input matrix, + /// @param[in] offset is the offset of the new matrix relative to the input matrix, + /// @param[in] size is the size of the new matrix relative to the offset, + /// @pre origin.isValid() + /// @pre size.isValid() + /// @pre origin + size <= mat.size() + MatrixRef(Matrix& mat, const GlobalElementIndex& offset, const GlobalElementSize& size) + : MatrixRef(mat, offset, size), mat_(mat) {} + + // TODO: default, copy, move construction? + MatrixRef() = delete; + + /// Returns a sender of the Tile with local index @p index. + /// + /// @pre index.isIn(distribution().localNrTiles()). + ReadWriteSenderType readwrite(const LocalTileIndex& index) noexcept { + DLAF_ASSERT(index.isIn(this->distribution().localNrTiles()), index, + this->distribution().localNrTiles()); + const GlobalTileIndex parent_index( + this->distribution().template globalTileFromLocalTile(index.row()), + this->distribution().template globalTileFromLocalTile(index.col())); + + // Note: the overload with GlobalTileIndex handles taking a subtile if needed + return readwrite(parent_index); + } + + /// Returns a sender of the Tile with global index @p index. + /// + /// @pre the global tile is stored in the current process, + /// @pre index.isIn(globalNrTiles()). + ReadWriteSenderType readwrite(const GlobalTileIndex& index) { + DLAF_ASSERT(index.isIn(this->distribution().nrTiles()), index, this->distribution().nrTiles()); + // TODO: add helpers for distribution vs. sub-distribution arithmetic + + const GlobalTileIndex parent_index(tile_offset_.row() + index.row(), + tile_offset_.col() + index.col()); + auto tile_sender = mat_.readwrite(parent_index); + + const auto parent_dist = mat_.distribution(); + const auto parent_tile_size = parent_dist.tileSize(parent_index); + const auto tile_size = this->tileSize(index); + + // If the corresponding tile in the parent distribution is exactly the same + // size as the tile in the sub-distribution, we don't need to take a subtile + // and can return the tile sender directly. + if (parent_tile_size == tile_size) { + return tile_sender; + } + + // Otherwise we have to extract a subtile from the tile in the parent + // distribution. + const TileElementIndex ij_tile{ + index.row() == 0 ? parent_dist.template tileElementFromGlobalElement(offset_.row()) + : 0, + index.col() == 0 ? parent_dist.template tileElementFromGlobalElement(offset_.col()) + : 0, + }; + + return splitTile(std::move(tile_sender), SubTileSpec{ij_tile, tile_size}); + } + +private: + Matrix& mat_; + using MatrixRef::offset_; + using MatrixRef::tile_offset_; +}; +} diff --git a/test/unit/matrix/CMakeLists.txt b/test/unit/matrix/CMakeLists.txt index 3ecf85b3e8..b9479da6b3 100644 --- a/test/unit/matrix/CMakeLists.txt +++ b/test/unit/matrix/CMakeLists.txt @@ -76,6 +76,14 @@ DLAF_addTest( MPIRANKS 6 ) +DLAF_addTest( + test_matrix_ref + SOURCES test_matrix_ref.cpp + LIBRARIES dlaf.core + USE_MAIN MPIPIKA + MPIRANKS 6 +) + DLAF_addTest( test_panel SOURCES test_panel.cpp diff --git a/test/unit/matrix/test_matrix_ref.cpp b/test/unit/matrix/test_matrix_ref.cpp new file mode 100644 index 0000000000..496b3d67f3 --- /dev/null +++ b/test/unit/matrix/test_matrix_ref.cpp @@ -0,0 +1,120 @@ +// +// Distributed Linear Algebra with Future (DLAF) +// +// Copyright (c) 2018-2023, ETH Zurich +// All rights reserved. +// +// Please, refer to the LICENSE file in the root directory. +// SPDX-License-Identifier: BSD-3-Clause +// + +#include +#include +#include + +#include + +#include +#include +#include +#include +#include + +#include + +#include +#include +#include + +using namespace std::chrono_literals; + +using namespace dlaf; +using namespace dlaf::matrix; +using namespace dlaf::matrix::test; +using namespace dlaf::test; +using namespace testing; + +namespace ex = pika::execution::experimental; +namespace tt = pika::this_thread::experimental; + +::testing::Environment* const comm_grids_env = + ::testing::AddGlobalTestEnvironment(new CommunicatorGrid6RanksEnvironment); + +template +struct MatrixRefTest : public TestWithCommGrids {}; + +TYPED_TEST_SUITE(MatrixRefTest, MatrixElementTypes); + +struct TestSubMatrix { + GlobalElementSize size; + TileElementSize block_size; + GlobalElementIndex sub_offset; + GlobalElementSize sub_size; +}; + +const std::vector tests_sub_matrix({ + // Empty matrix TODO: support? + // {{0, 0}, {1, 1}, {0, 0}, {0, 0}}, + // Empty sub-matrices + {{3, 4}, {3, 4}, {0, 0}, {0, 0}}, + {{3, 4}, {3, 4}, {2, 3}, {0, 0}}, + // Single-block matrix + {{3, 4}, {3, 4}, {0, 0}, {3, 4}}, + {{3, 4}, {3, 4}, {0, 0}, {2, 1}}, + {{3, 4}, {3, 4}, {1, 2}, {2, 1}}, + {{3, 4}, {8, 6}, {0, 0}, {3, 4}}, + {{3, 4}, {8, 6}, {0, 0}, {2, 1}}, + {{3, 4}, {8, 6}, {1, 2}, {2, 1}}, + // Larger matrices + {{10, 15}, {5, 5}, {6, 7}, {0, 0}}, + {{10, 15}, {5, 5}, {6, 7}, {0, 0}}, + {{10, 15}, {5, 5}, {1, 2}, {0, 0}}, + {{10, 15}, {5, 5}, {0, 0}, {10, 15}}, + {{10, 15}, {5, 5}, {0, 0}, {10, 15}}, + {{10, 15}, {5, 5}, {0, 0}, {10, 15}}, + {{10, 15}, {5, 5}, {6, 7}, {2, 2}}, + {{10, 15}, {5, 5}, {6, 7}, {4, 7}}, + {{10, 15}, {5, 5}, {1, 2}, {8, 7}}, +}); + +inline bool indexInSubMatrix(const GlobalElementIndex& index, const GlobalElementIndex& offset, + const GlobalElementSize& size) { + bool r = offset.row() <= index.row() && index.row() < offset.row() + size.rows() && + offset.col() <= index.col() && index.col() < offset.col() + size.cols(); + return r; +} + +TYPED_TEST(MatrixRefTest, Basic) { + using Type = TypeParam; + constexpr Device device = Device::CPU; + constexpr Type el_submatrix(1); + constexpr Type el_border(-1); + + const auto f_el_submatrix = [](const GlobalElementIndex&) { return el_submatrix; }; + const auto f_el_border = [](const GlobalElementIndex&) { return el_border; }; + + for (const auto& comm_grid : this->commGrids()) { + for (const auto& test : tests_sub_matrix) { + const auto f_el_full = [&](const GlobalElementIndex& index) { + return indexInSubMatrix(index, test.sub_offset, test.sub_size) ? el_submatrix : el_border; + }; + + Matrix mat_expected(test.size, test.block_size, comm_grid); + Matrix mat(test.size, test.block_size, comm_grid); + MatrixRef mat_ref(mat, test.sub_offset, test.sub_size); + + set(mat_expected, f_el_full); + set(mat, f_el_border); + for (const auto& ij_local : iterate_range2d(mat_ref.distribution().localNrTiles())) { + ex::start_detached(mat_ref.readwrite(ij_local) | + dlaf::internal::transform(dlaf::internal::Policy(), + [&](const auto& tile) { + set(tile, el_submatrix); + })); + } + + CHECK_MATRIX_EQ(f_el_full, mat); + CHECK_MATRIX_EQ(f_el_submatrix, mat_ref); + } + } +} From 493a29356d0784792dfa20c8fd7c619f2b87ca95 Mon Sep 17 00:00:00 2001 From: Mikael Simberg Date: Tue, 11 Jul 2023 14:41:41 +0200 Subject: [PATCH 03/18] Expand MatrixRef tests --- test/unit/matrix/test_matrix_ref.cpp | 88 ++++++++++++++++++++++++++++ 1 file changed, 88 insertions(+) diff --git a/test/unit/matrix/test_matrix_ref.cpp b/test/unit/matrix/test_matrix_ref.cpp index 496b3d67f3..ddd8399ef3 100644 --- a/test/unit/matrix/test_matrix_ref.cpp +++ b/test/unit/matrix/test_matrix_ref.cpp @@ -75,6 +75,7 @@ const std::vector tests_sub_matrix({ {{10, 15}, {5, 5}, {6, 7}, {2, 2}}, {{10, 15}, {5, 5}, {6, 7}, {4, 7}}, {{10, 15}, {5, 5}, {1, 2}, {8, 7}}, + {{256, 512}, {32, 16}, {45, 71}, {87, 55}}, }); inline bool indexInSubMatrix(const GlobalElementIndex& index, const GlobalElementIndex& offset, @@ -93,6 +94,38 @@ TYPED_TEST(MatrixRefTest, Basic) { const auto f_el_submatrix = [](const GlobalElementIndex&) { return el_submatrix; }; const auto f_el_border = [](const GlobalElementIndex&) { return el_border; }; + for (const auto& comm_grid : this->commGrids()) { + for (const auto& test : tests_sub_matrix) { + Matrix mat(test.size, test.block_size, comm_grid); + Matrix& mat_const = mat; + + MatrixRef mat_ref(mat, test.sub_offset, test.sub_size); + MatrixRef mat_const_ref1(mat, test.sub_offset, test.sub_size); + MatrixRef mat_const_ref2(mat_const, test.sub_offset, test.sub_size); + + EXPECT_EQ(mat_ref.distribution(), mat_const_ref1.distribution()); + EXPECT_EQ(mat_ref.distribution(), mat_const_ref2.distribution()); + EXPECT_EQ(mat_ref.size(), test.sub_size); + EXPECT_EQ(mat_ref.blockSize(), mat.blockSize()); + EXPECT_EQ(mat_ref.baseTileSize(), mat.baseTileSize()); + EXPECT_EQ(mat_ref.rankIndex(), mat.rankIndex()); + EXPECT_EQ(mat_ref.commGridSize(), mat.commGridSize()); + if (test.sub_offset.isIn(GlobalElementSize(test.block_size.rows(), test.block_size.cols()))) { + EXPECT_EQ(mat_ref.sourceRankIndex(), mat.sourceRankIndex()); + } + } + } +} + +TYPED_TEST(MatrixRefTest, NonConstRefFromNonConstMatrix) { + using Type = TypeParam; + constexpr Device device = Device::CPU; + constexpr Type el_submatrix(1); + constexpr Type el_border(-1); + + const auto f_el_submatrix = [](const GlobalElementIndex&) { return el_submatrix; }; + const auto f_el_border = [](const GlobalElementIndex&) { return el_border; }; + for (const auto& comm_grid : this->commGrids()) { for (const auto& test : tests_sub_matrix) { const auto f_el_full = [&](const GlobalElementIndex& index) { @@ -118,3 +151,58 @@ TYPED_TEST(MatrixRefTest, Basic) { } } } + +TYPED_TEST(MatrixRefTest, ConstRefFromNonConstMatrix) { + using Type = TypeParam; + constexpr Device device = Device::CPU; + constexpr Type el_submatrix(1); + constexpr Type el_border(-1); + + const auto f_el_submatrix = [](const GlobalElementIndex&) { return el_submatrix; }; + + for (const auto& comm_grid : this->commGrids()) { + for (const auto& test : tests_sub_matrix) { + const auto f_el_full = [&](const GlobalElementIndex& index) { + return indexInSubMatrix(index, test.sub_offset, test.sub_size) ? el_submatrix : el_border; + }; + + Matrix mat_expected(test.size, test.block_size, comm_grid); + Matrix mat(test.size, test.block_size, comm_grid); + MatrixRef mat_const_ref(mat, test.sub_offset, test.sub_size); + + set(mat_expected, f_el_full); + set(mat, f_el_full); + + CHECK_MATRIX_EQ(f_el_full, mat); + CHECK_MATRIX_EQ(f_el_submatrix, mat_const_ref); + } + } +} + +TYPED_TEST(MatrixRefTest, ConstRefFromConstMatrix) { + using Type = TypeParam; + constexpr Device device = Device::CPU; + constexpr Type el_submatrix(1); + constexpr Type el_border(-1); + + const auto f_el_submatrix = [](const GlobalElementIndex&) { return el_submatrix; }; + + for (const auto& comm_grid : this->commGrids()) { + for (const auto& test : tests_sub_matrix) { + const auto f_el_full = [&](const GlobalElementIndex& index) { + return indexInSubMatrix(index, test.sub_offset, test.sub_size) ? el_submatrix : el_border; + }; + + Matrix mat_expected(test.size, test.block_size, comm_grid); + Matrix mat(test.size, test.block_size, comm_grid); + Matrix& mat_const = mat; + MatrixRef mat_const_ref(mat_const, test.sub_offset, test.sub_size); + + set(mat_expected, f_el_full); + set(mat, f_el_full); + + CHECK_MATRIX_EQ(f_el_full, mat); + CHECK_MATRIX_EQ(f_el_submatrix, mat_const_ref); + } + } +} From 4fb2892277aaad565f9a7372d58c262f45c280dc Mon Sep 17 00:00:00 2001 From: Mikael Simberg Date: Tue, 11 Jul 2023 15:02:41 +0200 Subject: [PATCH 04/18] Support empty Matrix in MatrixRef --- include/dlaf/matrix/matrix_ref.h | 12 +++++------- test/unit/matrix/test_matrix_ref.cpp | 9 ++------- 2 files changed, 7 insertions(+), 14 deletions(-) diff --git a/include/dlaf/matrix/matrix_ref.h b/include/dlaf/matrix/matrix_ref.h index 906345c411..0b61f9c947 100644 --- a/include/dlaf/matrix/matrix_ref.h +++ b/include/dlaf/matrix/matrix_ref.h @@ -49,7 +49,7 @@ class MatrixRef : public internal::MatrixBase { /// @pre origin + size <= mat.size() MatrixRef(Matrix& mat, const GlobalElementIndex& offset, const GlobalElementSize& size) : internal::MatrixBase(Distribution(mat.distribution(), offset, size)), mat_const_(mat), - offset_(offset), tile_offset_(mat.distribution().globalTileIndex(offset)) {} + offset_(offset) {} // TODO: default, copy, move construction? // - default: no, don't want empty MatrixRef @@ -76,8 +76,8 @@ class MatrixRef : public internal::MatrixBase { ReadOnlySenderType read(const GlobalTileIndex& index) { DLAF_ASSERT(index.isIn(distribution().nrTiles()), index, distribution().nrTiles()); - const GlobalTileIndex parent_index(tile_offset_.row() + index.row(), - tile_offset_.col() + index.col()); + const GlobalTileIndex tile_offset = mat_const_.distribution().globalTileIndex(offset_); + const GlobalTileIndex parent_index(tile_offset.row() + index.row(), tile_offset.col() + index.col()); auto tile_sender = mat_const_.read(parent_index); const auto parent_dist = mat_const_.distribution(); @@ -108,7 +108,6 @@ class MatrixRef : public internal::MatrixBase { protected: GlobalElementIndex offset_; - GlobalTileIndex tile_offset_; }; template @@ -158,8 +157,8 @@ class MatrixRef : public MatrixRef { DLAF_ASSERT(index.isIn(this->distribution().nrTiles()), index, this->distribution().nrTiles()); // TODO: add helpers for distribution vs. sub-distribution arithmetic - const GlobalTileIndex parent_index(tile_offset_.row() + index.row(), - tile_offset_.col() + index.col()); + const GlobalTileIndex tile_offset = mat_.distribution().globalTileIndex(offset_); + const GlobalTileIndex parent_index(tile_offset.row() + index.row(), tile_offset.col() + index.col()); auto tile_sender = mat_.readwrite(parent_index); const auto parent_dist = mat_.distribution(); @@ -188,6 +187,5 @@ class MatrixRef : public MatrixRef { private: Matrix& mat_; using MatrixRef::offset_; - using MatrixRef::tile_offset_; }; } diff --git a/test/unit/matrix/test_matrix_ref.cpp b/test/unit/matrix/test_matrix_ref.cpp index ddd8399ef3..246e6cec1b 100644 --- a/test/unit/matrix/test_matrix_ref.cpp +++ b/test/unit/matrix/test_matrix_ref.cpp @@ -53,8 +53,8 @@ struct TestSubMatrix { }; const std::vector tests_sub_matrix({ - // Empty matrix TODO: support? - // {{0, 0}, {1, 1}, {0, 0}, {0, 0}}, + // Empty matrix + {{0, 0}, {1, 1}, {0, 0}, {0, 0}}, // Empty sub-matrices {{3, 4}, {3, 4}, {0, 0}, {0, 0}}, {{3, 4}, {3, 4}, {2, 3}, {0, 0}}, @@ -88,11 +88,6 @@ inline bool indexInSubMatrix(const GlobalElementIndex& index, const GlobalElemen TYPED_TEST(MatrixRefTest, Basic) { using Type = TypeParam; constexpr Device device = Device::CPU; - constexpr Type el_submatrix(1); - constexpr Type el_border(-1); - - const auto f_el_submatrix = [](const GlobalElementIndex&) { return el_submatrix; }; - const auto f_el_border = [](const GlobalElementIndex&) { return el_border; }; for (const auto& comm_grid : this->commGrids()) { for (const auto& test : tests_sub_matrix) { From 421692c490d2c82f137d2aa67fc5287469b1335f Mon Sep 17 00:00:00 2001 From: Mikael Simberg Date: Tue, 11 Jul 2023 15:30:21 +0200 Subject: [PATCH 05/18] Add TODO to MatrixRef --- include/dlaf/matrix/matrix_ref.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/include/dlaf/matrix/matrix_ref.h b/include/dlaf/matrix/matrix_ref.h index 0b61f9c947..397e675c33 100644 --- a/include/dlaf/matrix/matrix_ref.h +++ b/include/dlaf/matrix/matrix_ref.h @@ -57,6 +57,9 @@ class MatrixRef : public internal::MatrixBase { // - move: implement as copy, i.e. still refer to original matrix? MatrixRef() = delete; + // TODO: Do we need access to the original matrix? e.g: + // Matrix& get() const noexcept { return mat_const_; } + /// Returns a read-only sender of the Tile with local index @p index. /// /// @pre index.isIn(distribution().localNrTiles()). From 93982bf699d9f20292d9009d126822a3f24fc830 Mon Sep 17 00:00:00 2001 From: Mikael Simberg Date: Tue, 11 Jul 2023 15:37:43 +0200 Subject: [PATCH 06/18] Slightly simplify index arithmetic in MatrixRef --- include/dlaf/matrix/matrix_ref.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/dlaf/matrix/matrix_ref.h b/include/dlaf/matrix/matrix_ref.h index 397e675c33..74828772c4 100644 --- a/include/dlaf/matrix/matrix_ref.h +++ b/include/dlaf/matrix/matrix_ref.h @@ -80,7 +80,7 @@ class MatrixRef : public internal::MatrixBase { DLAF_ASSERT(index.isIn(distribution().nrTiles()), index, distribution().nrTiles()); const GlobalTileIndex tile_offset = mat_const_.distribution().globalTileIndex(offset_); - const GlobalTileIndex parent_index(tile_offset.row() + index.row(), tile_offset.col() + index.col()); + const GlobalTileIndex parent_index(tile_offset + sizeFromOrigin(index)); auto tile_sender = mat_const_.read(parent_index); const auto parent_dist = mat_const_.distribution(); @@ -161,7 +161,7 @@ class MatrixRef : public MatrixRef { // TODO: add helpers for distribution vs. sub-distribution arithmetic const GlobalTileIndex tile_offset = mat_.distribution().globalTileIndex(offset_); - const GlobalTileIndex parent_index(tile_offset.row() + index.row(), tile_offset.col() + index.col()); + const GlobalTileIndex parent_index(tile_offset + sizeFromOrigin(index)); auto tile_sender = mat_.readwrite(parent_index); const auto parent_dist = mat_.distribution(); From ade401d7e1b46b94bc3e67124cb56879297f21a4 Mon Sep 17 00:00:00 2001 From: Mikael Simberg Date: Tue, 11 Jul 2023 15:41:13 +0200 Subject: [PATCH 07/18] Slightly simplify index arithmetic in Distribution --- src/matrix/distribution.cpp | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/matrix/distribution.cpp b/src/matrix/distribution.cpp index 8b2cbe03cf..e033793f11 100644 --- a/src/matrix/distribution.cpp +++ b/src/matrix/distribution.cpp @@ -88,16 +88,15 @@ Distribution& Distribution::operator=(Distribution&& rhs) noexcept { return *this; } -Distribution::Distribution(Distribution rhs, const GlobalElementIndex& origin, +Distribution::Distribution(Distribution rhs, const GlobalElementIndex& sub_offset, const GlobalElementSize& size) : Distribution(std::move(rhs)) { - DLAF_ASSERT(origin.isValid(), origin); + DLAF_ASSERT(sub_offset.isValid(), sub_offset); DLAF_ASSERT(size.isValid(), size); - DLAF_ASSERT(origin.row() + size.rows() <= size_.rows(), origin, size_); - DLAF_ASSERT(origin.col() + size.cols() <= size_.cols(), origin, size_); + DLAF_ASSERT(sub_offset.row() + size.rows() <= size_.rows(), sub_offset, size_); + DLAF_ASSERT(sub_offset.col() + size.cols() <= size_.cols(), sub_offset, size_); - offset_ = GlobalElementIndex(offset_.get() + origin.get(), - offset_.get() + origin.get()); + offset_ = offset_ + sizeFromOrigin(sub_offset); size_ = size; computeGlobalAndLocalNrTilesAndLocalSize(); From 96499f4fbbe42bb30f38435e12f0458a177d73ca Mon Sep 17 00:00:00 2001 From: Mikael Simberg Date: Tue, 11 Jul 2023 16:14:47 +0200 Subject: [PATCH 08/18] Add more tests for sub-distributions --- test/unit/matrix/test_distribution.cpp | 45 ++++++++++++++++++-------- 1 file changed, 31 insertions(+), 14 deletions(-) diff --git a/test/unit/matrix/test_distribution.cpp b/test/unit/matrix/test_distribution.cpp index 0f0e7b3256..edf7161701 100644 --- a/test/unit/matrix/test_distribution.cpp +++ b/test/unit/matrix/test_distribution.cpp @@ -643,8 +643,8 @@ struct ParametersSubDistribution { comm::Index2D src_rank; GlobalElementIndex offset; // Sub-distribution settings - GlobalElementIndex suboffset; - GlobalElementSize subsize; + GlobalElementIndex sub_offset; + GlobalElementSize sub_size; // Valid indices GlobalElementIndex global_element; GlobalTileIndex global_tile; @@ -653,22 +653,37 @@ struct ParametersSubDistribution { }; const std::vector tests_sub_distribution = { - // {size, block_size, rank, grid_size, src_rank, offset, suboffset, subsize, + // {size, block_size, rank, grid_size, src_rank, offset, sub_offset, sub_size, // global_element, global_tile, rank_tile, local_tile} - - // TODO: More tests - + // Empty distribution + {{0, 0}, {2, 5}, {0, 0}, {1, 1}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0}}, + {{0, 0}, {2, 5}, {0, 0}, {1, 1}, {0, 0}, {4, 8}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0}}, + // Empty sub-distribution + {{3, 4}, {2, 2}, {0, 0}, {1, 1}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0}}, + {{3, 4}, {2, 2}, {0, 0}, {1, 1}, {0, 0}, {0, 0}, {2, 3}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0}}, + {{5, 9}, {3, 2}, {1, 1}, {2, 4}, {0, 2}, {1, 1}, {4, 5}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0}}, // Sub-distribution == distribution {{3, 4}, {2, 2}, {0, 0}, {1, 1}, {0, 0}, {0, 0}, {0, 0}, {3, 4}, {1, 3}, {0, 1}, {0, 0}, {0, 1}}, {{5, 9}, {3, 2}, {1, 1}, {2, 4}, {0, 2}, {1, 1}, {0, 0}, {5, 9}, {1, 3}, {0, 2}, {0, 0}, {-1, -1}}, + // clang-format off + {{123, 59}, {32, 16}, {3, 3}, {5, 7}, {3, 1}, {1, 1}, {0, 0}, {123, 59}, {30, 30}, {0, 1}, {3, 2}, {0, -1}}, + // clang-format on + // Other sub-distributions + {{3, 4}, {2, 2}, {0, 0}, {1, 1}, {0, 0}, {0, 0}, {1, 2}, {2, 1}, {0, 0}, {0, 0}, {0, 0}, {0, 0}}, + {{3, 4}, {2, 2}, {0, 0}, {1, 1}, {0, 0}, {0, 0}, {1, 2}, {2, 1}, {1, 0}, {1, 0}, {0, 0}, {1, 0}}, + {{5, 9}, {3, 2}, {1, 1}, {2, 4}, {0, 2}, {1, 1}, {3, 4}, {2, 3}, {0, 0}, {0, 0}, {1, 0}, {0, -1}}, + {{5, 9}, {3, 2}, {1, 1}, {2, 4}, {0, 2}, {1, 1}, {3, 4}, {2, 3}, {1, 2}, {0, 1}, {1, 1}, {0, 0}}, + // clang-format off + {{123, 59}, {32, 16}, {3, 3}, {5, 7}, {3, 1}, {1, 1}, {50, 17}, {40, 20}, {20, 10}, {1, 0}, {0, 2}, {-1, -1}}, + // clang-format on }; TEST(DistributionTest, SubDistribution) { for (const auto& test : tests_sub_distribution) { Distribution dist(test.size, test.block_size, test.grid_size, test.rank, test.src_rank, test.offset); - Distribution sub_dist(dist, test.suboffset, test.subsize); + Distribution sub_dist(dist, test.sub_offset, test.sub_size); - EXPECT_EQ(sub_dist.size(), test.subsize); + EXPECT_EQ(sub_dist.size(), test.sub_size); EXPECT_EQ(sub_dist.blockSize(), dist.blockSize()); EXPECT_EQ(sub_dist.baseTileSize(), dist.baseTileSize()); @@ -682,12 +697,14 @@ TEST(DistributionTest, SubDistribution) { EXPECT_LE(sub_dist.nrTiles().rows(), dist.nrTiles().rows()); EXPECT_LE(sub_dist.nrTiles().cols(), dist.nrTiles().cols()); - EXPECT_EQ(sub_dist.globalTileIndex(test.global_element), test.global_tile); - EXPECT_EQ(sub_dist.rankGlobalTile(sub_dist.globalTileIndex(test.global_element)), test.rank_tile); + if (!test.sub_size.isEmpty()) { + EXPECT_EQ(sub_dist.globalTileIndex(test.global_element), test.global_tile); + EXPECT_EQ(sub_dist.rankGlobalTile(sub_dist.globalTileIndex(test.global_element)), test.rank_tile); - EXPECT_EQ(sub_dist.localTileFromGlobalElement(test.global_element.get()), - test.local_tile[0]); - EXPECT_EQ(sub_dist.localTileFromGlobalElement(test.global_element.get()), - test.local_tile[1]); + EXPECT_EQ(sub_dist.localTileFromGlobalElement(test.global_element.get()), + test.local_tile[0]); + EXPECT_EQ(sub_dist.localTileFromGlobalElement(test.global_element.get()), + test.local_tile[1]); + } } } From ad68a4933e97bb40b1af2df692d8072943798cc6 Mon Sep 17 00:00:00 2001 From: Mikael Simberg Date: Tue, 11 Jul 2023 17:34:54 +0200 Subject: [PATCH 09/18] Add helpers for handling tiles and offsets in sub-distributions to Distribution --- include/dlaf/matrix/distribution.h | 40 +++++++++++++++++++++++ include/dlaf/matrix/matrix_ref.h | 51 ++++++++++-------------------- 2 files changed, 56 insertions(+), 35 deletions(-) diff --git a/include/dlaf/matrix/distribution.h b/include/dlaf/matrix/distribution.h index 588c41f2c1..7765f6ed29 100644 --- a/include/dlaf/matrix/distribution.h +++ b/include/dlaf/matrix/distribution.h @@ -501,6 +501,27 @@ class Distribution { localElementDistanceFromLocalTile(begin.col(), end.col())}; } + /// TODO + GlobalTileIndex globalTileIndexFromSubDistribution(const GlobalElementIndex& sub_offset, + const Distribution& sub_distribution, + const GlobalTileIndex& sub_index) const noexcept { + DLAF_ASSERT(sub_index.isIn(sub_distribution.nrTiles()), sub_index, sub_distribution.nrTiles()); + DLAF_ASSERT(isCompatibleSubDistribution(sub_offset, sub_distribution), ""); + const GlobalTileIndex tile_offset = globalTileIndex(sub_offset); + return tile_offset + common::sizeFromOrigin(sub_index); + } + + TileElementIndex tileElementOffsetFromSubDistribution( + const GlobalElementIndex& sub_offset, const Distribution& sub_distribution, + const GlobalTileIndex& sub_index) const noexcept { + DLAF_ASSERT(sub_index.isIn(sub_distribution.nrTiles()), sub_index, sub_distribution.nrTiles()); + DLAF_ASSERT(isCompatibleSubDistribution(sub_offset, sub_distribution), ""); + return { + sub_index.row() == 0 ? tileElementFromGlobalElement(sub_offset.row()) : 0, + sub_index.col() == 0 ? tileElementFromGlobalElement(sub_offset.col()) : 0, + }; + } + private: /// @pre block_size_, and tile_size_ are already set correctly. template @@ -575,6 +596,25 @@ class Distribution { /// @post offset_.row() < block_size_.rows() && offset_.col() < block_size_.cols() void normalizeSourceRankAndOffset() noexcept; + /// Checks if another distribution is a compatible sub-distribution of the current distribution. + /// + /// Compatible means that the block size, tile size, rank index, and grid size are equal. + /// Sub-distribution means that the source rank index of the sub-distribution is the rank index + /// of the tile at sub_offset in the current distribution. Additionally, the size and offset of + /// the sub-distribution must be within the size of the current distribution. + bool isCompatibleSubDistribution(const GlobalElementIndex& sub_offset, + const Distribution& sub_distribution) const noexcept { + const bool compatibleGrid = blockSize() == sub_distribution.blockSize() && + baseTileSize() == sub_distribution.baseTileSize() && + rankIndex() == sub_distribution.rankIndex() && + commGridSize() == sub_distribution.commGridSize(); + const bool compatibleSourceRankIndex = + rankGlobalTile(globalTileIndex(sub_offset)) == sub_distribution.sourceRankIndex(); + const bool compatibleSize = sub_offset.row() + sub_distribution.size().rows() <= size().rows() && + sub_offset.col() + sub_distribution.size().cols() <= size().cols(); + return compatibleGrid && compatibleSourceRankIndex && compatibleSize; + } + /// Sets default values. /// /// offset_ = {0, 0} diff --git a/include/dlaf/matrix/matrix_ref.h b/include/dlaf/matrix/matrix_ref.h index 74828772c4..52f5cc78a2 100644 --- a/include/dlaf/matrix/matrix_ref.h +++ b/include/dlaf/matrix/matrix_ref.h @@ -64,12 +64,9 @@ class MatrixRef : public internal::MatrixBase { /// /// @pre index.isIn(distribution().localNrTiles()). ReadOnlySenderType read(const LocalTileIndex& index) noexcept { - DLAF_ASSERT(index.isIn(distribution().localNrTiles()), index, distribution().localNrTiles()); - - // Note: the overload with GlobalTileIndex handles taking a subtile if needed - const GlobalTileIndex global_index(distribution().globalTileFromLocalTile(index.row()), - distribution().globalTileFromLocalTile(index.col())); - return read(global_index); + // Note: this forwards to the overload with GlobalTileIndex which will + // handle taking a subtile if needed + return read(distribution().globalTileIndex(index)); } /// Returns a read-only sender of the Tile with global index @p index. @@ -79,8 +76,8 @@ class MatrixRef : public internal::MatrixBase { ReadOnlySenderType read(const GlobalTileIndex& index) { DLAF_ASSERT(index.isIn(distribution().nrTiles()), index, distribution().nrTiles()); - const GlobalTileIndex tile_offset = mat_const_.distribution().globalTileIndex(offset_); - const GlobalTileIndex parent_index(tile_offset + sizeFromOrigin(index)); + const auto parent_index( + mat_const_.distribution().globalTileIndexFromSubDistribution(offset_, distribution(), index)); auto tile_sender = mat_const_.read(parent_index); const auto parent_dist = mat_const_.distribution(); @@ -89,20 +86,15 @@ class MatrixRef : public internal::MatrixBase { // If the corresponding tile in the parent distribution is exactly the same // size as the tile in the sub-distribution, we don't need to take a subtile - // and can return the tile sender directly. + // and can return the tile sender directly. This avoids unnecessary wrapping. if (parent_tile_size == tile_size) { return tile_sender; } // Otherwise we have to extract a subtile from the tile in the parent // distribution. - const TileElementIndex ij_tile{ - index.row() == 0 ? parent_dist.template tileElementFromGlobalElement(offset_.row()) - : 0, - index.col() == 0 ? parent_dist.template tileElementFromGlobalElement(offset_.col()) - : 0, - }; - + const auto ij_tile = + parent_dist.tileElementOffsetFromSubDistribution(offset_, distribution(), index); return splitTile(std::move(tile_sender), SubTileSpec{ij_tile, tile_size}); } @@ -142,14 +134,9 @@ class MatrixRef : public MatrixRef { /// /// @pre index.isIn(distribution().localNrTiles()). ReadWriteSenderType readwrite(const LocalTileIndex& index) noexcept { - DLAF_ASSERT(index.isIn(this->distribution().localNrTiles()), index, - this->distribution().localNrTiles()); - const GlobalTileIndex parent_index( - this->distribution().template globalTileFromLocalTile(index.row()), - this->distribution().template globalTileFromLocalTile(index.col())); - - // Note: the overload with GlobalTileIndex handles taking a subtile if needed - return readwrite(parent_index); + // Note: this forwards to the overload with GlobalTileIndex which will + // handle taking a subtile if needed + return readwrite(this->distribution().globalTileIndex(index)); } /// Returns a sender of the Tile with global index @p index. @@ -158,10 +145,9 @@ class MatrixRef : public MatrixRef { /// @pre index.isIn(globalNrTiles()). ReadWriteSenderType readwrite(const GlobalTileIndex& index) { DLAF_ASSERT(index.isIn(this->distribution().nrTiles()), index, this->distribution().nrTiles()); - // TODO: add helpers for distribution vs. sub-distribution arithmetic - const GlobalTileIndex tile_offset = mat_.distribution().globalTileIndex(offset_); - const GlobalTileIndex parent_index(tile_offset + sizeFromOrigin(index)); + const auto parent_index( + mat_.distribution().globalTileIndexFromSubDistribution(offset_, this->distribution(), index)); auto tile_sender = mat_.readwrite(parent_index); const auto parent_dist = mat_.distribution(); @@ -170,20 +156,15 @@ class MatrixRef : public MatrixRef { // If the corresponding tile in the parent distribution is exactly the same // size as the tile in the sub-distribution, we don't need to take a subtile - // and can return the tile sender directly. + // and can return the tile sender directly. This avoids unnecessary wrapping. if (parent_tile_size == tile_size) { return tile_sender; } // Otherwise we have to extract a subtile from the tile in the parent // distribution. - const TileElementIndex ij_tile{ - index.row() == 0 ? parent_dist.template tileElementFromGlobalElement(offset_.row()) - : 0, - index.col() == 0 ? parent_dist.template tileElementFromGlobalElement(offset_.col()) - : 0, - }; - + const auto ij_tile = + parent_dist.tileElementOffsetFromSubDistribution(offset_, this->distribution(), index); return splitTile(std::move(tile_sender), SubTileSpec{ij_tile, tile_size}); } From 2010844f372b9bd83c9a6685bb7170c433f44da1 Mon Sep 17 00:00:00 2001 From: Mikael Simberg Date: Wed, 12 Jul 2023 10:33:29 +0200 Subject: [PATCH 10/18] Add documentation for sub-distribution helpers in Distribution --- include/dlaf/matrix/distribution.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/include/dlaf/matrix/distribution.h b/include/dlaf/matrix/distribution.h index 7765f6ed29..fbc559bea0 100644 --- a/include/dlaf/matrix/distribution.h +++ b/include/dlaf/matrix/distribution.h @@ -501,7 +501,8 @@ class Distribution { localElementDistanceFromLocalTile(begin.col(), end.col())}; } - /// TODO + /// Returns the tile index in the current distribution corresponding to a tile index @p sub_index in a + /// sub-distribution (defined by @p sub_offset and @p sub_distribution) GlobalTileIndex globalTileIndexFromSubDistribution(const GlobalElementIndex& sub_offset, const Distribution& sub_distribution, const GlobalTileIndex& sub_index) const noexcept { @@ -511,6 +512,8 @@ class Distribution { return tile_offset + common::sizeFromOrigin(sub_index); } + /// Returns the element offset within the tile in the current distribution corresponding to a tile + /// index @p sub_index in a sub-distribution (defined by @p sub_offset and @p sub_distribution) TileElementIndex tileElementOffsetFromSubDistribution( const GlobalElementIndex& sub_offset, const Distribution& sub_distribution, const GlobalTileIndex& sub_index) const noexcept { From d17c9a127116d1b5e9ffc656908b36a53501bfe8 Mon Sep 17 00:00:00 2001 From: Mikael Simberg Date: Wed, 12 Jul 2023 10:39:47 +0200 Subject: [PATCH 11/18] Capture some variables in test_matrix_ref --- test/unit/matrix/test_matrix_ref.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/test/unit/matrix/test_matrix_ref.cpp b/test/unit/matrix/test_matrix_ref.cpp index 246e6cec1b..90c125e151 100644 --- a/test/unit/matrix/test_matrix_ref.cpp +++ b/test/unit/matrix/test_matrix_ref.cpp @@ -118,12 +118,12 @@ TYPED_TEST(MatrixRefTest, NonConstRefFromNonConstMatrix) { constexpr Type el_submatrix(1); constexpr Type el_border(-1); - const auto f_el_submatrix = [](const GlobalElementIndex&) { return el_submatrix; }; - const auto f_el_border = [](const GlobalElementIndex&) { return el_border; }; + const auto f_el_submatrix = [=](const GlobalElementIndex&) { return el_submatrix; }; + const auto f_el_border = [=](const GlobalElementIndex&) { return el_border; }; for (const auto& comm_grid : this->commGrids()) { for (const auto& test : tests_sub_matrix) { - const auto f_el_full = [&](const GlobalElementIndex& index) { + const auto f_el_full = [=](const GlobalElementIndex& index) { return indexInSubMatrix(index, test.sub_offset, test.sub_size) ? el_submatrix : el_border; }; @@ -136,7 +136,7 @@ TYPED_TEST(MatrixRefTest, NonConstRefFromNonConstMatrix) { for (const auto& ij_local : iterate_range2d(mat_ref.distribution().localNrTiles())) { ex::start_detached(mat_ref.readwrite(ij_local) | dlaf::internal::transform(dlaf::internal::Policy(), - [&](const auto& tile) { + [=](const auto& tile) { set(tile, el_submatrix); })); } @@ -153,11 +153,11 @@ TYPED_TEST(MatrixRefTest, ConstRefFromNonConstMatrix) { constexpr Type el_submatrix(1); constexpr Type el_border(-1); - const auto f_el_submatrix = [](const GlobalElementIndex&) { return el_submatrix; }; + const auto f_el_submatrix = [=](const GlobalElementIndex&) { return el_submatrix; }; for (const auto& comm_grid : this->commGrids()) { for (const auto& test : tests_sub_matrix) { - const auto f_el_full = [&](const GlobalElementIndex& index) { + const auto f_el_full = [=](const GlobalElementIndex& index) { return indexInSubMatrix(index, test.sub_offset, test.sub_size) ? el_submatrix : el_border; }; @@ -180,11 +180,11 @@ TYPED_TEST(MatrixRefTest, ConstRefFromConstMatrix) { constexpr Type el_submatrix(1); constexpr Type el_border(-1); - const auto f_el_submatrix = [](const GlobalElementIndex&) { return el_submatrix; }; + const auto f_el_submatrix = [=](const GlobalElementIndex&) { return el_submatrix; }; for (const auto& comm_grid : this->commGrids()) { for (const auto& test : tests_sub_matrix) { - const auto f_el_full = [&](const GlobalElementIndex& index) { + const auto f_el_full = [=](const GlobalElementIndex& index) { return indexInSubMatrix(index, test.sub_offset, test.sub_size) ? el_submatrix : el_border; }; From a8c848f3e176fdf3ac0b2b77022629880b60c795 Mon Sep 17 00:00:00 2001 From: Mikael Simberg Date: Wed, 12 Jul 2023 13:33:51 +0200 Subject: [PATCH 12/18] Explicitly instantiate MatrixRef --- include/dlaf/matrix/matrix_ref.h | 18 ++++++++++++++++++ src/CMakeLists.txt | 1 + src/matrix/matrix_ref.cpp | 26 ++++++++++++++++++++++++++ 3 files changed, 45 insertions(+) create mode 100644 src/matrix/matrix_ref.cpp diff --git a/include/dlaf/matrix/matrix_ref.h b/include/dlaf/matrix/matrix_ref.h index 52f5cc78a2..b2adfe4c23 100644 --- a/include/dlaf/matrix/matrix_ref.h +++ b/include/dlaf/matrix/matrix_ref.h @@ -172,4 +172,22 @@ class MatrixRef : public MatrixRef { Matrix& mat_; using MatrixRef::offset_; }; + +// ETI + +#define DLAF_MATRIX_REF_ETI(KWORD, DATATYPE, DEVICE) \ + KWORD template class MatrixRef; \ + KWORD template class MatrixRef; + +DLAF_MATRIX_REF_ETI(extern, float, Device::CPU) +DLAF_MATRIX_REF_ETI(extern, double, Device::CPU) +DLAF_MATRIX_REF_ETI(extern, std::complex, Device::CPU) +DLAF_MATRIX_REF_ETI(extern, std::complex, Device::CPU) + +#if defined(DLAF_WITH_GPU) +DLAF_MATRIX_REF_ETI(extern, float, Device::GPU) +DLAF_MATRIX_REF_ETI(extern, double, Device::GPU) +DLAF_MATRIX_REF_ETI(extern, std::complex, Device::GPU) +DLAF_MATRIX_REF_ETI(extern, std::complex, Device::GPU) +#endif } diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 39e9dc554a..6eb6526f09 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -193,6 +193,7 @@ DLAF_addSublibrary( init.cpp matrix/distribution.cpp matrix/layout_info.cpp + matrix/matrix_ref.cpp matrix/retiled_matrix.cpp matrix/tile.cpp matrix.cpp diff --git a/src/matrix/matrix_ref.cpp b/src/matrix/matrix_ref.cpp new file mode 100644 index 0000000000..a67c44bb0d --- /dev/null +++ b/src/matrix/matrix_ref.cpp @@ -0,0 +1,26 @@ +// +// Distributed Linear Algebra with Future (DLAF) +// +// Copyright (c) 2018-2023, ETH Zurich +// All rights reserved. +// +// Please, refer to the LICENSE file in the root directory. +// SPDX-License-Identifier: BSD-3-Clause +// + +#include + +namespace dlaf::matrix { + +DLAF_MATRIX_REF_ETI(, float, Device::CPU) +DLAF_MATRIX_REF_ETI(, double, Device::CPU) +DLAF_MATRIX_REF_ETI(, std::complex, Device::CPU) +DLAF_MATRIX_REF_ETI(, std::complex, Device::CPU) + +#if defined(DLAF_WITH_GPU) +DLAF_MATRIX_REF_ETI(, float, Device::GPU) +DLAF_MATRIX_REF_ETI(, double, Device::GPU) +DLAF_MATRIX_REF_ETI(, std::complex, Device::GPU) +DLAF_MATRIX_REF_ETI(, std::complex, Device::GPU) +#endif +} From a49a9acc30fb304ad8da4df3c29f80b545830b1f Mon Sep 17 00:00:00 2001 From: Mikael Simberg Date: Wed, 12 Jul 2023 14:25:51 +0200 Subject: [PATCH 13/18] Remove MatrixRef::get TODO --- include/dlaf/matrix/matrix_ref.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/include/dlaf/matrix/matrix_ref.h b/include/dlaf/matrix/matrix_ref.h index b2adfe4c23..47ff51b21f 100644 --- a/include/dlaf/matrix/matrix_ref.h +++ b/include/dlaf/matrix/matrix_ref.h @@ -57,9 +57,6 @@ class MatrixRef : public internal::MatrixBase { // - move: implement as copy, i.e. still refer to original matrix? MatrixRef() = delete; - // TODO: Do we need access to the original matrix? e.g: - // Matrix& get() const noexcept { return mat_const_; } - /// Returns a read-only sender of the Tile with local index @p index. /// /// @pre index.isIn(distribution().localNrTiles()). From 52a815ba1d58303baa4ef327a5e1086e83b79fcc Mon Sep 17 00:00:00 2001 From: Mikael Simberg Date: Wed, 12 Jul 2023 14:26:48 +0200 Subject: [PATCH 14/18] Reformat matrix_ref.h --- include/dlaf/matrix/matrix_ref.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/dlaf/matrix/matrix_ref.h b/include/dlaf/matrix/matrix_ref.h index 47ff51b21f..9529571efb 100644 --- a/include/dlaf/matrix/matrix_ref.h +++ b/include/dlaf/matrix/matrix_ref.h @@ -173,7 +173,7 @@ class MatrixRef : public MatrixRef { // ETI #define DLAF_MATRIX_REF_ETI(KWORD, DATATYPE, DEVICE) \ - KWORD template class MatrixRef; \ + KWORD template class MatrixRef; \ KWORD template class MatrixRef; DLAF_MATRIX_REF_ETI(extern, float, Device::CPU) From 1ff2f5355c95ce49aa9a82ab3cc1ccd75be2b2cb Mon Sep 17 00:00:00 2001 From: Mikael Simberg Date: Fri, 28 Jul 2023 12:23:20 +0200 Subject: [PATCH 15/18] Introduce SubDistributionSpec and SubMatrixSpec for specifying sub-distributions and sub-matrices, respectively --- include/dlaf/matrix/distribution.h | 21 +++++++----- include/dlaf/matrix/matrix_ref.h | 47 +++++++++++++------------- src/matrix/distribution.cpp | 15 ++++---- test/unit/matrix/test_distribution.cpp | 7 ++-- test/unit/matrix/test_matrix_ref.cpp | 33 ++++++++++-------- 5 files changed, 65 insertions(+), 58 deletions(-) diff --git a/include/dlaf/matrix/distribution.h b/include/dlaf/matrix/distribution.h index fbc559bea0..007201df8e 100644 --- a/include/dlaf/matrix/distribution.h +++ b/include/dlaf/matrix/distribution.h @@ -20,10 +20,15 @@ namespace dlaf { namespace matrix { +/// Contains information to create a sub-distribution. +struct SubDistributionSpec { + GlobalElementIndex origin; + GlobalElementSize size; +}; + /// Distribution contains the information about the size and distribution of a matrix. /// /// More details available in misc/matrix_distribution.md. - class Distribution { public: /// Constructs a distribution for a non distributed matrix of size {0, 0} and block size {1, 1}. @@ -119,16 +124,14 @@ class Distribution { Distribution& operator=(Distribution&& rhs) noexcept; - /// Constructs a sub-distribution based on the given distribution @p dist with - /// an @p offset and @p size. + /// Constructs a sub-distribution based on the given distribution @p dist specified by @p spec. /// /// @param[in] dist is the input distribution, - /// @param[in] offset is the offset of the new distribution relative to the input distribution, - /// @param[in] size is the size of the new distribution relative to the offset, - /// @pre origin.isValid() - /// @pre size.isValid() - /// @pre origin + size <= dist.size() - Distribution(Distribution dist, const GlobalElementIndex& offset, const GlobalElementSize& size); + /// @param[in] spec contains the origin and size of the new distribution relative to the input distribution, + /// @pre spec.origin.isValid() + /// @pre spec.size.isValid() + /// @pre spec.origin + spec.size <= dist.size() + Distribution(Distribution dist, const SubDistributionSpec& spec); bool operator==(const Distribution& rhs) const noexcept { return size_ == rhs.size_ && local_size_ == rhs.local_size_ && tile_size_ == rhs.tile_size_ && diff --git a/include/dlaf/matrix/matrix_ref.h b/include/dlaf/matrix/matrix_ref.h index 9529571efb..75289a56e9 100644 --- a/include/dlaf/matrix/matrix_ref.h +++ b/include/dlaf/matrix/matrix_ref.h @@ -19,6 +19,9 @@ #include namespace dlaf::matrix { +/// Contains information to create a sub-matrix. +using SubMatrixSpec = SubDistributionSpec; + /// A @c MatrixRef represents a sub-matrix of a @c Matrix. /// /// The class has reference semantics, meaning accesses to a @c MatrixRef and @@ -39,17 +42,16 @@ class MatrixRef : public internal::MatrixBase { using TileDataType = internal::TileData; using ReadOnlySenderType = ReadOnlyTileSender; - /// Create a sub-matrix of @p mat with an @p offset and @p size. + /// Create a sub-matrix of @p mat specified by @p spec. /// /// @param[in] mat is the input matrix, - /// @param[in] offset is the offset of the new matrix relative to the input matrix, - /// @param[in] size is the size of the new matrix relative to the offset, - /// @pre origin.isValid() - /// @pre size.isValid() - /// @pre origin + size <= mat.size() - MatrixRef(Matrix& mat, const GlobalElementIndex& offset, const GlobalElementSize& size) - : internal::MatrixBase(Distribution(mat.distribution(), offset, size)), mat_const_(mat), - offset_(offset) {} + /// @param[in] spec contains the origin and size of the new matrix relative to the input matrix, + /// @pre spec.origin.isValid(), + /// @pre spec.size.isValid(), + /// @pre spec.origin + spec.size <= mat.size(). + MatrixRef(Matrix& mat, const SubMatrixSpec& spec) + : internal::MatrixBase(Distribution(mat.distribution(), spec)), mat_const_(mat), + origin_(spec.origin) {} // TODO: default, copy, move construction? // - default: no, don't want empty MatrixRef @@ -74,7 +76,7 @@ class MatrixRef : public internal::MatrixBase { DLAF_ASSERT(index.isIn(distribution().nrTiles()), index, distribution().nrTiles()); const auto parent_index( - mat_const_.distribution().globalTileIndexFromSubDistribution(offset_, distribution(), index)); + mat_const_.distribution().globalTileIndexFromSubDistribution(origin_, distribution(), index)); auto tile_sender = mat_const_.read(parent_index); const auto parent_dist = mat_const_.distribution(); @@ -91,7 +93,7 @@ class MatrixRef : public internal::MatrixBase { // Otherwise we have to extract a subtile from the tile in the parent // distribution. const auto ij_tile = - parent_dist.tileElementOffsetFromSubDistribution(offset_, distribution(), index); + parent_dist.tileElementOffsetFromSubDistribution(origin_, distribution(), index); return splitTile(std::move(tile_sender), SubTileSpec{ij_tile, tile_size}); } @@ -99,7 +101,7 @@ class MatrixRef : public internal::MatrixBase { Matrix& mat_const_; protected: - GlobalElementIndex offset_; + GlobalElementIndex origin_; }; template @@ -113,16 +115,15 @@ class MatrixRef : public MatrixRef { using TileDataType = internal::TileData; using ReadWriteSenderType = ReadWriteTileSender; - /// Create a sub-matrix of @p mat with an @p offset and @p size. + /// Create a sub-matrix of @p mat specified by @p spec. /// /// @param[in] mat is the input matrix, - /// @param[in] offset is the offset of the new matrix relative to the input matrix, - /// @param[in] size is the size of the new matrix relative to the offset, - /// @pre origin.isValid() - /// @pre size.isValid() - /// @pre origin + size <= mat.size() - MatrixRef(Matrix& mat, const GlobalElementIndex& offset, const GlobalElementSize& size) - : MatrixRef(mat, offset, size), mat_(mat) {} + /// @param[in] spec contains the origin and size of the new matrix relative to the input matrix, + /// @pre spec.origin.isValid(), + /// @pre spec.size.isValid(), + /// @pre spec.origin + spec.size <= mat.size(). + MatrixRef(Matrix& mat, const SubMatrixSpec& spec) + : MatrixRef(mat, spec), mat_(mat) {} // TODO: default, copy, move construction? MatrixRef() = delete; @@ -144,7 +145,7 @@ class MatrixRef : public MatrixRef { DLAF_ASSERT(index.isIn(this->distribution().nrTiles()), index, this->distribution().nrTiles()); const auto parent_index( - mat_.distribution().globalTileIndexFromSubDistribution(offset_, this->distribution(), index)); + mat_.distribution().globalTileIndexFromSubDistribution(origin_, this->distribution(), index)); auto tile_sender = mat_.readwrite(parent_index); const auto parent_dist = mat_.distribution(); @@ -161,13 +162,13 @@ class MatrixRef : public MatrixRef { // Otherwise we have to extract a subtile from the tile in the parent // distribution. const auto ij_tile = - parent_dist.tileElementOffsetFromSubDistribution(offset_, this->distribution(), index); + parent_dist.tileElementOffsetFromSubDistribution(origin_, this->distribution(), index); return splitTile(std::move(tile_sender), SubTileSpec{ij_tile, tile_size}); } private: Matrix& mat_; - using MatrixRef::offset_; + using MatrixRef::origin_; }; // ETI diff --git a/src/matrix/distribution.cpp b/src/matrix/distribution.cpp index e033793f11..d4d96e0f72 100644 --- a/src/matrix/distribution.cpp +++ b/src/matrix/distribution.cpp @@ -88,16 +88,15 @@ Distribution& Distribution::operator=(Distribution&& rhs) noexcept { return *this; } -Distribution::Distribution(Distribution rhs, const GlobalElementIndex& sub_offset, - const GlobalElementSize& size) +Distribution::Distribution(Distribution rhs, const SubDistributionSpec& spec) : Distribution(std::move(rhs)) { - DLAF_ASSERT(sub_offset.isValid(), sub_offset); - DLAF_ASSERT(size.isValid(), size); - DLAF_ASSERT(sub_offset.row() + size.rows() <= size_.rows(), sub_offset, size_); - DLAF_ASSERT(sub_offset.col() + size.cols() <= size_.cols(), sub_offset, size_); + DLAF_ASSERT(spec.origin.isValid(), spec.origin); + DLAF_ASSERT(spec.size.isValid(), spec.size); + DLAF_ASSERT(spec.origin.row() + spec.size.rows() <= size_.rows(), spec.origin, spec.size, size_); + DLAF_ASSERT(spec.origin.col() + spec.size.cols() <= size_.cols(), spec.origin, spec.size, size_); - offset_ = offset_ + sizeFromOrigin(sub_offset); - size_ = size; + offset_ = offset_ + sizeFromOrigin(spec.origin); + size_ = spec.size; computeGlobalAndLocalNrTilesAndLocalSize(); } diff --git a/test/unit/matrix/test_distribution.cpp b/test/unit/matrix/test_distribution.cpp index edf7161701..6dd48be898 100644 --- a/test/unit/matrix/test_distribution.cpp +++ b/test/unit/matrix/test_distribution.cpp @@ -643,7 +643,7 @@ struct ParametersSubDistribution { comm::Index2D src_rank; GlobalElementIndex offset; // Sub-distribution settings - GlobalElementIndex sub_offset; + GlobalElementIndex sub_origin; GlobalElementSize sub_size; // Valid indices GlobalElementIndex global_element; @@ -653,7 +653,7 @@ struct ParametersSubDistribution { }; const std::vector tests_sub_distribution = { - // {size, block_size, rank, grid_size, src_rank, offset, sub_offset, sub_size, + // {size, block_size, rank, grid_size, src_rank, offset, sub_origin, sub_size, // global_element, global_tile, rank_tile, local_tile} // Empty distribution {{0, 0}, {2, 5}, {0, 0}, {1, 1}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0}}, @@ -681,7 +681,8 @@ const std::vector tests_sub_distribution = { TEST(DistributionTest, SubDistribution) { for (const auto& test : tests_sub_distribution) { Distribution dist(test.size, test.block_size, test.grid_size, test.rank, test.src_rank, test.offset); - Distribution sub_dist(dist, test.sub_offset, test.sub_size); + const SubDistributionSpec spec{test.sub_origin, test.sub_size}; + Distribution sub_dist(dist, spec); EXPECT_EQ(sub_dist.size(), test.sub_size); diff --git a/test/unit/matrix/test_matrix_ref.cpp b/test/unit/matrix/test_matrix_ref.cpp index 90c125e151..910ad5297e 100644 --- a/test/unit/matrix/test_matrix_ref.cpp +++ b/test/unit/matrix/test_matrix_ref.cpp @@ -48,7 +48,7 @@ TYPED_TEST_SUITE(MatrixRefTest, MatrixElementTypes); struct TestSubMatrix { GlobalElementSize size; TileElementSize block_size; - GlobalElementIndex sub_offset; + GlobalElementIndex sub_origin; GlobalElementSize sub_size; }; @@ -78,10 +78,9 @@ const std::vector tests_sub_matrix({ {{256, 512}, {32, 16}, {45, 71}, {87, 55}}, }); -inline bool indexInSubMatrix(const GlobalElementIndex& index, const GlobalElementIndex& offset, - const GlobalElementSize& size) { - bool r = offset.row() <= index.row() && index.row() < offset.row() + size.rows() && - offset.col() <= index.col() && index.col() < offset.col() + size.cols(); +inline bool indexInSubMatrix(const GlobalElementIndex& index, const SubMatrixSpec& spec) { + bool r = spec.origin.row() <= index.row() && index.row() < spec.origin.row() + spec.size.rows() && + spec.origin.col() <= index.col() && index.col() < spec.origin.col() + spec.size.cols(); return r; } @@ -94,9 +93,10 @@ TYPED_TEST(MatrixRefTest, Basic) { Matrix mat(test.size, test.block_size, comm_grid); Matrix& mat_const = mat; - MatrixRef mat_ref(mat, test.sub_offset, test.sub_size); - MatrixRef mat_const_ref1(mat, test.sub_offset, test.sub_size); - MatrixRef mat_const_ref2(mat_const, test.sub_offset, test.sub_size); + const SubMatrixSpec spec{test.sub_origin, test.sub_size}; + MatrixRef mat_ref(mat, spec); + MatrixRef mat_const_ref1(mat, spec); + MatrixRef mat_const_ref2(mat_const, spec); EXPECT_EQ(mat_ref.distribution(), mat_const_ref1.distribution()); EXPECT_EQ(mat_ref.distribution(), mat_const_ref2.distribution()); @@ -105,7 +105,7 @@ TYPED_TEST(MatrixRefTest, Basic) { EXPECT_EQ(mat_ref.baseTileSize(), mat.baseTileSize()); EXPECT_EQ(mat_ref.rankIndex(), mat.rankIndex()); EXPECT_EQ(mat_ref.commGridSize(), mat.commGridSize()); - if (test.sub_offset.isIn(GlobalElementSize(test.block_size.rows(), test.block_size.cols()))) { + if (test.sub_origin.isIn(GlobalElementSize(test.block_size.rows(), test.block_size.cols()))) { EXPECT_EQ(mat_ref.sourceRankIndex(), mat.sourceRankIndex()); } } @@ -123,13 +123,14 @@ TYPED_TEST(MatrixRefTest, NonConstRefFromNonConstMatrix) { for (const auto& comm_grid : this->commGrids()) { for (const auto& test : tests_sub_matrix) { + const SubMatrixSpec spec{test.sub_origin, test.sub_size}; const auto f_el_full = [=](const GlobalElementIndex& index) { - return indexInSubMatrix(index, test.sub_offset, test.sub_size) ? el_submatrix : el_border; + return indexInSubMatrix(index, spec) ? el_submatrix : el_border; }; Matrix mat_expected(test.size, test.block_size, comm_grid); Matrix mat(test.size, test.block_size, comm_grid); - MatrixRef mat_ref(mat, test.sub_offset, test.sub_size); + MatrixRef mat_ref(mat, spec); set(mat_expected, f_el_full); set(mat, f_el_border); @@ -157,13 +158,14 @@ TYPED_TEST(MatrixRefTest, ConstRefFromNonConstMatrix) { for (const auto& comm_grid : this->commGrids()) { for (const auto& test : tests_sub_matrix) { + const SubMatrixSpec spec{test.sub_origin, test.sub_size}; const auto f_el_full = [=](const GlobalElementIndex& index) { - return indexInSubMatrix(index, test.sub_offset, test.sub_size) ? el_submatrix : el_border; + return indexInSubMatrix(index, spec) ? el_submatrix : el_border; }; Matrix mat_expected(test.size, test.block_size, comm_grid); Matrix mat(test.size, test.block_size, comm_grid); - MatrixRef mat_const_ref(mat, test.sub_offset, test.sub_size); + MatrixRef mat_const_ref(mat, spec); set(mat_expected, f_el_full); set(mat, f_el_full); @@ -184,14 +186,15 @@ TYPED_TEST(MatrixRefTest, ConstRefFromConstMatrix) { for (const auto& comm_grid : this->commGrids()) { for (const auto& test : tests_sub_matrix) { + const SubMatrixSpec spec{test.sub_origin, test.sub_size}; const auto f_el_full = [=](const GlobalElementIndex& index) { - return indexInSubMatrix(index, test.sub_offset, test.sub_size) ? el_submatrix : el_border; + return indexInSubMatrix(index, spec) ? el_submatrix : el_border; }; Matrix mat_expected(test.size, test.block_size, comm_grid); Matrix mat(test.size, test.block_size, comm_grid); Matrix& mat_const = mat; - MatrixRef mat_const_ref(mat_const, test.sub_offset, test.sub_size); + MatrixRef mat_const_ref(mat_const, spec); set(mat_expected, f_el_full); set(mat, f_el_full); From 1a512bdd66eff46d0f8e66211d215fc8469dae2e Mon Sep 17 00:00:00 2001 From: Mikael Simberg Date: Fri, 28 Jul 2023 12:30:04 +0200 Subject: [PATCH 16/18] Make MatrixRef immovable --- include/dlaf/matrix/matrix_ref.h | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/include/dlaf/matrix/matrix_ref.h b/include/dlaf/matrix/matrix_ref.h index 75289a56e9..7c213917da 100644 --- a/include/dlaf/matrix/matrix_ref.h +++ b/include/dlaf/matrix/matrix_ref.h @@ -53,11 +53,11 @@ class MatrixRef : public internal::MatrixBase { : internal::MatrixBase(Distribution(mat.distribution(), spec)), mat_const_(mat), origin_(spec.origin) {} - // TODO: default, copy, move construction? - // - default: no, don't want empty MatrixRef - // - copy: implementable, still refer to the original matrix - // - move: implement as copy, i.e. still refer to original matrix? MatrixRef() = delete; + MatrixRef(MatrixRef&&) = delete; + MatrixRef(const MatrixRef&) = delete; + MatrixRef& operator=(MatrixRef&&) = delete; + MatrixRef& operator=(const MatrixRef&) = delete; /// Returns a read-only sender of the Tile with local index @p index. /// @@ -125,8 +125,11 @@ class MatrixRef : public MatrixRef { MatrixRef(Matrix& mat, const SubMatrixSpec& spec) : MatrixRef(mat, spec), mat_(mat) {} - // TODO: default, copy, move construction? MatrixRef() = delete; + MatrixRef(MatrixRef&&) = delete; + MatrixRef(const MatrixRef&) = delete; + MatrixRef& operator=(MatrixRef&&) = delete; + MatrixRef& operator=(const MatrixRef&) = delete; /// Returns a sender of the Tile with local index @p index. /// From 9b71fefdc4e242668d274bb28443e953cc88205f Mon Sep 17 00:00:00 2001 From: Mikael Simberg Date: Fri, 28 Jul 2023 12:31:32 +0200 Subject: [PATCH 17/18] Move MatrixRef to internal namespace --- include/dlaf/matrix/matrix_ref.h | 2 +- src/matrix/matrix_ref.cpp | 2 +- test/unit/matrix/test_matrix_ref.cpp | 1 + 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/include/dlaf/matrix/matrix_ref.h b/include/dlaf/matrix/matrix_ref.h index 7c213917da..d8fd64258a 100644 --- a/include/dlaf/matrix/matrix_ref.h +++ b/include/dlaf/matrix/matrix_ref.h @@ -18,7 +18,7 @@ #include #include -namespace dlaf::matrix { +namespace dlaf::matrix::internal { /// Contains information to create a sub-matrix. using SubMatrixSpec = SubDistributionSpec; diff --git a/src/matrix/matrix_ref.cpp b/src/matrix/matrix_ref.cpp index a67c44bb0d..6c9e759d18 100644 --- a/src/matrix/matrix_ref.cpp +++ b/src/matrix/matrix_ref.cpp @@ -10,7 +10,7 @@ #include -namespace dlaf::matrix { +namespace dlaf::matrix::internal { DLAF_MATRIX_REF_ETI(, float, Device::CPU) DLAF_MATRIX_REF_ETI(, double, Device::CPU) diff --git a/test/unit/matrix/test_matrix_ref.cpp b/test/unit/matrix/test_matrix_ref.cpp index 910ad5297e..1a0cdbbb7b 100644 --- a/test/unit/matrix/test_matrix_ref.cpp +++ b/test/unit/matrix/test_matrix_ref.cpp @@ -30,6 +30,7 @@ using namespace std::chrono_literals; using namespace dlaf; using namespace dlaf::matrix; +using namespace dlaf::matrix::internal; using namespace dlaf::matrix::test; using namespace dlaf::test; using namespace testing; From 0da1cd015a6a2083cf2703d626c287062fcd9f9f Mon Sep 17 00:00:00 2001 From: Mikael Simberg Date: Fri, 28 Jul 2023 12:32:14 +0200 Subject: [PATCH 18/18] Remove unused headers from MatrixRef test --- test/unit/matrix/test_matrix_ref.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/test/unit/matrix/test_matrix_ref.cpp b/test/unit/matrix/test_matrix_ref.cpp index 1a0cdbbb7b..0d29798685 100644 --- a/test/unit/matrix/test_matrix_ref.cpp +++ b/test/unit/matrix/test_matrix_ref.cpp @@ -8,8 +8,6 @@ // SPDX-License-Identifier: BSD-3-Clause // -#include -#include #include #include @@ -26,8 +24,6 @@ #include #include -using namespace std::chrono_literals; - using namespace dlaf; using namespace dlaf::matrix; using namespace dlaf::matrix::internal;