Skip to content

Commit

Permalink
review updates
Browse files Browse the repository at this point in the history 
Co-authored-by: Tobias Ribizel <ribizel@kit.edu>
Co-authored-by: Pratik Nayak <pratik.nayak@kit.edu>
  • Loading branch information
@MarcelKoch @upsj @pratikvn
3 people committed Mar 29, 2022
1 parent b1ee7d2 commit d050278
Show file tree
Hide file tree
Showing 6 changed files with 51 additions and 35 deletions.
16 changes: 8 additions & 8 deletions core/distributed/matrix.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,15 +83,15 @@ Matrix<ValueType, LocalIndexType, GlobalIndexType>::Matrix(
gather_idxs_{exec},
local_to_global_ghost_{exec},
one_scalar_{},
diag_mtx_{inner_matrix_type->create_default(exec)},
offdiag_mtx_{ghost_matrix_type->create_default(exec)}
diag_mtx_{inner_matrix_type->clone(exec)},
offdiag_mtx_{ghost_matrix_type->clone(exec)}
{
GKO_ASSERT(GKO_QUOTE(
dynamic_cast<ReadableFromMatrixData<ValueType, LocalIndexType>*>(
diag_mtx.get())));
GKO_ASSERT(GKO_QUOTE(
dynamic_cast<ReadableFromMatrixData<ValueType, LocalIndexType>*>(
offdiag_mtx.get())));
GKO_ASSERT(
(dynamic_cast<ReadableFromMatrixData<ValueType, LocalIndexType>*>(
diag_mtx_.get())));
GKO_ASSERT(
(dynamic_cast<ReadableFromMatrixData<ValueType, LocalIndexType>*>(
offdiag_mtx_.get())));
one_scalar_.init(exec, dim<2>{1, 1});
one_scalar_->fill(one<value_type>());
}
Expand Down
2 changes: 1 addition & 1 deletion core/test/utils/matrix_generator.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ matrix_data<ValueType, IndexType> generate_random_matrix_data(


/**
* Generates matrix data for a random matrix.
* Generates device matrix data for a random matrix.
*
* @see generate_random_matrix_data
*/
Expand Down
44 changes: 29 additions & 15 deletions include/ginkgo/core/distributed/matrix.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,16 @@ struct MatrixTypeBuilderFromValueAndIndex {
};


template <typename T, typename = void>
struct is_matrix_builder : std::false_type {};

template <typename T>
struct is_matrix_builder<
T, xstd::void_t<decltype(std::declval<T>().template create<double, int>(
std::declval<std::shared_ptr<const Executor>>()))>>
: std::true_type {};


} // namespace detail


Expand Down Expand Up @@ -140,7 +150,7 @@ class Vector;
*
* The matrix is stored in a row-wise distributed format.
* Each process owns a specific set of rows, where the assignment of rows is
* defined by a row @see Partition. The following depicts the distribution of
* defined by a row Partition. The following depicts the distribution of
* global rows according to their assigned part-id (which will usually be the
* owning process id):
* ```
Expand All @@ -161,14 +171,14 @@ class Vector;
* following:
* ```
* Part-Id Global Inner Ghost
* 0 | .. 1 : 2 .. : .. .. | | .. 1 | | 2 |
* 0 | 3 4 : .. .. : .. .. | | 3 4 | | .. |
* 0 | .. 1 ⁞ 2 .. ⁞ .. .. | | .. 1 | | 2 |
* 0 | 3 4 ⁞ .. .. ⁞ .. .. | | 3 4 | | .. |
* |-----------------------|
* 1 | .. 5 : 6 .. : 7 .. | ----> | 6 .. | | 5 7 .. |
* 1 | .. .. : .. 8 : .. 9 | ----> | 8 .. | | .. .. 9 |
* 1 | .. 5 ⁞ 6 .. ⁞ 7 .. | ----> | 6 .. | | 5 7 .. |
* 1 | .. .. ⁞ .. 8 ⁞ .. 9 | ----> | 8 .. | | .. .. 9 |
* |-----------------------|
* 2 | .. .. : .. 10 : 11 12 | | 11 12 | | .. 10 |
* 2 | 13 .. : .. .. : 14 .. | | 14 .. | | 13 .. |
* 2 | .. .. ⁞ .. 10 ⁞ 11 12 | | 11 12 | | .. 10 |
* 2 | 13 .. ⁞ .. .. ⁞ 14 .. | | 14 .. | | 13 .. |
* ```
* This uses the same ownership of the columns as for the rows.
* Additionally, the ownership of the columns may be explicitly defined with an
Expand All @@ -189,13 +199,13 @@ class Vector;
* ```
*
* The Matrix should be filled using the read_distributed method, e.g.
* ```c++
* ```
* auto part = Partition<...>::build_from_mapping(...);
* auto mat = Matrix<...>::create(exec, comm);
* mat->read_distributed(matrix_data, part);
* ```
* or if different partitions for the rows and columns are used:
* ```c++
* ```
* auto row_part = Partition<...>::build_from_mapping(...);
* auto col_part = Partition<...>::build_from_mapping(...);
* auto mat = Matrix<...>::create(exec, comm);
Expand Down Expand Up @@ -368,7 +378,7 @@ class Matrix
* @param comm Communicator associated with this matrix.
* @param matrix_type the local matrices will be constructed with the same
* type as `create` returns. It should be the return
* value of make_matrix_type.
* value of make_matrix_type.
*/
template <typename MatrixType>
explicit Matrix(std::shared_ptr<const Executor> exec,
Expand Down Expand Up @@ -421,6 +431,10 @@ class Matrix
/**
* Creates an empty distributed matrix with specified type
* for local matricies.
*
* @note It internally clones the passed in matrix_type. Therefore, this
* LinOp should be empty.
*
* @param exec Executor associated with this matrix.
* @param comm Communicator associated with this matrix.
* @param matrix_type the local matrices will be constructed with the same
Expand All @@ -433,8 +447,8 @@ class Matrix
* Creates an empty distributed matrix with specified types for the local
* inner matrix and the local ghost matrix.
*
* @note It calls Matrix(std::shared_ptr<const Executor>,mpi::communicator,
* const LinOp*,const LinOp*) underneath
* @note It internally clones the passed in inner_matrix_type and
* ghost_matrix_type. Therefore, these LinOps should be empty.
*
* @tparam InnerMatrixType A type that has a `create<ValueType,
* IndexType>(exec)` function to create an smart
Expand All @@ -454,11 +468,11 @@ class Matrix
const LinOp* ghost_matrix_type);

/**
* Asynchronously communicates the values of b that are shared with other
* processors.
* Starts a non-blocking communication of the values of b that are shared
* with other processors.
* @param local_b The full local vector to be communicated. The subset of
* shared values is automatically extracted.
* @return MPI request for the async communication.
* @return MPI request for the non-blocking communication.
*/
mpi::request communicate(const local_vector_type* local_b) const;

Expand Down
12 changes: 5 additions & 7 deletions omp/distributed/matrix_kernels.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,7 @@ void build_diag_offdiag(
return static_cast<size_type>(std::distance(range_bounds + 1, it));
}
};
auto map_to_local_row = [&](GlobalIndexType idx,
size_type range_id) -> LocalIndexType {
auto map_to_local_row = [&](GlobalIndexType idx, size_type range_id) {
auto range_bounds = row_partition->get_range_bounds();
auto range_starting_indices =
row_partition->get_range_starting_indices();
Expand All @@ -108,8 +107,7 @@ void build_diag_offdiag(
return static_cast<size_type>(std::distance(range_bounds + 1, it));
}
};
auto map_to_local_col = [&](GlobalIndexType idx,
size_type range_id) -> LocalIndexType {
auto map_to_local_col = [&](GlobalIndexType idx, size_type range_id) {
auto range_bounds = col_partition->get_range_bounds();
auto range_starting_indices =
col_partition->get_range_starting_indices();
Expand All @@ -118,10 +116,10 @@ void build_diag_offdiag(
};

// store offdiagonal columns and their range indices
std::map<GlobalIndexType, range_index_type> offdiag_cols;
map<GlobalIndexType, range_index_type> offdiag_cols(exec);
// store offdiagonal entries with global column idxs
std::vector<global_nonzero> global_offdiag_entries;
std::vector<local_nonzero> diag_entries;
vector<global_nonzero> global_offdiag_entries(exec);
vector<local_nonzero> diag_entries(exec);

auto num_threads = static_cast<size_type>(omp_get_max_threads());
auto num_input = input.get_num_elems();
Expand Down
6 changes: 6 additions & 0 deletions omp/test/distributed/matrix_kernels.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ TYPED_TEST(Matrix, BuildsDiagOffdiagEmptyIsSameAsRef)
using global_index_type = typename TestFixture::global_index_type;
gko::Array<comm_index_type> mapping{this->ref, {1, 0, 2, 2, 0, 1, 1, 2}};
comm_index_type num_parts = 3;

auto partition = gko::distributed::Partition<
local_index_type, global_index_type>::build_from_mapping(this->ref,
mapping,
Expand Down Expand Up @@ -186,6 +187,7 @@ TYPED_TEST(Matrix, BuildsLocalSmallIsEquivalentToRef)
std::uniform_int_distribution<int>(0, static_cast<int>(num_cols - 1)),
std::uniform_real_distribution<gko::remove_complex<value_type>>(0, 1),
this->engine, this->ref);

auto partition = gko::distributed::Partition<
local_index_type, global_index_type>::build_from_mapping(this->ref,
mapping,
Expand Down Expand Up @@ -221,6 +223,7 @@ TYPED_TEST(Matrix, BuildsLocalIsEquivalentToRef)
static_cast<int>(num_cols - 1)),
std::uniform_real_distribution<gko::remove_complex<value_type>>(0, 1),
this->engine, this->ref);

auto partition = gko::distributed::Partition<
local_index_type, global_index_type>::build_from_mapping(this->ref,
mapping,
Expand All @@ -245,6 +248,7 @@ TYPED_TEST(Matrix, BuildsDiagOffdiagEmptyWithColPartitionIsSameAsRef)
gko::Array<comm_index_type> col_mapping{this->ref,
{0, 0, 2, 2, 2, 1, 1, 1}};
comm_index_type num_parts = 3;

auto row_partition = gko::distributed::Partition<
local_index_type, global_index_type>::build_from_mapping(this->ref,
row_mapping,
Expand Down Expand Up @@ -295,6 +299,7 @@ TYPED_TEST(Matrix, BuildsLocalSmallWithColPartitionIsEquivalentToRef)
std::uniform_int_distribution<int>(0, static_cast<int>(num_cols - 1)),
std::uniform_real_distribution<gko::remove_complex<value_type>>(0, 1),
this->engine, this->ref);

auto row_partition = gko::distributed::Partition<
local_index_type, global_index_type>::build_from_mapping(this->ref,
row_mapping,
Expand Down Expand Up @@ -344,6 +349,7 @@ TYPED_TEST(Matrix, BuildsLocalWithColPartitionIsEquivalentToRef)
static_cast<int>(num_cols - 1)),
std::uniform_real_distribution<gko::remove_complex<value_type>>(0, 1),
this->engine, this->ref);

auto row_partition = gko::distributed::Partition<
local_index_type, global_index_type>::build_from_mapping(this->ref,
row_mapping,
Expand Down
6 changes: 2 additions & 4 deletions reference/distributed/matrix_kernels.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,7 @@ void build_diag_offdiag(
return static_cast<size_type>(std::distance(range_bounds + 1, it));
}
};
auto map_to_local_row = [&](GlobalIndexType idx,
size_type range_id) -> LocalIndexType {
auto map_to_local_row = [&](GlobalIndexType idx, size_type range_id) {
auto range_bounds = row_partition->get_range_bounds();
auto range_starting_indices =
row_partition->get_range_starting_indices();
Expand All @@ -102,8 +101,7 @@ void build_diag_offdiag(
return static_cast<size_type>(std::distance(range_bounds + 1, it));
}
};
auto map_to_local_col = [&](GlobalIndexType idx,
size_type range_id) -> LocalIndexType {
auto map_to_local_col = [&](GlobalIndexType idx, size_type range_id) {
auto range_bounds = col_partition->get_range_bounds();
auto range_starting_indices =
col_partition->get_range_starting_indices();
Expand Down

0 comments on commit d050278

Please sign in to comment.