Fix cmake for installation to a location

[JacobLotz]

EDITED 31-08-2023 Last Januari I have been working on two features: 1) to install libROM to a specified install location 2) to use MFEM from a specified location. This branch got out of sync with the master branch so I have re-emplemented this in the latest master of libROM. The changes mostly consist of changes in the cmake.

I am not an cmake expert and I learned this on the fly. The code works on my own local machine, but unfortunately more elaborate testing is difficult. I am creating this PR as it would be a pity if this would be lost and someone has to redo it again. However, this addition probably needs a thorough review of someone that actually knows what he is doing in cmake. If we cannot make that work it would be unfortunate. Then I will just use this in my forked version of libROM.

For feature 1) I have modified the cmake script with standard features such that it installs the binary and header files to a specified location. This location is specified in -DCMAKE_INSTALL_PREFIX. I have added standard options such that another cmake script is able to find it. The dependencies of libROM have to be added separately. I think there should be a way to account for the dependencies in libROM.config.in, but I could not figure out how to do that. I have added an example script on how to find libROM in cmake below.

For feature 2) I have added two options (which probably can be streamlined with other options or merged into one). These are -DUSE_EXTERNAL_MFEM=On and -DMFEM_DIR=$INSTALLDIR. This allows for usage of in external installation of MFEM, such that that you can compile libROM with the same version of MFEM you are already using in your code. In my workflow I will first compile and install MFEM, then compile and install libROM with this version of MFEM, then compile my code with MFEM and libROM.

An example shell script installation in such a workflow is as follows:

    mkdir build
    cd build
    cmake .. \
    -DCMAKE_INSTALL_PREFIX=$INSTALLDIR \
    -DCMAKE_CXX_COMPILER=/usr/bin/mpicxx \
    -DBUILD_STATIC=On \
    -DENABLE_EXAMPLES=Off \
    -DUSE_MFEM=Off \
    -DUSE_EXTERNAL_MFEM=On \
    -DMFEM_DIR=$INSTALLDIR
    make install -j $cores

An example script for cmake to find libROM would then be:

# Import libROM
find_package(librom REQUIRED NAMES libROM HINTS "${CMAKE_INSTALL_PREFIX}")
enable_language(${libROM_ENABLED_LANGUAGES})
link_libraries(${libROM_LIBRARIES})
# Librom dependencies
find_package(HDF5 1.8.0 REQUIRED)
find_package(BLAS 3.4.0 REQUIRED)
find_package(LAPACK 3.4.0 REQUIRED)
find_package(MPI 1.2 REQUIRED)
find_package(ZLIB 1.2.3 REQUIRED)
find_package(Doxygen 1.8.5)
find_package(GTest 1.6.0)

message(STATUS "Found libROM: in ${librom_DIR} (version ${librom_VERSION})")
message(STATUS "libROM_LIBRARIES = ${libROM_LIBRARIES}")
message(STATUS "libROM_ENABLED_LANGUAGES = ${libROM_ENABLED_LANGUAGES}")

[chldkdtn]

@dreamer2368 & @dylan-copeland can you review this PR?

[dreamer2368]

The code-style failure comes from IncrementalSVDBrand, which is already fixed in the current master branch. We might want to rebase this w.r.t. the current master branch.

[dylan-copeland]

In the past, whenever I needed to build libROM with a specific branch or commit of MFEM, I would just cd libROM/dependencies/mfem_parallel and checkout that branch, rebuild MFEM there, and then rebuild libROM with ./scripts/compile.sh -m. However, that is not working for me now. Something must have changed in the build system recently. Assuming we fix that, wouldn't that accomplish the same thing as this PR. If that is the case, is there still a good reason to merge this PR, or is the procedure I described sufficient, @JacobLotz?

[dreamer2368]

Assuming we fix that, wouldn't that accomplish the same thing as this PR. If that is the case, is there still a good reason to merge this PR, or is the procedure I described sufficient, @JacobLotz?

I'm wondering if I can jump in. I think it might be useful to allow users to have their custom mfem path. They may already using mfem somewhere else and wouldn't want to change the setting, or have a duplicate mfem?

Although, I also think some part is somewhat redundant, like USE_EXTERNAL_MFEM variable (which is commented in the code review).

[JacobLotz]

@dreamer2368: I have rebased with respect to the current master branch and the tests seem to pass now.

@dylan-copeland: I think you accomplish the same. However, I think that the method in this PR is more elegant and is less prone to errors as you are very sure that you use the same MFEM version.

@dreamer2368: Yes please jump in! I think you have good ideas and we should use them. I have given you access to my fork. I will have a look if I can move this branch to libROM repo (not the fork) itself, that might work a bit easier. EDIT: I think that is not possible.

BTW I think that we should not forget that the main feature of this branch is to be able to install libROM to a specific location.

[dreamer2368]

I forgot to finish the review to leave the comments.. To me, I think PR seems okay once the comments can be resolved.

[dylan-copeland]

@JacobLotz is there a way for me to checkout your PR from libROM? I cloned your fork and manually copied the changed files to my clone of master, which would not be convenient for a larger PR. Is there a better way?

[dylan-copeland]

@JacobLotz my attempt at a standard build with ./scripts/compile.sh -m resulted in the following error. I suppose you are building in a different way, but this script should still work.

CMake Error in lib/CMakeLists.txt: Target "ROM" INTERFACE_INCLUDE_DIRECTORIES property contains path: "libROM/dependencies/mfem" which is prefixed in the source directory.

[chldkdtn]

What is the status of this PR?