Skip to content
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

Optimized CNE and DE #90

Merged
merged 13 commits into from Mar 9, 2019

Conversation

@favre49
Copy link
Member

commented Mar 3, 2019

For CNE, i generated the population around the given starting point, which made it run in 1/6th of the time it took before on the Logistic regression test
For DE, i realized my implementation lacked termination for tolerance and objective change, which made it run for far far longer than it needed to. It now completes the logistic regression test in less than a second.
I also made the necessary documentation changes

@favre49

This comment has been minimized.

Copy link
Member Author

commented Mar 3, 2019

The check failing is the SPSA test, i believe @zoq PR #87 should fix that?

@@ -110,6 +114,15 @@ inline double DE::Optimize(DecomposableFunctionType& function,
population.slice(member) = iterate;
}

// Check for termination criteria.
if (lastBestFitness - fitnessValues.min() < objectiveChange)

This comment has been minimized.

Copy link
@zoq

zoq Mar 3, 2019

Member

I think we can just use threshold here, as we did for the SGD optimizer:

if (std::abs(lastObjective - overallObjective) < tolerance)
{
Info << "SGD: minimized within tolerance " << tolerance << "; "
<< "terminating optimization." << std::endl;
return overallObjective;
}

This comment has been minimized.

Copy link
@favre49

favre49 Mar 3, 2019

Author Member

Seeing as I implemented the termination criteria the same way as in CNE, should i change that as well?

This comment has been minimized.

Copy link
@zoq

zoq Mar 3, 2019

Member

Yes, thanks.

// Check for termination criteria.
if (tolerance >= lastBestFitness)
{
Info << "CNE::Optimize(): terminating. Given fitness criteria "

This comment has been minimized.

Copy link
@zoq

zoq Mar 3, 2019

Member

Do you mind to use the same wording as we used for the SGD class?

@rcurtin
Copy link
Member

left a comment

Nice, this looks like a nice improvement to me. 👍 Thanks! Do you think we should update the CNE documentation to mention that the initial point is used for the generation of the original population? Also, do you want to update HISTORY.txt?

I found a couple tiny issues; if you can handle them then from my end I think this is ready.


#### Examples:

```c++
RosenbrockFunction f;
arma::mat coordinates = f.GetInitialPoint();
CNE optimizer(200, 10000, 0.2, 0.2, 0.3, 65, 0.1e-4);
CNE optimizer(200, 10000, 0.2, 0.2, 0.3, 0.1e-4);

This comment has been minimized.

Copy link
@rcurtin

rcurtin Mar 4, 2019

Member

Why not 1e-5 here? That seems like a nice simplification we could make. :)

@@ -518,6 +517,7 @@ Differential Evolution is an evolutionary optimzation algorithm which selects be
* `DE(`_`populationSize, maxGenerations`_`)`
* `DE(`_`populationSize, maxGenerations, crossoverRate`_`)`
* `DE(`_`populationSize, maxGenerations, crossoverRate, differentialWeight`_`)`
* `DE(`_`populationSize, maxGenerations, mutationProb, mutationSize, tolerance`_`)`

This comment has been minimized.

Copy link
@rcurtin

rcurtin Mar 4, 2019

Member

Should this be populationSize, maxGenerations, crossoverRate, differentialWeight, tolerance?

// starting point.
population = arma::randn(iterate.n_rows, iterate.n_cols, populationSize);
for (size_t i = 0; i < populationSize; i++)
population.slice(i) += iterate;

This comment has been minimized.

Copy link
@rcurtin

rcurtin Mar 4, 2019

Member

I think also you could do population.each_slice() += iterate here.

@@ -120,6 +130,7 @@ inline double DE::Optimize(DecomposableFunctionType& function,
break;
}
}

This comment has been minimized.

Copy link
@rcurtin

rcurtin Mar 4, 2019

Member

No need to add an extra line here. 👍

@favre49

This comment has been minimized.

Copy link
Member Author

commented Mar 5, 2019

When i update HISTORY.md, am i meant to change the version number, or just add a new date and the PR? I'm sorry, i'm not really sure how it works

@zoq

This comment has been minimized.

Copy link
Member

commented Mar 5, 2019

You can put the update right after the Fixes for SPSA (#87). line.

@@ -526,18 +526,20 @@ Differential Evolution is an evolutionary optimzation algorithm which selects be
| `size_t` | **`populationSize`** | The number of candidates in the population. This should be at least 3 in size. | `100` |
| `size_t` | **`maxGenerations`** | The maximum number of generations allowed for DE. | `2000` |
| `double` | **`crossoverRate`** | Probability that a candidate will undergo crossover | `0.6` |
| `double` | **`differentialWeight`** | Amplification factor for differentiation | `0.02` |
| `double` | **`differentialWeight`** | Amplification factor for differentiation | `0.8` |

This comment has been minimized.

Copy link
@zoq

zoq Mar 6, 2019

Member

Looks like some parameter descriptions are missing a . at the end.


Attributes of the optimizer may also be changed via the member methods
`PopulationSize()`, `MaxGenerations()`, `CrossoverRate()`, `DifferentialWeight()`
and `Tolerance()`

This comment has been minimized.

Copy link
@zoq

zoq Mar 6, 2019

Member

Missing . at the end.

@@ -109,6 +111,11 @@ class DE
//! Modify differential weight.
double& DifferentialWeight() { return differentialWeight; }

//! Get the final objective value.

This comment has been minimized.

Copy link
@zoq

zoq Mar 6, 2019

Member

I think this will return the tolerance and not the final objective.

@rcurtin rcurtin referenced this pull request Mar 7, 2019
@@ -137,16 +133,11 @@ class CNE
//! Modify the selection percentage.
double& SelectionPercentage() { return selectPercent; }

//! Get the final objective value.
//! Get the final tolerance.

This comment has been minimized.

Copy link
@zoq

zoq Mar 7, 2019

Member

Pedantic style comment, do you mind to remove the "final" as I don't think the tolerance will change over time.

@rcurtin

rcurtin approved these changes Mar 8, 2019

Copy link
Member

left a comment

Everything here looks good to me, just two minor comments (and if you don't want to handle them we can do it doing merge, they should be really easy). Once we merge this I can release a new version of ensmallen with these (and other) fixes.

@@ -70,8 +68,10 @@ double CNE::Optimize(DecomposableFunctionType& function, arma::mat& iterate)
"children. Increase population size.");
}

// Set the population size and fill random values [0,1].
population = arma::randu(iterate.n_rows, iterate.n_cols, populationSize);
// Generate the population based on a Gaussian disribution around the given

This comment has been minimized.

Copy link
@rcurtin

rcurtin Mar 8, 2019

Member

Oops, tiny typo, this should be distribution. :)

This comment has been minimized.

Copy link
@favre49

favre49 Mar 8, 2019

Author Member

Just when i thought i had caught everything xD

`PopulationSize()`, `MaxGenerations()`, `MutationProb()`, `SelectPercent()`,
`Tolerance()`, and `ObjectiveChange()`.
`PopulationSize()`, `MaxGenerations()`, `MutationProb()`, `SelectPercent()`
and `Tolerance()`.

This comment has been minimized.

Copy link
@rcurtin

rcurtin Mar 8, 2019

Member

I think it could be useful to point out that CNE will generate the initial population using a random normal distribution centered at the given initial point. You could also point out this change in the changelog, noting that it should accelerate convergence.

This comment has been minimized.

Copy link
@favre49

favre49 Mar 8, 2019

Author Member

Done, I believe it should be ready to merge now

@@ -465,7 +465,7 @@ approxOptimizer.Optimize(f, coordinates);

*An optimizer for [arbitrary functions](#arbitrary-functions).*

Conventional Neural Evolution is an optimizer that works like biological evolution which selects best candidates based on their fitness scores and creates new generation by mutation and crossover of population. The initial population is generated based on a normal distibution centered at the given initial point.
Conventional Neural Evolution is an optimizer that works like biological evolution which selects best candidates based on their fitness scores and creates new generation by mutation and crossover of population. The initial population is generated based on a random normal distibution centered at the given initial point.

This comment has been minimized.

Copy link
@zoq

zoq Mar 8, 2019

Member

Should we use "a population" and "distribution" instead of "distibution"?

This comment has been minimized.

Copy link
@favre49

favre49 Mar 9, 2019

Author Member

Do you mean "a population" instead of "initial population"? If so, i think "initial population" is more descriptive, since subsequent populations are generated by crossover and mutation.

This comment has been minimized.

Copy link
@favre49

favre49 Mar 9, 2019

Author Member

Then again, I suppose you can look at it as new generations being created. not new populations. Seems to be an issue of semantics then, I'll go with your judgement.

@zoq

zoq approved these changes Mar 9, 2019

Copy link
Member

left a comment

Looks good to me, no further comments from my side.

@rcurtin

This comment has been minimized.

Copy link
Member

commented Mar 9, 2019

Nice to get this merged; thanks for the contribution! 👍 I'll release a new version of ensmallen now.

@rcurtin rcurtin merged commit 757f595 into mlpack:master Mar 9, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@favre49 favre49 deleted the favre49:optimizations branch Mar 10, 2019

@favre49 favre49 restored the favre49:optimizations branch Mar 10, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.