Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update PAPI SDE to support new versions #1321

Merged
merged 11 commits into from
Aug 14, 2023
Merged

Update PAPI SDE to support new versions #1321

merged 11 commits into from
Aug 14, 2023

Conversation

tcojean
Copy link
Member

@tcojean tcojean commented Apr 6, 2023 •
edited

As the title says.

I also reviewed a bit the external setup for non-CMake projects and at the same time slightly improved the hwloc setup.

For the PAPI SDE side, this depends on: https://bitbucket.org/icl/papi/issues/133/sde-problem-when-using-shutdown-for

@tcojean tcojean added the 1:ST:ready-for-review This PR is ready for review label Apr 6, 2023
@tcojean tcojean self-assigned this Apr 6, 2023
@ginkgo-bot ginkgo-bot added reg:build This is related to the build system. reg:testing This is related to testing. reg:ci-cd This is related to the continuous integration system. mod:core This is related to the core module. mod:reference This is related to the reference module. reg:example This is related to the examples. labels Apr 6, 2023
MarcelKoch
MarcelKoch reviewed Apr 6, 2023
View reviewed changes
Copy link
Member

@MarcelKoch MarcelKoch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, looks good. I only have some questions on the cmake set-up.

Is it now necessary to build PAPI ourselfs, or could we also use an installed one with the right version?

Did you check if the third party PAPI gets propagated to downstream users?

third_party/papi_sde/CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
yhmtsai
yhmtsai reviewed Apr 6, 2023
View reviewed changes
cmake/information_helpers.cmake Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
@upsj upsj self-requested a review April 6, 2023 15:57
@MarcelKoch
Copy link
Member

MarcelKoch commented Apr 12, 2023 •
edited

Side note: I build and installed ginkgo from current develop with a preinstalled papi (by spack), but the downstream project couldn't find the papi library. So we should make extra sure that it will work now. The issue is not restricted to PAPI, so ignore that.
Edit: The issue was caused by leaving GINKGO_INSTALL_RPATH_DEPENDENCIES=OFF. Switching that to ON fixed it. Perhaps we should discuss setting it to ON by default. That makes sense if ginkgo is built using dependencies from spack, since spack only relies on rpath and does not adjust LD_LIBRARY_PATH.

yhmtsai
yhmtsai reviewed Apr 18, 2023
View reviewed changes
cmake/DownloadNonCMakeCMakeLists.txt.in Outdated Show resolved Hide resolved
third_party/papi_sde/CMakeLists.txt Outdated Show resolved Hide resolved
tcojean added a commit that referenced this pull request Jun 16, 2023
@tcojean
Merge Release 1.6.0
1f1ed46 
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)
upsj
upsj approved these changes Aug 1, 2023
View reviewed changes
Copy link
Member

@upsj upsj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! But I'd honestly be happiest if we could assume hwloc and/or PAPI are available in the environment, and remove the custom build steps. That relates to another complexity in our CMake setup: Things like RPATH for third-party libraries, ROCm runtime etc. are concerns of the system configuration and not things that we should need to worry about.

.gitlab/scripts.yml Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
yhmtsai
yhmtsai reviewed Aug 1, 2023
View reviewed changes
cmake/DownloadNonCMakeCMakeLists.txt.in Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
MarcelKoch
MarcelKoch approved these changes Aug 1, 2023
View reviewed changes
Copy link
Member

@MarcelKoch MarcelKoch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would also prefer if we would not build PAPI or hwloc by ourselves. Since these two are not critical, it would be fine if they are just not available.

@tcojean
Copy link
Member Author

tcojean commented Aug 8, 2023

format!

upsj
upsj reviewed Aug 9, 2023
View reviewed changes
CMakeLists.txt Outdated Show resolved Hide resolved
upsj
upsj reviewed Aug 9, 2023
View reviewed changes
CMakeLists.txt Outdated Show resolved Hide resolved
@tcojean tcojean force-pushed the papi_sde branch 4 times, most recently from 7f3b927 to d26ddc6 Compare August 11, 2023 08:44
yhmtsai
yhmtsai reviewed Aug 11, 2023
View reviewed changes
CMakeLists.txt Outdated Show resolved Hide resolved
.gitlab/scripts.yml Show resolved Hide resolved
CMakeLists.txt Show resolved Hide resolved
cmake/Modules/FindPAPI.cmake Show resolved Hide resolved
include/ginkgo/core/log/papi.hpp Show resolved Hide resolved
cmake/Modules/FindPAPI.cmake Show resolved Hide resolved
cmake/autodetect_system_libs.cmake Show resolved Hide resolved
@tcojean 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 1:ST:run-full-test labels Aug 14, 2023
@tcojean tcojean merged commit f33e709 into develop Aug 14, 2023
12 of 14 checks passed
@tcojean tcojean deleted the papi_sde branch August 14, 2023 15:38
@sonarcloud
Copy link

sonarcloud bot commented Aug 14, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell E 17 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

warning 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

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@upsj upsj upsj approved these changes

@MarcelKoch MarcelKoch MarcelKoch approved these changes

@yhmtsai yhmtsai yhmtsai approved these changes

Assignees

@tcojean tcojean

Labels
1:ST:ready-to-merge This PR is ready to merge. mod:core This is related to the core module. mod:reference This is related to the reference module. reg:build This is related to the build system. reg:ci-cd This is related to the continuous integration system. reg:example This is related to the examples. reg:testing This is related to testing.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@tcojean @MarcelKoch @upsj @yhmtsai @ginkgo-bot