add more method for timer output #1294

yhmtsai · 2023-03-09T12:48:17Z

This PR adds more statistical methods like average, median, min, and max to compute the time result.

When the timing use x iteration which is more than 1 iteration, timer will record the average time x times.
Handle repetition_growth_factor <=1 case such that next_timing always be next iteration.
User can set it to get the same overhead operation in each repetitions

pratikvn

LGTM!

pratikvn · 2023-03-09T15:17:40Z

benchmark/utils/general.hpp

+     * @param same_rep  whether timer managed outside has the same repetitions
+     *                  as IterationControl. If the timer is managed outside and
+     *                  the timer does not have the same repetitions as
+     *                  InterationControl, the method is only allowed "average".


Suggested change

* InterationControl, the method is only allowed "average".

* IterationControl, the method is only allowed "average".

pratikvn · 2023-03-09T15:18:57Z

benchmark/utils/general.hpp

+     * @return the statistical time
+     */
+    double compute_time(const std::string& method = "average",
+                        bool same_rep = true) const


Do you mean to set this to false ?

I will delete them actually. I can check the iteration by the timer and IterationCount own

yanjen

LGTM, but I have some suggestions on the naming.

yanjen · 2023-03-13T08:28:16Z

benchmark/utils/timer_impl.hpp

@@ -63,13 +63,18 @@ class Timer {

    /**
     * Finish the timer
+     *
+     * @param num  the number of operation for the timing range


"the number of samples for the statistical timing" might be better?

yanjen · 2023-03-13T08:35:09Z

benchmark/utils/general.hpp

+            assert(method == "average");
+            return status_run_.managed_timer.get_total_time() /
+                   this->get_num_repetitions();
+        }
    }

    IndexType get_num_repetitions() const { return status_run_.cur_it; }


It was not from this pr but I think the word "samples" would describe better than the word "repetitions". Does anyone feel the same way?

I think the repetitions is better. Sample to me is like subset of data.

yanjen · 2023-03-13T08:37:16Z

benchmark/blas/blas_common.hpp

@@ -425,7 +425,7 @@ void apply_blas(const char* operation_name, std::shared_ptr<gko::Executor> exec,
        for (auto _ : ic.run()) {
            op->run();
        }
-        const auto runtime = ic.compute_average_time();
+        const auto runtime = ic.compute_time(FLAGS_timer_output);


FLAGS_timer_type or might be better? Same for the other places.

I change it to timer_method

MarcelKoch

I'm not sure if this makes sense. Currently, we are not timing each iteration individually, in order to reduce the timing overhead. Perhaps it makes sense to revisit that. But for now, without timings of the individual iterations, it doesn't make sense to compute the median, or min/max.

yhmtsai · 2023-03-13T13:51:21Z

@MarcelKoch Yes, if the iteration of timer does not match the iteration of IterationControl, it can only use average.
Does it make sense to use the average time for several iteration timing?
Put / times into vector, not put into vector.
By doing so, the iteration count will be the same in the adapting timing.
But the timing of each step does not share the same overhead of timing.

MarcelKoch

Currently, we take timestamps after a varying number of iterations, for example at repetition 1, 2, 4, 8, 16, 32, ... (not exactly but similar). So with the proposed changes we would take averages of a growing number of repetitions. That might have useful information, especially considering that the kernel launch and synchronization overhead is larger for fewer repetitions. But we wouldn't provide actual medians, or min/max.
Still, I won't hold this up.

yhmtsai · 2023-03-15T20:28:28Z

rebase!

benchmark/utils/general.hpp

benchmark/utils/timer_impl.hpp

- change timer_output -> timer_method - handle repetition_growth_factor <=1, next_timing will be next iteration - update timer_method to indicate the overhead difference - change best/worst -> min/max

sonarqubecloud · 2023-03-17T20:03:10Z

SonarCloud Quality Gate failed.

1 Bug
0 Vulnerabilities
0 Security Hotspots
3 Code Smells

0.0% Coverage
2.3% Duplication

codecov · 2023-03-17T20:09:59Z

Codecov Report

Patch coverage has no change and project coverage change: -0.40 ⚠️

Comparison is base (8b4c322) 91.20% compared to head (fdcf410) 90.81%.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #1294      +/-   ##
===========================================
- Coverage    91.20%   90.81%   -0.40%     
===========================================
  Files          570      570              
  Lines        48572    48593      +21     
===========================================
- Hits         44301    44130     -171     
- Misses        4271     4463     +192

see 5 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Release 1.6.0 of Ginkgo. The Ginkgo team is proud to announce the new Ginkgo minor release 1.6.0. This release brings new features such as: - Several building blocks for GPU-resident sparse direct solvers like symbolic and numerical LU and Cholesky factorization, ..., - A distributed Schwarz preconditioner, - New FGMRES and GCR solvers, - Distributed benchmarks for the SpMV operation, solvers, ... - Support for non-default streams in the CUDA and HIP backends, - Mixed precision support for the CSR SpMV, - A new profiling logger which integrates with NVTX, ROCTX, TAU and VTune to provide internal Ginkgo knowledge to most HPC profilers! and much 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.13+ + C++14 compliant compiler + Linux and macOS + GCC: 5.5+ + clang: 3.9+ + Intel compiler: 2018+ + Apple Clang: 14.0 is tested. Earlier versions might also work. + NVHPC: 22.7+ + Cray Compiler: 14.0.1+ + CUDA module: CUDA 9.2+ or NVHPC 22.7+ + HIP module: ROCm 4.5+ + DPC++ module: Intel OneAPI 2021.3+ with oneMKL and oneDPL. Set the CXX compiler to `dpcpp`. + Windows + MinGW: GCC 5.5+ + Microsoft Visual Studio: VS 2019+ + CUDA module: CUDA 9.2+, Microsoft Visual Studio + OpenMP module: MinGW. ### Version Support Changes + ROCm 4.0+ -> 4.5+ after [#1303](#1303) + Removed Cygwin pipeline and support [#1283](#1283) ### Interface Changes + Due to internal changes, `ConcreteExecutor::run` will now always throw if the corresponding module for the `ConcreteExecutor` is not build [#1234](#1234) + The constructor of `experimental::distributed::Vector` was changed to only accept local vectors as `std::unique_ptr` [#1284](#1284) + The default parameters for the `solver::MultiGrid` were improved. In particular, the smoother defaults to one iteration of `Ir` with `Jacobi` preconditioner, and the coarse grid solver uses the new direct solver with LU factorization. [#1291](#1291) [#1327](#1327) + The `iteration_complete` event gained a more expressive overload with additional parameters, the old overloads were deprecated. [#1288](#1288) [#1327](#1327) ### Deprecations + Deprecated less expressive `iteration_complete` event. Users are advised to now implement the function `void iteration_complete(const LinOp* solver, const LinOp* b, const LinOp* x, const size_type& it, const LinOp* r, const LinOp* tau, const LinOp* implicit_tau_sq, const array<stopping_status>* status, bool stopped)` [#1288](#1288) ### Added Features + A distributed Schwarz preconditioner. [#1248](#1248) + A GCR solver [#1239](#1239) + Flexible Gmres solver [#1244](#1244) + Enable Gmres solver for distributed matrices and vectors [#1201](#1201) + An example that uses Kokkos to assemble the system matrix [#1216](#1216) + A symbolic LU factorization allowing the `gko::experimental::factorization::Lu` and `gko::experimental::solver::Direct` classes to be used for matrices with non-symmetric sparsity pattern [#1210](#1210) + A numerical Cholesky factorization [#1215](#1215) + Symbolic factorizations in host-side operations are now wrapped in a host-side `Operation` to make their execution visible to loggers. This means that profiling loggers and benchmarks are no longer missing a separate entry for their runtime [#1232](#1232) + Symbolic factorization benchmark [#1302](#1302) + The `ProfilerHook` logger allows annotating the Ginkgo execution (apply, operations, ...) for profiling frameworks like NVTX, ROCTX and TAU. [#1055](#1055) + `ProfilerHook::created_(nested_)summary` allows the generation of a lightweight runtime profile over all Ginkgo functions written to a user-defined stream [#1270](#1270) for both host and device timing functionality [#1313](#1313) + It is now possible to enable host buffers for MPI communications at runtime even if the compile option `GINKGO_FORCE_GPU_AWARE_MPI` is set. [#1228](#1228) + A stencil matrices generator (5-pt, 7-pt, 9-pt, and 27-pt) for benchmarks [#1204](#1204) + Distributed benchmarks (multi-vector blas, SpMV, solver) [#1204](#1204) + Benchmarks for CSR sorting and lookup [#1219](#1219) + A timer for MPI benchmarks that reports the longest time [#1217](#1217) + A `timer_method=min|max|average|median` flag for benchmark timing summary [#1294](#1294) + Support for non-default streams in CUDA and HIP executors [#1236](#1236) + METIS integration for nested dissection reordering [#1296](#1296) + SuiteSparse AMD integration for fillin-reducing reordering [#1328](#1328) + Csr mixed-precision SpMV support [#1319](#1319) + A `with_loggers` function for all `Factory` parameters [#1337](#1337) ### Improvements + Improve naming of kernel operations for loggers [#1277](#1277) + Annotate solver iterations in `ProfilerHook` [#1290](#1290) + Allow using the profiler hooks and inline input strings in benchmarks [#1342](#1342) + Allow passing smart pointers in place of raw pointers to most matrix functions. This means that things like `vec->compute_norm2(x.get())` or `vec->compute_norm2(lend(x))` can be simplified to `vec->compute_norm2(x)` [#1279](#1279) [#1261](#1261) + Catch overflows in prefix sum operations, which makes Ginkgo's operations much less likely to crash. This also improves the performance of the prefix sum kernel [#1303](#1303) + Make the installed GinkgoConfig.cmake file relocatable and follow more best practices [#1325](#1325) ### Fixes + Fix OpenMPI version check [#1200](#1200) + Fix the mpi cxx type binding by c binding [#1306](#1306) + Fix runtime failures for one-sided MPI wrapper functions observed on some OpenMPI versions [#1249](#1249) + Disable thread pinning with GPU executors due to poor performance [#1230](#1230) + Fix hwloc version detection [#1266](#1266) + Fix PAPI detection in non-implicit include directories [#1268](#1268) + Fix PAPI support for newer PAPI versions: [#1321](#1321) + Fix pkg-config file generation for library paths outside prefix [#1271](#1271) + Fix various build failures with ROCm 5.4, CUDA 12, and OneAPI 6 [#1214](#1214), [#1235](#1235), [#1251](#1251) + Fix incorrect read for skew-symmetric MatrixMarket files with explicit diagonal entries [#1272](#1272) + Fix handling of missing diagonal entries in symbolic factorizations [#1263](#1263) + Fix segmentation fault in benchmark matrix construction [#1299](#1299) + Fix the stencil matrix creation for benchmarking [#1305](#1305) + Fix the additional residual check in IR [#1307](#1307) + Fix the cuSPARSE CSR SpMM issue on single strided vector when cuda >= 11.6 [#1322](#1322) [#1331](#1331) + Fix Isai generation for large sparsity powers [#1327](#1327) + Fix Ginkgo compilation and test with NVHPC >= 22.7 [#1331](#1331) + Fix Ginkgo compilation of 32 bit binaries with MSVC [#1349](#1349)

yhmtsai added the 1:ST:ready-for-review This PR is ready for review label Mar 9, 2023

yhmtsai self-assigned this Mar 9, 2023

ginkgo-bot added reg:benchmarking This is related to benchmarking. type:solver This is related to the solvers type:preconditioner This is related to the preconditioners labels Mar 9, 2023

yhmtsai force-pushed the more_timer_output branch from 5748092 to aa7b817 Compare March 9, 2023 12:56

pratikvn approved these changes Mar 9, 2023

View reviewed changes

yhmtsai force-pushed the more_timer_output branch 2 times, most recently from 57e0788 to 071b8dc Compare March 10, 2023 20:05

yanjen approved these changes Mar 13, 2023

View reviewed changes

MarcelKoch reviewed Mar 13, 2023

View reviewed changes

yhmtsai force-pushed the more_timer_output branch from 071b8dc to 7f53c6a Compare March 14, 2023 10:27

ginkgo-bot force-pushed the more_timer_output branch from 7f53c6a to d1ce63e Compare March 15, 2023 20:28

upsj reviewed Mar 15, 2023

View reviewed changes

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

upsj reviewed Mar 15, 2023

View reviewed changes

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

yhmtsai added 3 commits March 17, 2023 10:47

add more method for timer output

5704d09

record avg time with the number of operation

6db4d50

Update doc, handle repetition_growth_factor

fdcf410

- change timer_output -> timer_method - handle repetition_growth_factor <=1, next_timing will be next iteration - update timer_method to indicate the overhead difference - change best/worst -> min/max

yhmtsai force-pushed the more_timer_output branch from d1ce63e to fdcf410 Compare March 17, 2023 09:48

yhmtsai added 1:ST:ready-to-merge This PR is ready to merge. 1:ST:skip-full-test and removed 1:ST:ready-for-review This PR is ready for review labels Mar 17, 2023

yhmtsai merged commit fd3b425 into develop Mar 21, 2023

yhmtsai deleted the more_timer_output branch March 21, 2023 09:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add more method for timer output #1294

add more method for timer output #1294

yhmtsai commented Mar 9, 2023 •

edited

Loading

pratikvn left a comment

pratikvn Mar 9, 2023

pratikvn Mar 9, 2023

yhmtsai Mar 9, 2023

yanjen left a comment

yanjen Mar 13, 2023

yanjen Mar 13, 2023

yhmtsai Mar 13, 2023

yanjen Mar 13, 2023

yhmtsai Mar 14, 2023

MarcelKoch left a comment

yhmtsai commented Mar 13, 2023

MarcelKoch left a comment

yhmtsai commented Mar 15, 2023

sonarqubecloud bot commented Mar 17, 2023

codecov bot commented Mar 17, 2023

	* InterationControl, the method is only allowed "average".
	* IterationControl, the method is only allowed "average".

add more method for timer output #1294

add more method for timer output #1294

Conversation

yhmtsai commented Mar 9, 2023 • edited Loading

pratikvn 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

yanjen 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

MarcelKoch left a comment

Choose a reason for hiding this comment

yhmtsai commented Mar 13, 2023

MarcelKoch left a comment

Choose a reason for hiding this comment

yhmtsai commented Mar 15, 2023

sonarqubecloud bot commented Mar 17, 2023

codecov bot commented Mar 17, 2023

Codecov Report

yhmtsai commented Mar 9, 2023 •

edited

Loading