Make ginkgo.pc libraries be found recursively #923

tcojean · 2021-11-12T15:28:38Z

Small improvement for packages relying on our ginkgo.pc

This makes the libraries be found recursively so that -lhwloc which is also a dependency of ginkgo_device is found automatically. The aim is that this would also adapt to any other library structure change. This solution is obviously not bulletproof, but it should work in most cases.

Note that CMake on their side is working on pkg-config file creating support, but it's still ongoing AFAIK.
https://gitlab.kitware.com/cmake/cmake/-/issues/22621

This is required for OpenCARP and I already use it there:
1dd2dc7

thoasm

LGTM!

upsj

LGTM! Seems to work like it is supposed to, I guess the order of the recursion takes care of the correct link order?

cmake/information_helpers.cmake

yhmtsai

why do we need to find the hwloc of device? Isn't it required automatically when we link lib ginkgo_device?

yhmtsai · 2021-11-12T15:49:26Z

cmake/information_helpers.cmake

+        if (TARGET ${_libs})
+            list(APPEND GINKGO_INTERFACE_LIBS_FOUND "-l${_libs}")
+            get_target_property(GINKGO_LIBS_INTERFACE_LIBS "${_libs}" INTERFACE_LINK_LIBRARIES)
+            ginkgo_interface_libraries_recursively("${GINKGO_LIBS_INTERFACE_LIBS}")
+        endif()


infinite loop? if -lliba -llibb -lliba for some specific cases
b is in a interface and a is also in b interface

sonarqubecloud · 2021-11-16T13:39:22Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

codecov · 2021-11-16T13:43:15Z

Codecov Report

Merging #923 (cbd70da) into develop (029e0f8) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           develop     #923      +/-   ##
===========================================
+ Coverage    93.35%   93.37%   +0.02%     
===========================================
  Files          485      485              
  Lines        40709    40712       +3     
===========================================
+ Hits         38004    38016      +12     
+ Misses        2705     2696       -9

Impacted Files	Coverage Δ
core/solver/bicg.cpp	`88.88% <100.00%> (+0.12%)`	⬆️
core/solver/bicgstab.cpp	`97.80% <100.00%> (+0.02%)`	⬆️
core/solver/cg.cpp	`98.50% <100.00%> (+0.02%)`	⬆️
core/solver/cgs.cpp	`98.71% <100.00%> (+0.01%)`	⬆️
core/solver/fcg.cpp	`98.57% <100.00%> (+0.02%)`	⬆️
core/solver/idr.cpp	`99.03% <100.00%> (+<0.01%)`	⬆️
core/matrix/dense.cpp	`94.04% <0.00%> (+1.45%)`	⬆️

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 fda151c...cbd70da. Read the comment docs.

tcojean · 2021-12-22T10:40:18Z

This has been updated and complexified by quite a margin, in particular, to deal properly with MPI and include more link options.

This is what is now generated in a simple hwloc+MPI setting:

prefix=/home/user/0.INSTALL/ginkgo
libdir=${prefix}/lib64
includedir=${prefix}/include

Name:
Description: A numerical linear algebra library targeting many-core architectures
Version: 1.5.0
URL: https://ginkgo-project.github.io/

Requires:
Libs:  -L/home/user/0.INSTALL/ginkgo/lib64 -lginkgo -lginkgo_device /usr/lib64/libhwloc.so -lginkgo_omp -lginkgo_cuda -lginkgo_reference -lginkgo_hip -lginkgo_dpcpp -Wl,-rpath -Wl,/usr/lib64 -Wl,-rpath -Wl,/usr/lib64 -Wl,--enable-new-dtags -pthread /usr/lib64/libmpi.so
Cflags: -I/home/user/0.INSTALL/ginkgo/include -I/usr/include -pthread

upsj

There seem to be a few issues when running CMake on my system

CMakeLists.txt

cmake/information_helpers.cmake

sonarqubecloud · 2022-02-01T19:15:28Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

upsj

I tested with all major CMake versions since 3.13 and different configs (with MVAPICH, OpenMPI or no MPI, with vendored and installed HWLOC).

There are still two issues with the implementation

When using vendored hwloc, the pkg-config file contains the the build location of hwloc, not the installed location
The installed file probably needs to be renamed

cmake/install_helpers.cmake

yhmtsai

hwloc is a target and then it contains the hwloc.so file.
use it directly because the first condition only handles ginkgo lib.
recursive_search(hwloc.so) existed condition path, add it directly into link flag.
Is it the expected behavior?

cmake/information_helpers.cmake

stale

upsj

LGTM! I checked across all minor CMake versions we support and all 3 cases (no hwloc, vendored hwloc, installed hwloc), and the results are consistent.

stale

yhmtsai

LGTM. some nit. If you have examples about the SHELL, that will be great. @upsj already check all cmake output, so I do not wonder about the correctness.

cmake/information_helpers.cmake

yhmtsai · 2022-04-12T16:03:57Z

cmake/information_helpers.cmake

+    # without (v3.16.x). Sometimes, it also has extra arguments. We need to do
+    # at least a greedy regex also consuming the final `>` if present before
+    # doing a non greedy one for the leftovers.


do you have some example input here?

This comes from the CMake code itself, mostly from FindMPI
https://gitlab.kitware.com/cmake/cmake/-/blob/v3.20.5/Modules/FindMPI.cmake#L1186
https://gitlab.kitware.com/cmake/cmake/-/blob/v3.16.3/Modules/FindMPI.cmake#L1150

thoasm

LGTM

cmake/ginkgo.pc.in

+ Add support for multiple build types + Remove more generator expressions

tcojean · 2022-04-13T10:26:23Z

If there is no more issues, I would merge this once the pipeline succeeds.

upsj · 2022-04-13T13:08:12Z

Multi-Config generators currently don't generate the correct library name for debug builds due to CMAKE_DEBUG_SUFFIX. A fix for that would be using TARGET_LINKER_FILE_BASE_NAME, but that requires CMake 3.15, so we will update it in a later version. Corresponding diff:

diff --git a/cmake/information_helpers.cmake b/cmake/information_helpers.cmake
index 5cf80368c8..037f39e4a7 100644
--- a/cmake/information_helpers.cmake
+++ b/cmake/information_helpers.cmake
@@ -20,8 +20,8 @@ macro(ginkgo_interface_libraries_recursively INTERFACE_LIBS)
         if (NOT "${_libs}" IN_LIST GINKGO_INTERFACE_LIBS_FOUND
                 AND NOT "-l${_libs}" IN_LIST GINKGO_INTERFACE_LIBS_FOUND)
             if (TARGET ${_libs})
-                if (upper_CMAKE_BUILD_TYPE STREQUAL "DEBUG" AND "${_libs}" MATCHES "ginkgo.*")
-                    set(GINKGO_INTERFACE_LIB_NAME "-l${_libs}${CMAKE_DEBUG_POSTFIX}")
+                if ("${_libs}" MATCHES "ginkgo.*")
+                    set(GINKGO_INTERFACE_LIB_NAME "-l$<TARGET_LINKER_FILE_BASE_NAME:${_libs}>")
                 elseif("${_libs}" MATCHES "ginkgo.*") # Ginkgo libs are appended in the form -l
                     set(GINKGO_INTERFACE_LIB_NAME "-l${_libs}")
                 endif()
@@ -81,12 +81,7 @@ macro(ginkgo_interface_information)
     unset(GINKGO_INTERFACE_LIBS_FOUND)
     unset(GINKGO_INTERFACE_CFLAGS_FOUND)
     # Prepare recursively populated library list
-    string(TOUPPER "${CMAKE_BUILD_TYPE}" upper_CMAKE_BUILD_TYPE)
-    if (upper_CMAKE_BUILD_TYPE STREQUAL "DEBUG")
-        list(APPEND GINKGO_INTERFACE_LIBS_FOUND "-lginkgo${CMAKE_DEBUG_POSTFIX}")
-    else()
-        list(APPEND GINKGO_INTERFACE_LIBS_FOUND "-lginkgo")
-    endif()
+    list(APPEND GINKGO_INTERFACE_LIBS_FOUND "-l$<TARGET_LINKER_FILE_BASE_NAME:ginkgo>")
     # Prepare recursively populated include directory list
     list(APPEND GINKGO_INTERFACE_CFLAGS_FOUND
         "-I${CMAKE_INSTALL_PREFIX}/${GINKGO_INSTALL_INCLUDE_DIR}")

sonarqubecloud · 2022-04-13T14:54:32Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

Advertise release 1.5.0 and last changes + Add changelog, + Update third party libraries + A small fix to a CMake file See PR: #1195 The Ginkgo team is proud to announce the new Ginkgo minor release 1.5.0. This release brings many important new features such as: - MPI-based multi-node support for all matrix formats and most solvers; - full DPC++/SYCL support, - functionality and interface for GPU-resident sparse direct solvers, - an interface for wrapping solvers with scaling and reordering applied, - a new algebraic Multigrid solver/preconditioner, - improved mixed-precision support, - support for device matrix assembly, 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 LLVM: 8.0+ + NVHPC: 22.7+ + Cray Compiler: 14.0.1+ + CUDA module: CUDA 9.2+ or NVHPC 22.7+ + HIP module: ROCm 4.0+ + DPC++ module: Intel OneAPI 2021.3 with oneMKL and oneDPL. Set the CXX compiler to `dpcpp`. + Windows + MinGW and Cygwin: GCC 5.5+ + Microsoft Visual Studio: VS 2019 + CUDA module: CUDA 9.2+, Microsoft Visual Studio + OpenMP module: MinGW or Cygwin. Algorithm and important feature additions: + Add MPI-based multi-node for all matrix formats and solvers (except GMRES and IDR). ([#676](#676), [#908](#908), [#909](#909), [#932](#932), [#951](#951), [#961](#961), [#971](#971), [#976](#976), [#985](#985), [#1007](#1007), [#1030](#1030), [#1054](#1054), [#1100](#1100), [#1148](#1148)) + Porting the remaining algorithms (preconditioners like ISAI, Jacobi, Multigrid, ParILU(T) and ParIC(T)) to DPC++/SYCL, update to SYCL 2020, and improve support and performance ([#896](#896), [#924](#924), [#928](#928), [#929](#929), [#933](#933), [#943](#943), [#960](#960), [#1057](#1057), [#1110](#1110), [#1142](#1142)) + Add a Sparse Direct interface supporting GPU-resident numerical LU factorization, symbolic Cholesky factorization, improved triangular solvers, and more ([#957](#957), [#1058](#1058), [#1072](#1072), [#1082](#1082)) + Add a ScaleReordered interface that can wrap solvers and automatically apply reorderings and scalings ([#1059](#1059)) + Add a Multigrid solver and improve the aggregation based PGM coarsening scheme ([#542](#542), [#913](#913), [#980](#980), [#982](#982), [#986](#986)) + Add infrastructure for unified, lambda-based, backend agnostic, kernels and utilize it for some simple kernels ([#833](#833), [#910](#910), [#926](#926)) + Merge different CUDA, HIP, DPC++ and OpenMP tests under a common interface ([#904](#904), [#973](#973), [#1044](#1044), [#1117](#1117)) + Add a device_matrix_data type for device-side matrix assembly ([#886](#886), [#963](#963), [#965](#965)) + Add support for mixed real/complex BLAS operations ([#864](#864)) + Add a FFT LinOp for all but DPC++/SYCL ([#701](#701)) + Add FBCSR support for NVIDIA and AMD GPUs and CPUs with OpenMP ([#775](#775)) + Add CSR scaling ([#848](#848)) + Add array::const_view and equivalent to create constant matrices from non-const data ([#890](#890)) + Add a RowGatherer LinOp supporting mixed precision to gather dense matrix rows ([#901](#901)) + Add mixed precision SparsityCsr SpMV support ([#970](#970)) + Allow creating CSR submatrix including from (possibly discontinuous) index sets ([#885](#885), [#964](#964)) + Add a scaled identity addition (M <- aI + bM) feature interface and impls for Csr and Dense ([#942](#942)) Deprecations and important changes: + Deprecate AmgxPgm in favor of the new Pgm name. ([#1149](#1149)). + Deprecate specialized residual norm classes in favor of a common `ResidualNorm` class ([#1101](#1101)) + Deprecate CamelCase non-polymorphic types in favor of snake_case versions (like array, machine_topology, uninitialized_array, index_set) ([#1031](#1031), [#1052](#1052)) + Bug fix: restrict gko::share to rvalue references (*possible interface break*) ([#1020](#1020)) + Bug fix: when using cuSPARSE's triangular solvers, specifying the factory parameter `num_rhs` is now required when solving for more than one right-hand side, otherwise an exception is thrown ([#1184](#1184)). + Drop official support for old CUDA < 9.2 ([#887](#887)) Improved performance additions: + Reuse tmp storage in reductions in solvers and add a mutable workspace to all solvers ([#1013](#1013), [#1028](#1028)) + Add HIP unsafe atomic option for AMD ([#1091](#1091)) + Prefer vendor implementations for Dense dot, conj_dot and norm2 when available ([#967](#967)). + Tuned OpenMP SellP, COO, and ELL SpMV kernels for a small number of RHS ([#809](#809)) Fixes: + Fix various compilation warnings ([#1076](#1076), [#1183](#1183), [#1189](#1189)) + Fix issues with hwloc-related tests ([#1074](#1074)) + Fix include headers for GCC 12 ([#1071](#1071)) + Fix for simple-solver-logging example ([#1066](#1066)) + Fix for potential memory leak in Logger ([#1056](#1056)) + Fix logging of mixin classes ([#1037](#1037)) + Improve value semantics for LinOp types, like moved-from state in cross-executor copy/clones ([#753](#753)) + Fix some matrix SpMV and conversion corner cases ([#905](#905), [#978](#978)) + Fix uninitialized data ([#958](#958)) + Fix CUDA version requirement for cusparseSpSM ([#953](#953)) + Fix several issues within bash-script ([#1016](#1016)) + Fixes for `NVHPC` compiler support ([#1194](#1194)) Other additions: + Simplify and properly name GMRES kernels ([#861](#861)) + Improve pkg-config support for non-CMake libraries ([#923](#923), [#1109](#1109)) + Improve gdb pretty printer ([#987](#987), [#1114](#1114)) + Add a logger highlighting inefficient allocation and copy patterns ([#1035](#1035)) + Improved and optimized test random matrix generation ([#954](#954), [#1032](#1032)) + Better CSR strategy defaults ([#969](#969)) + Add `move_from` to `PolymorphicObject` ([#997](#997)) + Remove unnecessary device_guard usage ([#956](#956)) + Improvements to the generic accessor for mixed-precision ([#727](#727)) + Add a naive lower triangular solver implementation for CUDA ([#764](#764)) + Add support for int64 indices from CUDA 11 onward with SpMV and SpGEMM ([#897](#897)) + Add a L1 norm implementation ([#900](#900)) + Add reduce_add for arrays ([#831](#831)) + Add utility to simplify Dense View creation from an existing Dense vector ([#1136](#1136)). + Add a custom transpose implementation for Fbcsr and Csr transpose for unsupported vendor types ([#1123](#1123)) + Make IDR random initilization deterministic ([#1116](#1116)) + Move the algorithm choice for triangular solvers from Csr::strategy_type to a factory parameter ([#1088](#1088)) + Update CUDA archCoresPerSM ([#1175](#1116)) + Add kernels for Csr sparsity pattern lookup ([#994](#994)) + Differentiate between structural and numerical zeros in Ell/Sellp ([#1027](#1027)) + Add a binary IO format for matrix data ([#984](#984)) + Add a tuple zip_iterator implementation ([#966](#966)) + Simplify kernel stubs and declarations ([#888](#888)) + Simplify GKO_REGISTER_OPERATION with lambdas ([#859](#859)) + Simplify copy to device in tests and examples ([#863](#863)) + More verbose output to array assertions ([#858](#858)) + Allow parallel compilation for Jacobi kernels ([#871](#871)) + Change clang-format pointer alignment to left ([#872](#872)) + Various improvements and fixes to the benchmarking framework ([#750](#750), [#759](#759), [#870](#870), [#911](#911), [#1033](#1033), [#1137](#1137)) + Various documentation improvements ([#892](#892), [#921](#921), [#950](#950), [#977](#977), [#1021](#1021), [#1068](#1068), [#1069](#1069), [#1080](#1080), [#1081](#1081), [#1108](#1108), [#1153](#1153), [#1154](#1154)) + Various CI improvements ([#868](#868), [#874](#874), [#884](#884), [#889](#889), [#899](#899), [#903](#903), [#922](#922), [#925](#925), [#930](#930), [#936](#936), [#937](#937), [#958](#958), [#882](#882), [#1011](#1011), [#1015](#1015), [#989](#989), [#1039](#1039), [#1042](#1042), [#1067](#1067), [#1073](#1073), [#1075](#1075), [#1083](#1083), [#1084](#1084), [#1085](#1085), [#1139](#1139), [#1178](#1178), [#1187](#1187))

tcojean added the 1:ST:ready-for-review This PR is ready for review label Nov 12, 2021

tcojean requested a review from a team November 12, 2021 15:28

tcojean self-assigned this Nov 12, 2021

ginkgo-bot added the reg:build This is related to the build system. label Nov 12, 2021

thoasm previously approved these changes Nov 12, 2021

View reviewed changes

upsj approved these changes Nov 12, 2021

View reviewed changes

cmake/information_helpers.cmake Outdated Show resolved Hide resolved

yhmtsai reviewed Nov 12, 2021

View reviewed changes

tcojean force-pushed the pkgconfig_recursive branch from 1213cee to b84440d Compare November 15, 2021 22:48

tcojean added 1:ST:WIP This PR is a work in progress. Not ready for review. and removed 1:ST:ready-for-review This PR is ready for review labels Dec 21, 2021

tcojean force-pushed the pkgconfig_recursive branch from b84440d to 49d72cf Compare December 22, 2021 10:38

tcojean added 1:ST:ready-for-review This PR is ready for review and removed 1:ST:WIP This PR is a work in progress. Not ready for review. labels Dec 22, 2021

tcojean requested review from yhmtsai, upsj and thoasm December 22, 2021 10:45

upsj requested changes Jan 11, 2022

View reviewed changes

CMakeLists.txt Outdated Show resolved Hide resolved

cmake/information_helpers.cmake Outdated Show resolved Hide resolved

cmake/information_helpers.cmake Outdated Show resolved Hide resolved

upsj mentioned this pull request Jan 28, 2022

Cannot init any type of Executor #959

Closed

tcojean force-pushed the pkgconfig_recursive branch 2 times, most recently from 2f1c566 to 68689da Compare February 1, 2022 10:01

tcojean requested review from upsj February 4, 2022 14:48

upsj previously requested changes Feb 7, 2022

View reviewed changes

cmake/install_helpers.cmake Outdated Show resolved Hide resolved

yhmtsai reviewed Feb 7, 2022

View reviewed changes

cmake/information_helpers.cmake Show resolved Hide resolved

cmake/information_helpers.cmake Outdated Show resolved Hide resolved

tcojean added this to the Ginkgo 1.5.0 milestone Feb 10, 2022

tcojean removed the 1:ST:ready-for-review This PR is ready for review label Mar 17, 2022

tcojean added the 1:ST:WIP This PR is a work in progress. Not ready for review. label Mar 17, 2022

tcojean force-pushed the pkgconfig_recursive branch from 68689da to 6776248 Compare March 30, 2022 16:56

tcojean requested a review from yhmtsai March 30, 2022 16:56

tcojean added 1:ST:ready-for-review This PR is ready for review and removed 1:ST:WIP This PR is a work in progress. Not ready for review. labels Mar 30, 2022

upsj approved these changes Apr 12, 2022

View reviewed changes

yhmtsai approved these changes Apr 12, 2022

View reviewed changes

thoasm approved these changes Apr 12, 2022

View reviewed changes

cmake/ginkgo.pc.in Show resolved Hide resolved

tcojean added 5 commits April 13, 2022 12:14

Make ginkgo.pc libraries be found recursively

418fa49

Deal with more complex settings.

fcb4058

Review updates.

6c2b296

+ Add support for multiple build types + Remove more generator expressions

Take care of hwloc corner cases

fee3be7

Do not add duplicates to the list.

cbd70da

tcojean force-pushed the pkgconfig_recursive branch from 6776248 to cbd70da Compare April 13, 2022 10:14

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 Apr 13, 2022

tcojean merged commit 8619e77 into develop Apr 13, 2022

tcojean deleted the pkgconfig_recursive branch April 13, 2022 15:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make ginkgo.pc libraries be found recursively #923

Make ginkgo.pc libraries be found recursively #923

tcojean commented Nov 12, 2021

thoasm left a comment

upsj left a comment

yhmtsai left a comment

yhmtsai Nov 12, 2021 •

edited

Loading

sonarqubecloud bot commented Nov 16, 2021

codecov bot commented Nov 16, 2021 •

edited

Loading

tcojean commented Dec 22, 2021 •

edited

Loading

upsj left a comment

sonarqubecloud bot commented Feb 1, 2022

upsj left a comment

yhmtsai left a comment

upsj left a comment

yhmtsai left a comment

yhmtsai Apr 12, 2022

tcojean Apr 13, 2022

thoasm left a comment

tcojean commented Apr 13, 2022

upsj commented Apr 13, 2022

sonarqubecloud bot commented Apr 13, 2022

Make ginkgo.pc libraries be found recursively #923

Make ginkgo.pc libraries be found recursively #923

Conversation

tcojean commented Nov 12, 2021

thoasm left a comment

Choose a reason for hiding this comment

upsj left a comment

Choose a reason for hiding this comment

yhmtsai left a comment

Choose a reason for hiding this comment

yhmtsai Nov 12, 2021 • edited Loading

Choose a reason for hiding this comment

sonarqubecloud bot commented Nov 16, 2021

codecov bot commented Nov 16, 2021 • edited Loading

Codecov Report

tcojean commented Dec 22, 2021 • edited Loading

upsj left a comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Feb 1, 2022

upsj left a comment

Choose a reason for hiding this comment

yhmtsai left a comment

Choose a reason for hiding this comment

upsj left a comment

Choose a reason for hiding this comment

yhmtsai left a comment

Choose a reason for hiding this comment

yhmtsai Apr 12, 2022

Choose a reason for hiding this comment

tcojean Apr 13, 2022

Choose a reason for hiding this comment

thoasm left a comment

Choose a reason for hiding this comment

tcojean commented Apr 13, 2022

upsj commented Apr 13, 2022

sonarqubecloud bot commented Apr 13, 2022

yhmtsai Nov 12, 2021 •

edited

Loading

codecov bot commented Nov 16, 2021 •

edited

Loading

tcojean commented Dec 22, 2021 •

edited

Loading