[joss-review] Minimal CMake example is missing some dependencies
Hi, while trying to build and install htool as described in the docs (which worked without problems), I also wanted to try out the CMake example. It lists these packages as dependencies:
find_package(MPI REQUIRED)
find_package(BLAS REQUIRED)
I had to additionally specify
find_package(LAPACK REQUIRED)
find_package(OpenMP REQUIRED)
It seems that all those packages are hard dependencies of htool, so I was wondering if it would make more sense if the HtoolConfig.cmake just does the find_package calls? Then a simple find_package(Htool REQUIRED) would be sufficient. I am not an expert in CMake, so if there are some disadvantages to this, then the example should still be updated (I'd be happy to create a PR for that but I'm not sure where the docs can even be edited, I guess they are in a separate repo?).
cc: openjournals/joss-reviews#9279
Hi,
Thank you for your feedback. The only hard-dependency used everywhere would be BLAS a priori, then it depends on what the user actually wants to use.
- MPI is only used when using the
DistributedOperatorand/orSolvercomponents. This way, users can useHMatrixwithout a MPI implementation (some people are only interested by the compression). - OpenMP is only used by the
HMatrixcomponent, but it has sequential version of all parallelized operations. Besides, even the call to parallelized operations with OpenMP should compile without OpenMP and should run sequentially. This is because every OpenMP pragma is prefixed with#if defined(_OPENMP). - Lapack is only used when the compressor
SVDis chosen, factorisation or recompression are used. It is also used for building the coarse space with dense matrices. But for example, one would not need it if he only wants to use aHMatrixwith another compression, or even aDistributedOperatorto get a distributed matrix vector product. - HPDDM is required for
Solver.
That being said, I checked how to put these find_package calls into the CMake target and it seems possible to add them in here. It would be easy for Lapack, MPI and Lapack, but the issue would be HPDDM which only have a custom (and not very robust) FindHPDDM I did for building my test-suite and the examples.
In any case, I could improve the CMake example with something like:
find_package(BLAS REQUIRED)
find_package(MPI) # required for DistributedOperator and Solver
find_package(LAPACK) # required for SVD compression or recompression, and GenEO coarse space
find_package(OpenMP) # required for shared parallelism
# other optional packages
what do you think?
The problem is that htool itself has find_package(OpenMP) in its CMakeLists.txt, so when I build htool and it finds OpenMP, then the Htool::htool target in HtoolTargets.cmake will link against OpenMP::OpenMP_CXX. So at this point OpenMP is not an optional dependency anymore and I need to have find_package(OpenMP) (ideally find_package(OpenMP REQUIRED)) in my CMakeLists.txt if I want to use Htool::htool.
So I think this
That being said, I checked how to put these find_package calls into the CMake target and it seems possible to add them in here.
would be the best solution. And you could ship the FindHPDDM.cmake with the other CMake files and use it there as well.
But I think clarifying the example is also fine.
I will add the find_package in cmake config file. But for HPDDM I prefer not shipping my FindHPDDM because it is really not robust (it just looks for hpddm in the folder next to the current one).
Should I modify the joss_paper branch or the main branch ?
Could you maybe fix this in main and then rebase (or merge main in) joss_paper?
Ideally I would fix in main and rebase joss_paper, but I just want to make sure it would not break anything on JOSS side.
This is fixed in #70. I will rebase joss_paper once this is merged