Fix for CMA-ES inconsistencies. #193

gaurav-singh1998 · 2020-04-04T11:48:20Z

Issue reference CMA-ES inconsistencies #70
This pull request aims to resolve inconsistencies that currently exist in CMA-ES implementation. I have added the boundary constraints as per this code snippet which is a part of CMA-ES implementation in C by N. Hansen. I will make the appropriate changes in documentation also after a member's approval of every change I made. Thank you.

gaurav-singh1998 · 2020-04-05T17:46:15Z

Hi, @zoq kindly have a look at this whenever you get a chance. Thanks.

include/ensmallen_bits/cmaes/cmaes.hpp

include/ensmallen_bits/cmaes/cmaes_impl.hpp

Co-Authored-By: Marcus Edel <marcus.edel@fu-berlin.de>

gaurav-singh1998 · 2020-04-08T07:57:53Z

Hi, @zoq as per this discussion, I have implemented the constructor in this pull request but wanted to ask how should mark this constructor deprecated. Should I use Info here?

favre49 · 2020-04-08T08:29:47Z

You can use ens_deprecated to mark a constructor as deprecated. Make sure to add it to the comments as well. See the diff of #185 to see how the deprecated constructors were marked.

favre49 · 2020-04-08T08:32:56Z

Does it still qualify as breaking reverse compatibility though? Since you made initialSigma the last member of the constructor, I don't think it does (though the behavior if the function does change slightly).

gaurav-singh1998 · 2020-04-08T11:51:11Z

Oh! okay, thanks for the clarification @favre49

zoq

Looks good, can you update the HISTORY.md file as well.

include/ensmallen_bits/cmaes/cmaes_impl.hpp

gaurav-singh1998 · 2020-05-09T19:45:40Z

Hi, @zoq I have made the relevant changes in the code, HISTORY.md and in the documentation as well. Kindly have a look at it whenever you have a chance. Thanks.

zoq · 2020-05-12T20:31:43Z

include/ensmallen_bits/cmaes/cmaes_impl.hpp

+      // Boundary transformation shift into feasible pre-image.
+      if (y < xlow)
+      {
+        y += (ElemType)(r * (1 + (xlow - y) / r));


Shouldn't this be y += (ElemType)(r * (1 + (int)(xlow - y) / r)); also is the cast to ElemType needed, not sure I can see the reason to do that?

Hi, @zoq sorry for replying so late. I did it to ensure that every element of the matrix are of same types if not required I'll remove it. Thanks.

If you don't mind let's remove the extra cast.

include/ensmallen_bits/cmaes/cmaes_impl.hpp

zoq · 2020-05-14T20:58:48Z

You can use assert, here is an example:

ensmallen/include/ensmallen_bits/pso/init_policies/default_init.hpp

Line 91 in 45efb3e

assert(lowerBound.n_rows == iterate.n_rows && "The dimensions of "

let me know if that doesn't work.

gaurav-singh1998 · 2020-05-15T08:28:06Z

@zoq, I have made the relevant changes kindly have a look at it whenever you get a chance. Thanks.

favre49

Before we merge, I'd also like to test how robust the changed algorithm is w.r.t the tests.

include/ensmallen_bits/cmaes/cmaes.hpp

favre49 · 2020-05-15T08:37:47Z

include/ensmallen_bits/cmaes/cmaes_impl.hpp

+    initialSigma(initialSigma)
+{
+  assert(this->lowerBound != this->upperBound && "The values of "
+        "lowerbound and upperbound must be different.");


Either camelcase the variable names or say lower bound.

favre49 · 2020-05-15T08:40:14Z

tests/cmaes_test.cpp

@@ -50,7 +50,7 @@ TEST_CASE("CMAESLogisticRegressionTest", "[CMAESTest]")
        responses, testResponses, shuffledResponses);


Should we add a test that makes sure CMAES keeps the answers within the given bounds?

Hi, @favre49 sorry for being late on this. Can you please elaborate on the test for this should be written? Should I consider an armadillo matrix which is out of bounds and then apply the transformation to see if the values are within the specified range? Sorry, that I didn't get the gist of what you were saying.

I meant we should add a test that ensures the optimized coordinates are within the given bounds [-20,20]. Something like REQUIRE(coordinates[i] <=20 && coordinates[i] >= -20)

Hi, @favre49 the test has been added kindly have a look whenever you get a chance. Thanks.

Co-authored-by: favre49 <40389657+favre49@users.noreply.github.com>

gaurav-singh1998 · 2020-05-24T20:37:07Z

Hello, @zoq kindly have a look at this whenever you get a chance. I have made all the necessary changes required. Thanks.

zoq · 2020-05-29T19:58:49Z

include/ensmallen_bits/cmaes/cmaes_impl.hpp

+  const double diff = (upperBound - lowerBound) / 2.0;
+  const double al = std::min(diff, (1 + std::abs(lowerBound)) / 20.0);
+  const double au = std::min(diff, (1 + std::abs(upperBound)) / 20.0);
+  const double xlow = lowerBound - 2 * al - diff;


Do you think we should make t possible to define bounds for all coordinates similar to what has been done for the PSO method:

ensmallen/include/ensmallen_bits/pso/pso.hpp

Lines 91 to 92 in 70a2be8

const arma::mat& lowerBound = arma::ones(1, 1),

const arma::mat& upperBound = arma::ones(1, 1),

It uses a 1x1 matrix as a default, so the current implementation but you use a matrix of the same size as the coordinates as well. Let me know what you think. We can also implement the change another time and merge this as is.

Hi @zoq, I implemented this boundary handing function after referring the N Hansen's implementation and in my opinion, I think we should merge this as it is and if there are any issues in the future I will try to change this accordingly. Thanks.

Okay, fine for me.

gaurav-singh1998 · 2020-06-08T20:26:07Z

Hi, @zoq Are there any more changes required for this?

zoq · 2020-06-09T20:10:22Z

No I think this is fine, I just have to double check the windows issue, but for some reason I can't access the page right now.

zoq · 2020-06-09T23:52:53Z

Looks like the decomposition failed on the windows build:

[00:11:38] 1/1 Test #1: ensmallen_tests ..................***Failed  283.19 sec
[00:11:38] ensmallen version: 2.12.1 (Stir Crazy)
[00:11:38] armadillo version: 8.400.0 (Entropy Bandit)
[00:11:38] 
[00:11:38] warning: chol(): decomposition failed
[00:11:38] 
[00:11:38] warning: chol(): decomposition failed
[00:11:38] 
[00:11:38] warning: chol(): decomposition failed
[00:11:38] 
[00:11:38] warning: chol(): decomposition failed
[00:11:38] 
[00:11:38] ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[00:11:38] ensmallen_tests.exe is a Catch v2.4.1 host application.
[00:11:38] Run with -? for options
[00:11:38] 
[00:11:38] -------------------------------------------------------------------------------
[00:11:38] BoundaryConditionTestFunction
[00:11:38] -------------------------------------------------------------------------------
[00:11:38] C:\projects\ensmallen\tests\cmaes_test.cpp(180)
[00:11:38] ...............................................................................
[00:11:38] 
[00:11:38] C:\projects\ensmallen\tests\cmaes_test.cpp(194): FAILED:
[00:11:38]   REQUIRE( success == true )
[00:11:38] with expansion:
[00:11:38]   false == true
[00:11:38] 
[00:11:38] ===============================================================================
[00:11:38] test cases:   265 |   264 passed | 1 failed
[00:11:38] assertions: 12149 | 12148 passed | 1 failed

mlpack-bot · 2020-07-10T00:16:21Z

This issue has been automatically marked as stale because it has not had any recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions! 👍

gaurav-singh1998 · 2020-07-10T11:40:36Z

Hi, @zoq please keep this open I want to work on it whenever I get a chance. Thanks

zoq · 2020-07-13T17:08:12Z

@mlpack-jenkins test this please

zoq · 2020-07-14T21:22:47Z

tests/cmaes_test.cpp

+ */
+TEST_CASE("BoundaryConditionTestFunction", "[CMAESTest]")
+{
+  SGDTestFunction f;


Do you mind to change the object function here, to something more complex, the SGD function has only 3 parameters.

@gaurav-singh1998 let me know if you need any help with this.

Hi @zoq, sorry for being so late on this(life got messed up). I want to take this up again. Can you provide some help as to what objective functions I need to use and what other changes do I need to make? Thanks.

@gaurav-singh1998 No worries I hope everything is going well. Let's test this one on the LogisticRegression function, here is an example -

ensmallen/tests/cmaes_test.cpp

Lines 40 to 66 in 60a99f7

TEST_CASE("CMAESLogisticRegressionTest", "[CMAESTest]")

{

const size_t trials = 3;

bool success = false;

for (size_t trial = 0; trial < trials; ++trial)

{

arma::mat data, testData, shuffledData;

arma::Row<size_t> responses, testResponses, shuffledResponses;

LogisticRegressionTestData(data, testData, shuffledData,

responses, testResponses, shuffledResponses);

LogisticRegression<> lr(shuffledData, shuffledResponses, 0.5);

CMAES<> cmaes(0, -1, 1, 32, 200, 1e-3);

arma::mat coordinates = lr.GetInitialPoint();

cmaes.Optimize(lr, coordinates);

// Ensure that the error is close to zero.

const double acc = lr.ComputeAccuracy(data, responses, coordinates);

const double testAcc = lr.ComputeAccuracy(testData, testResponses,

coordinates);

if (acc >= 99.7 && testAcc >= 99.4)

{

success = true;

break;

}

}

.

…llen into cmaes-fix

gaurav-singh1998 · 2020-10-04T09:23:59Z

Hi, @zoq made the relevant changes in the test function. Kindly have a look at it now.

zoq · 2020-10-12T21:50:32Z

include/ensmallen_bits/cmaes/cmaes_impl.hpp

+  BaseMatType initialVal;
+  initialVal.randu(iterate.n_rows, iterate.n_cols);
+  initialVal += (BaseMatType)(iterateIn);
+  mPosition[0] = initialVal;


I think it's still valid to apply the lower/upper bound here.

Hi, @zoq so should I remove the boundary transformation from here and implement it like it was before like here. Thanks

zoq · 2021-02-04T01:35:59Z

Hi, @zoq made the relevant changes in the test function. Kindly have a look at it now.

Sorry for the slow response, I'll review the code over the weekend.

zoq · 2021-02-07T00:07:04Z

include/ensmallen_bits/cmaes/cmaes_impl.hpp

+// Transforms the candidate into the given bounds.
+template<typename SelectionPolicyType>
+template<typename BaseMatType>
+void CMAES<SelectionPolicyType>::BoundaryTransform(BaseMatType& matrix)


So I dived into the code and I think it makes sense to introduce another policy e.g. TransformationPolicy so we can enable/disable the boundary constraint. Let me know what you think, I can push the changes to you branch, if you like?

Hi, @zoq thanks for the review. I will try and implement the TransformationPolicy this week. Can you point to some source code to which I can refer before implementing this? Thank you.

So basically it's similar to how the update policies are implemented https://github.com/mlpack/ensmallen/tree/master/include/ensmallen_bits/sgd/update_policies.

We basically add a new template type called TransformationPolicy or something like that and define an interface. I think we just need two functions at this point Transform and Inverse. So the class policy class for the BoundaryBoxConstraint could look like:

class BoundaryBoxConstraint { public: void Transform(...) { } void Inverse(...) { } }

we also a policy class that does nothing:

class EmptyTransformation { public: void Transform(...) { } void Inverse(...) { } }

in the CMAES class we can than use an instantiation of the TransformationPolicy and call the Transform and Inverse methods. Hopefully what I wrote above makes sense, happy to clarify anything.

Initial commit.

f428f71

mlpack-bot bot added s: needs review s: unanswered s: unlabeled labels Apr 4, 2020

Changes in cmaes test.

c0a5f28

favre49 added c: optimizers t: bugfix and removed s: unanswered s: unlabeled labels Apr 4, 2020

zoq reviewed Apr 7, 2020

View reviewed changes

gaurav-singh1998 and others added 6 commits April 8, 2020 12:25

Update include/ensmallen_bits/cmaes/cmaes_impl.hpp

1e848fd

Co-Authored-By: Marcus Edel <marcus.edel@fu-berlin.de>

Update include/ensmallen_bits/cmaes/cmaes_impl.hpp

2e503f1

Co-Authored-By: Marcus Edel <marcus.edel@fu-berlin.de>

Update include/ensmallen_bits/cmaes/cmaes_impl.hpp

93146c5

Co-Authored-By: Marcus Edel <marcus.edel@fu-berlin.de>

Update include/ensmallen_bits/cmaes/cmaes_impl.hpp

1edbfe7

Co-Authored-By: Marcus Edel <marcus.edel@fu-berlin.de>

Update include/ensmallen_bits/cmaes/cmaes_impl.hpp

7de865b

Co-Authored-By: Marcus Edel <marcus.edel@fu-berlin.de>

Changes in implementation.

659d0c6

gaurav-singh1998 added 2 commits April 8, 2020 14:06

Minor style fixes.

748bbd4

Changes in inverse implementation.

9ad2852

zoq reviewed May 7, 2020

View reviewed changes

include/ensmallen_bits/cmaes/cmaes_impl.hpp Outdated Show resolved Hide resolved

gaurav-singh1998 added 4 commits May 10, 2020 00:26

Merge branch 'master' into cmaes-fix

8d7598c

Changes in Documentation and some other minor changes.

63aed56

Merge branch 'master' into cmaes-fix

77a89ca

Changes in HISTORY.md

881de8a

zoq reviewed May 12, 2020

View reviewed changes

Changing ens::Warn to assert.

c5bebd6

favre49 reviewed May 15, 2020

View reviewed changes

gaurav-singh1998 and others added 2 commits May 15, 2020 17:04

Update include/ensmallen_bits/cmaes/cmaes.hpp

89fe19c

Co-authored-by: favre49 <40389657+favre49@users.noreply.github.com>

Adding boundary transformation test.

2b86192

zoq reviewed May 29, 2020

View reviewed changes

mlpack-bot bot added the s: stale label Jul 10, 2020

mlpack-bot bot removed the s: stale label Jul 10, 2020

zoq added the s: keep open label Jul 13, 2020

Merge branch 'master' into cmaes-fix

838e47c

zoq reviewed Jul 14, 2020

View reviewed changes

gaurav-singh1998 added 3 commits October 4, 2020 14:25

Changed the test function from SGD to Logistic Regression.

486930a

Merge branch 'cmaes-fix' of https://github.com/gaurav-singh1998/ensma…

bd793ed

…llen into cmaes-fix

Merge branch 'master' into cmaes-fix

04ab903

zoq reviewed Oct 12, 2020

View reviewed changes

zoq reviewed Feb 7, 2021

View reviewed changes

conradsnicta added s: stale and removed s: keep open labels Dec 21, 2021

mlpack-bot bot closed this Dec 28, 2021

SuvarshaChennareddy mentioned this pull request Jan 2, 2023

An attempt to fix the current CMAES inconsistencies #351

Merged

		@@ -50,7 +50,7 @@ TEST_CASE("CMAESLogisticRegressionTest", "[CMAESTest]")
		responses, testResponses, shuffledResponses);

	const arma::mat& lowerBound = arma::ones(1, 1),
	const arma::mat& upperBound = arma::ones(1, 1),

	TEST_CASE("CMAESLogisticRegressionTest", "[CMAESTest]")
	{
	const size_t trials = 3;
	bool success = false;
	for (size_t trial = 0; trial < trials; ++trial)
	{
	arma::mat data, testData, shuffledData;
	arma::Row<size_t> responses, testResponses, shuffledResponses;

	LogisticRegressionTestData(data, testData, shuffledData,
	responses, testResponses, shuffledResponses);
	LogisticRegression<> lr(shuffledData, shuffledResponses, 0.5);

	CMAES<> cmaes(0, -1, 1, 32, 200, 1e-3);
	arma::mat coordinates = lr.GetInitialPoint();
	cmaes.Optimize(lr, coordinates);

	// Ensure that the error is close to zero.
	const double acc = lr.ComputeAccuracy(data, responses, coordinates);
	const double testAcc = lr.ComputeAccuracy(testData, testResponses,
	coordinates);
	if (acc >= 99.7 && testAcc >= 99.4)
	{
	success = true;
	break;
	}
	}

Fix for CMA-ES inconsistencies. #193

Fix for CMA-ES inconsistencies. #193

Conversation

gaurav-singh1998 commented Apr 4, 2020

gaurav-singh1998 commented Apr 5, 2020

gaurav-singh1998 commented Apr 8, 2020 • edited Loading

favre49 commented Apr 8, 2020

favre49 commented Apr 8, 2020

gaurav-singh1998 commented Apr 8, 2020

zoq left a comment

Choose a reason for hiding this comment

gaurav-singh1998 commented May 9, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zoq commented May 14, 2020

gaurav-singh1998 commented May 15, 2020

favre49 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gaurav-singh1998 commented May 24, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gaurav-singh1998 commented Jun 8, 2020

zoq commented Jun 9, 2020

zoq commented Jun 9, 2020

mlpack-bot bot commented Jul 10, 2020

gaurav-singh1998 commented Jul 10, 2020

zoq commented Jul 13, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gaurav-singh1998 commented Oct 4, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zoq commented Feb 4, 2021

Choose a reason for hiding this comment

gaurav-singh1998 Feb 7, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gaurav-singh1998 commented Apr 8, 2020 •

edited

Loading

gaurav-singh1998 Feb 7, 2021 •

edited

Loading