-
Notifications
You must be signed in to change notification settings - Fork 79
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
change all interfaces from double* + size to vector<double> #65
Comments
FYI, the CMAParameters constructor has been updated some time ago, so at least there's a new one: CMAParameters(const std::vector<double> &x0,
CMAParameters(const std::vector<double> &x0,
const double &sigma,
const int &lambda=-1,
const uint64_t &seed=0,
const TGenoPheno &gp=GenoPheno<NoBoundStrategy>()); For now, I have been keeping both the CMAParameters constructor and the FitFunc type with double* for the following reasons:
These may be insufficient reasons and I am ready to revise them, I just wanted to make clear that my silence was because I was pondering ! |
I am closing this one for now as I believe pointers still make best sense at the moment. We could use move semantics for passing the user vector to the function without copying it, but this would empty the original vector, which may be detrimental to some applications. |
i am puzzled. this is clearly a c++ library, even more, it's a c++11 library. this thing does not work with c anyway, so i don't see the point. we keep treating stl as some kind of external library, which is not right. stl is part of the c++ language. having pointer interfaces facing user side is just bad design since it is not clear how and whether ownership is transferred. a reference has the smallness of the pointer and tells the user that he is the owner of the arguments. i don't understand why you think about move semantics, the vector reference is constant anyway and you're not allowed to modify it. there is no copy made with a reference so i am really confused... |
The work to be done on the interface regarding input data can be broken down as follows:
As a side note, the library does work with C as long as C is compiled with g++. While this is not the dream use case, this is happening from time to time. A useful example is the use of the BBOB benchmark C code, and its linking to libcmaes, see https://github.com/beniz/libcmaes/blob/master/tests/bbobexperiment.cc Unless I am missing something, the points above are in practice cleared or transferred to other tickets for resolution. |
using a pair of double* and an int for size is very antiquated and very c-style. there is probably no reason why cmaes intefarces should not use vector. this includes for instance the following:
CMAParameters<>(unsigned int n, const double* const p, const double sigma);
and the minimizer lambda
FitFunc Likelihood = [](const double* const p, const unsigned int n) { ... };
which could be e.g. simply
CMAParameters<>(const vector& p, const double sigma);
FitFunc Likelihood = [](const vector& p) { ... };
The text was updated successfully, but these errors were encountered: