Use same matrix list in benchmark and warmup/repetitions in solver #366

yhmtsai · 2019-10-15T14:53:27Z

Move the creations and comand-line arguments of matrix format into utils/common.hpp
every program under benchmark uses the same matrix format list.
Split timing detailed components and computing residuals in solver.
Solver will use warmup and repetitions argument.
(solver has two extra apply for timing detailed components and computing residuals, so the total number is warmup + 2 + repetitions)

preconditioner uses csr as default matrix format which is same as previous setting, and others use coo.
solver uses 1 as repetitions, and others use 10

pratikvn

Some minor things.

benchmark/preconditioner/preconditioner.cpp

benchmark/solver/solver.cpp

codecov · 2019-10-16T07:58:14Z

Codecov Report

❗ No coverage uploaded for pull request base (develop@e26285b). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             develop     #366   +/-   ##
==========================================
  Coverage           ?   89.98%           
==========================================
  Files              ?      136           
  Lines              ?     6078           
  Branches           ?        0           
==========================================
  Hits               ?     5469           
  Misses             ?      609           
  Partials           ?        0

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e26285b...76cb7c1. Read the comment docs.

tcojean · 2019-10-16T11:58:47Z

You seem to have a minor problem:
https://gitlab.com/ginkgo-project/ginkgo-public-ci/-/jobs/322217541

Also, sonarqube finds some relatively important issues:
https://sonarcloud.io/project/issues?id=ginkgo-project_ginkgo&pullRequest=366&resolved=false&types=BUG

pratikvn · 2019-10-16T13:32:24Z

benchmark/utils/common.hpp

+// If define DISABLE_FORMATS_COMMAND, do not define this argument.
+#ifndef DISABLE_FORMATS_COMMAND
+DEFINE_string(
+    formats, "coo",


If you want the default as csr, then I think you should set it here instead.

benchmark/solver/solver.cpp

yhmtsai · 2019-10-16T15:27:13Z

It depends on what is the default value.
I change the default to keep the same behavior.
Or, I can use same default value and modify the run_all_benchmark script?

tcojean

Some small changes. In general LGTM, but I would really like if this was fixed:
https://gitlab.com/ginkgo-project/ginkgo-public-ci/-/jobs/322217541

benchmark/solver/CMakeLists.txt

benchmark/solver/solver.cpp

tcojean

Thanks, LGTM.

pratikvn

LGTM!

The Ginkgo team is proud to announce the new minor release of Ginkgo version 1.1.0. This release brings several performance improvements, adds Windows support, adds support for factorizations inside Ginkgo and a new ILU preconditioner based on ParILU algorithm, among other things. For detailed information, check the respective issue. Supported systems and requirements: + For all platforms, cmake 3.9+ + Linux and MacOS + gcc: 5.3+, 6.3+, 7.3+, 8.1+ + clang: 3.9+ + Intel compiler: 2017+ + Apple LLVM: 8.0+ + CUDA module: CUDA 9.0+ + Windows + MinGW and CygWin: gcc 5.3+, 6.3+, 7.3+, 8.1+ + Microsoft Visual Studio: VS 2017 15.7+ + CUDA module: CUDA 9.0+, Microsoft Visual Studio + OpenMP module: MinGW or CygWin. The current known issues can be found in the [known issues page](https://github.com/ginkgo-project/ginkgo/wiki/Known-Issues). Additions: + Upper and lower triangular solvers ([#327](#327), [#336](#336), [#341](#341), [#342](#342)) + New factorization support in Ginkgo, and addition of the ParILU algorithm ([#305](#305), [#315](#315), [#319](#319), [#324](#324)) + New ILU preconditioner ([#348](#348), [#353](#353)) + Windows MinGW and Cygwin support ([#347](#347)) + Windows Visual studio support ([#351](#351)) + New example showing how to use ParILU as a preconditioner ([#358](#358)) + New example on using loggers for debugging ([#360](#360)) + Add two new 9pt and 27pt stencil examples ([#300](#300), [#306](#306)) + Allow benchmarking CuSPARSE spmv formats through Ginkgo's benchmarks ([#303](#303)) + New benchmark for sparse matrix format conversions ([#312](https://github.com/ginkgo-project/ginkgo/issues/312)[#317](https://github.com/ginkgo-project/ginkgo/issues/317)) + Add conversions between CSR and Hybrid formats ([#302](#302), [#310](#310)) + Support for sorting rows in the CSR format by column idices ([#322](#322)) + Addition of a CUDA COO SpMM kernel for improved performance ([#345](#345)) + Addition of a LinOp to handle perturbations of the form (identity + scalar * basis * projector) ([#334](#334)) + New sparsity matrix representation format with Reference and OpenMP kernels ([#349](#349), [#350](#350)) Fixes: + Accelerate GMRES solver for CUDA executor ([#363](#363)) + Fix BiCGSTAB solver convergence ([#359](#359)) + Fix CGS logging by reporting the residual for every sub iteration ([#328](#328)) + Fix CSR,Dense->Sellp conversion's memory access violation ([#295](#295)) + Accelerate CSR->Ell,Hybrid conversions on CUDA ([#313](#313), [#318](#318)) + Fixed slowdown of COO SpMV on OpenMP ([#340](#340)) + Fix gcc 6.4.0 internal compiler error ([#316](#316)) + Fix compilation issue on Apple clang++ 10 ([#322](#322)) + Make Ginkgo able to compile on Intel 2017 and above ([#337](#337)) + Make the benchmarks spmv/solver use the same matrix formats ([#366](#366)) + Fix self-written isfinite function ([#348](#348)) + Fix Jacobi issues shown by cuda-memcheck Tools and ecosystem: + Multiple improvements to the CI system and tools ([#296](#296), [#311](#311), [#365](#365)) + Multiple improvements to the Ginkgo containers ([#328](#328), [#361](#361)) + Add sonarqube analysis to Ginkgo ([#304](#304), [#308](#308), [#309](#309)) + Add clang-tidy and iwyu support to Ginkgo ([#298](#298)) + Improve Ginkgo's support of xSDK M12 policy by adding the `TPL_` arguments to CMake ([#300](#300)) + Add support for the xSDK R7 policy ([#325](#325)) + Fix examples in html documentation ([#367](#367))

The Ginkgo team is proud to announce the new minor release of Ginkgo version 1.1.0. This release brings several performance improvements, adds Windows support, adds support for factorizations inside Ginkgo and a new ILU preconditioner based on ParILU algorithm, among other things. For detailed information, check the respective issue. Supported systems and requirements: + For all platforms, cmake 3.9+ + Linux and MacOS + gcc: 5.3+, 6.3+, 7.3+, 8.1+ + clang: 3.9+ + Intel compiler: 2017+ + Apple LLVM: 8.0+ + CUDA module: CUDA 9.0+ + Windows + MinGW and Cygwin: gcc 5.3+, 6.3+, 7.3+, 8.1+ + Microsoft Visual Studio: VS 2017 15.7+ + CUDA module: CUDA 9.0+, Microsoft Visual Studio + OpenMP module: MinGW or Cygwin. The current known issues can be found in the [known issues page](https://github.com/ginkgo-project/ginkgo/wiki/Known-Issues). ### Additions + Upper and lower triangular solvers ([#327](#327), [#336](#336), [#341](#341), [#342](#342)) + New factorization support in Ginkgo, and addition of the ParILU algorithm ([#305](#305), [#315](#315), [#319](#319), [#324](#324)) + New ILU preconditioner ([#348](#348), [#353](#353)) + Windows MinGW and Cygwin support ([#347](#347)) + Windows Visual Studio support ([#351](#351)) + New example showing how to use ParILU as a preconditioner ([#358](#358)) + New example on using loggers for debugging ([#360](#360)) + Add two new 9pt and 27pt stencil examples ([#300](#300), [#306](#306)) + Allow benchmarking CuSPARSE spmv formats through Ginkgo's benchmarks ([#303](#303)) + New benchmark for sparse matrix format conversions ([#312](https://github.com/ginkgo-project/ginkgo/issues/312)[#317](https://github.com/ginkgo-project/ginkgo/issues/317)) + Add conversions between CSR and Hybrid formats ([#302](#302), [#310](#310)) + Support for sorting rows in the CSR format by column idices ([#322](#322)) + Addition of a CUDA COO SpMM kernel for improved performance ([#345](#345)) + Addition of a LinOp to handle perturbations of the form (identity + scalar * basis * projector) ([#334](#334)) + New sparsity matrix representation format with Reference and OpenMP kernels ([#349](#349), [#350](#350)) ### Fixes + Accelerate GMRES solver for CUDA executor ([#363](#363)) + Fix BiCGSTAB solver convergence ([#359](#359)) + Fix CGS logging by reporting the residual for every sub iteration ([#328](#328)) + Fix CSR,Dense->Sellp conversion's memory access violation ([#295](#295)) + Accelerate CSR->Ell,Hybrid conversions on CUDA ([#313](#313), [#318](#318)) + Fixed slowdown of COO SpMV on OpenMP ([#340](#340)) + Fix gcc 6.4.0 internal compiler error ([#316](#316)) + Fix compilation issue on Apple clang++ 10 ([#322](#322)) + Make Ginkgo able to compile on Intel 2017 and above ([#337](#337)) + Make the benchmarks spmv/solver use the same matrix formats ([#366](#366)) + Fix self-written isfinite function ([#348](#348)) + Fix Jacobi issues shown by cuda-memcheck ### Tools and ecosystem improvements + Multiple improvements to the CI system and tools ([#296](#296), [#311](#311), [#365](#365)) + Multiple improvements to the Ginkgo containers ([#328](#328), [#361](#361)) + Add sonarqube analysis to Ginkgo ([#304](#304), [#308](#308), [#309](#309)) + Add clang-tidy and iwyu support to Ginkgo ([#298](#298)) + Improve Ginkgo's support of xSDK M12 policy by adding the `TPL_` arguments to CMake ([#300](#300)) + Add support for the xSDK R7 policy ([#325](#325)) + Fix examples in html documentation ([#367](#367)) Related PR: #370

yhmtsai added the reg:benchmarking This is related to benchmarking. label Oct 15, 2019

yhmtsai requested review from pratikvn, thoasm, hartwiganzt and tcojean October 15, 2019 14:53

yhmtsai self-assigned this Oct 15, 2019

yhmtsai added the 1:ST:ready-for-review This PR is ready for review label Oct 15, 2019

pratikvn requested changes Oct 15, 2019

View reviewed changes

benchmark/preconditioner/preconditioner.cpp Outdated Show resolved Hide resolved

benchmark/solver/solver.cpp Show resolved Hide resolved

benchmark/solver/solver.cpp Outdated Show resolved Hide resolved

tcojean added this to the v1.1.0 milestone Oct 16, 2019

pratikvn reviewed Oct 16, 2019

View reviewed changes

tcojean requested changes Oct 17, 2019

View reviewed changes

benchmark/solver/CMakeLists.txt Show resolved Hide resolved

benchmark/solver/solver.cpp Outdated Show resolved Hide resolved

tcojean approved these changes Oct 17, 2019

View reviewed changes

hartwiganzt approved these changes Oct 17, 2019

View reviewed changes

yhmtsai force-pushed the modify_benchmark_solver branch 2 times, most recently from 01e8440 to ae258a8 Compare October 17, 2019 16:09

yhmtsai added 11 commits October 18, 2019 00:35

move the spmv and add the solver name

f5bd868

keep the same format as spmv's

acc1f32

fix compile error

8d16aa7

split residual computation and timing component

05b1d3c

keep same matrix format argument

e8c53a4

add warmup and repetitions in solver

a03e81b

delete gflags declare

5ae78a2

fix warning

a72aeae

fix sonar issues

18df523

create func to handle duplication and rm disable

a28219c

fix throw in destructor

76cb7c1

yhmtsai force-pushed the modify_benchmark_solver branch from ae258a8 to 76cb7c1 Compare October 17, 2019 22:36

tcojean requested a review from pratikvn October 18, 2019 07:24

pratikvn approved these changes Oct 18, 2019

View reviewed changes

tcojean 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 18, 2019

yhmtsai merged commit b40150e into develop Oct 18, 2019

pratikvn deleted the modify_benchmark_solver branch October 20, 2019 11:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use same matrix list in benchmark and warmup/repetitions in solver #366

Use same matrix list in benchmark and warmup/repetitions in solver #366

yhmtsai commented Oct 15, 2019

pratikvn left a comment

codecov bot commented Oct 16, 2019 •

edited

Loading

tcojean commented Oct 16, 2019 •

edited

Loading

pratikvn Oct 16, 2019

yhmtsai commented Oct 16, 2019

tcojean left a comment

tcojean left a comment

pratikvn left a comment

Use same matrix list in benchmark and warmup/repetitions in solver #366

Use same matrix list in benchmark and warmup/repetitions in solver #366

Conversation

yhmtsai commented Oct 15, 2019

pratikvn left a comment

Choose a reason for hiding this comment

codecov bot commented Oct 16, 2019 • edited Loading

Codecov Report

tcojean commented Oct 16, 2019 • edited Loading

pratikvn Oct 16, 2019

Choose a reason for hiding this comment

yhmtsai commented Oct 16, 2019

tcojean left a comment

Choose a reason for hiding this comment

tcojean left a comment

Choose a reason for hiding this comment

pratikvn left a comment

Choose a reason for hiding this comment

codecov bot commented Oct 16, 2019 •

edited

Loading

tcojean commented Oct 16, 2019 •

edited

Loading