BallisticLA / RandBLAS

A header-only C++ library for sketching in randomized linear algebra
https://randblas.readthedocs.io/en/latest/
Other
66 stars 4 forks source link

RandBLAS.hh creation issue. #35

Closed TeachRaccooon closed 1 year ago

TeachRaccooon commented 1 year ago

RandBLAS.hh does not get properly created at installation time. Projects accessing RandBLAS.hh will then have no way of viewing this file. With RandLAPACK, this issue appears once I try building benchmark code. The issue is resolved by manually moving RandBLAS.hh into install include folder.

The view of a directory after a fresh install:

-bash-4.2$ pwd
/home/mmelnich/libs/RandLAPACK-install/include
-bash-4.2$ ls
RandBLAS  RandLAPACK  RandLAPACK.hh
-bash-4.2$ cd RandBLAS/
-bash-4.2$ ls
base.hh  config.h  dense.hh  exceptions.hh  random_gen.hh  sparse.hh  test_util.hh  util.hh

The view of the directory after manually copying RandBLAS.hh:

-bash-4.2$ ls
RandBLAS  RandBLAS.hh  RandLAPACK  RandLAPACK.hh
rileyjmurray commented 1 year ago

@burlen we aren't sure why this issue doesn't show up in RandLAPACK tests. It probably has something to do with the fact that RandBLAS is a subproject of RandLAPACK from a CMake perspective. When Max says the issue only appears when building benchmark code, he means that it only appears when using CMake to build this project: https://github.com/BallisticLA/RandLAPACK/blob/main/benchmark/CMakeLists.txt. I suspect that any "third-party" project that tries to use RandBLAS in this way will run into the same error as Max.

rileyjmurray commented 1 year ago

I think the problem traces back to these lines:

https://github.com/BallisticLA/RandBLAS/blob/68116d6825ee89d7b0af911dfb0d39234e3d98dd/RandBLAS/CMakeLists.txt#L30-L31

CMake interprets that statement from within the RandBLAS folder of this repo. Meanwhile, RandBLAS.hh is located at the top level of this repo (at the same level of the RandBLAS folder). So that command doesn't tell RandBLAS's CMake to install RandBLAS.hh. I was able to fix this by adding the following lines right before the installation command linked above:

install(
    FILES "${CMAKE_CURRENT_SOURCE_DIR}/../RandBLAS.hh"
    DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}
)

(Although, I'll note that the exact location of this command probably doesn't matter. Before or after should be fine.)

After this change, I've verified that RandBLAS gets installed correctly when installed as a subproject of RandLAPACK.

I'd like to know why RandLAPACK is able to build and successfully pass tests given that RandBLAS's installation is broken without this change. I think this happens because CMake is recognizing that some RandLAPACK header files require RandBLAS.hh, and the makefile that CMake generates makes a point to include this file in the compilation statements that rely on it. This would let RandBLAS tests and benchmarks compile correctly even though the installation of RandLAPACK remains broken from the broken RandBLAS installation.

rileyjmurray commented 1 year ago

Resolved by #39