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

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
@SuryodayBasak SuryodayBasak changed the title Introducing LBest PSO Introducing LBest PSO for Constrained Optimization Problems Mar 12, 2019
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
localBestIndices.imbue([&]() { return index++; });

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me know what you think.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay we can do this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@SuryodayBasak
Copy link
Contributor Author

@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
Copy link
Contributor Author

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.

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/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 Show resolved Hide resolved
upperBound.fill(50);

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, let's comment the test for now.

@zoq
Copy link
Member

zoq commented Jul 22, 2019

@SuryodayBasak
Copy link
Contributor Author

Yes, I will do that gladly! :)

@zoq
Copy link
Member

zoq 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.

doc/optimizers.md Outdated Show resolved Hide resolved
doc/optimizers.md Outdated Show resolved Hide resolved
@zoq
Copy link
Member

zoq commented Jul 25, 2019

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

@SuryodayBasak
Copy link
Contributor Author

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

@zoq
Copy link
Member

zoq 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),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

@SuryodayBasak SuryodayBasak Jul 28, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the pointers. I've done this!

Copy link
Member

@zoq zoq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link

@mlpack-bot mlpack-bot bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Second approval provided automatically after 24 hours. 👍

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

zoq 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
Development

Successfully merging this pull request may close these issues.

4 participants