Skip to content

Updated support for CMA-ES minimizer based on libcmaes #507

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

Closed
wants to merge 103 commits into from

Conversation

beniz
Copy link

@beniz beniz commented Apr 18, 2017

This is an update to PR #40 that includes:

  • a rebase with all CMA-ES commits at the tip of master (as of 04/17/2017)
  • a set of small fixes as requested by @vgvassilev

As a reminder, this PR fetches, builds and wraps https://github.com/beniz/libcmaes/ with ROOT.

This PR builds ROOT with CMA-ES support and I've been able to run some of the tests, indicating that it is working fine.

Issues
However, some caveats remain, on which help is required, at the moment

  • build/include/libcmaes/cmaes.h and build/include/Eigen are not properly passed to the compiler at build time. I cannot find how to do it properly. At the moment I am using symlinks as a temporary hack (see how to build below)

  • To access the inner option of the CMA-ES Minimizer, I was using code similar to

ROOT::Math::IOptions &opts = ROOT::Math::MinimizerOptions::Default(fitter);
opts.SetIntValue("lambda",lambda);

Code above now appears to fail with errors such as:

root [0] .L tutorials/fit/cmaesFitBench.C++g
Info in <TUnixSystem::ACLiC>: creating shared library /home/beniz/research/siminole/dev/tmp/root/build/./tutorials/fit/cmaesFitBench_C.so
In file included from input_line_11:9:
././tutorials/fit/cmaesFitBench.C:58:16: error: no type named 'IOptions' in namespace 'ROOT::Math'
   ROOT::Math::IOptions *opts = ROOT::Math::MinimizerOptions::Default(fitter);
   ~~~~~~~~~~~~^
././tutorials/fit/cmaesFitBench.C:58:45: error: no member named 'MinimizerOptions' in namespace 'ROOT::Math'
   ROOT::Math::IOptions *opts = ROOT::Math::MinimizerOptions::Default(fitter);
                                ~~~~~~~~~~~~^
Error in <ACLiC>: Dictionary generation failed!

Help is needed to fix the above.

How to build

cd build
cmake ../ -Dminuit2=on -Dtesting=on -Dlibcmaes=on
make

The build will fail because of the header issue mentioned above, so do:

cd include
ln -s eigen3/Eigen .
ln -s eigen3/unsupported .
cd ..
make

You can then use the newly built ROOT and test that CMA-ES is working:

.L tutorials/fit/cmaesGausFit.C++g
cmaesGausFit()

Please see instructions and links from PR #40 for more tests, performance checks, etc...

Once everything is fine, I'll be able to squash all commits into a single one if needed.

beniz and others added 30 commits April 17, 2017 20:40
…ce + added control of fixed variables and auto-maxiter stopping criteria
@phsft-bot
Copy link

Can one of the admins verify this patch?

@vgvassilev
Copy link
Member

vgvassilev commented Apr 19, 2017

@beniz, could you address the coding conventions issues reported by travis?

About the error, it seems you are missing #include "Math/IOptions.h" in cmaesFitBench.C

@beniz
Copy link
Author

beniz commented Apr 26, 2017

@vgvassilev FYI I'm pretty busy over the next couple of weeks, if you can test before I fix the coding conventions, please do. It'd be best to fix major issues first. Thanks.

@phsft-bot
Copy link

Can one of the admins verify this patch?

@vgvassilev
Copy link
Member

I will try to look at it. I am quite busy, too.

@etejedor
Copy link
Collaborator

@beniz @lmoneta @vgvassilev, what would you like to do with this PR?

@beniz
Copy link
Author

beniz commented Feb 20, 2018

@etejedor Hi, I'd be happy to help get the PR into trunk, I just couldn't find time to struggle with coding conventions.

As of today, I can propose the following:

  • I do rebase to current master
  • Someone from ROOT helps me through, including to fix coding conventions and anything that can get into the way.

My time is unfortunately scarce, but I'm willing to allocate time to bring this into production.

@oshadura
Copy link
Contributor

@beniz, can you rebase please?

@beniz
Copy link
Author

beniz commented Mar 21, 2018

@oshadura Hi, I can, though it will not be immediate, stay tuned, thanks.

@lmoneta
Copy link
Member

lmoneta commented Mar 23, 2018

@oshadura , we don't need to rebase this one now. We need first to test well this PR first.
After that, then we could rebase it.

@beniz
Copy link
Author

beniz commented Mar 26, 2018

@lmoneta @oshadura hi, let me know whether I can help you test the PR thoroughly. I'd understand that rebasing could make it easier, depending of the additional tools / testbeds you'd like to use for the testing phase.
I believe libcmaes is sound, so it should really be about the integration into ROOT structures and framework.

@etejedor
Copy link
Collaborator

@lmoneta could you provide an answer to @beniz about how to test this PR?

1 similar comment
@etejedor
Copy link
Collaborator

@lmoneta could you provide an answer to @beniz about how to test this PR?

@lmoneta
Copy link
Member

lmoneta commented Jan 14, 2021

This PR is now replaced by #7044

@lmoneta lmoneta closed this Jan 14, 2021
@phsft-bot phsft-bot mentioned this pull request Sep 13, 2022
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants