Support reordering in all benchmarks #1408

upsj · 2023-09-22T10:07:34Z

This adds another reorder parameter to the input JSON, allowing users to specify a reordering algorithm to be applied on the matrix before proceeding. The reordering time itself is not being measured in the benchmark (Should it be?). I am also a bit unsure where to document this.

For consistency, I also added a new interface for RCM that creates a Permutation

Fixes #1374

MarcelKoch · 2023-09-22T10:13:28Z

I think this is a good time to clarify when a parameter for the benchmark is part of the json config or part of the program arguments. IIRC, currently we only have the matrix file or stencil config, and the optimal spmv setting, as part of the json, while other parameters are arguments. I think the most similar parameter is the matrix format, which is an argument.

I'm not saying the reordered parameter should also be an argument, I'm just curious about argument for or against it.

upsj · 2023-09-22T10:33:53Z

I agree that this is an important issue, I guess at the core of it is that we take a "cross-product" approach to solver-preconditioner combinations, adding another factor to this product and integrating it into the solver benchmark seems not the right path at this time to me. Also, reorderings matter for most if not all benchmarks - cache effects will have impact on all algorithms that operate on the matrix (spmv, sparse_blas, conversions), numerical and fill-in consideration impact all factorization-based preconditioners and solvers.

upsj · 2023-09-24T12:54:42Z

Thinking about it again, I don't think we need to support multiple reorderings at the same time. If we want to benchmark multiple, we can do that in separate benchmark invocations. This also fits with what seems like the normal approach to benchmarking we use. Duplicating the same filename with multiple reorderings seems hard to handle in post-processing.
So I see two possible approaches:

make reorder an input JSON object parameter
make reorder a command-line parameter

An unrelated issue here is that the reordering doesn't affect the RHS, so we may need additional processing for the solver benchmark.

MarcelKoch

For handling the RHS in the solver, it might be necessary to store the permutation in the generator. Otherwise it would need to be regenerated.

Also, the changelog comment should not mention all benchmarks, since this only works for the non-distributed ones.

benchmark/utils/generator.hpp

pratikvn

I agree that we don't need to have the "cross-product" for reordering. It does not really lend itself to all options: For example, it requires the CSR format.

I think it would be quite useful to have the runtime of the reordering itself to evaluate the overall effectiveness when comparing different reorderings.

benchmark/utils/generator.hpp

include/ginkgo/core/reorder/rcm.hpp

pratikvn

Two things, but otherwise looks good.

I think you should separate the updated Rcm interface class to a new PR and only have the benchmark changes in this PR.
Not sure if necessary, but maybe it is good to ensure row major ordering after reordering the data.

benchmark/utils/general_matrix.hpp

include/ginkgo/core/reorder/rcm.hpp

pratikvn

LGTM!

MarcelKoch · 2023-10-10T09:40:41Z

Do you want to base this PR on the new RCM PR?

upsj · 2023-10-10T10:00:00Z

@MarcelKoch no, it works independently, and only requires ~4 lines changed, so I will merge this first

yhmtsai

I am not sure how the reordering is used in practice.
Unless the reordered solver is used several times for different right hand side, should the solving time also include reorder time?
Because the flag only accepts one reorder algorithm, there's still a need for several files to store the result when comparing the reordering effect. For me, the cross product is easier to test several reordering, but the multiple runs should also work with additional script

benchmark/sparse_blas/sparse_blas.cpp

benchmark/utils/general_matrix.hpp

tcojean

LGTM

tcojean · 2023-10-10T10:03:15Z

benchmark/conversion/conversion.cpp

+ // no reordering here, as it doesn't impact conversions beyond
+ // dense-sparse conversions


Now that we have neat classes, we could document this, and generally what the benchmark is doing, by documenting the main struct?
This is rather a comment for the future.

Do you mean the individual benchmarks, or the benchmark runner?

benchmark/utils/general_matrix.hpp

benchmark/preconditioner/preconditioner.cpp

Co-authored-by: Yuhsiang M. Tsai <[email protected]>

sonarcloud · 2023-10-11T02:45:05Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
8 Code Smells

87.5% Coverage
0.0% Duplication

The version of Java (11.0.3) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17.
Read more here

Release 1.7.0 to master The Ginkgo team is proud to announce the new Ginkgo minor release 1.7.0. This release brings new features such as: - Complete GPU-resident sparse direct solvers feature set and interfaces, - Improved Cholesky factorization performance, - A new MC64 reordering, - Batched iterative solver support with the BiCGSTAB solver with batched Dense and ELL matrix types, - MPI support for the SYCL backend, - Improved ParILU(T)/ParIC(T) preconditioner convergence, and more! If you face an issue, please first check our [known issues page](https://github.com/ginkgo-project/ginkgo/wiki/Known-Issues) and the [open issues list](https://github.com/ginkgo-project/ginkgo/issues) and if you do not find a solution, feel free to [open a new issue](https://github.com/ginkgo-project/ginkgo/issues/new/choose) or ask a question using the [github discussions](https://github.com/ginkgo-project/ginkgo/discussions). Supported systems and requirements: + For all platforms, CMake 3.16+ + C++14 compliant compiler + Linux and macOS + GCC: 5.5+ + clang: 3.9+ + Intel compiler: 2019+ + Apple Clang: 14.0 is tested. Earlier versions might also work. + NVHPC: 22.7+ + Cray Compiler: 14.0.1+ + CUDA module: CMake 3.18+, and CUDA 10.1+ or NVHPC 22.7+ + HIP module: ROCm 4.5+ + DPC++ module: Intel oneAPI 2022.1+ with oneMKL and oneDPL. Set the CXX compiler to `dpcpp` or `icpx`. + MPI: standard version 3.1+, ideally GPU Aware, for best performance + Windows + MinGW: GCC 5.5+ + Microsoft Visual Studio: VS 2019+ + CUDA module: CUDA 10.1+, Microsoft Visual Studio + OpenMP module: MinGW. ### Version support changes + CUDA 9.2 is no longer supported and 10.0 is untested [#1382](#1382) + Ginkgo now requires CMake version 3.16 (and 3.18 for CUDA) [#1368](#1368) ### Interface changes + `const` Factory parameters can no longer be modified through `with_*` functions, as this breaks const-correctness [#1336](#1336) [#1439](#1439) ### New Deprecations + The `device_reset` parameter of CUDA and HIP executors no longer has an effect, and its `allocation_mode` parameters have been deprecated in favor of the `Allocator` interface. [#1315](#1315) + The CMake parameter `GINKGO_BUILD_DPCPP` has been deprecated in favor of `GINKGO_BUILD_SYCL`. [#1350](#1350) + The `gko::reorder::Rcm` interface has been deprecated in favor of `gko::experimental::reorder::Rcm` based on `Permutation`. [#1418](#1418) + The Permutation class' `permute_mask` functionality. [#1415](#1415) + Multiple functions with typos (`set_complex_subpsace()`, range functions such as `conj_operaton` etc). [#1348](#1348) ### Summary of previous deprecations + `gko::lend()` is not necessary anymore. + The classes `RelativeResidualNorm` and `AbsoluteResidualNorm` are deprecated in favor of `ResidualNorm`. + The class `AmgxPgm` is deprecated in favor of `Pgm`. + Default constructors for the CSR `load_balance` and `automatical` strategies + The PolymorphicObject's move-semantic `copy_from` variant + The templated `SolverBase` class. + The class `MachineTopology` is deprecated in favor of `machine_topology`. + Logger constructors and create functions with the `executor` parameter. + The virtual, protected, Dense functions `compute_norm1_impl`, `add_scaled_impl`, etc. + Logger events for solvers and criterion without the additional `implicit_tau_sq` parameter. + The global `gko::solver::default_krylov_dim`, use instead `gko::solver::gmres_default_krylov_dim`. ### Added features + Adds a batch::BatchLinOp class that forms a base class for batched linear operators such as batched matrix formats, solver and preconditioners [#1379](#1379) + Adds a batch::MultiVector class that enables operations such as dot, norm, scale on batched vectors [#1371](#1371) + Adds a batch::Dense matrix format that stores batched dense matrices and provides gemv operations for these dense matrices. [#1413](#1413) + Adds a batch::Ell matrix format that stores batched Ell matrices and provides spmv operations for these batched Ell matrices. [#1416](#1416) [#1437](#1437) + Add a batch::Bicgstab solver (class, core, and reference kernels) that enables iterative solution of batched linear systems [#1438](#1438). + Add device kernels (CUDA, HIP, and DPCPP) for batch::Bicgstab solver. [#1443](#1443). + New MC64 reordering algorithm which optimizes the diagonal product or sum of a matrix by permuting the rows, and computes additional scaling factors for equilibriation [#1120](#1120) + New interface for (non-symmetric) permutation and scaled permutation of Dense and Csr matrices [#1415](#1415) + LU and Cholesky Factorizations can now be separated into their factors [#1432](#1432) + New symbolic LU factorization algorithm that is optimized for matrices with an almost-symmetric sparsity pattern [#1445](#1445) + Sorting kernels for SparsityCsr on all backends [#1343](#1343) + Allow passing pre-generated local solver as factory parameter for the distributed Schwarz preconditioner [#1426](#1426) + Add DPCPP kernels for Partition [#1034](#1034), and CSR's `check_diagonal_entries` and `add_scaled_identity` functionality [#1436](#1436) + Adds a helper function to create a partition based on either local sizes, or local ranges [#1227](#1227) + Add function to compute arithmetic mean of dense and distributed vectors [#1275](#1275) + Adds `icpx` compiler supports [#1350](#1350) + All backends can be built simultaneously [#1333](#1333) + Emits a CMake warning in downstream projects that use different compilers than the installed Ginkgo [#1372](#1372) + Reordering algorithms in sparse_blas benchmark [#1354](#1354) + Benchmarks gained an `-allocator` parameter to specify device allocators [#1385](#1385) + Benchmarks gained an `-input_matrix` parameter that initializes the input JSON based on the filename [#1387](#1387) + Benchmark inputs can now be reordered as a preprocessing step [#1408](#1408) ### Improvements + Significantly improve Cholesky factorization performance [#1366](#1366) + Improve parallel build performance [#1378](#1378) + Allow constrained parallel test execution using CTest resources [#1373](#1373) + Use arithmetic type more inside mixed precision ELL [#1414](#1414) + Most factory parameters of factory type no longer need to be constructed explicitly via `.on(exec)` [#1336](#1336) [#1439](#1439) + Improve ParILU(T)/ParIC(T) convergence by using more appropriate atomic operations [#1434](#1434) ### Fixes + Fix an over-allocation for OpenMP reductions [#1369](#1369) + Fix DPCPP's common-kernel reduction for empty input sizes [#1362](#1362) + Fix several typos in the API and documentation [#1348](#1348) + Fix inconsistent `Threads` between generations [#1388](#1388) + Fix benchmark median condition [#1398](#1398) + Fix HIP 5.6.0 compilation [#1411](#1411) + Fix missing destruction of rand_generator from cuda/hip [#1417](#1417) + Fix PAPI logger destruction order [#1419](#1419) + Fix TAU logger compilation [#1422](#1422) + Fix relative criterion to not iterate if the residual is already zero [#1079](#1079) + Fix memory_order invocations with C++20 changes [#1402](#1402) + Fix `check_diagonal_entries_exist` report correctly when only missing diagonal value in the last rows. [#1440](#1440) + Fix checking OpenMPI version in cross-compilation settings [#1446](#1446) + Fix false-positive deprecation warnings in Ginkgo, especially for the old Rcm (it doesn't emit deprecation warnings anymore as a result but is still considered deprecated) [#1444](#1444) ### Related PR: #1451

Release 1.7.0 to develop The Ginkgo team is proud to announce the new Ginkgo minor release 1.7.0. This release brings new features such as: - Complete GPU-resident sparse direct solvers feature set and interfaces, - Improved Cholesky factorization performance, - A new MC64 reordering, - Batched iterative solver support with the BiCGSTAB solver with batched Dense and ELL matrix types, - MPI support for the SYCL backend, - Improved ParILU(T)/ParIC(T) preconditioner convergence, and more! If you face an issue, please first check our [known issues page](https://github.com/ginkgo-project/ginkgo/wiki/Known-Issues) and the [open issues list](https://github.com/ginkgo-project/ginkgo/issues) and if you do not find a solution, feel free to [open a new issue](https://github.com/ginkgo-project/ginkgo/issues/new/choose) or ask a question using the [github discussions](https://github.com/ginkgo-project/ginkgo/discussions). Supported systems and requirements: + For all platforms, CMake 3.16+ + C++14 compliant compiler + Linux and macOS + GCC: 5.5+ + clang: 3.9+ + Intel compiler: 2019+ + Apple Clang: 14.0 is tested. Earlier versions might also work. + NVHPC: 22.7+ + Cray Compiler: 14.0.1+ + CUDA module: CMake 3.18+, and CUDA 10.1+ or NVHPC 22.7+ + HIP module: ROCm 4.5+ + DPC++ module: Intel oneAPI 2022.1+ with oneMKL and oneDPL. Set the CXX compiler to `dpcpp` or `icpx`. + MPI: standard version 3.1+, ideally GPU Aware, for best performance + Windows + MinGW: GCC 5.5+ + Microsoft Visual Studio: VS 2019+ + CUDA module: CUDA 10.1+, Microsoft Visual Studio + OpenMP module: MinGW. ### Version support changes + CUDA 9.2 is no longer supported and 10.0 is untested [#1382](#1382) + Ginkgo now requires CMake version 3.16 (and 3.18 for CUDA) [#1368](#1368) ### Interface changes + `const` Factory parameters can no longer be modified through `with_*` functions, as this breaks const-correctness [#1336](#1336) [#1439](#1439) ### New Deprecations + The `device_reset` parameter of CUDA and HIP executors no longer has an effect, and its `allocation_mode` parameters have been deprecated in favor of the `Allocator` interface. [#1315](#1315) + The CMake parameter `GINKGO_BUILD_DPCPP` has been deprecated in favor of `GINKGO_BUILD_SYCL`. [#1350](#1350) + The `gko::reorder::Rcm` interface has been deprecated in favor of `gko::experimental::reorder::Rcm` based on `Permutation`. [#1418](#1418) + The Permutation class' `permute_mask` functionality. [#1415](#1415) + Multiple functions with typos (`set_complex_subpsace()`, range functions such as `conj_operaton` etc). [#1348](#1348) ### Summary of previous deprecations + `gko::lend()` is not necessary anymore. + The classes `RelativeResidualNorm` and `AbsoluteResidualNorm` are deprecated in favor of `ResidualNorm`. + The class `AmgxPgm` is deprecated in favor of `Pgm`. + Default constructors for the CSR `load_balance` and `automatical` strategies + The PolymorphicObject's move-semantic `copy_from` variant + The templated `SolverBase` class. + The class `MachineTopology` is deprecated in favor of `machine_topology`. + Logger constructors and create functions with the `executor` parameter. + The virtual, protected, Dense functions `compute_norm1_impl`, `add_scaled_impl`, etc. + Logger events for solvers and criterion without the additional `implicit_tau_sq` parameter. + The global `gko::solver::default_krylov_dim`, use instead `gko::solver::gmres_default_krylov_dim`. ### Added features + Adds a batch::BatchLinOp class that forms a base class for batched linear operators such as batched matrix formats, solver and preconditioners [#1379](#1379) + Adds a batch::MultiVector class that enables operations such as dot, norm, scale on batched vectors [#1371](#1371) + Adds a batch::Dense matrix format that stores batched dense matrices and provides gemv operations for these dense matrices. [#1413](#1413) + Adds a batch::Ell matrix format that stores batched Ell matrices and provides spmv operations for these batched Ell matrices. [#1416](#1416) [#1437](#1437) + Add a batch::Bicgstab solver (class, core, and reference kernels) that enables iterative solution of batched linear systems [#1438](#1438). + Add device kernels (CUDA, HIP, and DPCPP) for batch::Bicgstab solver. [#1443](#1443). + New MC64 reordering algorithm which optimizes the diagonal product or sum of a matrix by permuting the rows, and computes additional scaling factors for equilibriation [#1120](#1120) + New interface for (non-symmetric) permutation and scaled permutation of Dense and Csr matrices [#1415](#1415) + LU and Cholesky Factorizations can now be separated into their factors [#1432](#1432) + New symbolic LU factorization algorithm that is optimized for matrices with an almost-symmetric sparsity pattern [#1445](#1445) + Sorting kernels for SparsityCsr on all backends [#1343](#1343) + Allow passing pre-generated local solver as factory parameter for the distributed Schwarz preconditioner [#1426](#1426) + Add DPCPP kernels for Partition [#1034](#1034), and CSR's `check_diagonal_entries` and `add_scaled_identity` functionality [#1436](#1436) + Adds a helper function to create a partition based on either local sizes, or local ranges [#1227](#1227) + Add function to compute arithmetic mean of dense and distributed vectors [#1275](#1275) + Adds `icpx` compiler supports [#1350](#1350) + All backends can be built simultaneously [#1333](#1333) + Emits a CMake warning in downstream projects that use different compilers than the installed Ginkgo [#1372](#1372) + Reordering algorithms in sparse_blas benchmark [#1354](#1354) + Benchmarks gained an `-allocator` parameter to specify device allocators [#1385](#1385) + Benchmarks gained an `-input_matrix` parameter that initializes the input JSON based on the filename [#1387](#1387) + Benchmark inputs can now be reordered as a preprocessing step [#1408](#1408) ### Improvements + Significantly improve Cholesky factorization performance [#1366](#1366) + Improve parallel build performance [#1378](#1378) + Allow constrained parallel test execution using CTest resources [#1373](#1373) + Use arithmetic type more inside mixed precision ELL [#1414](#1414) + Most factory parameters of factory type no longer need to be constructed explicitly via `.on(exec)` [#1336](#1336) [#1439](#1439) + Improve ParILU(T)/ParIC(T) convergence by using more appropriate atomic operations [#1434](#1434) ### Fixes + Fix an over-allocation for OpenMP reductions [#1369](#1369) + Fix DPCPP's common-kernel reduction for empty input sizes [#1362](#1362) + Fix several typos in the API and documentation [#1348](#1348) + Fix inconsistent `Threads` between generations [#1388](#1388) + Fix benchmark median condition [#1398](#1398) + Fix HIP 5.6.0 compilation [#1411](#1411) + Fix missing destruction of rand_generator from cuda/hip [#1417](#1417) + Fix PAPI logger destruction order [#1419](#1419) + Fix TAU logger compilation [#1422](#1422) + Fix relative criterion to not iterate if the residual is already zero [#1079](#1079) + Fix memory_order invocations with C++20 changes [#1402](#1402) + Fix `check_diagonal_entries_exist` report correctly when only missing diagonal value in the last rows. [#1440](#1440) + Fix checking OpenMPI version in cross-compilation settings [#1446](#1446) + Fix false-positive deprecation warnings in Ginkgo, especially for the old Rcm (it doesn't emit deprecation warnings anymore as a result but is still considered deprecated) [#1444](#1444) ### Related PR: #1454

upsj added the 1:ST:ready-for-review This PR is ready for review label Sep 22, 2023

upsj requested a review from a team September 22, 2023 10:07

upsj self-assigned this Sep 22, 2023

ginkgo-bot added reg:testing This is related to testing. reg:benchmarking This is related to benchmarking. labels Sep 22, 2023

MarcelKoch reviewed Sep 25, 2023

View reviewed changes

benchmark/utils/generator.hpp Outdated Show resolved Hide resolved

benchmark/utils/generator.hpp Outdated Show resolved Hide resolved

pratikvn reviewed Sep 25, 2023

View reviewed changes

benchmark/utils/generator.hpp Outdated Show resolved Hide resolved

stanisic mentioned this pull request Oct 4, 2023

Evaluating solver performance in BENCHMARK with different reordering algorithms #1374

Closed

upsj force-pushed the benchmark_reorder_matrix branch from d102aff to 899dbe4 Compare October 8, 2023 19:50

upsj changed the base branch from develop to scaled_permutation October 8, 2023 19:51

upsj requested review from MarcelKoch and pratikvn October 8, 2023 19:51

upsj force-pushed the benchmark_reorder_matrix branch from 899dbe4 to 5d019bb Compare October 10, 2023 08:26

upsj changed the base branch from scaled_permutation to develop October 10, 2023 08:26

MarcelKoch reviewed Oct 10, 2023

View reviewed changes

include/ginkgo/core/reorder/rcm.hpp Outdated Show resolved Hide resolved

pratikvn requested changes Oct 10, 2023

View reviewed changes

benchmark/utils/general_matrix.hpp Show resolved Hide resolved

include/ginkgo/core/reorder/rcm.hpp Outdated Show resolved Hide resolved

upsj mentioned this pull request Oct 10, 2023

Add new interface for RCM #1418

Merged

upsj force-pushed the benchmark_reorder_matrix branch from eb6a2a5 to a067443 Compare October 10, 2023 09:30

upsj requested a review from pratikvn October 10, 2023 09:30

pratikvn approved these changes Oct 10, 2023

View reviewed changes

yhmtsai reviewed Oct 10, 2023

View reviewed changes

benchmark/sparse_blas/sparse_blas.cpp Show resolved Hide resolved

benchmark/utils/general_matrix.hpp Show resolved Hide resolved

tcojean approved these changes Oct 10, 2023

View reviewed changes

upsj added 1:ST:ready-to-merge This PR is ready to merge. and removed 1:ST:ready-for-review This PR is ready for review labels Oct 10, 2023

add reorderings to benchmarks

a41002e

upsj and others added 2 commits October 10, 2023 14:31

revert to old permutation interface for now

046dd58

disable reordering flag for distributed benchmarks

558ec16

Co-authored-by: Yuhsiang M. Tsai <[email protected]>

upsj force-pushed the benchmark_reorder_matrix branch from a067443 to 558ec16 Compare October 10, 2023 12:31

yhmtsai approved these changes Oct 10, 2023

View reviewed changes

upsj merged commit fbae694 into develop Oct 11, 2023
13 of 14 checks passed

upsj deleted the benchmark_reorder_matrix branch October 11, 2023 02:04

tcojean mentioned this pull request Nov 6, 2023

Release 1.7.0 to master #1451

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support reordering in all benchmarks #1408

Support reordering in all benchmarks #1408

upsj commented Sep 22, 2023 •

edited

Loading

MarcelKoch commented Sep 22, 2023

upsj commented Sep 22, 2023 •

edited

Loading

upsj commented Sep 24, 2023

MarcelKoch left a comment

pratikvn left a comment

pratikvn left a comment

pratikvn left a comment

MarcelKoch commented Oct 10, 2023

upsj commented Oct 10, 2023

yhmtsai left a comment

tcojean left a comment

tcojean Oct 10, 2023

upsj Oct 10, 2023

sonarcloud bot commented Oct 11, 2023

		// no reordering here, as it doesn't impact conversions beyond
		// dense-sparse conversions

Support reordering in all benchmarks #1408

Support reordering in all benchmarks #1408

Conversation

upsj commented Sep 22, 2023 • edited Loading

MarcelKoch commented Sep 22, 2023

upsj commented Sep 22, 2023 • edited Loading

upsj commented Sep 24, 2023

MarcelKoch left a comment

Choose a reason for hiding this comment

pratikvn left a comment

Choose a reason for hiding this comment

pratikvn left a comment

Choose a reason for hiding this comment

pratikvn left a comment

Choose a reason for hiding this comment

MarcelKoch commented Oct 10, 2023

upsj commented Oct 10, 2023

yhmtsai left a comment

Choose a reason for hiding this comment

tcojean left a comment

Choose a reason for hiding this comment

tcojean Oct 10, 2023

Choose a reason for hiding this comment

upsj Oct 10, 2023

Choose a reason for hiding this comment

sonarcloud bot commented Oct 11, 2023

upsj commented Sep 22, 2023 •

edited

Loading

upsj commented Sep 22, 2023 •

edited

Loading