search

mlpack / ensmallen

A header-only C++ library for numerical optimization --
http://ensmallen.org
Other
742 stars 120 forks source link

Implementation of Active CMAES #367

Closed SuvarshaChennareddy closed 1 year ago

SuvarshaChennareddy commented 1 year ago

This pull request is an initial implementation of Active CMAES, which is part of my Google Summer of Code (GSoC) project. Tests have not been added yet, but they will be included soon.

SuvarshaChennareddy commented 1 year ago

It's just the (Vanilla) CMAES tests that are failing. I'm fine tuning the parameters used in the tests.

zoq commented 1 year ago

@mlpack-jenkins test this please

SuvarshaChennareddy commented 1 year ago

Thank you, @zoq! There is one thing I would like to address. Wouldn't removing the deprecation go against this. I also think we should make changes (fixing style and changing element types) the to Vanilla CMAES for consistency.

zoq commented 1 year ago

If we use BoundaryBoxConstraint as the default transformation policy wouldn't that be the same as using the other constructor?

SuvarshaChennareddy commented 1 year ago

If we use BoundaryBoxConstraint as the default transformation policy wouldn't that be the same as using the other constructor?

Yes, but the default is EmptyTransformation.

zoq commented 1 year ago

Do you see any problem with using BoundaryBoxConstraint as the default for the constructor?

zoq commented 1 year ago

We could even provide another constructor for thee empty transformation.

zoq commented 1 year ago

Looking at it again, if we keep the constructor, it's backward compatible, since I can still use the "old" constructor.

SuvarshaChennareddy commented 1 year ago

Looking at it again, if we keep the constructor, it's backward compatible, since I can still use the "old" constructor.

Yes, that's why it was marked as deprecated instead of being removed altogether. We could make BoundaryBoxConstraint the default constructor; however, CMA-ES itself isn't bounded. So, I think it would make more sense to keep EmptyTransformation as the default. Since it would be the default, keeping it deprecated would also make sense, wouldn't it? These are just my thoughts.

zoq commented 1 year ago

I think both interface are fine, the old interface is closer to what we provide for the other optimizers, so I like to keep it, the new one offers the ability to support more transformations so I like to keep it as well.

SuvarshaChennareddy commented 1 year ago

Alright. We should probably modify Vanilla CMA-ES as well then (the element types should be changed as well for consistency).

zoq commented 1 year ago

Makes sense, I'll resolve the failing test and merge this PR.

zoq commented 1 year ago

Thanks for the great contribution.