CMA-ES / libcmaes

libcmaes is a multithreaded C++11 library with Python bindings for high performance blackbox stochastic optimization using the CMA-ES algorithm for Covariance Matrix Adaptation Evolution Strategy
Other
315 stars 79 forks source link

Cmake updates #210

Closed jwittbrodt closed 3 years ago

jwittbrodt commented 3 years ago

Hi,

I updated the CMake setup to export the proper targets both on install and when libcmaes is used through add_subdirectory.

This closes #157

  1. It is now possible to use libcmaes directly as a subdirectory of another cmake project through add_subdirectory(libcmaes). In this case the library target is accessible through the libcmaes::cmaes alias. Additionally, if this use case is detected the tests, examples and python interface are not built (i.e. the corresponding options default to off). To make this usage less error-prone I replaced all global properties (like CMAKE_CXX_FLAGS or calls to includedirectories) with the corresponding target properties on the cmaes target. I also renamed to options by prefixing them all with `LIBCMAES` to avoid any option name clashes. In particular you can now use FetchContent to download and compile libcmaes as

    FetchContent_Declare(
      libcmaes
      GIT_REPOSITORY https://github.com/beniz/libcmaes.git
      GIT_TAG master)
    FetchContent_GetProperties(libcmaes)
    if(NOT libcmaes_POPULATED)
      FetchContent_Populate(libcmaes)
      add_subdirectory(${libcmaes_SOURCE_DIR} ${libcmaes_BINARY_DIR})
    endif()
    # ...
    target_link_library(yourTarget PRIVATE libcmaes::cmaes)
  2. Installing libcmaes now installs a libcmaesConfig.cmake file in the appropriate location. This way (if the install prefix is standard or matches the libcmaes install prefix) you can simply link against libcmaes as

    find_package(libcmaes)
    target_link_libraries(yourTarget PRIVATE libcmaes::cmaes)

    The dependencies to Eigen and OpenMP are handled automatically (in libcmaesConfig.cmake.in).

  1. A libcmaesConfig.cmake file is also generated in the build directory. If pointed to this file (by setting libcmaes_DIR before calling find_package(libcmaes)) the library from the build directory will be used directly without having to be installed.

I bumped the cmake_version_required from 2.8 to 3.1 to make all of this considerably easier. This version is still ancient, and cmake is really easy to upgrade, so I don't think this will impact many users.

I also updated the Find modules. For Eigen3, I just updated to the latest official one, which defines the proper interface targets and for numpy I rewrote the existing one.

Cheers, Jonas

jwittbrodt commented 3 years ago

One issue remains that I don't want to address without feedback by the authors: libcmaes (correctly) installs it's headers to a libcmaes subfolder of the include directory, but this directory layout is not mimicked in the source tree. This means that when using method 2 the libcmaes headers will be available as e.g.

#include <libcmaes/cmaes.h>

while using the other two methods only

#include "cmaes.h"

will work. Ideally, this would be fixed by moving all header files from the src/ directory into include/libcmaes/ to allow a consistent usage through all methods.

beniz commented 3 years ago

Hi @jwittbrodt thanks for the useful and very professional work! I do agree that moving the headers into include/libcmaes in the source tree is a good practice.

Users of previous versions would have to update the way they include the library headers. Maybe there could be a way or two to facilitate the migration, e.g. by adding a small section to the README and/or wiki, or by adding a special cmaes.h file into the src/ directory somehow. If it is applicable, the later would make the migration actually invisible.

Let me know your thoughts.

As a side note, you may want to squash all commits once the PR is finished!

jwittbrodt commented 3 years ago

Great. Keeping a cmaes.h in the src directory that just includes the real cmaes.hshould indeed be a good solution. I will open a separate pull request for the restructuring in a few weeks when I'm back at work, so feel free to merge this one.

I think you can just tick a box and the commits will be squashed on merge, right?

phbasler commented 3 years ago

@jwittbrodt @beniz Basically you're just thinking of src/XXX.h with a content #include <libcmaes/XXX.h> , right? I can do that at some point over the weekend and provide a seperate PR for that.

beniz commented 3 years ago

@phbasler this sounds as easy as this simple header yes. Thanks.

phbasler commented 3 years ago

@beniz I seem to have some trouble with the autogen installation. An easy way without having to create additional files after moving them to include/libcmaes/*.h would be to adapt the Include routines to both have -I path/to/include/libcmaes and -I path/to/include , but this is not really a nice solution. I will try to fix this

jwittbrodt commented 3 years ago

I am back now and will also take a look at the restructuring. @beniz any particular reason this one isn't merged yet? Something I still need to do?

beniz commented 3 years ago

I was away. Let me know if I should merge and if a release is needed.

jwittbrodt commented 3 years ago

Ok, great. In that case it would be nice if you could merge this, then @phbasler and/or me can do a separate pull request for the header file move. You could probably do another release once that one is done as well.

Cheers, Jonas

beniz commented 3 years ago

Hi, please look at #213 as there seems to be smallish issues with the current cmake build.