From d6a2c8be07641f263416bec197304b3b6e20e59c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Gr=C3=BCtzmacher?= Date: Thu, 15 Oct 2020 17:53:12 +0200 Subject: [PATCH 01/13] Add sorting for the Jacobi preconditioner Add utility header which (later) will be used to unsort matrices --- benchmark/utils/preconditioners.hpp | 7 ++ core/preconditioner/jacobi.cpp | 23 +++++- core/test/utils/shuffle_matrix.hpp | 79 +++++++++++++++++++ include/ginkgo/core/preconditioner/jacobi.hpp | 34 ++++++-- 4 files changed, 134 insertions(+), 9 deletions(-) create mode 100644 core/test/utils/shuffle_matrix.hpp diff --git a/benchmark/utils/preconditioners.hpp b/benchmark/utils/preconditioners.hpp index 9f61c99f9a7..f55b9b0e13a 100644 --- a/benchmark/utils/preconditioners.hpp +++ b/benchmark/utils/preconditioners.hpp @@ -78,6 +78,7 @@ const std::map( .with_storage_optimization( parse_storage_optimization(FLAGS_jacobi_storage)) .with_accuracy(FLAGS_jacobi_accuracy) + .with_skip_sorting(true) .on(exec); }}, {"parict", @@ -87,6 +88,7 @@ const std::map( .with_iterations(FLAGS_parilu_iterations) .with_approximate_select(FLAGS_parilut_approx_select) .with_fill_in_limit(FLAGS_parilut_limit) + .with_skip_sorting(true) .on(exec)); return gko::preconditioner::Ilu<>::build() .with_factorization_factory(fact) @@ -97,6 +99,7 @@ const std::map( auto fact = gko::share(gko::factorization::ParIlu<>::build() .with_iterations(FLAGS_parilu_iterations) + .with_skip_sorting(true) .on(exec)); return gko::preconditioner::Ilu<>::build() .with_factorization_factory(fact) @@ -109,6 +112,7 @@ const std::map( .with_iterations(FLAGS_parilu_iterations) .with_approximate_select(FLAGS_parilut_approx_select) .with_fill_in_limit(FLAGS_parilut_limit) + .with_skip_sorting(true) .on(exec)); return gko::preconditioner::Ilu<>::build() .with_factorization_factory(fact) @@ -129,6 +133,7 @@ const std::map( .with_iterations(FLAGS_parilu_iterations) .with_approximate_select(FLAGS_parilut_approx_select) .with_fill_in_limit(FLAGS_parilut_limit) + .with_skip_sorting(true) .on(exec)); auto lisai = gko::share(gko::preconditioner::LowerIsai<>::build() .with_sparsity_power(FLAGS_isai_power) @@ -149,6 +154,7 @@ const std::map( auto fact = gko::share(gko::factorization::ParIlu<>::build() .with_iterations(FLAGS_parilu_iterations) + .with_skip_sorting(true) .on(exec)); auto lisai = gko::share(gko::preconditioner::LowerIsai<>::build() .with_sparsity_power(FLAGS_isai_power) @@ -171,6 +177,7 @@ const std::map( .with_iterations(FLAGS_parilu_iterations) .with_approximate_select(FLAGS_parilut_approx_select) .with_fill_in_limit(FLAGS_parilut_limit) + .with_skip_sorting(true) .on(exec)); auto lisai = gko::share(gko::preconditioner::LowerIsai<>::build() .with_sparsity_power(FLAGS_isai_power) diff --git a/core/preconditioner/jacobi.cpp b/core/preconditioner/jacobi.cpp index f7351cd779c..a8a8ad7ba2f 100644 --- a/core/preconditioner/jacobi.cpp +++ b/core/preconditioner/jacobi.cpp @@ -33,6 +33,9 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include +#include + + #include #include #include @@ -201,12 +204,26 @@ void Jacobi::detect_blocks( template -void Jacobi::generate(const LinOp *system_matrix) +void Jacobi::generate(const LinOp *system_matrix, + bool skip_sorting) { GKO_ASSERT_IS_SQUARE_MATRIX(system_matrix); + using csr_type = matrix::Csr; const auto exec = this->get_executor(); - const auto csr_mtx = copy_and_convert_to>( - exec, system_matrix); + decltype(copy_and_convert_to(exec, system_matrix)) csr_mtx{}; + + if (skip_sorting) { + csr_mtx = copy_and_convert_to>( + exec, system_matrix); + } else { + auto editable_csr = csr_type::create(exec); + as>(system_matrix) + ->convert_to(lend(editable_csr)); + // Maybe check if it is sorted first + editable_csr->sort_by_column_index(); + csr_mtx = decltype(csr_mtx){editable_csr.release(), + std::default_delete{}}; + } if (parameters_.block_pointers.get_data() == nullptr) { this->detect_blocks(csr_mtx.get()); diff --git a/core/test/utils/shuffle_matrix.hpp b/core/test/utils/shuffle_matrix.hpp new file mode 100644 index 00000000000..44bcc45e502 --- /dev/null +++ b/core/test/utils/shuffle_matrix.hpp @@ -0,0 +1,79 @@ +/************************************************************* +Copyright (c) 2017-2020, the Ginkgo authors +All rights reserved. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions +are met: + +1. Redistributions of source code must retain the above copyright +notice, this list of conditions and the following disclaimer. + +2. Redistributions in binary form must reproduce the above copyright +notice, this list of conditions and the following disclaimer in the +documentation and/or other materials provided with the distribution. + +3. Neither the name of the copyright holder nor the names of its +contributors may be used to endorse or promote products derived from +this software without specific prior written permission. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS +IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED +TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A +PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +*************************************************************/ + +#ifndef GKO_CORE_TEST_UTILS_MATRIX_SHUFFLE_HPP_ +#define GKO_CORE_TEST_UTILS_MATRIX_SHUFFLE_HPP_ + +#include +#include + + +#include + + +namespace gko { +namespace test { + + +// Plan for now: shuffle values and column indices to unsort the given matrix +// without changing the represented matrix. +template +void shuffle_nz_storage(matrix::Csr *mtx, + RandomEngine &&engine) +{ + using value_type = ValueType; + using index_type = IndexType; + auto size = mtx->get_size(); + auto vals = mtx->get_values(); + auto row_ptrs = mtx->get_row_ptrs(); + auto cols = mtx->get_col_idxs(); + for (index_type row = 0; row < size[0]; ++row) { + auto start = row_ptrs[row]; + auto end = row_ptrs[row + 1] - 1; + std::uniform_int_distribution dist{start, end}; + for (index_type i = start; i < start + (start + end) / 2; ++i) { + auto a = dist(engine); + auto b = dist(engine); + using std::swap; + if (a != b) { + swap(vals[a], vals[b]); + swap(cols[a], cols[b]); + } + } + } +} + + +} // namespace test +} // namespace gko + +#endif // GKO_CORE_TEST_UTILS_MATRIX_SHUFFLE_HPP_ diff --git a/include/ginkgo/core/preconditioner/jacobi.hpp b/include/ginkgo/core/preconditioner/jacobi.hpp index be82373c37c..60433b2f2cc 100644 --- a/include/ginkgo/core/preconditioner/jacobi.hpp +++ b/include/ginkgo/core/preconditioner/jacobi.hpp @@ -107,8 +107,8 @@ struct block_interleaved_storage_scheme { * blocks is not known, for a special input `size_type{} - 1` * the method returns `0` to avoid overallocation of memory. */ - GKO_ATTRIBUTES size_type compute_storage_space(size_type num_blocks) const - noexcept + GKO_ATTRIBUTES size_type + compute_storage_space(size_type num_blocks) const noexcept { return (num_blocks + 1 == size_type{0}) ? size_type{0} @@ -146,8 +146,8 @@ struct block_interleaved_storage_scheme { * * @return the offset of the block with ID `block_id` */ - GKO_ATTRIBUTES IndexType get_global_block_offset(IndexType block_id) const - noexcept + GKO_ATTRIBUTES IndexType + get_global_block_offset(IndexType block_id) const noexcept { return this->get_group_offset(block_id) + this->get_block_offset(block_id); @@ -313,6 +313,25 @@ class Jacobi : public EnableLinOp>, */ uint32 GKO_FACTORY_PARAMETER_SCALAR(max_block_stride, 0u); + /** + * @brief `true` means it is known that the matrix given to this + * factory will be sorted first by row, then by column index, + * `false` means it is unknown or not sorted, so an additional + * sorting step will be performed during the preconditioner + * generation (it will not change the matrix given). + * The matrix must be sorted for this preconditioner to work. + * + * The `system_matrix`, which will be given to this factory, must be + * sorted (first by row, then by column) in order for the algorithm + * to work. If it is known that the matrix will be sorted, this + * parameter can be set to `true` to skip the sorting (therefore, + * shortening the runtime). + * However, if it is unknown or if the matrix is known to be not sorted, + * it must remain `false`, otherwise, this preconditioner might be + * incorrect. + */ + bool GKO_FACTORY_PARAMETER_SCALAR(skip_sorting, false); + /** * Starting (row / column) indexes of individual blocks. * @@ -509,7 +528,7 @@ class Jacobi : public EnableLinOp>, parameters_.block_pointers.set_executor(this->get_executor()); parameters_.storage_optimization.block_wise.set_executor( this->get_executor()); - this->generate(lend(system_matrix)); + this->generate(lend(system_matrix), parameters_.skip_sorting); } /** @@ -559,8 +578,11 @@ class Jacobi : public EnableLinOp>, * * @param system_matrix the source matrix used to generate the * preconditioner + * @param skip_sorting determines if the sorting of system_matrix can be + * skipped (therefore, marking that it is already + * sorted) */ - void generate(const LinOp *system_matrix); + void generate(const LinOp *system_matrix, bool skip_sorting); /** * Detects the diagonal blocks and allocates the memory needed to store the From 46d23900204fc9a5a243c3bf7e08735f20c33352 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Gr=C3=BCtzmacher?= Date: Tue, 20 Oct 2020 22:18:07 +0200 Subject: [PATCH 02/13] Add proper tests for unsorting --- core/test/utils/CMakeLists.txt | 3 +- core/test/utils/shuffle_matrix.hpp | 79 ------------ core/test/utils/unsort_matrix.hpp | 137 +++++++++++++++++++++ core/test/utils/unsort_matrix_test.cpp | 163 +++++++++++++++++++++++++ 4 files changed, 302 insertions(+), 80 deletions(-) delete mode 100644 core/test/utils/shuffle_matrix.hpp create mode 100644 core/test/utils/unsort_matrix.hpp create mode 100644 core/test/utils/unsort_matrix_test.cpp diff --git a/core/test/utils/CMakeLists.txt b/core/test/utils/CMakeLists.txt index 223dd62e1f4..84e3d46958d 100644 --- a/core/test/utils/CMakeLists.txt +++ b/core/test/utils/CMakeLists.txt @@ -1,2 +1,3 @@ -ginkgo_create_test(matrix_generator_test) ginkgo_create_test(assertions_test) +ginkgo_create_test(matrix_generator_test) +ginkgo_create_test(unsort_matrix_test) diff --git a/core/test/utils/shuffle_matrix.hpp b/core/test/utils/shuffle_matrix.hpp deleted file mode 100644 index 44bcc45e502..00000000000 --- a/core/test/utils/shuffle_matrix.hpp +++ /dev/null @@ -1,79 +0,0 @@ -/************************************************************* -Copyright (c) 2017-2020, the Ginkgo authors -All rights reserved. - -Redistribution and use in source and binary forms, with or without -modification, are permitted provided that the following conditions -are met: - -1. Redistributions of source code must retain the above copyright -notice, this list of conditions and the following disclaimer. - -2. Redistributions in binary form must reproduce the above copyright -notice, this list of conditions and the following disclaimer in the -documentation and/or other materials provided with the distribution. - -3. Neither the name of the copyright holder nor the names of its -contributors may be used to endorse or promote products derived from -this software without specific prior written permission. - -THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS -IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED -TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A -PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT -HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, -SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT -LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, -DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY -THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT -(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE -OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -*************************************************************/ - -#ifndef GKO_CORE_TEST_UTILS_MATRIX_SHUFFLE_HPP_ -#define GKO_CORE_TEST_UTILS_MATRIX_SHUFFLE_HPP_ - -#include -#include - - -#include - - -namespace gko { -namespace test { - - -// Plan for now: shuffle values and column indices to unsort the given matrix -// without changing the represented matrix. -template -void shuffle_nz_storage(matrix::Csr *mtx, - RandomEngine &&engine) -{ - using value_type = ValueType; - using index_type = IndexType; - auto size = mtx->get_size(); - auto vals = mtx->get_values(); - auto row_ptrs = mtx->get_row_ptrs(); - auto cols = mtx->get_col_idxs(); - for (index_type row = 0; row < size[0]; ++row) { - auto start = row_ptrs[row]; - auto end = row_ptrs[row + 1] - 1; - std::uniform_int_distribution dist{start, end}; - for (index_type i = start; i < start + (start + end) / 2; ++i) { - auto a = dist(engine); - auto b = dist(engine); - using std::swap; - if (a != b) { - swap(vals[a], vals[b]); - swap(cols[a], cols[b]); - } - } - } -} - - -} // namespace test -} // namespace gko - -#endif // GKO_CORE_TEST_UTILS_MATRIX_SHUFFLE_HPP_ diff --git a/core/test/utils/unsort_matrix.hpp b/core/test/utils/unsort_matrix.hpp new file mode 100644 index 00000000000..907fcfa5a41 --- /dev/null +++ b/core/test/utils/unsort_matrix.hpp @@ -0,0 +1,137 @@ +/************************************************************* +Copyright (c) 2017-2020, the Ginkgo authors +All rights reserved. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions +are met: + +1. Redistributions of source code must retain the above copyright +notice, this list of conditions and the following disclaimer. + +2. Redistributions in binary form must reproduce the above copyright +notice, this list of conditions and the following disclaimer in the +documentation and/or other materials provided with the distribution. + +3. Neither the name of the copyright holder nor the names of its +contributors may be used to endorse or promote products derived from +this software without specific prior written permission. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS +IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED +TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A +PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +*************************************************************/ + +#ifndef GKO_CORE_TEST_UTILS_UNSORT_MATRIX_HPP_ +#define GKO_CORE_TEST_UTILS_UNSORT_MATRIX_HPP_ + +#include +#include + + +#include + + +#include "core/base/iterator_factory.hpp" + + +namespace gko { +namespace test { + + +// Plan for now: shuffle values and column indices to unsort the given matrix +// without changing the represented matrix. +template +void unsort_matrix(matrix::Csr *mtx, + RandomEngine &&engine) +{ + using value_type = ValueType; + using index_type = IndexType; + auto size = mtx->get_size(); + if (mtx->get_num_stored_elements() <= 0) { + return; + } + const auto &exec = mtx->get_executor(); + const auto &master = mtx->get_executor()->get_master(); + + if (exec != master) { + auto h_mtx = mtx->clone(master); + unsort_matrix(lend(h_mtx), engine); + mtx->copy_from(lend(h_mtx)); + return; + } + + auto vals = mtx->get_values(); + auto row_ptrs = mtx->get_row_ptrs(); + auto cols = mtx->get_col_idxs(); + + for (index_type row = 0; row < size[0]; ++row) { + auto start = row_ptrs[row]; + auto end = row_ptrs[row + 1]; + auto iterator = detail::IteratorFactory( + cols + start, vals + start, end - start); + std::shuffle(iterator.begin(), iterator.end(), engine); + } +} + + +// Plan for now: shuffle values and column indices to unsort the given matrix +// without changing the represented matrix. +// Note: This expects COO to be properly sorted by row index (which is required +// by the Ginkgo COO format) +template +void unsort_matrix(matrix::Coo *mtx, + RandomEngine &&engine) +{ + using value_type = ValueType; + using index_type = IndexType; + auto nnz = mtx->get_num_stored_elements(); + + if (nnz <= 0) { + return; + } + + const auto &exec = mtx->get_executor(); + const auto &master = mtx->get_executor()->get_master(); + + if (exec != master) { + auto h_mtx = mtx->clone(master); + unsort_matrix(lend(h_mtx), engine); + mtx->copy_from(lend(h_mtx)); + return; + } + + auto size = mtx->get_size(); + auto vals = mtx->get_values(); + auto rows = mtx->get_row_idxs(); + auto cols = mtx->get_col_idxs(); + + auto current_row = rows[0]; + for (IndexType i = 0; i < nnz;) { + auto start = i; + while (i < nnz && rows[i] == current_row) { + ++i; + } + current_row = rows[i]; + auto end = i; + auto iterator = detail::IteratorFactory( + cols + start, vals + start, end - start); + // since the row entries are supposed to be the same, there is no need + // to swap + std::shuffle(iterator.begin(), iterator.end(), engine); + } +} + + +} // namespace test +} // namespace gko + +#endif // GKO_CORE_TEST_UTILS_UNSORT_MATRIX_HPP_ diff --git a/core/test/utils/unsort_matrix_test.cpp b/core/test/utils/unsort_matrix_test.cpp new file mode 100644 index 00000000000..9e80cdca83e --- /dev/null +++ b/core/test/utils/unsort_matrix_test.cpp @@ -0,0 +1,163 @@ +/************************************************************* +Copyright (c) 2017-2020, the Ginkgo authors +All rights reserved. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions +are met: + +1. Redistributions of source code must retain the above copyright +notice, this list of conditions and the following disclaimer. + +2. Redistributions in binary form must reproduce the above copyright +notice, this list of conditions and the following disclaimer in the +documentation and/or other materials provided with the distribution. + +3. Neither the name of the copyright holder nor the names of its +contributors may be used to endorse or promote products derived from +this software without specific prior written permission. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS +IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED +TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A +PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +*************************************************************/ + +#include "core/test/utils/unsort_matrix.hpp" + + +#include +#include + + +#include + + +#include +#include +#include +#include +#include +#include + + +#include "core/test/utils.hpp" + + +namespace { + + +class UnsortMatrix : public ::testing::Test { +protected: + using value_type = double; + using index_type = gko::int32; + using Csr = gko::matrix::Csr; + using Coo = gko::matrix::Coo; + using Dense = gko::matrix::Dense; + UnsortMatrix() + : exec(gko::ReferenceExecutor::create()), + rand_engine(42), + mtx(gko::initialize({{1, 2, 0, 0, 0}, + {0, 0, 0, 0, 0}, + {3, 4, 5, 6, 0}, + {0, 0, 7, 0, 0}, + {0, 0, 8, 9, 10}}, + exec)), + empty(Dense::create(exec, gko::dim<2>(0, 0))) + {} + + bool is_coo_matrix_sorted(Coo *mtx) + { + auto size = mtx->get_size(); + auto vals = mtx->get_values(); + auto rows = mtx->get_row_idxs(); + auto cols = mtx->get_col_idxs(); + auto nnz = mtx->get_num_stored_elements(); + + if (nnz <= 0) { + return true; + } + + auto prev_row = rows[0]; + auto prev_col = cols[0]; + for (index_type i = 0; i < nnz; ++i) { + auto cur_row = rows[i]; + auto cur_col = cols[i]; + if (prev_row == cur_row && prev_col > cur_col) { + return false; + } + prev_row = cur_row; + prev_col = cur_col; + } + return true; + } + + std::shared_ptr exec; + std::ranlux48 rand_engine; + std::unique_ptr mtx; + std::unique_ptr empty; +}; + + +TEST_F(UnsortMatrix, CsrWorks) +{ + auto csr = Csr::create(exec); + mtx->convert_to(gko::lend(csr)); + bool was_sorted = csr->is_sorted_by_column_index(); + + gko::test::unsort_matrix(gko::lend(csr), rand_engine); + + ASSERT_FALSE(csr->is_sorted_by_column_index()); + ASSERT_TRUE(was_sorted); + GKO_ASSERT_MTX_NEAR(csr, mtx, 0.); +} + + +TEST_F(UnsortMatrix, CsrWorksWithEmpty) +{ + auto csr = Csr::create(exec); + empty->convert_to(gko::lend(csr)); + bool was_sorted = csr->is_sorted_by_column_index(); + + gko::test::unsort_matrix(gko::lend(csr), rand_engine); + + ASSERT_TRUE(was_sorted); + GKO_ASSERT_MTX_NEAR(csr, empty, 0.); +} + + +TEST_F(UnsortMatrix, CooWorks) +{ + auto coo = Coo::create(exec); + mtx->convert_to(gko::lend(coo)); + const bool was_sorted = is_coo_matrix_sorted(gko::lend(coo)); + + gko::test::unsort_matrix(gko::lend(coo), rand_engine); + + ASSERT_FALSE(is_coo_matrix_sorted(gko::lend(coo))); + ASSERT_TRUE(was_sorted); + GKO_ASSERT_MTX_NEAR(coo, mtx, 0.); +} + + +TEST_F(UnsortMatrix, CooWorksWithEmpty) +{ + auto coo = Coo::create(exec); + empty->convert_to(gko::lend(coo)); + const bool was_sorted = is_coo_matrix_sorted(gko::lend(coo)); + + gko::test::unsort_matrix(gko::lend(coo), rand_engine); + + ASSERT_TRUE(was_sorted); + GKO_ASSERT_MTX_NEAR(coo, empty, 0.); +} + + +} // namespace From f313921821134ba72ea81819cf30bef590db96c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Gr=C3=BCtzmacher?= Date: Wed, 21 Oct 2020 09:15:12 +0200 Subject: [PATCH 03/13] Add CUDA tests for unsorted CSR --- cuda/test/matrix/csr_kernels.cpp | 66 ++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/cuda/test/matrix/csr_kernels.cpp b/cuda/test/matrix/csr_kernels.cpp index f0a1416b6bd..e938d926680 100644 --- a/cuda/test/matrix/csr_kernels.cpp +++ b/cuda/test/matrix/csr_kernels.cpp @@ -52,6 +52,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include "core/matrix/csr_kernels.hpp" +#include "core/test/utils/unsort_matrix.hpp" #include "cuda/test/utils.hpp" @@ -219,6 +220,18 @@ TEST_F(Csr, SimpleApplyIsEquivalentToRefWithLoadBalance) } +TEST_F(Csr, SimpleApplyIsEquivalentToRefWithLoadBalanceUnsorted) +{ + set_up_apply_data(std::make_shared(cuda)); + gko::test::unsort_matrix(dmtx.get(), rand_engine); + + mtx->apply(y.get(), expected.get()); + dmtx->apply(dy.get(), dresult.get()); + + GKO_ASSERT_MTX_NEAR(dresult, expected, 1e-14); +} + + TEST_F(Csr, AdvancedApplyIsEquivalentToRefWithLoadBalance) { set_up_apply_data(std::make_shared(cuda)); @@ -241,6 +254,18 @@ TEST_F(Csr, SimpleApplyIsEquivalentToRefWithCusparse) } +TEST_F(Csr, SimpleApplyIsEquivalentToRefWithCusparseUnsorted) +{ + set_up_apply_data(std::make_shared()); + gko::test::unsort_matrix(dmtx.get(), rand_engine); + + mtx->apply(y.get(), expected.get()); + dmtx->apply(dy.get(), dresult.get()); + + GKO_ASSERT_MTX_NEAR(dresult, expected, 1e-14); +} + + TEST_F(Csr, AdvancedApplyIsEquivalentToRefWithCusparse) { set_up_apply_data(std::make_shared()); @@ -263,6 +288,18 @@ TEST_F(Csr, SimpleApplyIsEquivalentToRefWithMergePath) } +TEST_F(Csr, SimpleApplyIsEquivalentToRefWithMergePathUnsorted) +{ + set_up_apply_data(std::make_shared()); + gko::test::unsort_matrix(dmtx.get(), rand_engine); + + mtx->apply(y.get(), expected.get()); + dmtx->apply(dy.get(), dresult.get()); + + GKO_ASSERT_MTX_NEAR(dresult, expected, 1e-14); +} + + TEST_F(Csr, AdvancedApplyIsEquivalentToRefWithMergePath) { set_up_apply_data(std::make_shared()); @@ -285,6 +322,18 @@ TEST_F(Csr, SimpleApplyIsEquivalentToRefWithClassical) } +TEST_F(Csr, SimpleApplyIsEquivalentToRefWithClassicalUnsorted) +{ + set_up_apply_data(std::make_shared()); + gko::test::unsort_matrix(dmtx.get(), rand_engine); + + mtx->apply(y.get(), expected.get()); + dmtx->apply(dy.get(), dresult.get()); + + GKO_ASSERT_MTX_NEAR(dresult, expected, 1e-14); +} + + TEST_F(Csr, AdvancedApplyIsEquivalentToRefWithClassical) { set_up_apply_data(std::make_shared()); @@ -403,6 +452,23 @@ TEST_F(Csr, SimpleApplyToCsrMatrixIsEquivalentToRef) } +TEST_F(Csr, SimpleApplyToCsrMatrixIsEquivalentToRefUnsorted) +{ + set_up_apply_data(std::make_shared()); + auto trans = mtx->transpose(); + auto d_trans = dmtx->transpose(); + gko::test::unsort_matrix(static_cast(dmtx.get()), rand_engine); + gko::test::unsort_matrix(static_cast(d_trans.get()), rand_engine); + + mtx->apply(trans.get(), square_mtx.get()); + dmtx->apply(d_trans.get(), square_dmtx.get()); + + GKO_ASSERT_MTX_NEAR(square_dmtx, square_mtx, 1e-14); + GKO_ASSERT_MTX_EQ_SPARSITY(square_dmtx, square_mtx); + ASSERT_TRUE(square_dmtx->is_sorted_by_column_index()); +} + + TEST_F(Csr, AdvancedApplyToIdentityMatrixIsEquivalentToRef) { set_up_apply_data(std::make_shared()); From 50bee3cde8f70a0bdc2ffd61a6c122f4366c53b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Gr=C3=BCtzmacher?= Date: Mon, 26 Oct 2020 07:59:30 +0100 Subject: [PATCH 04/13] Add OMP and HIP tests --- core/test/utils/unsort_matrix.hpp | 4 +- cuda/test/matrix/coo_kernels.cpp | 18 +++++ cuda/test/matrix/csr_kernels.cpp | 56 ++++---------- hip/test/matrix/coo_kernels.hip.cpp | 19 +++++ hip/test/matrix/csr_kernels.hip.cpp | 105 +++++++++++++++++--------- omp/test/matrix/coo_kernels.cpp | 41 +++++++++- omp/test/matrix/csr_kernels.cpp | 33 ++++---- reference/test/matrix/coo_kernels.cpp | 58 ++++++++++++++ 8 files changed, 237 insertions(+), 97 deletions(-) diff --git a/core/test/utils/unsort_matrix.hpp b/core/test/utils/unsort_matrix.hpp index 907fcfa5a41..ff2ff500e0e 100644 --- a/core/test/utils/unsort_matrix.hpp +++ b/core/test/utils/unsort_matrix.hpp @@ -76,7 +76,7 @@ void unsort_matrix(matrix::Csr *mtx, for (index_type row = 0; row < size[0]; ++row) { auto start = row_ptrs[row]; auto end = row_ptrs[row + 1]; - auto iterator = detail::IteratorFactory( + auto iterator = gko::detail::IteratorFactory( cols + start, vals + start, end - start); std::shuffle(iterator.begin(), iterator.end(), engine); } @@ -122,7 +122,7 @@ void unsort_matrix(matrix::Coo *mtx, } current_row = rows[i]; auto end = i; - auto iterator = detail::IteratorFactory( + auto iterator = gko::detail::IteratorFactory( cols + start, vals + start, end - start); // since the row entries are supposed to be the same, there is no need // to swap diff --git a/cuda/test/matrix/coo_kernels.cpp b/cuda/test/matrix/coo_kernels.cpp index 1c65df1b692..d436cad06d5 100644 --- a/cuda/test/matrix/coo_kernels.cpp +++ b/cuda/test/matrix/coo_kernels.cpp @@ -48,6 +48,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include "core/matrix/coo_kernels.hpp" +#include "core/test/utils/unsort_matrix.hpp" #include "cuda/test/utils.hpp" @@ -104,6 +105,11 @@ class Coo : public ::testing::Test { dbeta->copy_from(beta.get()); } + void unsort_mtx() + { + gko::test::unsort_matrix(mtx.get(), rand_engine); + dmtx->copy_from(mtx.get()); + } std::shared_ptr ref; std::shared_ptr cuda; @@ -135,6 +141,18 @@ TEST_F(Coo, SimpleApplyIsEquivalentToRef) } +TEST_F(Coo, SimpleApplyIsEquivalentToRefUnsorted) +{ + set_up_apply_data(); + unsort_mtx(); + + mtx->apply(y.get(), expected.get()); + dmtx->apply(dy.get(), dresult.get()); + + GKO_ASSERT_MTX_NEAR(dresult, expected, 1e-14); +} + + TEST_F(Coo, AdvancedApplyIsEquivalentToRef) { set_up_apply_data(); diff --git a/cuda/test/matrix/csr_kernels.cpp b/cuda/test/matrix/csr_kernels.cpp index e938d926680..29b7f04f292 100644 --- a/cuda/test/matrix/csr_kernels.cpp +++ b/cuda/test/matrix/csr_kernels.cpp @@ -140,36 +140,10 @@ class Csr : public ::testing::Test { complex_dmtx->copy_from(complex_mtx.get()); } - struct matrix_pair { - std::unique_ptr ref; - std::unique_ptr cuda; - }; - - matrix_pair gen_unsorted_mtx() + void unsort_mtx() { - constexpr int min_nnz_per_row = 2; // Must be at least 2 - auto local_mtx_ref = - gen_mtx(mtx_size[0], mtx_size[1], min_nnz_per_row); - for (size_t row = 0; row < mtx_size[0]; ++row) { - const auto row_ptrs = local_mtx_ref->get_const_row_ptrs(); - const auto start_row = row_ptrs[row]; - auto col_idx = local_mtx_ref->get_col_idxs() + start_row; - auto vals = local_mtx_ref->get_values() + start_row; - const auto nnz_in_this_row = row_ptrs[row + 1] - row_ptrs[row]; - auto swap_idx_dist = - std::uniform_int_distribution<>(0, nnz_in_this_row - 1); - // shuffle `nnz_in_this_row / 2` times - for (size_t perm = 0; perm < nnz_in_this_row; perm += 2) { - const auto idx1 = swap_idx_dist(rand_engine); - const auto idx2 = swap_idx_dist(rand_engine); - std::swap(col_idx[idx1], col_idx[idx2]); - std::swap(vals[idx1], vals[idx2]); - } - } - auto local_mtx_cuda = Mtx::create(cuda); - local_mtx_cuda->copy_from(local_mtx_ref.get()); - - return {std::move(local_mtx_ref), std::move(local_mtx_cuda)}; + gko::test::unsort_matrix(mtx.get(), rand_engine); + dmtx->copy_from(mtx.get()); } std::shared_ptr ref; @@ -223,7 +197,7 @@ TEST_F(Csr, SimpleApplyIsEquivalentToRefWithLoadBalance) TEST_F(Csr, SimpleApplyIsEquivalentToRefWithLoadBalanceUnsorted) { set_up_apply_data(std::make_shared(cuda)); - gko::test::unsort_matrix(dmtx.get(), rand_engine); + unsort_mtx(); mtx->apply(y.get(), expected.get()); dmtx->apply(dy.get(), dresult.get()); @@ -257,7 +231,7 @@ TEST_F(Csr, SimpleApplyIsEquivalentToRefWithCusparse) TEST_F(Csr, SimpleApplyIsEquivalentToRefWithCusparseUnsorted) { set_up_apply_data(std::make_shared()); - gko::test::unsort_matrix(dmtx.get(), rand_engine); + unsort_mtx(); mtx->apply(y.get(), expected.get()); dmtx->apply(dy.get(), dresult.get()); @@ -291,7 +265,7 @@ TEST_F(Csr, SimpleApplyIsEquivalentToRefWithMergePath) TEST_F(Csr, SimpleApplyIsEquivalentToRefWithMergePathUnsorted) { set_up_apply_data(std::make_shared()); - gko::test::unsort_matrix(dmtx.get(), rand_engine); + unsort_mtx(); mtx->apply(y.get(), expected.get()); dmtx->apply(dy.get(), dresult.get()); @@ -325,7 +299,7 @@ TEST_F(Csr, SimpleApplyIsEquivalentToRefWithClassical) TEST_F(Csr, SimpleApplyIsEquivalentToRefWithClassicalUnsorted) { set_up_apply_data(std::make_shared()); - gko::test::unsort_matrix(dmtx.get(), rand_engine); + unsort_mtx(); mtx->apply(y.get(), expected.get()); dmtx->apply(dy.get(), dresult.get()); @@ -837,12 +811,13 @@ TEST_F(Csr, RecognizeSortedMatrixIsEquivalentToRef) TEST_F(Csr, RecognizeUnsortedMatrixIsEquivalentToRef) { - auto uns_mtx = gen_unsorted_mtx(); + set_up_apply_data(std::make_shared()); + unsort_mtx(); bool is_sorted_cuda{}; bool is_sorted_ref{}; - is_sorted_ref = uns_mtx.ref->is_sorted_by_column_index(); - is_sorted_cuda = uns_mtx.cuda->is_sorted_by_column_index(); + is_sorted_ref = mtx->is_sorted_by_column_index(); + is_sorted_cuda = dmtx->is_sorted_by_column_index(); ASSERT_EQ(is_sorted_ref, is_sorted_cuda); } @@ -862,13 +837,14 @@ TEST_F(Csr, SortSortedMatrixIsEquivalentToRef) TEST_F(Csr, SortUnsortedMatrixIsEquivalentToRef) { - auto uns_mtx = gen_unsorted_mtx(); + set_up_apply_data(std::make_shared()); + unsort_mtx(); - uns_mtx.ref->sort_by_column_index(); - uns_mtx.cuda->sort_by_column_index(); + mtx->sort_by_column_index(); + dmtx->sort_by_column_index(); // Values must be unchanged, therefore, tolerance is `0` - GKO_ASSERT_MTX_NEAR(uns_mtx.ref, uns_mtx.cuda, 0); + GKO_ASSERT_MTX_NEAR(mtx, dmtx, 0); } diff --git a/hip/test/matrix/coo_kernels.hip.cpp b/hip/test/matrix/coo_kernels.hip.cpp index b2dd2039926..e722c6cfdb7 100644 --- a/hip/test/matrix/coo_kernels.hip.cpp +++ b/hip/test/matrix/coo_kernels.hip.cpp @@ -48,6 +48,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include "core/matrix/coo_kernels.hpp" +#include "core/test/utils/unsort_matrix.hpp" #include "hip/test/utils.hip.hpp" @@ -104,6 +105,12 @@ class Coo : public ::testing::Test { dbeta->copy_from(beta.get()); } + void unsort_mtx() + { + gko::test::unsort_matrix(mtx.get(), rand_engine); + dmtx->copy_from(mtx.get()); + } + std::shared_ptr ref; std::shared_ptr hip; @@ -135,6 +142,18 @@ TEST_F(Coo, SimpleApplyIsEquivalentToRef) } +TEST_F(Coo, SimpleApplyIsEquivalentToRefUnsorted) +{ + set_up_apply_data(); + unsort_mtx(); + + mtx->apply(y.get(), expected.get()); + dmtx->apply(dy.get(), dresult.get()); + + GKO_ASSERT_MTX_NEAR(dresult, expected, 1e-14); +} + + TEST_F(Coo, AdvancedApplyIsEquivalentToRef) { set_up_apply_data(); diff --git a/hip/test/matrix/csr_kernels.hip.cpp b/hip/test/matrix/csr_kernels.hip.cpp index 1fb01640dfa..fea89e3ff6d 100644 --- a/hip/test/matrix/csr_kernels.hip.cpp +++ b/hip/test/matrix/csr_kernels.hip.cpp @@ -52,6 +52,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include "core/matrix/csr_kernels.hpp" +#include "core/test/utils/unsort_matrix.hpp" #include "hip/test/utils.hip.hpp" @@ -139,36 +140,10 @@ class Csr : public ::testing::Test { complex_dmtx->copy_from(complex_mtx.get()); } - struct matrix_pair { - std::unique_ptr ref; - std::unique_ptr hip; - }; - - matrix_pair gen_unsorted_mtx() + void unsort_mtx() { - constexpr int min_nnz_per_row = 2; // Must be at least 2 - auto local_mtx_ref = - gen_mtx(mtx_size[0], mtx_size[1], min_nnz_per_row); - for (size_t row = 0; row < mtx_size[0]; ++row) { - const auto row_ptrs = local_mtx_ref->get_const_row_ptrs(); - const auto start_row = row_ptrs[row]; - auto col_idx = local_mtx_ref->get_col_idxs() + start_row; - auto vals = local_mtx_ref->get_values() + start_row; - const auto nnz_in_this_row = row_ptrs[row + 1] - row_ptrs[row]; - auto swap_idx_dist = - std::uniform_int_distribution<>(0, nnz_in_this_row - 1); - // shuffle `nnz_in_this_row / 2` times - for (size_t perm = 0; perm < nnz_in_this_row; perm += 2) { - const auto idx1 = swap_idx_dist(rand_engine); - const auto idx2 = swap_idx_dist(rand_engine); - std::swap(col_idx[idx1], col_idx[idx2]); - std::swap(vals[idx1], vals[idx2]); - } - } - auto local_mtx_hip = Mtx::create(hip); - local_mtx_hip->copy_from(local_mtx_ref.get()); - - return {std::move(local_mtx_ref), std::move(local_mtx_hip)}; + gko::test::unsort_matrix(mtx.get(), rand_engine); + dmtx->copy_from(mtx.get()); } std::shared_ptr ref; @@ -219,6 +194,18 @@ TEST_F(Csr, SimpleApplyIsEquivalentToRefWithLoadBalance) } +TEST_F(Csr, SimpleApplyIsEquivalentToRefWithLoadBalanceUnsorted) +{ + set_up_apply_data(std::make_shared(hip)); + unsort_mtx(); + + mtx->apply(y.get(), expected.get()); + dmtx->apply(dy.get(), dresult.get()); + + GKO_ASSERT_MTX_NEAR(dresult, expected, 1e-14); +} + + TEST_F(Csr, AdvancedApplyIsEquivalentToRefWithLoadBalance) { set_up_apply_data(std::make_shared(hip)); @@ -241,6 +228,18 @@ TEST_F(Csr, SimpleApplyIsEquivalentToRefWithHipsparse) } +TEST_F(Csr, SimpleApplyIsEquivalentToRefWithHipsparseUnsorted) +{ + set_up_apply_data(std::make_shared()); + unsort_mtx(); + + mtx->apply(y.get(), expected.get()); + dmtx->apply(dy.get(), dresult.get()); + + GKO_ASSERT_MTX_NEAR(dresult, expected, 1e-14); +} + + TEST_F(Csr, AdvancedApplyIsEquivalentToRefWithHipsparse) { set_up_apply_data(std::make_shared()); @@ -263,6 +262,18 @@ TEST_F(Csr, SimpleApplyIsEquivalentToRefWithMergePath) } +TEST_F(Csr, SimpleApplyIsEquivalentToRefWithMergePathUnsorted) +{ + set_up_apply_data(std::make_shared()); + unsort_mtx(); + + mtx->apply(y.get(), expected.get()); + dmtx->apply(dy.get(), dresult.get()); + + GKO_ASSERT_MTX_NEAR(dresult, expected, 1e-14); +} + + TEST_F(Csr, AdvancedApplyIsEquivalentToRefWithMergePath) { set_up_apply_data(std::make_shared()); @@ -285,6 +296,18 @@ TEST_F(Csr, SimpleApplyIsEquivalentToRefWithClassical) } +TEST_F(Csr, SimpleApplyIsEquivalentToRefWithClassicalUnsorted) +{ + set_up_apply_data(std::make_shared()); + unsort_mtx(); + + mtx->apply(y.get(), expected.get()); + dmtx->apply(dy.get(), dresult.get()); + + GKO_ASSERT_MTX_NEAR(dresult, expected, 1e-14); +} + + TEST_F(Csr, AdvancedApplyIsEquivalentToRefWithClassical) { set_up_apply_data(std::make_shared()); @@ -307,6 +330,18 @@ TEST_F(Csr, SimpleApplyIsEquivalentToRefWithAutomatical) } +TEST_F(Csr, SimpleApplyIsEquivalentToRefWithAutomaticalUnsorted) +{ + set_up_apply_data(std::make_shared(hip)); + unsort_mtx(); + + mtx->apply(y.get(), expected.get()); + dmtx->apply(dy.get(), dresult.get()); + + GKO_ASSERT_MTX_NEAR(dresult, expected, 1e-14); +} + + TEST_F(Csr, SimpleApplyToDenseMatrixIsEquivalentToRefWithLoadBalance) { set_up_apply_data(std::make_shared(hip), 3); @@ -770,7 +805,8 @@ TEST_F(Csr, RecognizeSortedMatrixIsEquivalentToRef) TEST_F(Csr, RecognizeUnsortedMatrixIsEquivalentToRef) { - auto uns_mtx = gen_unsorted_mtx(); + set_up_apply_data(std::make_shared()); + unsort_mtx(); bool is_sorted_hip{}; bool is_sorted_ref{}; @@ -795,13 +831,14 @@ TEST_F(Csr, SortSortedMatrixIsEquivalentToRef) TEST_F(Csr, SortUnsortedMatrixIsEquivalentToRef) { - auto uns_mtx = gen_unsorted_mtx(); + set_up_apply_data(std::make_shared()); + unsort_mtx(); - uns_mtx.ref->sort_by_column_index(); - uns_mtx.hip->sort_by_column_index(); + mtx->sort_by_column_index(); + dmtx->sort_by_column_index(); // Values must be unchanged, therefore, tolerance is `0` - GKO_ASSERT_MTX_NEAR(uns_mtx.ref, uns_mtx.hip, 0); + GKO_ASSERT_MTX_NEAR(mtx, dmtx, 0); } diff --git a/omp/test/matrix/coo_kernels.cpp b/omp/test/matrix/coo_kernels.cpp index fae58704ab2..9f0dcfb7e73 100644 --- a/omp/test/matrix/coo_kernels.cpp +++ b/omp/test/matrix/coo_kernels.cpp @@ -49,6 +49,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include "core/matrix/coo_kernels.hpp" #include "core/test/utils.hpp" +#include "core/test/utils/unsort_matrix.hpp" namespace { @@ -60,7 +61,7 @@ class Coo : public ::testing::Test { using Vec = gko::matrix::Dense<>; using ComplexVec = gko::matrix::Dense>; - Coo() : rand_engine(42) {} + Coo() : mtx_size(532, 231), rand_engine(42) {} void SetUp() { @@ -88,9 +89,9 @@ class Coo : public ::testing::Test { void set_up_apply_data(int num_vectors = 1) { mtx = Mtx::create(ref); - mtx->copy_from(gen_mtx(532, 231, 1)); - expected = gen_mtx(532, num_vectors, 1); - y = gen_mtx(231, num_vectors, 1); + mtx->copy_from(gen_mtx(mtx_size[0], mtx_size[1], 1)); + expected = gen_mtx(mtx_size[0], num_vectors, 1); + y = gen_mtx(mtx_size[1], num_vectors, 1); alpha = gko::initialize({2.0}, ref); beta = gko::initialize({-1.0}, ref); dmtx = Mtx::create(omp); @@ -105,10 +106,30 @@ class Coo : public ::testing::Test { dbeta->copy_from(beta.get()); } + struct matrix_pair { + std::unique_ptr ref; + std::unique_ptr omp; + }; + + matrix_pair gen_unsorted_mtx() + { + constexpr int min_nnz_per_row{2}; + auto local_mtx_ref = Mtx::create(ref); + auto generated = gen_mtx(mtx_size[0], mtx_size[1], min_nnz_per_row); + local_mtx_ref->copy_from(generated.get()); + gko::test::unsort_matrix(local_mtx_ref.get(), rand_engine); + + auto local_mtx_omp = Mtx::create(omp); + local_mtx_omp->copy_from(local_mtx_ref.get()); + + return {std::move(local_mtx_ref), std::move(local_mtx_omp)}; + } + std::shared_ptr ref; std::shared_ptr omp; + const gko::dim<2> mtx_size; std::ranlux48 rand_engine; std::unique_ptr mtx; @@ -202,6 +223,18 @@ TEST_F(Coo, SimpleApplyAddToDenseMatrixIsEquivalentToRef) } +TEST_F(Coo, SimpleApplyAddToDenseMatrixIsEquivalentToRefUnsorted) +{ + set_up_apply_data(3); + auto pair = gen_unsorted_mtx(); + + pair.ref->apply2(y.get(), expected.get()); + pair.omp->apply2(dy.get(), dresult.get()); + + GKO_ASSERT_MTX_NEAR(dresult, expected, 1e-14); +} + + TEST_F(Coo, AdvancedApplyAddToDenseMatrixIsEquivalentToRef) { set_up_apply_data(3); diff --git a/omp/test/matrix/csr_kernels.cpp b/omp/test/matrix/csr_kernels.cpp index 064a129b9eb..26af3397262 100644 --- a/omp/test/matrix/csr_kernels.cpp +++ b/omp/test/matrix/csr_kernels.cpp @@ -53,6 +53,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include "core/matrix/csr_kernels.hpp" #include "core/test/utils.hpp" +#include "core/test/utils/unsort_matrix.hpp" namespace { @@ -139,25 +140,11 @@ class Csr : public ::testing::Test { matrix_pair gen_unsorted_mtx() { - constexpr int min_nnz_per_row = 2; // Must be at least 2 + constexpr int min_nnz_per_row{2}; auto local_mtx_ref = gen_mtx(mtx_size[0], mtx_size[1], min_nnz_per_row); - for (size_t row = 0; row < mtx_size[0]; ++row) { - const auto row_ptrs = local_mtx_ref->get_const_row_ptrs(); - const auto start_row = row_ptrs[row]; - auto col_idx = local_mtx_ref->get_col_idxs() + start_row; - auto vals = local_mtx_ref->get_values() + start_row; - const auto nnz_in_this_row = row_ptrs[row + 1] - row_ptrs[row]; - auto swap_idx_dist = - std::uniform_int_distribution<>(0, nnz_in_this_row - 1); - // shuffle `nnz_in_this_row / 2` times - for (size_t perm = 0; perm < nnz_in_this_row; perm += 2) { - const auto idx1 = swap_idx_dist(rand_engine); - const auto idx2 = swap_idx_dist(rand_engine); - std::swap(col_idx[idx1], col_idx[idx2]); - std::swap(vals[idx1], vals[idx2]); - } - } + gko::test::unsort_matrix(gko::lend(local_mtx_ref), rand_engine); + auto local_mtx_omp = Mtx::create(omp); local_mtx_omp->copy_from(local_mtx_ref.get()); @@ -225,6 +212,18 @@ TEST_F(Csr, SimpleApplyToDenseMatrixIsEquivalentToRef) } +TEST_F(Csr, SimpleApplyToDenseMatrixIsEquivalentToRefUnsorted) +{ + set_up_apply_data(3); + auto pair = gen_unsorted_mtx(); + + pair.ref->apply(y.get(), expected.get()); + pair.omp->apply(dy.get(), dresult.get()); + + GKO_ASSERT_MTX_NEAR(dresult, expected, 1e-14); +} + + TEST_F(Csr, AdvancedApplyToCsrMatrixIsEquivalentToRef) { set_up_apply_data(); diff --git a/reference/test/matrix/coo_kernels.cpp b/reference/test/matrix/coo_kernels.cpp index 68657a8702e..2e7d7764c99 100644 --- a/reference/test/matrix/coo_kernels.cpp +++ b/reference/test/matrix/coo_kernels.cpp @@ -33,6 +33,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include +#include #include @@ -70,6 +71,11 @@ class Coo : public ::testing::Test { mtx = gko::initialize({{1.0, 3.0, 2.0}, {0.0, 5.0, 0.0}}, exec); // clang-format on + uns_mtx = gko::clone(exec, mtx); + auto cols = uns_mtx->get_col_idxs(); + auto vals = uns_mtx->get_values(); + std::swap(cols[0], cols[1]); + std::swap(vals[0], vals[1]); } void assert_equal_to_mtx_in_csr_format(const Csr *m) @@ -94,6 +100,7 @@ class Coo : public ::testing::Test { std::shared_ptr exec; std::unique_ptr mtx; + std::unique_ptr uns_mtx; }; TYPED_TEST_SUITE(Coo, gko::test::ValueIndexTypes); @@ -199,6 +206,23 @@ TYPED_TEST(Coo, ConvertsToDense) } +TYPED_TEST(Coo, ConvertsToDenseUnsorted) +{ + using value_type = typename TestFixture::value_type; + using index_type = typename TestFixture::index_type; + using Dense = typename TestFixture::Vec; + auto dense_mtx = Dense::create(this->mtx->get_executor()); + + this->uns_mtx->convert_to(dense_mtx.get()); + + // clang-format off + GKO_ASSERT_MTX_NEAR(dense_mtx, + l({{1.0, 3.0, 2.0}, + {0.0, 5.0, 0.0}}), 0.0); + // clang-format on +} + + TYPED_TEST(Coo, MovesToDense) { using value_type = typename TestFixture::value_type; @@ -325,6 +349,18 @@ TYPED_TEST(Coo, AppliesToDenseVector) } +TYPED_TEST(Coo, AppliesToDenseVectorUnsorted) +{ + using Vec = typename TestFixture::Vec; + auto x = gko::initialize({2.0, 1.0, 4.0}, this->exec); + auto y = Vec::create(this->exec, gko::dim<2>{2, 1}); + + this->uns_mtx->apply(x.get(), y.get()); + + GKO_ASSERT_MTX_NEAR(y, l({13.0, 5.0}), 0.0); +} + + TYPED_TEST(Coo, AppliesToDenseMatrix) { using Vec = typename TestFixture::Vec; @@ -347,6 +383,28 @@ TYPED_TEST(Coo, AppliesToDenseMatrix) } +TYPED_TEST(Coo, AppliesToDenseMatrixUnsorted) +{ + using Vec = typename TestFixture::Vec; + using T = typename TestFixture::value_type; + // clang-format off + auto x = gko::initialize( + {I{2.0, 3.0}, + I{1.0, -1.5}, + I{4.0, 2.5}}, this->exec); + // clang-format on + auto y = Vec::create(this->exec, gko::dim<2>{2, 2}); + + this->uns_mtx->apply(x.get(), y.get()); + + // clang-format off + GKO_ASSERT_MTX_NEAR(y, + l({{13.0, 3.5}, + { 5.0, -7.5}}), 0.0); + // clang-format on +} + + TYPED_TEST(Coo, AppliesLinearCombinationToDenseVector) { using Vec = typename TestFixture::Vec; From 4e50e1f6ae36e9d0655107628b7f63cdca2a3134 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Gr=C3=BCtzmacher?= Date: Thu, 29 Oct 2020 17:57:59 +0100 Subject: [PATCH 05/13] Add sorting to ILU factorization --- core/factorization/ilu.cpp | 6 ++- core/test/utils/unsort_matrix.hpp | 18 ++++++++- cuda/test/factorization/ilu_kernels.cpp | 45 ++++++++++++++++++++--- hip/test/factorization/ilu_kernels.cpp | 44 +++++++++++++++++++--- include/ginkgo/core/factorization/ilu.hpp | 27 +++++++++++++- reference/test/preconditioner/jacobi.cpp | 25 ++++++++----- 6 files changed, 140 insertions(+), 25 deletions(-) diff --git a/core/factorization/ilu.cpp b/core/factorization/ilu.cpp index c2f397151d3..22655b463eb 100644 --- a/core/factorization/ilu.cpp +++ b/core/factorization/ilu.cpp @@ -63,7 +63,7 @@ GKO_REGISTER_OPERATION(initialize_l_u, factorization::initialize_l_u); template std::unique_ptr> Ilu::generate_l_u( - const std::shared_ptr &system_matrix) const + const std::shared_ptr &system_matrix, bool skip_sorting) const { GKO_ASSERT_IS_SQUARE_MATRIX(system_matrix); @@ -75,6 +75,10 @@ std::unique_ptr> Ilu::generate_l_u( as>(system_matrix.get()) ->convert_to(local_system_matrix.get()); + if (!skip_sorting) { + local_system_matrix->sort_by_column_index(); + } + // Add explicit diagonal zero elements if they are missing exec->run(ilu_factorization::make_add_diagonal_elements( local_system_matrix.get(), false)); diff --git a/core/test/utils/unsort_matrix.hpp b/core/test/utils/unsort_matrix.hpp index ff2ff500e0e..9e0759c8097 100644 --- a/core/test/utils/unsort_matrix.hpp +++ b/core/test/utils/unsort_matrix.hpp @@ -37,6 +37,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include +#include #include @@ -60,9 +61,24 @@ void unsort_matrix(matrix::Csr *mtx, return; } const auto &exec = mtx->get_executor(); - const auto &master = mtx->get_executor()->get_master(); + auto master = mtx->get_executor()->get_master(); + // If exec is not the master/host, extract the master and perform the + // unsorting there, followed by copying it back if (exec != master) { + constexpr int max_depth{10}; // Max depth that is searched for master + bool actual_master_found{false}; + for (int i = 0; i < max_depth; ++i) { + const auto new_master = master->get_master(); + if (new_master == master) { + actual_master_found = true; + break; + } + master = new_master; + } + if (!actual_master_found) { + GKO_NOT_SUPPORTED(exec); + } auto h_mtx = mtx->clone(master); unsort_matrix(lend(h_mtx), engine); mtx->copy_from(lend(h_mtx)); diff --git a/cuda/test/factorization/ilu_kernels.cpp b/cuda/test/factorization/ilu_kernels.cpp index 4c1d356b0d0..f0025651a48 100644 --- a/cuda/test/factorization/ilu_kernels.cpp +++ b/cuda/test/factorization/ilu_kernels.cpp @@ -46,6 +46,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include +#include "core/test/utils/unsort_matrix.hpp" #include "cuda/test/utils.hpp" #include "matrices/config.hpp" @@ -61,12 +62,14 @@ class Ilu : public ::testing::Test { std::shared_ptr ref; std::shared_ptr cuda; + std::ranlux48 rand_engine; std::shared_ptr csr_ref; std::shared_ptr csr_cuda; Ilu() : ref(gko::ReferenceExecutor::create()), - cuda(gko::CudaExecutor::create(0, ref)) + cuda(gko::CudaExecutor::create(0, ref)), + rand_engine(1337) {} void SetUp() override @@ -84,12 +87,42 @@ class Ilu : public ::testing::Test { }; -TEST_F(Ilu, ComputeILUIsEquivalentToRef) +TEST_F(Ilu, ComputeILUIsEquivalentToRefSorted) { - auto ref_fact = - gko::factorization::ParIlu<>::build().on(ref)->generate(csr_ref); - auto cuda_fact = - gko::factorization::Ilu<>::build().on(cuda)->generate(csr_cuda); + auto ref_fact = gko::factorization::ParIlu<>::build() + .with_skip_sorting(true) + .on(ref) + ->generate(csr_ref); + auto cuda_fact = gko::factorization::Ilu<>::build() + .with_skip_sorting(true) + .on(cuda) + ->generate(csr_cuda); + + GKO_ASSERT_MTX_NEAR(ref_fact->get_l_factor(), cuda_fact->get_l_factor(), + 1e-14); + GKO_ASSERT_MTX_NEAR(ref_fact->get_u_factor(), cuda_fact->get_u_factor(), + 1e-14); + GKO_ASSERT_MTX_EQ_SPARSITY(ref_fact->get_l_factor(), + cuda_fact->get_l_factor()); + GKO_ASSERT_MTX_EQ_SPARSITY(ref_fact->get_u_factor(), + cuda_fact->get_u_factor()); +} + + +TEST_F(Ilu, ComputeILUIsEquivalentToRefUnsorted) +{ + gko::test::unsort_matrix(gko::lend(csr_ref), rand_engine); + csr_cuda->copy_from(gko::lend(csr_ref)); + + // TODO: Remove sorting, just for testing here! + auto ref_fact = gko::factorization::ParIlu<>::build() + .with_skip_sorting(true) + .on(ref) + ->generate(csr_ref); + auto cuda_fact = gko::factorization::Ilu<>::build() + .with_skip_sorting(true) + .on(cuda) + ->generate(csr_cuda); GKO_ASSERT_MTX_NEAR(ref_fact->get_l_factor(), cuda_fact->get_l_factor(), 1e-14); diff --git a/hip/test/factorization/ilu_kernels.cpp b/hip/test/factorization/ilu_kernels.cpp index b0bffcdd430..ce69b38602e 100644 --- a/hip/test/factorization/ilu_kernels.cpp +++ b/hip/test/factorization/ilu_kernels.cpp @@ -61,12 +61,14 @@ class Ilu : public ::testing::Test { std::shared_ptr ref; std::shared_ptr hip; + std::randlux48 rand_engine; std::shared_ptr csr_ref; std::shared_ptr csr_hip; Ilu() : ref(gko::ReferenceExecutor::create()), - hip(gko::HipExecutor::create(0, ref)) + hip(gko::HipExecutor::create(0, ref)), + rand_engine(1337) {} void SetUp() override @@ -84,12 +86,42 @@ class Ilu : public ::testing::Test { }; -TEST_F(Ilu, ComputeILUIsEquivalentToRef) +TEST_F(Ilu, ComputeILUIsEquivalentToRefSorted) { - auto ref_fact = - gko::factorization::ParIlu<>::build().on(ref)->generate(csr_ref); - auto hip_fact = - gko::factorization::Ilu<>::build().on(hip)->generate(csr_hip); + auto ref_fact = gko::factorization::ParIlu<>::build() + .with_skip_sorting(true) + .on(ref) + ->generate(csr_ref); + auto hip_fact = gko::factorization::Ilu<>::build() + .with_skip_sorting(true) + .on(hip) + ->generate(csr_hip); + + GKO_ASSERT_MTX_NEAR(ref_fact->get_l_factor(), hip_fact->get_l_factor(), + 1e-14); + GKO_ASSERT_MTX_NEAR(ref_fact->get_u_factor(), hip_fact->get_u_factor(), + 1e-14); + GKO_ASSERT_MTX_EQ_SPARSITY(ref_fact->get_l_factor(), + hip_fact->get_l_factor()); + GKO_ASSERT_MTX_EQ_SPARSITY(ref_fact->get_u_factor(), + hip_fact->get_u_factor()); +} + + +TEST_F(Ilu, ComputeILUIsEquivalentToRefUnsorted) +{ + gko::test::unsort_matrix(gko::lend(csr_ref), rand_engine); + csr_hip->copy_from(gko::lend(csr_ref)); + + // TODO: Remove sorting, just for testing here! + auto ref_fact = gko::factorization::ParIlu<>::build() + .with_skip_sorting(true) + .on(ref) + ->generate(csr_ref); + auto hip_fact = gko::factorization::Ilu<>::build() + .with_skip_sorting(true) + .on(hip) + ->generate(csr_hip); GKO_ASSERT_MTX_NEAR(ref_fact->get_l_factor(), hip_fact->get_l_factor(), 1e-14); diff --git a/include/ginkgo/core/factorization/ilu.hpp b/include/ginkgo/core/factorization/ilu.hpp index ee71beb56af..ece4e2aeb87 100644 --- a/include/ginkgo/core/factorization/ilu.hpp +++ b/include/ginkgo/core/factorization/ilu.hpp @@ -109,6 +109,25 @@ class Ilu : public Composition { */ std::shared_ptr GKO_FACTORY_PARAMETER_SCALAR(u_strategy, nullptr); + + /** + * @brief `true` means it is known that the matrix given to this + * factory will be sorted first by row, then by column index, + * `false` means it is unknown or not sorted, so an additional + * sorting step will be performed during the factorization + * (it will not change the matrix given). + * The matrix must be sorted for this factorization to work. + * + * The `system_matrix`, which will be given to this factory, must be + * sorted (first by row, then by column) in order for the algorithm + * to work. If it is known that the matrix will be sorted, this + * parameter can be set to `true` to skip the sorting (therefore, + * shortening the runtime). + * However, if it is unknown or if the matrix is known to be not sorted, + * it must remain `false`, otherwise, this factorization might be + * incorrect. + */ + bool GKO_FACTORY_PARAMETER_SCALAR(skip_sorting, false); }; GKO_ENABLE_LIN_OP_FACTORY(Ilu, parameters, Factory); GKO_ENABLE_BUILD_METHOD(Factory); @@ -126,7 +145,7 @@ class Ilu : public Composition { parameters_.u_strategy = std::make_shared(); } - generate_l_u(system_matrix)->move_to(this); + generate_l_u(system_matrix, parameters_.skip_sorting)->move_to(this); } /** @@ -138,11 +157,15 @@ class Ilu : public Composition { * @param system_matrix the source matrix used to generate the factors. * @note: system_matrix must be convertible to a Csr * Matrix, otherwise, an exception is thrown. + * @param skip_sorting determines if the sorting of system_matrix can be + * skipped (therefore, marking that it is already + * sorted) * @return A Composition, containing the incomplete LU factors for the * given system_matrix (first element is L, then U) */ std::unique_ptr> generate_l_u( - const std::shared_ptr &system_matrix) const; + const std::shared_ptr &system_matrix, + bool skip_sorting) const; }; diff --git a/reference/test/preconditioner/jacobi.cpp b/reference/test/preconditioner/jacobi.cpp index 586825a2d3b..af0e9d9005b 100644 --- a/reference/test/preconditioner/jacobi.cpp +++ b/reference/test/preconditioner/jacobi.cpp @@ -314,16 +314,11 @@ TYPED_TEST(Jacobi, CanBeClearedWithAdaptivePrecision) GKO_EXPECT_NEAR(first.value, second.value, tol) -TYPED_TEST(Jacobi, GeneratesCorrectMatrixData) +template +void assert_matrix_data(gko::matrix_data data) { - using value_type = typename TestFixture::value_type; - using index_type = typename TestFixture::index_type; - using tpl = typename gko::matrix_data::nonzero_type; - auto tol = r::value; - gko::matrix_data data; - - this->bj->write(data); - + using tpl = typename decltype(data)::nonzero_type; + auto tol = r::value; ASSERT_EQ(data.size, gko::dim<2>{5}); ASSERT_EQ(data.nonzeros.size(), 13); GKO_EXPECT_NONZERO_NEAR(data.nonzeros[0], tpl(0, 0, 4.0 / 14), tol); @@ -342,6 +337,18 @@ TYPED_TEST(Jacobi, GeneratesCorrectMatrixData) } +TYPED_TEST(Jacobi, GeneratesCorrectMatrixData) +{ + using value_type = typename TestFixture::value_type; + using index_type = typename TestFixture::index_type; + gko::matrix_data data; + + this->bj->write(data); + + assert_matrix_data(data); +} + + TYPED_TEST(Jacobi, GeneratesCorrectMatrixDataWithAdaptivePrecision) { using value_type = typename TestFixture::value_type; From 1c20e2bc098c38eeec22c33f916bcb493ffc9f36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Gr=C3=BCtzmacher?= Date: Fri, 30 Oct 2020 10:12:06 +0100 Subject: [PATCH 06/13] Add Jacobi preconditioner test for unsorted Mtx --- cuda/test/factorization/ilu_kernels.cpp | 13 ++++-------- cuda/test/preconditioner/jacobi_kernels.cpp | 22 +++++++++++++++++++-- hip/test/factorization/ilu_kernels.cpp | 16 ++++++--------- hip/test/preconditioner/jacobi_kernels.cpp | 20 +++++++++++++++++-- omp/test/preconditioner/jacobi_kernels.cpp | 22 +++++++++++++++++++-- 5 files changed, 68 insertions(+), 25 deletions(-) diff --git a/cuda/test/factorization/ilu_kernels.cpp b/cuda/test/factorization/ilu_kernels.cpp index f0025651a48..bcb18de9fae 100644 --- a/cuda/test/factorization/ilu_kernels.cpp +++ b/cuda/test/factorization/ilu_kernels.cpp @@ -114,15 +114,10 @@ TEST_F(Ilu, ComputeILUIsEquivalentToRefUnsorted) gko::test::unsort_matrix(gko::lend(csr_ref), rand_engine); csr_cuda->copy_from(gko::lend(csr_ref)); - // TODO: Remove sorting, just for testing here! - auto ref_fact = gko::factorization::ParIlu<>::build() - .with_skip_sorting(true) - .on(ref) - ->generate(csr_ref); - auto cuda_fact = gko::factorization::Ilu<>::build() - .with_skip_sorting(true) - .on(cuda) - ->generate(csr_cuda); + auto ref_fact = + gko::factorization::ParIlu<>::build().on(ref)->generate(csr_ref); + auto cuda_fact = + gko::factorization::Ilu<>::build().on(cuda)->generate(csr_cuda); GKO_ASSERT_MTX_NEAR(ref_fact->get_l_factor(), cuda_fact->get_l_factor(), 1e-14); diff --git a/cuda/test/preconditioner/jacobi_kernels.cpp b/cuda/test/preconditioner/jacobi_kernels.cpp index 05ea7d766e8..2d90883a852 100644 --- a/cuda/test/preconditioner/jacobi_kernels.cpp +++ b/cuda/test/preconditioner/jacobi_kernels.cpp @@ -44,6 +44,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include "core/test/utils.hpp" +#include "core/test/utils/unsort_matrix.hpp" namespace { @@ -75,7 +76,7 @@ class Jacobi : public ::testing::Test { std::initializer_list block_precisions, std::initializer_list condition_numbers, gko::uint32 max_block_size, int min_nnz, int max_nnz, int num_rhs = 1, - double accuracy = 0.1) + double accuracy = 0.1, bool skip_sorting = true) { std::ranlux48 engine(42); const auto dim = *(end(block_pointers) - 1); @@ -101,10 +102,12 @@ class Jacobi : public ::testing::Test { bj_factory = Bj::build() .with_max_block_size(max_block_size) .with_block_pointers(block_ptrs) + .with_skip_sorting(skip_sorting) .on(ref); d_bj_factory = Bj::build() .with_max_block_size(max_block_size) .with_block_pointers(block_ptrs) + .with_skip_sorting(skip_sorting) .on(cuda); } else { bj_factory = Bj::build() @@ -112,12 +115,14 @@ class Jacobi : public ::testing::Test { .with_block_pointers(block_ptrs) .with_storage_optimization(block_prec) .with_accuracy(accuracy) + .with_skip_sorting(skip_sorting) .on(ref); d_bj_factory = Bj::build() .with_max_block_size(max_block_size) .with_block_pointers(block_ptrs) .with_storage_optimization(block_prec) .with_accuracy(accuracy) + .with_skip_sorting(skip_sorting) .on(cuda); } b = gko::test::generate_random_matrix( @@ -290,7 +295,7 @@ TEST_F(Jacobi, } -TEST_F(Jacobi, CudaPreconditionerEquivalentToRefWithBlockSize32) +TEST_F(Jacobi, CudaPreconditionerEquivalentToRefWithBlockSize32Sorted) { initialize_data({0, 32, 64, 96, 128}, {}, {}, 32, 100, 110); @@ -301,6 +306,19 @@ TEST_F(Jacobi, CudaPreconditionerEquivalentToRefWithBlockSize32) } +TEST_F(Jacobi, CudaPreconditionerEquivalentToRefWithBlockSize32Unsorted) +{ + std::ranlux48 engine(42); + initialize_data({0, 32, 64, 96, 128}, {}, {}, 32, 100, 110, 1, 0.1, false); + gko::test::unsort_matrix(mtx.get(), engine); + + auto bj = bj_factory->generate(mtx); + auto d_bj = d_bj_factory->generate(mtx); + + GKO_ASSERT_MTX_NEAR(gko::as(d_bj.get()), gko::as(bj.get()), 1e-13); +} + + TEST_F(Jacobi, CudaPreconditionerEquivalentToRefWithDifferentBlockSize) { initialize_data({0, 11, 24, 33, 45, 55, 67, 70, 80, 92, 100}, {}, {}, 32, diff --git a/hip/test/factorization/ilu_kernels.cpp b/hip/test/factorization/ilu_kernels.cpp index ce69b38602e..3cd8f2be763 100644 --- a/hip/test/factorization/ilu_kernels.cpp +++ b/hip/test/factorization/ilu_kernels.cpp @@ -46,6 +46,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include +#include "core/test/utils/unsort_matrix.hpp" #include "hip/test/utils.hip.hpp" #include "matrices/config.hpp" @@ -61,7 +62,7 @@ class Ilu : public ::testing::Test { std::shared_ptr ref; std::shared_ptr hip; - std::randlux48 rand_engine; + std::ranlux48 rand_engine; std::shared_ptr csr_ref; std::shared_ptr csr_hip; @@ -113,15 +114,10 @@ TEST_F(Ilu, ComputeILUIsEquivalentToRefUnsorted) gko::test::unsort_matrix(gko::lend(csr_ref), rand_engine); csr_hip->copy_from(gko::lend(csr_ref)); - // TODO: Remove sorting, just for testing here! - auto ref_fact = gko::factorization::ParIlu<>::build() - .with_skip_sorting(true) - .on(ref) - ->generate(csr_ref); - auto hip_fact = gko::factorization::Ilu<>::build() - .with_skip_sorting(true) - .on(hip) - ->generate(csr_hip); + auto ref_fact = + gko::factorization::ParIlu<>::build().on(ref)->generate(csr_ref); + auto hip_fact = + gko::factorization::Ilu<>::build().on(hip)->generate(csr_hip); GKO_ASSERT_MTX_NEAR(ref_fact->get_l_factor(), hip_fact->get_l_factor(), 1e-14); diff --git a/hip/test/preconditioner/jacobi_kernels.cpp b/hip/test/preconditioner/jacobi_kernels.cpp index 868e10fbbad..c6a48f2c0c3 100644 --- a/hip/test/preconditioner/jacobi_kernels.cpp +++ b/hip/test/preconditioner/jacobi_kernels.cpp @@ -75,7 +75,7 @@ class Jacobi : public ::testing::Test { std::initializer_list block_precisions, std::initializer_list condition_numbers, gko::uint32 max_block_size, int min_nnz, int max_nnz, int num_rhs = 1, - double accuracy = 0.1) + double accuracy = 0.1, bool skip_sorting = true) { std::ranlux48 engine(42); const auto dim = *(end(block_pointers) - 1); @@ -103,6 +103,7 @@ class Jacobi : public ::testing::Test { .with_max_block_size(max_block_size) .with_block_pointers(block_ptrs) .with_max_block_stride(gko::uint32(hip->get_warp_size())) + .with_skip_sorting(skip_sorting) .on(ref); d_bj_factory = Bj::build() .with_max_block_size(max_block_size) @@ -116,12 +117,14 @@ class Jacobi : public ::testing::Test { .with_max_block_stride(gko::uint32(hip->get_warp_size())) .with_storage_optimization(block_prec) .with_accuracy(accuracy) + .with_skip_sorting(skip_sorting) .on(ref); d_bj_factory = Bj::build() .with_max_block_size(max_block_size) .with_block_pointers(block_ptrs) .with_storage_optimization(block_prec) .with_accuracy(accuracy) + .with_skip_sorting(skip_sorting) .on(hip); } b = gko::test::generate_random_matrix( @@ -294,7 +297,7 @@ TEST_F(Jacobi, } -TEST_F(Jacobi, HipPreconditionerEquivalentToRefWithBlockSize32) +TEST_F(Jacobi, HipPreconditionerEquivalentToRefWithBlockSize32Sorted) { initialize_data({0, 32, 64, 96, 128}, {}, {}, 32, 100, 110); @@ -305,6 +308,19 @@ TEST_F(Jacobi, HipPreconditionerEquivalentToRefWithBlockSize32) } +TEST_F(Jacobi, HipPreconditionerEquivalentToRefWithBlockSize32Unsorted) +{ + std::ranlux48 engine(43); + initialize_data({0, 32, 64, 96, 128}, {}, {}, 32, 100, 110, 0.1, false); + gko::test::unsort_matrix(mtx.get(), engine); + + auto bj = bj_factory->generate(mtx); + auto d_bj = d_bj_factory->generate(mtx); + + GKO_ASSERT_MTX_NEAR(gko::as(d_bj.get()), gko::as(bj.get()), 1e-13); +} + + #if GINKGO_HIP_PLATFORM_HCC TEST_F(Jacobi, HipPreconditionerEquivalentToRefWithBlockSize64) { diff --git a/omp/test/preconditioner/jacobi_kernels.cpp b/omp/test/preconditioner/jacobi_kernels.cpp index 0753d22f008..c1d3a03ee32 100644 --- a/omp/test/preconditioner/jacobi_kernels.cpp +++ b/omp/test/preconditioner/jacobi_kernels.cpp @@ -44,6 +44,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include "core/test/utils.hpp" +#include "core/test/utils/unsort_matrix.hpp" namespace { @@ -74,7 +75,7 @@ class Jacobi : public ::testing::Test { std::initializer_list block_precisions, std::initializer_list condition_numbers, gko::uint32 max_block_size, int min_nnz, int max_nnz, int num_rhs = 1, - double accuracy = 0.1) + double accuracy = 0.1, bool skip_sorting = true) { std::ranlux48 engine(42); const auto dim = *(end(block_pointers) - 1); @@ -100,10 +101,12 @@ class Jacobi : public ::testing::Test { bj_factory = Bj::build() .with_max_block_size(max_block_size) .with_block_pointers(block_ptrs) + .with_skip_sorting(skip_sorting) .on(ref); d_bj_factory = Bj::build() .with_max_block_size(max_block_size) .with_block_pointers(block_ptrs) + .with_skip_sorting(skip_sorting) .on(omp); } else { bj_factory = Bj::build() @@ -111,12 +114,14 @@ class Jacobi : public ::testing::Test { .with_block_pointers(block_ptrs) .with_storage_optimization(block_prec) .with_accuracy(accuracy) + .with_skip_sorting(skip_sorting) .on(ref); d_bj_factory = Bj::build() .with_max_block_size(max_block_size) .with_block_pointers(block_ptrs) .with_storage_optimization(block_prec) .with_accuracy(accuracy) + .with_skip_sorting(skip_sorting) .on(omp); } b = gko::test::generate_random_matrix( @@ -289,7 +294,7 @@ TEST_F(Jacobi, } -TEST_F(Jacobi, OmpPreconditionerEquivalentToRefWithBlockSize32) +TEST_F(Jacobi, OmpPreconditionerEquivalentToRefWithBlockSize32Sorted) { initialize_data({0, 32, 64, 96, 128}, {}, {}, 32, 100, 110); @@ -300,6 +305,19 @@ TEST_F(Jacobi, OmpPreconditionerEquivalentToRefWithBlockSize32) } +TEST_F(Jacobi, OmpPreconditionerEquivalentToRefWithBlockSize32Unsorted) +{ + std::ranlux48 engine(43); + initialize_data({0, 32, 64, 96, 128}, {}, {}, 32, 100, 110, 0.1, false); + gko::test::unsort_matrix(mtx.get(), engine); + + auto bj = bj_factory->generate(mtx); + auto d_bj = d_bj_factory->generate(mtx); + + GKO_ASSERT_MTX_NEAR(gko::as(d_bj.get()), gko::as(bj.get()), 1e-14); +} + + TEST_F(Jacobi, OmpPreconditionerEquivalentToRefWithDifferentBlockSize) { initialize_data({0, 11, 24, 33, 45, 55, 67, 70, 80, 92, 100}, {}, {}, 32, From d43044743464e6782eda02258a872c84c5e09aa6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Gr=C3=BCtzmacher?= Date: Fri, 30 Oct 2020 12:34:41 +0100 Subject: [PATCH 07/13] Fix typo in HIP CSR test --- hip/test/matrix/csr_kernels.hip.cpp | 4 ++-- hip/test/preconditioner/jacobi_kernels.cpp | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/hip/test/matrix/csr_kernels.hip.cpp b/hip/test/matrix/csr_kernels.hip.cpp index fea89e3ff6d..ea3b3a2395f 100644 --- a/hip/test/matrix/csr_kernels.hip.cpp +++ b/hip/test/matrix/csr_kernels.hip.cpp @@ -810,8 +810,8 @@ TEST_F(Csr, RecognizeUnsortedMatrixIsEquivalentToRef) bool is_sorted_hip{}; bool is_sorted_ref{}; - is_sorted_ref = uns_mtx.ref->is_sorted_by_column_index(); - is_sorted_hip = uns_mtx.hip->is_sorted_by_column_index(); + is_sorted_ref = mtx->is_sorted_by_column_index(); + is_sorted_hip = dmtx->is_sorted_by_column_index(); ASSERT_EQ(is_sorted_ref, is_sorted_hip); } diff --git a/hip/test/preconditioner/jacobi_kernels.cpp b/hip/test/preconditioner/jacobi_kernels.cpp index c6a48f2c0c3..d44716534f4 100644 --- a/hip/test/preconditioner/jacobi_kernels.cpp +++ b/hip/test/preconditioner/jacobi_kernels.cpp @@ -43,6 +43,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include +#include "core/test/utils/unsort_matrix.hpp" #include "hip/test/utils.hip.hpp" @@ -311,7 +312,7 @@ TEST_F(Jacobi, HipPreconditionerEquivalentToRefWithBlockSize32Sorted) TEST_F(Jacobi, HipPreconditionerEquivalentToRefWithBlockSize32Unsorted) { std::ranlux48 engine(43); - initialize_data({0, 32, 64, 96, 128}, {}, {}, 32, 100, 110, 0.1, false); + initialize_data({0, 32, 64, 96, 128}, {}, {}, 32, 100, 110, 1, 0.1, false); gko::test::unsort_matrix(mtx.get(), engine); auto bj = bj_factory->generate(mtx); From 4bd07af7cce93a92540509d294601b74672df502 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Gr=C3=BCtzmacher?= Date: Fri, 6 Nov 2020 13:00:20 +0100 Subject: [PATCH 08/13] Make core-tests independent of executor Additionally, address review comments. Co-authored-by: Terry Cojean --- core/preconditioner/jacobi.cpp | 1 - core/test/utils/unsort_matrix_test.cpp | 36 +++++++++++++++++++++----- 2 files changed, 29 insertions(+), 8 deletions(-) diff --git a/core/preconditioner/jacobi.cpp b/core/preconditioner/jacobi.cpp index a8a8ad7ba2f..6d52c5b4838 100644 --- a/core/preconditioner/jacobi.cpp +++ b/core/preconditioner/jacobi.cpp @@ -219,7 +219,6 @@ void Jacobi::generate(const LinOp *system_matrix, auto editable_csr = csr_type::create(exec); as>(system_matrix) ->convert_to(lend(editable_csr)); - // Maybe check if it is sorted first editable_csr->sort_by_column_index(); csr_mtx = decltype(csr_mtx){editable_csr.release(), std::default_delete{}}; diff --git a/core/test/utils/unsort_matrix_test.cpp b/core/test/utils/unsort_matrix_test.cpp index 9e80cdca83e..afeb22ae246 100644 --- a/core/test/utils/unsort_matrix_test.cpp +++ b/core/test/utils/unsort_matrix_test.cpp @@ -75,10 +75,8 @@ class UnsortMatrix : public ::testing::Test { bool is_coo_matrix_sorted(Coo *mtx) { - auto size = mtx->get_size(); - auto vals = mtx->get_values(); - auto rows = mtx->get_row_idxs(); - auto cols = mtx->get_col_idxs(); + auto rows = mtx->get_const_row_idxs(); + auto cols = mtx->get_const_col_idxs(); auto nnz = mtx->get_num_stored_elements(); if (nnz <= 0) { @@ -99,6 +97,30 @@ class UnsortMatrix : public ::testing::Test { return true; } + bool is_csr_matrix_sorted(Csr *mtx) + { + auto size = mtx->get_size(); + auto rows = mtx->get_const_row_ptrs(); + auto cols = mtx->get_const_col_idxs(); + auto nnz = mtx->get_num_stored_elements(); + + if (nnz <= 0) { + return true; + } + + for (index_type row = 0; row < size[1]; ++row) { + auto prev_col = cols[rows[row]]; + for (index_type i = rows[row]; i < rows[row + 1]; ++i) { + auto cur_col = cols[i]; + if (prev_col > cur_col) { + return false; + } + prev_col = cur_col; + } + } + return true; + } + std::shared_ptr exec; std::ranlux48 rand_engine; std::unique_ptr mtx; @@ -110,11 +132,11 @@ TEST_F(UnsortMatrix, CsrWorks) { auto csr = Csr::create(exec); mtx->convert_to(gko::lend(csr)); - bool was_sorted = csr->is_sorted_by_column_index(); + bool was_sorted = is_csr_matrix_sorted(gko::lend(csr)); gko::test::unsort_matrix(gko::lend(csr), rand_engine); - ASSERT_FALSE(csr->is_sorted_by_column_index()); + ASSERT_FALSE(is_csr_matrix_sorted(gko::lend(csr))); ASSERT_TRUE(was_sorted); GKO_ASSERT_MTX_NEAR(csr, mtx, 0.); } @@ -124,7 +146,7 @@ TEST_F(UnsortMatrix, CsrWorksWithEmpty) { auto csr = Csr::create(exec); empty->convert_to(gko::lend(csr)); - bool was_sorted = csr->is_sorted_by_column_index(); + bool was_sorted = is_csr_matrix_sorted(gko::lend(csr)); gko::test::unsort_matrix(gko::lend(csr), rand_engine); From 12ed74461285883f7d2d0db3feac8a9ca188ab29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Gr=C3=BCtzmacher?= Date: Fri, 6 Nov 2020 13:10:01 +0100 Subject: [PATCH 09/13] Search the proper master executor for COO matrices Additionally, remove unnecessary variables in CSR and COO `unsort_matrix`. --- core/test/utils/unsort_matrix.hpp | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/core/test/utils/unsort_matrix.hpp b/core/test/utils/unsort_matrix.hpp index 9e0759c8097..1ad95a02c25 100644 --- a/core/test/utils/unsort_matrix.hpp +++ b/core/test/utils/unsort_matrix.hpp @@ -116,16 +116,31 @@ void unsort_matrix(matrix::Coo *mtx, } const auto &exec = mtx->get_executor(); - const auto &master = mtx->get_executor()->get_master(); + auto master = mtx->get_executor()->get_master(); + // If exec is not the master/host, extract the master and perform the + // unsorting there, followed by copying it back if (exec != master) { + constexpr int max_depth{10}; // Max depth that is searched for master + bool actual_master_found{false}; + for (int i = 0; i < max_depth; ++i) { + const auto new_master = master->get_master(); + if (new_master == master) { + actual_master_found = true; + break; + } + master = new_master; + } + if (!actual_master_found) { + GKO_NOT_SUPPORTED(exec); + } auto h_mtx = mtx->clone(master); unsort_matrix(lend(h_mtx), engine); mtx->copy_from(lend(h_mtx)); return; } - auto size = mtx->get_size(); + auto vals = mtx->get_values(); auto rows = mtx->get_row_idxs(); auto cols = mtx->get_col_idxs(); From 556d5d11bdaaba38d9d368f3f3789e5d5e205b74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Gr=C3=BCtzmacher?= Date: Fri, 6 Nov 2020 17:15:52 +0100 Subject: [PATCH 10/13] Remove recursive check for master in unsort_matrix --- core/test/utils/unsort_matrix.hpp | 32 ++----------------------------- 1 file changed, 2 insertions(+), 30 deletions(-) diff --git a/core/test/utils/unsort_matrix.hpp b/core/test/utils/unsort_matrix.hpp index 1ad95a02c25..9256177d200 100644 --- a/core/test/utils/unsort_matrix.hpp +++ b/core/test/utils/unsort_matrix.hpp @@ -61,24 +61,11 @@ void unsort_matrix(matrix::Csr *mtx, return; } const auto &exec = mtx->get_executor(); - auto master = mtx->get_executor()->get_master(); + const auto &master = exec->get_master(); // If exec is not the master/host, extract the master and perform the // unsorting there, followed by copying it back if (exec != master) { - constexpr int max_depth{10}; // Max depth that is searched for master - bool actual_master_found{false}; - for (int i = 0; i < max_depth; ++i) { - const auto new_master = master->get_master(); - if (new_master == master) { - actual_master_found = true; - break; - } - master = new_master; - } - if (!actual_master_found) { - GKO_NOT_SUPPORTED(exec); - } auto h_mtx = mtx->clone(master); unsort_matrix(lend(h_mtx), engine); mtx->copy_from(lend(h_mtx)); @@ -110,37 +97,22 @@ void unsort_matrix(matrix::Coo *mtx, using value_type = ValueType; using index_type = IndexType; auto nnz = mtx->get_num_stored_elements(); - if (nnz <= 0) { return; } const auto &exec = mtx->get_executor(); - auto master = mtx->get_executor()->get_master(); + const auto &master = exec->get_master(); // If exec is not the master/host, extract the master and perform the // unsorting there, followed by copying it back if (exec != master) { - constexpr int max_depth{10}; // Max depth that is searched for master - bool actual_master_found{false}; - for (int i = 0; i < max_depth; ++i) { - const auto new_master = master->get_master(); - if (new_master == master) { - actual_master_found = true; - break; - } - master = new_master; - } - if (!actual_master_found) { - GKO_NOT_SUPPORTED(exec); - } auto h_mtx = mtx->clone(master); unsort_matrix(lend(h_mtx), engine); mtx->copy_from(lend(h_mtx)); return; } - auto vals = mtx->get_values(); auto rows = mtx->get_row_idxs(); auto cols = mtx->get_col_idxs(); From 0ab246004f69abf1ebdf8ac5f8be318c22756d27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Gr=C3=BCtzmacher?= Date: Wed, 11 Nov 2020 12:40:12 +0100 Subject: [PATCH 11/13] Fix unsort_matrix core test by removing the kernel dependency with hand-initializing the matrices. --- core/test/utils/unsort_matrix.hpp | 4 +- core/test/utils/unsort_matrix_test.cpp | 67 ++++++++++++++++---------- 2 files changed, 44 insertions(+), 27 deletions(-) diff --git a/core/test/utils/unsort_matrix.hpp b/core/test/utils/unsort_matrix.hpp index 9256177d200..9b2074086c5 100644 --- a/core/test/utils/unsort_matrix.hpp +++ b/core/test/utils/unsort_matrix.hpp @@ -79,9 +79,9 @@ void unsort_matrix(matrix::Csr *mtx, for (index_type row = 0; row < size[0]; ++row) { auto start = row_ptrs[row]; auto end = row_ptrs[row + 1]; - auto iterator = gko::detail::IteratorFactory( + auto sort_wrapper = gko::detail::IteratorFactory( cols + start, vals + start, end - start); - std::shuffle(iterator.begin(), iterator.end(), engine); + std::shuffle(sort_wrapper.begin(), sort_wrapper.end(), engine); } } diff --git a/core/test/utils/unsort_matrix_test.cpp b/core/test/utils/unsort_matrix_test.cpp index afeb22ae246..c090686116d 100644 --- a/core/test/utils/unsort_matrix_test.cpp +++ b/core/test/utils/unsort_matrix_test.cpp @@ -34,6 +34,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include +#include #include @@ -64,14 +65,34 @@ class UnsortMatrix : public ::testing::Test { UnsortMatrix() : exec(gko::ReferenceExecutor::create()), rand_engine(42), - mtx(gko::initialize({{1, 2, 0, 0, 0}, - {0, 0, 0, 0, 0}, - {3, 4, 5, 6, 0}, - {0, 0, 7, 0, 0}, - {0, 0, 8, 9, 10}}, - exec)), - empty(Dense::create(exec, gko::dim<2>(0, 0))) + /* + */ + csr_empty(Csr::create(exec, gko::dim<2>(0, 0))), + coo_empty(Coo::create(exec, gko::dim<2>(0, 0))) {} + /* + Matrix used for both CSR and COO: + 1, 2, 0, 0, 0 + 0, 0, 0, 0, 0 + 3, 4, 5, 6, 0 + 0, 0, 7, 0, 0 + 0, 0, 8, 9, 10 + */ + std::unique_ptr get_sorted_csr() + { + return Csr::create(exec, gko::dim<2>{5, 5}, + I{1, 2, 3, 4, 5, 6, 7, 8, 9, 10}, + I{0, 1, 0, 1, 2, 3, 2, 2, 3, 4}, + I{0, 2, 2, 6, 7, 10}); + } + + std::unique_ptr get_sorted_coo() + { + return Coo::create(exec, gko::dim<2>{5, 5}, + I{1, 2, 3, 4, 5, 6, 7, 8, 9, 10}, + I{0, 1, 0, 1, 2, 3, 2, 2, 3, 4}, + I{0, 0, 2, 2, 2, 2, 3, 4, 4, 4}); + } bool is_coo_matrix_sorted(Coo *mtx) { @@ -123,62 +144,58 @@ class UnsortMatrix : public ::testing::Test { std::shared_ptr exec; std::ranlux48 rand_engine; - std::unique_ptr mtx; - std::unique_ptr empty; + std::unique_ptr csr_empty; + std::unique_ptr coo_empty; }; TEST_F(UnsortMatrix, CsrWorks) { - auto csr = Csr::create(exec); - mtx->convert_to(gko::lend(csr)); + auto csr = get_sorted_csr(); + const auto ref_mtx = get_sorted_csr(); bool was_sorted = is_csr_matrix_sorted(gko::lend(csr)); gko::test::unsort_matrix(gko::lend(csr), rand_engine); ASSERT_FALSE(is_csr_matrix_sorted(gko::lend(csr))); ASSERT_TRUE(was_sorted); - GKO_ASSERT_MTX_NEAR(csr, mtx, 0.); + GKO_ASSERT_MTX_NEAR(csr, ref_mtx, 0.); } TEST_F(UnsortMatrix, CsrWorksWithEmpty) { - auto csr = Csr::create(exec); - empty->convert_to(gko::lend(csr)); - bool was_sorted = is_csr_matrix_sorted(gko::lend(csr)); + const bool was_sorted = is_csr_matrix_sorted(gko::lend(csr_empty)); - gko::test::unsort_matrix(gko::lend(csr), rand_engine); + gko::test::unsort_matrix(gko::lend(csr_empty), rand_engine); ASSERT_TRUE(was_sorted); - GKO_ASSERT_MTX_NEAR(csr, empty, 0.); + ASSERT_EQ(csr_empty->get_num_stored_elements(), 0); } TEST_F(UnsortMatrix, CooWorks) { - auto coo = Coo::create(exec); - mtx->convert_to(gko::lend(coo)); + auto coo = get_sorted_coo(); + const auto ref_mtx = get_sorted_coo(); const bool was_sorted = is_coo_matrix_sorted(gko::lend(coo)); gko::test::unsort_matrix(gko::lend(coo), rand_engine); ASSERT_FALSE(is_coo_matrix_sorted(gko::lend(coo))); ASSERT_TRUE(was_sorted); - GKO_ASSERT_MTX_NEAR(coo, mtx, 0.); + GKO_ASSERT_MTX_NEAR(coo, ref_mtx, 0.); } TEST_F(UnsortMatrix, CooWorksWithEmpty) { - auto coo = Coo::create(exec); - empty->convert_to(gko::lend(coo)); - const bool was_sorted = is_coo_matrix_sorted(gko::lend(coo)); + const bool was_sorted = is_coo_matrix_sorted(gko::lend(coo_empty)); - gko::test::unsort_matrix(gko::lend(coo), rand_engine); + gko::test::unsort_matrix(gko::lend(coo_empty), rand_engine); ASSERT_TRUE(was_sorted); - GKO_ASSERT_MTX_NEAR(coo, empty, 0.); + ASSERT_EQ(coo_empty->get_num_stored_elements(), 0); } From b1ddd4d5c8a93178b3b00bee47115bd946ec7dd4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Gr=C3=BCtzmacher?= Date: Thu, 12 Nov 2020 10:51:51 +0100 Subject: [PATCH 12/13] Add convert_to_with_sorting helper Additionally, use this helper function everywhere a sorting with conditional sorting is required. --- core/base/utils.hpp | 171 +++++++++++++++++++++++++- core/factorization/par_ict.cpp | 27 +---- core/factorization/par_ilu.cpp | 23 ++-- core/factorization/par_ilut.cpp | 29 ++--- core/preconditioner/isai.cpp | 45 +------ core/preconditioner/jacobi.cpp | 16 +-- reference/test/base/CMakeLists.txt | 1 + reference/test/base/utils.cpp | 187 +++++++++++++++++++++++++++++ 8 files changed, 383 insertions(+), 116 deletions(-) create mode 100644 reference/test/base/utils.cpp diff --git a/core/base/utils.hpp b/core/base/utils.hpp index 4e6fbc1dfce..ace930bd116 100644 --- a/core/base/utils.hpp +++ b/core/base/utils.hpp @@ -33,7 +33,15 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #ifndef GKO_INTERNAL_CORE_BASE_UTILS_HPP_ #define GKO_INTERNAL_CORE_BASE_UTILS_HPP_ + +#include +#include + + +#include #include +#include +#include namespace gko { @@ -50,7 +58,168 @@ GKO_ATTRIBUTES GKO_INLINE ValueType checked_load(const ValueType *p, } // namespace kernels + + +namespace detail { + + +template +struct conversion_sort_helper { +}; + +template +struct conversion_sort_helper> { + using mtx_type = matrix::Csr; + template + static std::unique_ptr get_sorted_conversion( + std::shared_ptr &exec, Source *source) + { + auto editable_mtx = mtx_type::create(exec); + as>(source)->convert_to(lend(editable_mtx)); + editable_mtx->sort_by_column_index(); + return editable_mtx; + } +}; + + +template +std::unique_ptr> convert_to_with_sorting_impl( + std::shared_ptr &exec, Source *obj, bool skip_sorting) +{ + if (skip_sorting) { + return copy_and_convert_to(exec, obj); + } else { + using decay_dest = std::decay_t; + auto sorted_mtx = + detail::conversion_sort_helper::get_sorted_conversion( + exec, obj); + return {sorted_mtx.release(), std::default_delete()}; + } +} + +template +std::shared_ptr convert_to_with_sorting_impl( + std::shared_ptr &exec, std::shared_ptr obj, + bool skip_sorting) +{ + if (skip_sorting) { + return copy_and_convert_to(exec, obj); + } else { + using decay_dest = std::decay_t; + auto sorted_mtx = + detail::conversion_sort_helper::get_sorted_conversion( + exec, obj.get()); + return {std::move(sorted_mtx)}; + } +} + + +} // namespace detail + + +/** + * @internal + * + * Helper function that converts the given matrix to the Dest format with + * additional sorting if requested. + * + * If the given matrix was already sorted, is on the same executor and with a + * dynamic type of `Dest`, the same pointer is returned with an empty + * deleter. + * In all other cases, a new matrix is created, which stores the converted + * matrix. + * + * @tparam Dest the type to which the object should be converted + * @tparam Source the type of the source object + * + * @param exec the executor where the result should be placed + * @param obj the source object that should be converted + * @param skip_sorting indicator if the resulting matrix should be sorted or + * not + */ +template +std::unique_ptr> convert_to_with_sorting( + std::shared_ptr exec, Source *obj, bool skip_sorting) +{ + return detail::convert_to_with_sorting_impl(exec, obj, skip_sorting); +} + +/** + * @copydoc convert_to_with_sorting(std::shared_ptr, + * Source *, bool) + * + * @note This version adds the const qualifier for the result since the input is + * also const + */ +template +std::unique_ptr> +convert_to_with_sorting(std::shared_ptr exec, const Source *obj, + bool skip_sorting) +{ + return detail::convert_to_with_sorting_impl(exec, obj, + skip_sorting); +} + +/** + * @copydoc convert_to_with_sorting(std::shared_ptr, + * Source *, bool) + * + * @note This version has a unique_ptr as the source instead of a plain pointer + */ +template +std::unique_ptr> convert_to_with_sorting( + std::shared_ptr exec, const std::unique_ptr &obj, + bool skip_sorting) +{ + return detail::convert_to_with_sorting_impl(exec, obj.get(), + skip_sorting); +} + +/** + * @internal + * + * Helper function that converts the given matrix to the Dest format with + * additional sorting if requested. + * + * If the given matrix was already sorted, is on the same executor and with a + * dynamic type of `Dest`, the same pointer is returned. + * In all other cases, a new matrix is created, which stores the converted + * matrix. + * + * @tparam Dest the type to which the object should be converted + * @tparam Source the type of the source object + * + * @param exec the executor where the result should be placed + * @param obj the source object that should be converted + * @param skip_sorting indicator if the resulting matrix should be sorted or + * not + */ +template +std::shared_ptr convert_to_with_sorting( + std::shared_ptr exec, std::shared_ptr obj, + bool skip_sorting) +{ + return detail::convert_to_with_sorting_impl(exec, obj, skip_sorting); +} + +/** + * @copydoc convert_to_with_sorting(std::shared_ptr, + * std::shared_ptr, bool) + * + * @note This version adds the const qualifier for the result since the input is + * also const + */ +template +std::shared_ptr convert_to_with_sorting( + std::shared_ptr exec, std::shared_ptr obj, + bool skip_sorting) +{ + return detail::convert_to_with_sorting_impl(exec, obj, + skip_sorting); +} + + } // namespace gko -#endif // GKO_INTERNAL_CORE_BASE_UTILS_HPP_ \ No newline at end of file +#endif // GKO_INTERNAL_CORE_BASE_UTILS_HPP_ diff --git a/core/factorization/par_ict.cpp b/core/factorization/par_ict.cpp index a1a1408fb79..40d14a730d5 100644 --- a/core/factorization/par_ict.cpp +++ b/core/factorization/par_ict.cpp @@ -44,6 +44,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include +#include "core/base/utils.hpp" #include "core/factorization/factorization_kernels.hpp" #include "core/factorization/par_ict_kernels.hpp" #include "core/factorization/par_ilu_kernels.hpp" @@ -175,30 +176,14 @@ ParIct::generate_l_lt( const auto exec = this->get_executor(); // convert and/or sort the matrix if necessary - std::unique_ptr csr_system_matrix_unique_ptr{}; - auto csr_system_matrix = - dynamic_cast(system_matrix.get()); - if (csr_system_matrix == nullptr || - csr_system_matrix->get_executor() != exec) { - csr_system_matrix_unique_ptr = CsrMatrix::create(exec); - as>(system_matrix.get()) - ->convert_to(csr_system_matrix_unique_ptr.get()); - csr_system_matrix = csr_system_matrix_unique_ptr.get(); - } - if (!parameters_.skip_sorting) { - if (csr_system_matrix_unique_ptr == nullptr) { - csr_system_matrix_unique_ptr = CsrMatrix::create(exec); - csr_system_matrix_unique_ptr->copy_from(csr_system_matrix); - } - csr_system_matrix_unique_ptr->sort_by_column_index(); - csr_system_matrix = csr_system_matrix_unique_ptr.get(); - } + auto csr_system_matrix = convert_to_with_sorting( + exec, system_matrix, parameters_.skip_sorting); // initialize the L matrix data structures const auto num_rows = csr_system_matrix->get_size()[0]; Array l_row_ptrs_array{exec, num_rows + 1}; auto l_row_ptrs = l_row_ptrs_array.get_data(); - exec->run(make_initialize_row_ptrs_l(csr_system_matrix, l_row_ptrs)); + exec->run(make_initialize_row_ptrs_l(csr_system_matrix.get(), l_row_ptrs)); auto l_nnz = static_cast(exec->copy_val_to_host(l_row_ptrs + num_rows)); @@ -209,14 +194,14 @@ ParIct::generate_l_lt( std::move(l_row_ptrs_array)); // initialize L - exec->run(make_initialize_l(csr_system_matrix, l.get(), true)); + exec->run(make_initialize_l(csr_system_matrix.get(), l.get(), true)); // compute limit #nnz for L auto l_nnz_limit = static_cast(l_nnz * parameters_.fill_in_limit); ParIctState state{exec, - csr_system_matrix, + csr_system_matrix.get(), std::move(l), l_nnz_limit, parameters_.approximate_select, diff --git a/core/factorization/par_ilu.cpp b/core/factorization/par_ilu.cpp index d61a27747af..fc984415c41 100644 --- a/core/factorization/par_ilu.cpp +++ b/core/factorization/par_ilu.cpp @@ -83,10 +83,9 @@ ParIlu::generate_l_u( // Converts the system matrix to CSR. // Throws an exception if it is not convertible. - auto csr_system_matrix_unique_ptr = CsrMatrix::create(exec); + auto csr_system_matrix = CsrMatrix::create(exec); as>(system_matrix.get()) - ->convert_to(csr_system_matrix_unique_ptr.get()); - auto csr_system_matrix = csr_system_matrix_unique_ptr.get(); + ->convert_to(csr_system_matrix.get()); // If necessary, sort it if (!skip_sorting) { csr_system_matrix->sort_by_column_index(); @@ -94,14 +93,14 @@ ParIlu::generate_l_u( // Add explicit diagonal zero elements if they are missing exec->run(par_ilu_factorization::make_add_diagonal_elements( - csr_system_matrix, true)); + csr_system_matrix.get(), true)); const auto matrix_size = csr_system_matrix->get_size(); const auto number_rows = matrix_size[0]; Array l_row_ptrs{exec, number_rows + 1}; Array u_row_ptrs{exec, number_rows + 1}; exec->run(par_ilu_factorization::make_initialize_row_ptrs_l_u( - csr_system_matrix, l_row_ptrs.get_data(), u_row_ptrs.get_data())); + csr_system_matrix.get(), l_row_ptrs.get_data(), u_row_ptrs.get_data())); // Get nnz from device memory auto l_nnz = static_cast( @@ -123,7 +122,7 @@ ParIlu::generate_l_u( std::move(u_row_ptrs), u_strategy); exec->run(par_ilu_factorization::make_initialize_l_u( - csr_system_matrix, l_factor.get(), u_factor.get())); + csr_system_matrix.get(), l_factor.get(), u_factor.get())); // We use `transpose()` here to convert the Csr format to Csc. auto u_factor_transpose_lin_op = u_factor->transpose(); @@ -140,18 +139,10 @@ ParIlu::generate_l_u( // If it was not, and we already own a CSR `system_matrix`, // we can move the Csr matrix to Coo, which has very little overhead. - // Otherwise, we convert from the Csr matrix, since it is the conversion - // with the least overhead. - // We also have to convert / move from the CSR matrix if it was not already - // sorted (in which case we definitively own a CSR `system_matrix`). + // We also have to move from the CSR matrix if it was not already sorted. if (!skip_sorting || coo_system_matrix_ptr == nullptr) { coo_system_matrix_unique_ptr = CooMatrix::create(exec); - if (csr_system_matrix_unique_ptr == nullptr) { - csr_system_matrix->convert_to(coo_system_matrix_unique_ptr.get()); - } else { - csr_system_matrix_unique_ptr->move_to( - coo_system_matrix_unique_ptr.get()); - } + csr_system_matrix->move_to(coo_system_matrix_unique_ptr.get()); coo_system_matrix_ptr = coo_system_matrix_unique_ptr.get(); } diff --git a/core/factorization/par_ilut.cpp b/core/factorization/par_ilut.cpp index 1eb3dfeb950..a68a7d11c91 100644 --- a/core/factorization/par_ilut.cpp +++ b/core/factorization/par_ilut.cpp @@ -44,6 +44,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include +#include "core/base/utils.hpp" #include "core/factorization/factorization_kernels.hpp" #include "core/factorization/par_ilu_kernels.hpp" #include "core/factorization/par_ilut_kernels.hpp" @@ -191,24 +192,8 @@ ParIlut::generate_l_u( const auto exec = this->get_executor(); // convert and/or sort the matrix if necessary - std::unique_ptr csr_system_matrix_unique_ptr{}; - auto csr_system_matrix = - dynamic_cast(system_matrix.get()); - if (csr_system_matrix == nullptr || - csr_system_matrix->get_executor() != exec) { - csr_system_matrix_unique_ptr = CsrMatrix::create(exec); - as>(system_matrix.get()) - ->convert_to(csr_system_matrix_unique_ptr.get()); - csr_system_matrix = csr_system_matrix_unique_ptr.get(); - } - if (!parameters_.skip_sorting) { - if (csr_system_matrix_unique_ptr == nullptr) { - csr_system_matrix_unique_ptr = CsrMatrix::create(exec); - csr_system_matrix_unique_ptr->copy_from(csr_system_matrix); - } - csr_system_matrix_unique_ptr->sort_by_column_index(); - csr_system_matrix = csr_system_matrix_unique_ptr.get(); - } + auto csr_system_matrix = convert_to_with_sorting( + exec, system_matrix, parameters_.skip_sorting); // initialize the L and U matrix data structures const auto num_rows = csr_system_matrix->get_size()[0]; @@ -216,7 +201,7 @@ ParIlut::generate_l_u( Array u_row_ptrs_array{exec, num_rows + 1}; auto l_row_ptrs = l_row_ptrs_array.get_data(); auto u_row_ptrs = u_row_ptrs_array.get_data(); - exec->run(make_initialize_row_ptrs_l_u(csr_system_matrix, l_row_ptrs, + exec->run(make_initialize_row_ptrs_l_u(csr_system_matrix.get(), l_row_ptrs, u_row_ptrs)); auto l_nnz = @@ -233,7 +218,7 @@ ParIlut::generate_l_u( std::move(u_row_ptrs_array)); // initialize L and U - exec->run(make_initialize_l_u(csr_system_matrix, l.get(), u.get())); + exec->run(make_initialize_l_u(csr_system_matrix.get(), l.get(), u.get())); // compute limit #nnz for L and U auto l_nnz_limit = @@ -242,7 +227,7 @@ ParIlut::generate_l_u( static_cast(u_nnz * parameters_.fill_in_limit); ParIlutState state{exec, - csr_system_matrix, + csr_system_matrix.get(), std::move(l), std::move(u), l_nnz_limit, @@ -352,4 +337,4 @@ GKO_INSTANTIATE_FOR_EACH_VALUE_AND_INDEX_TYPE(GKO_DECLARE_PAR_ILUT); } // namespace factorization -} // namespace gko \ No newline at end of file +} // namespace gko diff --git a/core/preconditioner/isai.cpp b/core/preconditioner/isai.cpp index 0b8738c5594..108ca300acc 100644 --- a/core/preconditioner/isai.cpp +++ b/core/preconditioner/isai.cpp @@ -46,6 +46,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include +#include "core/base/utils.hpp" #include "core/preconditioner/isai_kernels.hpp" @@ -62,48 +63,6 @@ GKO_REGISTER_OPERATION(scatter_excess_solution, isai::scatter_excess_solution); } // namespace isai -/** - * @internal - * - * Helper function that converts the given matrix to the (const) CSR format with - * additional sorting. - * - * If the given matrix was already sorted, is on the same executor and with a - * dynamic type of `const Csr`, the same pointer is returned with an empty - * deleter. - * In all other cases, a new matrix is created, which stores the converted Csr - * matrix. - * If `skip_sorting` is false, the matrix will be sorted by column index, - * otherwise, it will not be sorted. - */ -template -std::shared_ptr convert_to_csr_and_sort( - std::shared_ptr &exec, std::shared_ptr mtx, - bool skip_sorting) -{ - static_assert( - std::is_same>::value, - "The given `Csr` type must be of type `matrix::Csr`!"); - if (skip_sorting && exec == mtx->get_executor()) { - auto csr_mtx = std::dynamic_pointer_cast(mtx); - if (csr_mtx) { - // Here, we can just forward the pointer with an empty deleter - // since it is already sorted and in the correct format - return csr_mtx; - } - } - auto copy = Csr::create(exec); - as>(mtx)->convert_to(lend(copy)); - // Here, we assume that a sorted matrix converted to CSR will also be - // sorted - if (!skip_sorting) { - copy->sort_by_column_index(); - } - return {std::move(copy)}; -} - - /** * @internal * @@ -156,7 +115,7 @@ void Isai::generate_inverse( using UpperTrs = solver::UpperTrs; GKO_ASSERT_IS_SQUARE_MATRIX(input); auto exec = this->get_executor(); - auto to_invert = convert_to_csr_and_sort(exec, input, skip_sorting); + auto to_invert = convert_to_with_sorting(exec, input, skip_sorting); auto inverted = extend_sparsity(exec, to_invert, power); auto num_rows = inverted->get_size()[0]; auto is_lower = IsaiType == isai_type::lower; diff --git a/core/preconditioner/jacobi.cpp b/core/preconditioner/jacobi.cpp index 6d52c5b4838..b59a6289a46 100644 --- a/core/preconditioner/jacobi.cpp +++ b/core/preconditioner/jacobi.cpp @@ -45,6 +45,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include "core/base/extended_float.hpp" +#include "core/base/utils.hpp" #include "core/preconditioner/jacobi_kernels.hpp" #include "core/preconditioner/jacobi_utils.hpp" @@ -210,19 +211,8 @@ void Jacobi::generate(const LinOp *system_matrix, GKO_ASSERT_IS_SQUARE_MATRIX(system_matrix); using csr_type = matrix::Csr; const auto exec = this->get_executor(); - decltype(copy_and_convert_to(exec, system_matrix)) csr_mtx{}; - - if (skip_sorting) { - csr_mtx = copy_and_convert_to>( - exec, system_matrix); - } else { - auto editable_csr = csr_type::create(exec); - as>(system_matrix) - ->convert_to(lend(editable_csr)); - editable_csr->sort_by_column_index(); - csr_mtx = decltype(csr_mtx){editable_csr.release(), - std::default_delete{}}; - } + auto csr_mtx = + convert_to_with_sorting(exec, system_matrix, skip_sorting); if (parameters_.block_pointers.get_data() == nullptr) { this->detect_blocks(csr_mtx.get()); diff --git a/reference/test/base/CMakeLists.txt b/reference/test/base/CMakeLists.txt index 3b86589507e..3386bb01e20 100644 --- a/reference/test/base/CMakeLists.txt +++ b/reference/test/base/CMakeLists.txt @@ -1,3 +1,4 @@ ginkgo_create_test(combination) ginkgo_create_test(composition) ginkgo_create_test(perturbation) +ginkgo_create_test(utils) diff --git a/reference/test/base/utils.cpp b/reference/test/base/utils.cpp new file mode 100644 index 00000000000..6a4b7d4b9bb --- /dev/null +++ b/reference/test/base/utils.cpp @@ -0,0 +1,187 @@ +/************************************************************* +Copyright (c) 2017-2020, the Ginkgo authors +All rights reserved. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions +are met: + +1. Redistributions of source code must retain the above copyright +notice, this list of conditions and the following disclaimer. + +2. Redistributions in binary form must reproduce the above copyright +notice, this list of conditions and the following disclaimer in the +documentation and/or other materials provided with the distribution. + +3. Neither the name of the copyright holder nor the names of its +contributors may be used to endorse or promote products derived from +this software without specific prior written permission. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS +IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED +TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A +PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +*************************************************************/ + +#include "core/base/utils.hpp" + + +#include + + +#include + + +#include +#include +#include +#include +#include +#include +#include + + +#include "core/test/utils.hpp" +#include "core/test/utils/unsort_matrix.hpp" + + +namespace { + + +class ConvertToWithSorting : public ::testing::Test { +protected: + using value_type = double; + using index_type = gko::int32; + using Dense = gko::matrix::Dense; + using Csr = gko::matrix::Csr; + using Coo = gko::matrix::Coo; + + ConvertToWithSorting() + : ref{gko::ReferenceExecutor::create()}, + mtx{gko::initialize({{1, 2, 3}, {6, 0, 7}, {-1, 8, 0}}, ref)}, + unsorted_coo{Coo::create(ref, gko::dim<2>{3, 3}, + I{1, 3, 2, 7, 6, -1, 8}, + I{0, 2, 1, 2, 0, 0, 1}, + I{0, 0, 0, 1, 1, 2, 2})}, + unsorted_csr{Csr::create( + ref, gko::dim<2>{3, 3}, I{1, 3, 2, 7, 6, -1, 8}, + I{0, 2, 1, 2, 0, 0, 1}, I{0, 3, 5, 7})} + + {} + + std::shared_ptr ref; + std::unique_ptr mtx; + std::unique_ptr unsorted_coo; + std::unique_ptr unsorted_csr; +}; + + +TEST_F(ConvertToWithSorting, SortWithUniquePtr) +{ + auto result = gko::convert_to_with_sorting(ref, unsorted_coo, false); + + ASSERT_TRUE(result->is_sorted_by_column_index()); + GKO_ASSERT_MTX_NEAR(result.get(), mtx.get(), 0.); +} + + +TEST_F(ConvertToWithSorting, DontSortWithUniquePtr) +{ + auto result = gko::convert_to_with_sorting(ref, unsorted_csr, true); + + ASSERT_EQ(result.get(), unsorted_csr.get()); + GKO_ASSERT_MTX_NEAR(result.get(), mtx.get(), 0.); +} + + +TEST_F(ConvertToWithSorting, SortWithSharedPtr) +{ + std::shared_ptr shared = gko::share(unsorted_csr); + + auto result = gko::convert_to_with_sorting(ref, shared, false); + + ASSERT_TRUE(result->is_sorted_by_column_index()); + GKO_ASSERT_MTX_NEAR(result.get(), mtx.get(), 0.); +} + + +TEST_F(ConvertToWithSorting, DontSortWithSharedPtr) +{ + std::shared_ptr shared = gko::share(unsorted_csr); + + auto result = gko::convert_to_with_sorting(ref, shared, true); + + ASSERT_EQ(result.get(), shared.get()); + GKO_ASSERT_MTX_NEAR(result.get(), mtx.get(), 0.); +} + + +TEST_F(ConvertToWithSorting, SortWithSharedConstPtr) +{ + std::shared_ptr shared = gko::share(unsorted_coo); + + auto result = gko::convert_to_with_sorting(ref, shared, false); + + ASSERT_TRUE(result->is_sorted_by_column_index()); + GKO_ASSERT_MTX_NEAR(result.get(), mtx.get(), 0.); +} + + +TEST_F(ConvertToWithSorting, DontSortWithSharedConstPtr) +{ + std::shared_ptr shared = gko::share(unsorted_coo); + + auto result = gko::convert_to_with_sorting(ref, shared, true); + + GKO_ASSERT_MTX_NEAR(result.get(), mtx.get(), 0.); +} + + +TEST_F(ConvertToWithSorting, SortWithRawPtr) +{ + auto result = + gko::convert_to_with_sorting(ref, unsorted_coo.get(), false); + + ASSERT_TRUE(result->is_sorted_by_column_index()); + GKO_ASSERT_MTX_NEAR(result.get(), mtx.get(), 0.); +} + + +TEST_F(ConvertToWithSorting, DontSortWithRawPtr) +{ + auto result = + gko::convert_to_with_sorting(ref, unsorted_coo.get(), true); + + GKO_ASSERT_MTX_NEAR(result.get(), mtx.get(), 0.); +} + + +TEST_F(ConvertToWithSorting, SortWithConstRawPtr) +{ + const Coo *cptr = unsorted_coo.get(); + + auto result = gko::convert_to_with_sorting(ref, cptr, false); + + ASSERT_TRUE(result->is_sorted_by_column_index()); + GKO_ASSERT_MTX_NEAR(result.get(), mtx.get(), 0.); +} + + +TEST_F(ConvertToWithSorting, DontSortWithConstRawPtr) +{ + const auto cptr = mtx.get(); + + auto result = gko::convert_to_with_sorting(ref, cptr, true); + + GKO_ASSERT_MTX_NEAR(result.get(), mtx.get(), 0.); +} + + +} // namespace From 42c7adee2c7a81e6c8317450174c061cca505d60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Gr=C3=BCtzmacher?= Date: Thu, 12 Nov 2020 11:45:33 +0100 Subject: [PATCH 13/13] Change the unsorting of COO matrices by using matrix_data: 1. shuffling all non-zeros 2. sorting by row This takes longer, but does not rely on the COO sorting by row index --- core/test/utils/unsort_matrix.hpp | 31 ++++++++++--------------------- 1 file changed, 10 insertions(+), 21 deletions(-) diff --git a/core/test/utils/unsort_matrix.hpp b/core/test/utils/unsort_matrix.hpp index 9b2074086c5..3a4b83b6b4e 100644 --- a/core/test/utils/unsort_matrix.hpp +++ b/core/test/utils/unsort_matrix.hpp @@ -38,6 +38,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include +#include #include @@ -88,8 +89,6 @@ void unsort_matrix(matrix::Csr *mtx, // Plan for now: shuffle values and column indices to unsort the given matrix // without changing the represented matrix. -// Note: This expects COO to be properly sorted by row index (which is required -// by the Ginkgo COO format) template void unsort_matrix(matrix::Coo *mtx, RandomEngine &&engine) @@ -112,25 +111,15 @@ void unsort_matrix(matrix::Coo *mtx, mtx->copy_from(lend(h_mtx)); return; } - - auto vals = mtx->get_values(); - auto rows = mtx->get_row_idxs(); - auto cols = mtx->get_col_idxs(); - - auto current_row = rows[0]; - for (IndexType i = 0; i < nnz;) { - auto start = i; - while (i < nnz && rows[i] == current_row) { - ++i; - } - current_row = rows[i]; - auto end = i; - auto iterator = gko::detail::IteratorFactory( - cols + start, vals + start, end - start); - // since the row entries are supposed to be the same, there is no need - // to swap - std::shuffle(iterator.begin(), iterator.end(), engine); - } + matrix_data data; + mtx->write(data); + auto &nonzeros = data.nonzeros; + using nz_type = typename decltype(data)::nonzero_type; + + std::shuffle(nonzeros.begin(), nonzeros.end(), engine); + std::stable_sort(nonzeros.begin(), nonzeros.end(), + [](nz_type a, nz_type b) { return a.row < b.row; }); + mtx->read(data); }