Improve benchmark setup #1323

upsj · 2023-04-15T12:32:50Z

I needed to find something lightweight to do to get my mind off the Cholesky issues, so here we are 😄

Refactor benchmarks to use a common framework for handling JSON etc.
Replace RapidJSON by nlohmann_json

yhmtsai

There are several empty files for distribution tests
From cuda, there are two part of profiling: nsight system and nsight compute. (not sure aboth the other vendor)
I do not know the reason for splitting, but nsight system is for tracing (timeline) and nsight compute is for profiling kernels.
For tracing, annotations on the repetition should give a better timeline overview after the warmup.
For profiling kernels, the cuProfilerStart and cuProfilerStop or filter by annotation should help here.
It's also based on that we have some workspace avoiding reallocation and skipping some operations in the second run.
Only profiling the first run may lead us always see the reallocation overhead.

benchmark/blas/blas_common.hpp

benchmark/utils/generator.hpp

benchmark/utils/general.hpp

benchmark/conversions/conversions.cpp

benchmark/test/compare.py

benchmark/test/preconditioner.py

upsj · 2023-04-18T12:44:15Z

The distributed benchmarks are tested now as well. The -profile flag is meant as a shortcut, if users are interested in the difference between hot and cold calls, they can still see the individual generate and apply calls in the timeline by controlling repetitions themselves. We could consider adding ranges to the timer iterations if repetitions > 1?

MarcelKoch · 2023-05-17T09:32:23Z

I feel like this PR mixes quite a few things, which could stand as their own PR. I think splitting this up into 3 PRs

CLI changes (-profile, -input, ...)
test framework
JSON changes
would make each part simpler to review. If that is too inconvenient, I would suggest to at least extract the test framework changes.

yhmtsai

LGTM in general. currently the pull request has some content from the other pull request and is not rebased

benchmark/test/reference/blas.simple.stderr

yhmtsai · 2023-06-13T08:13:44Z

CMakeLists.txt

@@ -272,7 +272,7 @@ if(GINKGO_BUILD_TESTS)
 endif()
 if(GINKGO_BUILD_BENCHMARKS)
    find_package(gflags 2.2.2 QUIET)
-    find_package(RapidJSON 1.1.0 QUIET)
+    find_package(nlohmann_json 3.9.1 QUIET)


any reason for picking 3.9.1?

I think I needed this particular minimum version to provide ordered_json support

benchmark/blas/blas.cpp

benchmark/solver/distributed/solver.cpp

benchmark/utils/generator.hpp

yhmtsai · 2023-06-13T08:49:20Z

benchmark/utils/json.hpp

-    value.Accept(writer);
-    return os;
-}
+using json = nlohmann::ordered_json;


Is it mainly for testing purposes?

RapidJSON has the same property, I wanted to preserve it, since it also makes the output more stable.

benchmark/utils/runner.hpp

benchmark/conversion/conversion.cpp

codecov · 2023-08-22T22:23:54Z

Codecov Report

Patch has no changes to coverable lines.

📢 Thoughts on this report? Let us know!.

MarcelKoch

Only a few minor comments left.

.gitlab/image.yml

benchmark/solver/solver_common.hpp

benchmark/test/multi_vector_distributed.py

benchmark/test/reference/conversion.profile.stderr

benchmark/utils/loggers.hpp

benchmark/test/test_framework.py.in

MarcelKoch

LGTM, but don't forget to update the CI.

This reverts commit 0dab762. Additionally replaces the JSON test case output by their description

they are sometimes implementation-dependent for libstdc++ types

- rename 'determinize' -> 'sanitize' - use empty struct for empty benchmark state - use version tag instead of commit ID - use std::endl where appropriate Co-authored-by: Marcel Koch <[email protected]>

- remove unnecessary stdin in tests - simplify validate_config - consistently use pointer members instead of reference members Co-authored-by: Marcel Koch <[email protected]>

benchmark/test/CMakeLists.txt

benchmark/test/reference/blas.simple.stdout

benchmark/test/reference/conversion.simple.stderr

yhmtsai · 2023-08-29T10:07:32Z

benchmark/test/CMakeLists.txt

+    add_benchmark_test(multi_vector_distributed)
+    add_benchmark_test(spmv_distributed)


I think it had the issue from unstable output from MPI. Is it solved now?

Yes, the instability came from the fact that multiple ranks were printing output. This is now fixed thanks to the do_print variables that are set everywhere.

yhmtsai · 2023-08-29T10:24:52Z

benchmark/test/reference/preconditioner.simple.stdout

 [
    {
-        "size": 125,
+        "size": 100,


size meaning is changed now? from matrix size to stecil point?

Yes, before the matrix dimensions were written into the size field, now they are being written into rows and cols to avoid overwriting the input size specified for the stencil.

benchmark/utils/generator.hpp

benchmark/preconditioner/preconditioner.cpp

benchmark/spmv/spmv_common.hpp

yhmtsai · 2023-08-29T13:53:35Z

benchmark/test/reference/conversion.profile.stderr

-DEBUG: begin components::aos_to_soa
-DEBUG: end   components::aos_to_soa


aos_to_soa -> fill_array + copy + convert_idxs_to_ptrs
any idea?

I think this has to do with changing the benchmark from using matrix_data to using device_matrix_data, so the AOS-SOA conversion only happens once.

benchmark/utils/runner.hpp

third_party/nlohmann_json/CMakeLists.txt

- don't install nlohmann-json - simplify code - improve config description formatting Co-authored-by: Yuhsiang M. Tsai <[email protected]>

sonarcloud · 2023-08-30T22:06:12Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
71 Code Smells

87.7% Coverage
2.8% 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

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

upsj requested a review from a team April 15, 2023 12:32

upsj self-assigned this Apr 15, 2023

yhmtsai reviewed Apr 17, 2023

View reviewed changes

upsj force-pushed the improve_benchmarks branch 3 times, most recently from a2f11db to 715f4ab Compare April 18, 2023 13:25

upsj force-pushed the improve_benchmarks branch from 2b71f7a to 386cfd7 Compare May 21, 2023 07:41

upsj mentioned this pull request May 21, 2023

Add tests for benchmarks #1341

Merged

upsj force-pushed the improve_benchmarks branch from 386cfd7 to df98da7 Compare May 21, 2023 08:03

upsj changed the base branch from develop to benchmark_tests May 21, 2023 08:04

MarcelKoch self-requested a review May 22, 2023 12:12

upsj force-pushed the benchmark_tests branch from 08ff62e to 16608a3 Compare May 25, 2023 07:07

upsj mentioned this pull request Jun 1, 2023

Add support for profiler and file input to benchmarks #1342

Merged

upsj force-pushed the benchmark_tests branch from 16608a3 to 857ca5b Compare June 4, 2023 14:16

yhmtsai reviewed Jun 13, 2023

View reviewed changes

upsj force-pushed the benchmark_tests branch 2 times, most recently from 6ab159f to 22e12e9 Compare June 21, 2023 09:39

upsj force-pushed the benchmark_tests branch from f512133 to 0dab762 Compare July 19, 2023 13:22

Base automatically changed from benchmark_tests to develop July 20, 2023 07:27

upsj force-pushed the improve_benchmarks branch from df98da7 to 49c4342 Compare July 27, 2023 21:46

upsj force-pushed the improve_benchmarks branch from 9cdc64a to 8b07b48 Compare August 16, 2023 11:51

upsj added the 1:ST:run-full-test label Aug 22, 2023

upsj requested review from MarcelKoch and yhmtsai August 22, 2023 15:53

MarcelKoch requested changes Aug 23, 2023

View reviewed changes

MarcelKoch approved these changes Aug 24, 2023

View reviewed changes

upsj and others added 11 commits August 28, 2023 09:52

nlohmann_json refactor

b469ad6

add distributed tests again

c1cee35

This reverts commit 0dab762. Additionally replaces the JSON test case output by their description

handle JSON and non-JSON test output separately

ca3fccf

benchmark reads on device_matrix_data

d27b507

remove allocations from output

e3af029

they are sometimes implementation-dependent for libstdc++ types

update matrix outputs

9cd278c

review updates

8adf765

- rename 'determinize' -> 'sanitize' - use empty struct for empty benchmark state - use version tag instead of commit ID - use std::endl where appropriate Co-authored-by: Marcel Koch <[email protected]>

annotate repetitions

8d52ec8

update test output

e2f2996

update documentation

49ffd96

review updates

a725d3c

- remove unnecessary stdin in tests - simplify validate_config - consistently use pointer members instead of reference members Co-authored-by: Marcel Koch <[email protected]>

upsj force-pushed the improve_benchmarks branch from 1335b29 to a725d3c Compare August 28, 2023 07:52

yhmtsai approved these changes Aug 29, 2023

View reviewed changes

yhmtsai reviewed Aug 29, 2023

View reviewed changes

third_party/nlohmann_json/CMakeLists.txt Show resolved Hide resolved

review updates

7b482dc

- don't install nlohmann-json - simplify code - improve config description formatting Co-authored-by: Yuhsiang M. Tsai <[email protected]>

upsj force-pushed the improve_benchmarks branch from 17389b0 to 7b482dc Compare August 29, 2023 17:26

upsj added 1:ST:no-changelog-entry Skip the wiki check for changelog update 1:ST:ready-to-merge This PR is ready to merge. and removed 1:ST:ready-for-review This PR is ready for review 1:ST:run-full-test labels Aug 29, 2023

keep trailing EOL

fe3789c

upsj merged commit 1100cbd into develop Aug 30, 2023
12 of 14 checks passed

upsj deleted the improve_benchmarks branch August 30, 2023 12:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve benchmark setup #1323

Improve benchmark setup #1323

upsj commented Apr 15, 2023 •

edited

Loading

yhmtsai left a comment

upsj commented Apr 18, 2023

MarcelKoch commented May 17, 2023

yhmtsai left a comment

yhmtsai Jun 13, 2023

upsj Jun 13, 2023

yhmtsai Jun 13, 2023

upsj Jun 13, 2023

codecov bot commented Aug 22, 2023 •

edited

Loading

MarcelKoch left a comment

MarcelKoch left a comment

yhmtsai Aug 29, 2023

upsj Aug 29, 2023

yhmtsai Aug 29, 2023

upsj Aug 29, 2023

yhmtsai Aug 29, 2023

upsj Aug 29, 2023

sonarcloud bot commented Aug 30, 2023

		add_benchmark_test(multi_vector_distributed)
		add_benchmark_test(spmv_distributed)

		DEBUG: begin components::aos_to_soa
		DEBUG: end components::aos_to_soa

Improve benchmark setup #1323

Improve benchmark setup #1323

Conversation

upsj commented Apr 15, 2023 • edited Loading

yhmtsai left a comment

Choose a reason for hiding this comment

upsj commented Apr 18, 2023

MarcelKoch commented May 17, 2023

yhmtsai left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Aug 22, 2023 • edited Loading

Codecov Report

MarcelKoch left a comment

Choose a reason for hiding this comment

MarcelKoch left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented Aug 30, 2023

upsj commented Apr 15, 2023 •

edited

Loading

codecov bot commented Aug 22, 2023 •

edited

Loading