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

Introducing LBest PSO for Unconstrained Optimization Problems #86

Merged
merged 53 commits into from Aug 1, 2019

Conversation

@SuryodayBasak
Copy link
Contributor

commented Feb 28, 2019

I have included code that implements LBest PSO. This code is from the fork of mlpack owned by chintans111 on GitHub that I made compatible in ensmallen. I've included tests from his fork of mlpack as well.

@SuryodayBasak SuryodayBasak changed the title Pso dev Introducing LBest PSO Mar 1, 2019

Merge branch 'master' of git://github.com/mlpack/ensmallen into pso-dev
Need to get commits fixing SPSA parameters.

@SuryodayBasak SuryodayBasak changed the title Introducing LBest PSO Introducing LBest PSO for Constrained Optimization Problems Mar 12, 2019

Show resolved Hide resolved include/ensmallen_bits/pso/pso.hpp Outdated
Show resolved Hide resolved include/ensmallen_bits/pso/pso.hpp Outdated
Show resolved Hide resolved include/ensmallen_bits/pso/pso.hpp
Show resolved Hide resolved include/ensmallen_bits/pso/pso.hpp Outdated
Show resolved Hide resolved include/ensmallen_bits/pso/pso.hpp Outdated
Show resolved Hide resolved include/ensmallen_bits/pso/update_policies/lbest_update.hpp Outdated
Show resolved Hide resolved include/ensmallen_bits/pso/update_policies/lbest_update.hpp Outdated
localBestIndices.imbue([&]() { return index++; });

// Set sizes r1 and r2.
r1.set_size(iterate.n_rows, iterate.n_cols);

This comment has been minimized.

Copy link
@zoq

zoq Mar 24, 2019

Member

I think we can remove the two lines here and use arma::randu(particleVelocities.n_rows, particleVelocities.n_colsin the Update step.

This comment has been minimized.

Copy link
@SuryodayBasak

SuryodayBasak Jul 2, 2019

Author Contributor

Take a look at this one more time in the code, @zoq .

r1 and r2 are random vectors, and these need to change in every iteration.

In lines 127 and 128, we have r1.randu(); and r2.randu();. Thus, in every update iteration, r1 and r2 will have different values, as is necessary in the PSO algorithm.

This comment has been minimized.

Copy link
@SuryodayBasak

SuryodayBasak Jul 2, 2019

Author Contributor

Let me know what you think.

This comment has been minimized.

Copy link
@zoq

zoq Jul 2, 2019

Member

We just change the expression in the update step from:

r1.randu();

to

arma::randu(particleVelocities.n_rows, particleVelocities.n_cols);

it would be the same, just that we don't have need to set the size before. If you prefer the current solution that's fine with me as well.

This comment has been minimized.

Copy link
@SuryodayBasak

SuryodayBasak Jul 2, 2019

Author Contributor

Okay we can do this.

This comment has been minimized.

Copy link
@SuryodayBasak

SuryodayBasak Jul 17, 2019

Author Contributor

I think that if we follow the previous solution, the code will not have to reinitialize the variables r1 and r2, but would only have to fill them with new random numbers. That's why I prefer the previous one.

Show resolved Hide resolved include/ensmallen_bits/pso/init_policies/constrained_init.hpp Outdated
Show resolved Hide resolved include/ensmallen_bits/pso/init_policies/constrained_init.hpp Outdated
@SuryodayBasak

This comment has been minimized.

Copy link
Contributor Author

commented Jun 20, 2019

@zoq - If you could look at my documentation style in ensmallen_bits/pso/update_policies/lbest_update.hpp and let me know what you think, it would be really helpful. I've done some extra bits and was just wondering if it is okay.

@SuryodayBasak

This comment has been minimized.

Copy link
Contributor Author

commented Jul 16, 2019

Okay, I've done the needful. I think this is a fairly stable implementation. In the latest push, I've implemented the lookback-horizon using std::queue. @zoq : thanks for the suggestion!

Commenting throughout the code is left to be done. I'll start working on that. Besides anything regarding the commenting, let me know if you would like anything more implemented in this version.

Show resolved Hide resolved include/ensmallen_bits/pso/init_policies/default_init.hpp Outdated
Show resolved Hide resolved include/ensmallen_bits/pso/init_policies/default_init.hpp Outdated
Show resolved Hide resolved include/ensmallen_bits/pso/init_policies/default_init.hpp Outdated
Show resolved Hide resolved include/ensmallen_bits/pso/init_policies/default_init.hpp Outdated
Show resolved Hide resolved include/ensmallen_bits/pso/init_policies/default_init.hpp
Show resolved Hide resolved include/ensmallen_bits/pso/pso_impl.hpp
Show resolved Hide resolved tests/pso_test.cpp Outdated
Show resolved Hide resolved tests/pso_test.cpp Outdated
Show resolved Hide resolved tests/pso_test.cpp Outdated
Show resolved Hide resolved tests/pso_test.cpp Outdated
upperBound.fill(50);

LBestPSO s(15000, lowerBound, upperBound, 3000, 200, 1e-20, 1.5, 3.0);
arma::mat coordinates = arma::mat("0; 10");

This comment has been minimized.

Copy link
@zoq

zoq Jul 19, 2019

Member

Perhaps it makes sense to use an initial starting point that is closer to the solution.

This comment has been minimized.

Copy link
@SuryodayBasak

SuryodayBasak Jul 19, 2019

Author Contributor

That's the one thing I did not want to do. I wanted to test PSO for its robustness with non-convex problems. As a dynamical system, I'm not sure if selecting a closer initial point in a problem this hard will make much of a difference. Regardless, let me try this out. If it works, then we'll have a new observation. Also, let me try to initialize the solutions over a larger area that encompasses the solution. That's one thing that we haven't tried.

This comment has been minimized.

Copy link
@zoq

zoq Jul 19, 2019

Member

Yeah, ideally we can stabilise the travis build, without bumping the number of iterations.

This comment has been minimized.

Copy link
@SuryodayBasak

SuryodayBasak Jul 19, 2019

Author Contributor

I've been trying for the past two hours with many different values. Can't seem to land a 100% success rate. If you're okay with it, may we exclude it for the time being?

This comment has been minimized.

Copy link
@zoq

zoq Jul 19, 2019

Member

Sure, let's comment the test for now.

@zoq

This comment has been minimized.

Copy link
Member

commented Jul 22, 2019

@SuryodayBasak

This comment has been minimized.

Copy link
Contributor Author

commented Jul 22, 2019

Yes, I will do that gladly! :)

@zoq

This comment has been minimized.

Copy link
Member

commented Jul 24, 2019

Looks like there is a swap file ("include/ensmallen_bits/pso/.pso.hpp.swp") that we should remove from the PR.

Show resolved Hide resolved doc/optimizers.md Outdated
Show resolved Hide resolved doc/optimizers.md Outdated
@zoq

This comment has been minimized.

Copy link
Member

commented Jul 25, 2019

This looks good, will run multiple test iterations, to make sure the pass rate is reasonable.

@SuryodayBasak

This comment has been minimized.

Copy link
Contributor Author

commented Jul 25, 2019

Okay. I have already tested it multiple times, but take your time to test it!!

@zoq

This comment has been minimized.

Copy link
Member

commented Jul 26, 2019

Always good to test it on multiple machines, with different seeds each time. But haven't seen a single failure in 100+ runs.

* @param explorationFactor Influence of the neighbours of the particle.
*/
PSOType(const size_t numParticles = 64,
arma::vec lowerBound = arma::ones<arma::vec>(1),

This comment has been minimized.

Copy link
@zoq

zoq Jul 26, 2019

Member

Just noted, we should probably use arma::vec& lowerBound same for the upperBound parameter, sorry missed this.

This comment has been minimized.

Copy link
@SuryodayBasak

SuryodayBasak Jul 26, 2019

Author Contributor

Yeah, that makes sense. I'm sorry for missing this, too.

This comment has been minimized.

Copy link
@SuryodayBasak

SuryodayBasak Jul 28, 2019

Author Contributor

There's one problem - if we use arma::vec& lowerBound, then we cannot use a default value for lowerBound and upperBound. Let me know what you think would be the right thing to do.

This comment has been minimized.

Copy link
@zoq

zoq Jul 28, 2019

Member

const arma::vec& lowerBound = arma::ones<arma::vec>(1) should work.

This comment has been minimized.

Copy link
@SuryodayBasak

SuryodayBasak Jul 28, 2019

Author Contributor

Thanks for the pointers. I've done this!

SuryodayBasak added some commits Jul 28, 2019

@zoq

zoq approved these changes Jul 29, 2019

Copy link
Member

left a comment

This looks good to me, there are some minor style issue, that I can fix during the merge process.

@mlpack-bot

mlpack-bot bot approved these changes Jul 30, 2019

Copy link

left a comment

Second approval provided automatically after 24 hours. 👍

@zoq zoq merged commit 948fba6 into mlpack:master Aug 1, 2019

2 checks passed

Static Code Analysis Checks Build finished.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@zoq

This comment has been minimized.

Copy link
Member

commented Aug 1, 2019

Thanks for a great contribution, excited to use this in practice.

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