Improvise NSGA2 #263

jonpsy · 2021-03-04T10:25:36Z

Continuation of #262 .

The goal of this PR is to increase the efficiency of this code, enhance its expressibility and reducing its code size as much as possible.

Summary of my goals:

Minimize usage of raw for loops
Employing std & arma algorithms whenever possible
Potential error fixes.
Introduce OMP parallelism [https://ieeexplore.ieee.org/document/6968779] whether on this class or a separate class.

I have a few questions before I do that, let me know your thoughts on that.

Providing logs for the convenience of the reader. Stay tuned for more progress updates!

LOGS

Main Loop

Initial population should be within (lowerBound, upperBound) so that they exist inside the solution space link
Incorporate CheckArbitraryFunctionTypeAPI<> for NSGA2.
Add RequireDenseFloatingType for NSGA2

Fast Non Dominated Sorting

Discard the empty vector appended to the front. link
fronts.size() > 0 ==> !fronts.empty()

Crowding Distance Assignment

Pull the crowdingDistance 0 initialisation out of the function
Remove fronts.size() > 0 check, since empty front isn't passed at all. Refer the above section.
crowdingDistance is filled using objective values not by taking average of crowdingDistance
~~The calculation seems to be very roundabout, need to confirm with @say4n. If you're reading this, can you check this patch~~ As discussed, going with the patch.

Testing

Check for different types of matrices, for ex: fmat

jonpsy · 2021-03-04T10:51:35Z

include/ensmallen_bits/nsga2/nsga2_impl.hpp

+  //TODO: Shouldn't we check if they're inside the upperBound && lowerBound?
  for (size_t i = 0; i < populationSize; i++)
  {
    population.push_back(arma::randu<MatType>(iterate.n_rows,


Shouldn't we check if they're inside the upperBound && lowerBound? Am I missing something?

An example https://github.com/baopng/NSGA-II/blob/29a6ec33f87b32a7fb2596091e1a51c897106e7b/nsga2/problem.py#L20

I have to check the paper, not sure if the constrained was only for the mutation/crossover step or for the initial population as well. Also might be good to reference the paper instead of another python implementation.

/*if limits are specified it generates the value in range of minimum and maximum value of the variable*/ else { pop_ptr->ind[i].xreal[j] = d*(lim_r[j][1] - lim_r[j][0])+lim_r[j][0]; }

Source: realinit.h, Original Implementation in C, IITK Kanpur Genetics Laboratory.

include/ensmallen_bits/nsga2/nsga2_impl.hpp

jonpsy · 2021-03-04T14:39:02Z

Requesting reviews @rcurtin @zoq @ShikharJ since you've mentored the original PR. If I've gone ahead of myself with these changes, please do tell me :).

jonpsy · 2021-03-04T14:43:21Z

include/ensmallen_bits/nsga2/nsga2_impl.hpp

+  //Remove the empty final set
+  fronts.pop_back();


Caught a big fish here. Allow me to explain myself,

The while loop breaks when the newFront is empty. Note that the newFront is pushed regardless of any of the conditions.

At the terminate stage of our while loop, the nextFront is not updated at all! That means its an empty vector which gets pushed into our front

This causes the while loop to terminate

Result?

You got an additional empty vector in your front set. Which causes smelly code.

Nice catch!

include/ensmallen_bits/nsga2/nsga2_impl.hpp

zoq · 2021-03-04T22:18:54Z

include/ensmallen_bits/nsga2/nsga2_impl.hpp

+  //TODO: Shouldn't we check if they're inside the upperBound && lowerBound?
  for (size_t i = 0; i < populationSize; i++)
  {
    population.push_back(arma::randu<MatType>(iterate.n_rows,


I have to check the paper, not sure if the constrained was only for the mutation/crossover step or for the initial population as well. Also might be good to reference the paper instead of another python implementation.

zoq · 2021-03-04T22:22:41Z

Summary of my goals:

Minimize usage of raw for loops

Employing std & arma algorithms whenever possible

Potential error fixes.

Introduce OMP parallelism [https://ieeexplore.ieee.org/document/6968779] whether on this class or a separate class.

Sounds good to me, if we can show that an OpenMP solution is faster let's integrate it into the current method, but maybe armadillo build against openblas is already faster, haven't read the paper yet.

include/ensmallen_bits/nsga2/nsga2_impl.hpp

zoq · 2021-03-09T00:56:41Z

include/ensmallen_bits/nsga2/nsga2_impl.hpp

@@ -150,7 +161,7 @@ typename MatType::elem_type NSGA2::Optimize(

    // Perform crowding distance assignment.
    crowdingDistance.resize(population.size());
-
+    std::fill(crowdingDistance.begin(), crowdingDistance.end(), 0.f);


I guess before the reset to zero, we got lucky, because in the CrowdingDistanceAssignment method we use front[i + 1] which should be undefined:

crowdingDistance[front[i]] += (crowdingDistance[front[i - 1]] - crowdingDistance[front[i + 1]])

Ahh, nevermind we set it to zero before as well.

@zoq Regarding this, I think this implementation is wrong. Here's an excerpt from the paper

Notice the equation should've been

crowdingDistance[front[i]] += (fValue[i+1] - fValue[i-1])/(maxFval - minFval)

and not the average of crowdingDistances

I've implemented it in this patch

Good, can you make the patch part of the PR?

I've no problem with that. Thing is, @say4n has implemented assignment first and sorting later which I think is wrong but I guess he must have some reason. I was waiting for his review, very well, I'll add this patch on my PR.

I think I had unrolled the first iteration of some parts of the optimisation loop to be outside the for loop. This had the benefit of instantiating some items as far as I recall. I don't remember all the details now so do take this with a grain of salt!

Also, sorry for the delays, I have a bunch of impending exams and a ton of work. 😢

Thanks for clarifying :). I'd say let's merge my patch for now as it is straightforward & adheres to the research paper directly. We can then try your optimization method and see how it all fits in.

Yes, for sure! Great work btw!!

zoq · 2021-03-09T00:59:06Z

include/ensmallen_bits/nsga2/nsga2_impl.hpp

@@ -150,7 +161,7 @@ typename MatType::elem_type NSGA2::Optimize(

    // Perform crowding distance assignment.
    crowdingDistance.resize(population.size());
-
+    std::fill(crowdingDistance.begin(), crowdingDistance.end(), 0.f);


Ahh, nevermind we set it to zero before as well.

include/ensmallen_bits/nsga2/nsga2_impl.hpp

jonpsy · 2021-03-11T15:33:21Z

All the tests pass, Wohoo!. All the tasks are done as well. @zoq Do you have any comments?

zoq · 2021-03-12T18:53:58Z

All the tests pass, Wohoo!. All the tasks are done as well. @zoq Do you have any comments?

I'll do another pass over the PR, in the meantime can you update the HISTORY.md so it passes the Travis CI check.

include/ensmallen_bits/nsga2/nsga2.hpp

include/ensmallen_bits/nsga2/nsga2_impl.hpp

jonpsy · 2021-03-15T18:29:58Z

AppVeyor is failing, but the tests work, can you throw some light on this please?

zoq · 2021-03-18T02:27:56Z

Strange, I have to debug this on a local machine, but looks like the windows build never passed on this PR.

jonpsy · 2021-03-18T16:49:23Z

On second thought, I think its better to keep calculatedObjectives because fitness doesn't always equate to calculated objective value. For ex: SPEA-II has a clearly distinguished fitness and objective value.

jonpsy · 2021-04-01T04:30:38Z

Hm, not sure I can see the fix in the last commits, was this just a strange windows fluke?

Your question is well-founded, it looks like Appveyor cmd does not repeat for 3000 times, even after putting ---repeat-until-fail command. So I went ahead and run it manually for like ~50 times for MSVC15. Here's the pastebin (https://pastebin.com/k641DjC7). For 100 times: https://pastebin.com/t4yMGdec.

Let me know if you require further proof.

zoq · 2021-04-01T23:38:53Z

Hm, not sure I can see the fix in the last commits, was this just a strange windows fluke?

Your question is well-founded, it looks like Appveyor cmd does not repeat for 3000 times, even after putting ---repeat-until-fail command. So I went ahead and run it manually for like ~50 times for MSVC15. Here's the pastebin (https://pastebin.com/k641DjC7). For 100 times: https://pastebin.com/t4yMGdec.

Let me know if you require further proof.

Thanks, I just was just wondering what it was, thanks for tracking this down.

zoq · 2021-04-01T23:42:54Z

I restarted the windows build. because the build failed, not sure it's because of the RDP session.

jonpsy · 2021-04-02T00:54:51Z

The build fails because of RDP. Its waiting for us to access it, and terminates when we dont. @zoq , do we agree the algorithm is working? If so, I'll revert RDP changes and the build would pass.

zoq · 2021-04-02T01:02:33Z

The build fails because of RDP. Its waiting for us to access it, and terminates when we dont. @zoq , do we agree the algorithm is working? If so, I'll revert RDP changes and the build would pass.

Ahh, right I have seen you removed it from one section, but looks like not from the other one. This looks good, so let's remove it.

jonpsy · 2021-04-03T12:17:54Z

Hi @zoq , is this GTM?

zoq

This looks really good to me, just one more really minor style issue.

include/ensmallen_bits/nsga2/nsga2_impl.hpp

jonpsy · 2021-04-08T17:05:09Z

Once #283 is merged, I will incorporate that feature along with the rebase changes + static analysis fix changes.

…to nsga2-better

zoq · 2021-04-10T17:35:32Z

Once #283 is merged, I will incorporate that feature along with the rebase changes + static analysis fix changes.

Nice, just merged #283.

white space

jonpsy · 2021-04-10T18:36:11Z

All right, waiting for the green tick :) .

jonpsy · 2021-04-10T22:55:02Z

All green! 🥳

zoq

Thanks for another great contribution 👍

mlpack-bot

Second approval provided automatically after 24 hours. 👍

zoq · 2021-04-14T00:05:16Z

Thanks for digging into the optimization method, really nice improvements.

mlpack-bot bot added s: needs review s: unanswered s: unlabeled labels Mar 4, 2021

jonpsy commented Mar 4, 2021

View reviewed changes

include/ensmallen_bits/nsga2/nsga2_impl.hpp Outdated Show resolved Hide resolved

jonpsy commented Mar 4, 2021

View reviewed changes

include/ensmallen_bits/nsga2/nsga2_impl.hpp Show resolved Hide resolved

jonpsy commented Mar 4, 2021

View reviewed changes

include/ensmallen_bits/nsga2/nsga2_impl.hpp Outdated Show resolved Hide resolved

zoq mentioned this pull request Mar 4, 2021

Nits on NSGA2 #262

Merged

zoq added c: optimizers and removed s: unanswered s: unlabeled labels Mar 4, 2021

zoq reviewed Mar 4, 2021

View reviewed changes

conradsnicta reviewed Mar 7, 2021

View reviewed changes

include/ensmallen_bits/nsga2/nsga2_impl.hpp Outdated Show resolved Hide resolved

conradsnicta reviewed Mar 7, 2021

View reviewed changes

include/ensmallen_bits/nsga2/nsga2_impl.hpp Outdated Show resolved Hide resolved

zoq reviewed Mar 9, 2021

View reviewed changes

jonpsy changed the title ~~[WIP] Improvise NSGA2~~ Improvise NSGA2 Mar 10, 2021

jonpsy force-pushed the nsga2-better branch from 94ef5e5 to 79231b4 Compare March 12, 2021 19:08

zoq reviewed Mar 13, 2021

View reviewed changes

jonpsy force-pushed the nsga2-better branch from fd5ecd0 to 3a1f6a8 Compare March 14, 2021 07:28

jonpsy mentioned this pull request Mar 18, 2021

SchafferN1 Initial Point fix #274

Merged

jonpsy force-pushed the nsga2-better branch from b1a02d6 to 6fb0d79 Compare March 19, 2021 04:43

jonpsy added 2 commits April 2, 2021 07:06

Revert changes from RDP

7eafb8a

appveyor from master

02d8d94

zoq reviewed Apr 7, 2021

View reviewed changes

include/ensmallen_bits/nsga2/nsga2_impl.hpp Outdated Show resolved Hide resolved

indent fix

c6e76e2

zoq mentioned this pull request Apr 7, 2021

Code analyzer adjustments #284

Merged

sanity fix

850612f

jonpsy and others added 2 commits April 9, 2021 00:19

Merge branch 'nsga2-better' of https://github.com/jonpsy/ensmallen in…

6714a17

…to nsga2-better

Merge branch 'nsga2-better' of https://github.com/jonpsy/ensmallen in…

6be8403

…to nsga2-better

jonpsy and others added 3 commits April 10, 2021 23:56

Merge branch 'master' into nsga2-better

a4f320a

Update HISTORY.md

9463b15

white space

check arbitrary

33974ce

zoq approved these changes Apr 10, 2021

View reviewed changes

mlpack-bot bot approved these changes Apr 11, 2021

View reviewed changes

mlpack-bot bot removed the s: needs review label Apr 11, 2021

zoq merged commit 1e70823 into mlpack:master Apr 14, 2021

jonpsy deleted the nsga2-better branch April 14, 2021 04:36

This was referenced Jul 5, 2021

Release version 2.17.0: "25 Din Mein Paisa Double" #308

Closed

Release version 2.17.0: "Pachis Din Me Pesa Double" #310

Merged

Improvise NSGA2 #263

Improvise NSGA2 #263

Conversation

jonpsy commented Mar 4, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonpsy Mar 5, 2021 • edited Loading

Choose a reason for hiding this comment

jonpsy commented Mar 4, 2021

jonpsy Mar 4, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zoq commented Mar 4, 2021

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

say4n Mar 11, 2021 • edited Loading

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

jonpsy commented Mar 11, 2021

zoq commented Mar 12, 2021

jonpsy commented Mar 15, 2021

zoq commented Mar 18, 2021

jonpsy commented Mar 18, 2021

jonpsy commented Apr 1, 2021 • edited Loading

zoq commented Apr 1, 2021

zoq commented Apr 1, 2021

jonpsy commented Apr 2, 2021

zoq commented Apr 2, 2021

jonpsy commented Apr 3, 2021

zoq left a comment

Choose a reason for hiding this comment

jonpsy commented Apr 8, 2021

zoq commented Apr 10, 2021

jonpsy commented Apr 10, 2021

jonpsy commented Apr 10, 2021

zoq left a comment

Choose a reason for hiding this comment

mlpack-bot bot left a comment

Choose a reason for hiding this comment

zoq commented Apr 14, 2021

jonpsy commented Mar 4, 2021 •

edited

Loading

jonpsy Mar 5, 2021 •

edited

Loading

jonpsy Mar 4, 2021 •

edited

Loading

say4n Mar 11, 2021 •

edited

Loading

jonpsy commented Apr 1, 2021 •

edited

Loading