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
321 stars 78 forks source link

Don't define template functions in library C++ files #223

Closed intractabilis closed 3 years ago

intractabilis commented 3 years ago

It's kind of a very basic C++ rule of thumb. If you define your template functions in C++ files included into a library, there is a guarantee it's not going to work, because the compiler doesn't know how to instantiate the templates.

Surely it doesn't work with your library too. Follow your build instructions, then try to compile and link with your library your own example:

#include <iostream>
#include <vector>

#include <cmaes.h>

using namespace libcmaes;

FitFunc fsphere = [](const double *x, const int N)
{
    double val = 0.0;
    for (int i=0; i < N; i++) {
        val += x[i]*x[i];
    }
    return val;
};

int main(int argc, char *argv[])
{
    int dim = 10; // problem dimensions.
    double sigma = 0.1;
    double lbounds[dim], ubounds[dim]; // arrays for lower and upper parameter bounds, respectively

    for (int i = 0; i < dim; i++) {
        lbounds[i] = -2.0;
        ubounds[i] = 2.0;
    }

    std::vector<double> x0(dim,1.0); // beware that x0 is within bounds.
    GenoPheno<pwqBoundStrategy> gp(lbounds,ubounds,dim); // genotype / phenotype transform associated to bounds.
    CMAParameters<GenoPheno<pwqBoundStrategy>> cmaparams(x0,sigma,-1,0,gp); // -1 for automatically
                                                                            // decided lambda, 0 is for random seeding of the internal generator.
    cmaparams.set_algo(aCMAES);
    CMASolutions cmasols = cmaes<GenoPheno<pwqBoundStrategy>>(fsphere,cmaparams);
    std::cout << "best solution: ";
    cmasols.print(std::cout,0,gp);
    std::cout << std::endl;
    std::cout << "optimization took " << cmasols.elapsed_time() / 1000.0 << " seconds\n";
    return cmasols.run_status();
}

You will see a multitude of link errors:

undefined reference to `libcmaes::ESOStrategy<libcmaes::CMAParameters<libcmaes::GenoPheno<libcmaes::pwqBoundStrategy, libcmaes::NoScalingStrategy> >, libcmaes::CMASolutions, libcmaes::CMAStopCriteria<libcmaes::GenoPheno<libcmaes::pwqBoundStrategy, libcmaes::NoScalingStrategy> > >::eval(Eigen::Matrix<double, -1, -1, 1, -1, -1> const&, Eigen::Matrix<double, -1, -1, 1, -1, -1> const&)'
undefined reference to `libcmaes::IPOPCMAStrategy<libcmaes::CovarianceUpdate, libcmaes::GenoPheno<libcmaes::pwqBoundStrategy, libcmaes::NoScalingStrategy> >::optimize(std::function<void (Eigen::Matrix<double, -1, -1, 1, -1, -1> const&, Eigen::Matrix<double, -1, -1, 1, -1, -1> const&)> const&, std::function<Eigen::Matrix<double, -1, -1, 1, -1, -1> ()> const&, std::function<void ()> const&)'
...

You must define template functions in header files, NEVER in C++ files.

beniz commented 3 years ago

Hi, you may want to share your building/linking setup and/or files, templates in C++ files are usually not an issue and allow for faster development/rebuild.

intractabilis commented 3 years ago

Building and linking of the library is as described in README.md. Toolset is GCC 11. Linking with the library uses your libcmaes.pc.

All of this is irrelevant. Templates must be defined in header files, period.

allow for faster development/rebuild

How so if it doesn't work? I'd rather spend a couple of extra seconds of compilation than hours of search what explicit templates I have to instantiate and then recompile the library, or moving definitions to the header files. Nobody does it in a bizarre way you have done it.

If you would like to shorten your compilation time for development purposes, use precompiled header files. I recommend Meson for that. Setting up a precompiled header is one liner in Meson.

Another thing: don't enforce C++11 in libcmaes.pc. It's an ancient standard, most people use C++14 or C++17 nowadays.

beniz commented 3 years ago

All of this is irrelevant. Templates must be defined in header files, period.

This sounds nice :)

If you feel like marking your point by providing a PR, feel free. Until then, we do prefer action over reaction.

intractabilis commented 3 years ago

If you feel like marking your point by providing a PR, feel free. Until then, we do prefer action over reaction.

Good to know. Some people don't like others touch their code. I will revamp the whole library. I hope you won't complain on a lot of changes.