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

Adding ZDT Test Suite #273

Merged
merged 30 commits into from Jun 14, 2021
Merged

Adding ZDT Test Suite #273

merged 30 commits into from Jun 14, 2021

Conversation

jonpsy
Copy link
Member

@jonpsy jonpsy commented Mar 16, 2021

Pizza!

Declared files

  • zdt1_function.hpp
  • zdt2_function.hpp
  • zdt3_function.hpp
  • zdt4_function.hpp
  • zdt5_function.hpp [skipping for now]
  • zdt6_function.hpp

TODO

  • Write docs for each declared files.
  • Write the interface for each declared files.
  • Test working against pagmo
  • Test numerical stability and corner cases & add to test case.
  • Add in nsga2_test.cpp
  • Add Pareto Front for each problem suite.

Pareto Fronts plotting: https://colab.research.google.com/drive/1Hgmvj4ZQVdT6OgopLPabldeyaOm0DInI?usp=sharing

@jonpsy
Copy link
Member Author

jonpsy commented Mar 16, 2021

All this in just one day! Phew!

@rcurtin, @zoq If you find the time, please review it :) . Meanwhile, I'll test try and make them more robust to corner cases and try and optimize it. Cheers!

@jonpsy
Copy link
Member Author

jonpsy commented Mar 19, 2021

Few updates:
a) I've tested all the ZDT(1-6 excluding 5) against pagmo, results match. I can create a dummy repository if one wants to verify this.
b) Interestingly, NSGA-II (current implementation) fails on this, but #263 passes, mostly owing to the population being out-of-bounds.
c) I think this is good to merge, critique's welcome.

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.

Looks pretty good to me, I made some comments about the style, I'll check the function implementation next, the optimizer is already able to solve the problems, so that is a good sign.

include/ensmallen_bits/problems/zdt1_function.hpp Outdated Show resolved Hide resolved
include/ensmallen_bits/problems/zdt1_function.hpp Outdated Show resolved Hide resolved
include/ensmallen_bits/problems/zdt1_function.hpp Outdated Show resolved Hide resolved

return arma::Col<ElemType>(numVariables, 1, arma::fill::zeros);
}

Copy link
Member

Choose a reason for hiding this comment

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

Might be a good idea to add a comment to ObjectiveF1, ObjectiveF2.

include/ensmallen_bits/problems/zdt2_function.hpp Outdated Show resolved Hide resolved
include/ensmallen_bits/problems/zdt3_function.hpp Outdated Show resolved Hide resolved
@jonpsy
Copy link
Member Author

jonpsy commented Mar 24, 2021

Looks pretty good to me, I made some comments about the style, I'll check the function implementation next, the optimizer is already able to solve the problems, so that is a good sign.

Just to make your job easier, I've created a repository comparing pagmo results to mine. Ofcourse, these are trivial cases, if you have any edge cases in mind do let me know so I can test and compare them. Cheers!

@zoq
Copy link
Member

zoq commented Mar 25, 2021

Looks pretty good to me, I made some comments about the style, I'll check the function implementation next, the optimizer is already able to solve the problems, so that is a good sign.

Just to make your job easier, I've created a repository comparing pagmo results to mine. Ofcourse, these are trivial cases, if you have any edge cases in mind do let me know so I can test and compare them. Cheers!

Usually I take a look at the function definition (paper, wikipedia) and compare it with the implementation, since there is a chance that an existing implementation does something different; but it's a nice basis.

@jonpsy
Copy link
Member Author

jonpsy commented Mar 25, 2021

I see. Here's the link to the research paper: https://www.mitpressjournals.org/doi/abs/10.1162/106365600568202

@jonpsy
Copy link
Member Author

jonpsy commented Apr 11, 2021

Now that NSGA-II fix is close to merge, we can start working on this. ZDT5 is also remaining, but since it only works on bitstring type I would need to use std::enable_if < is_bitset<....>>, something along these lines.

jonpsy and others added 11 commits April 14, 2021 11:54
=> Documentation done.
=> Declared the class.
1) ZDT1
2) ZDT2
3) ZDT3
4) ZDT4
5) ZDT6
=> Indent fix
Co-authored-by: Marcus Edel <marcus.edel@fu-berlin.de>
Co-authored-by: Marcus Edel <marcus.edel@fu-berlin.de>
@jonpsy
Copy link
Member Author

jonpsy commented Apr 18, 2021

Keep open

@jonpsy
Copy link
Member Author

jonpsy commented May 1, 2021

Updates on this: pymoo has implemented the Pareto front for each of these so I have included the Ipynb notebook in this pull request. I was thinking we could test if the Pareto Front from the Pymoo implementation is the same as ours, see if the outputs match? Your thoughts @zoq ?

@zoq
Copy link
Member

zoq commented May 25, 2021

Looking at the Travis output, this is an easy issue to fix.

@jonpsy
Copy link
Member Author

jonpsy commented May 26, 2021

Indeed. Currently, two things bother me:

a) How do we evaluate the performance on ZDT?

In this regard, the paper has defined the g objective and its optimal value on any given coordinates. We can use coords and evaluate it with g such that it is approximately equal to g*.

For instance, look at this code, this test passed when asserted 3000 times with random seed. We will have to create an API to expose the g function instead of manually writing, for eg.

 ...
 opt.Optimize(objectives, coords);
 double gValue = ZDT_ONE.GetGValue();
 double expectedGValue = ZDT_ONE.GetOptimalGValue();

 REQUIRE(gValue == Approx(expectedGValue).margin(0.99));

b) ZDT5

This has been bothering me for a while now, the paper, pymoo and pagmo all have very different interpretations on the bitstring thingy and its confusing me a lot. Still working on it.

@zoq
Copy link
Member

zoq commented May 26, 2021

a) How do we evaluate the performance on ZDT?

In this regard, the paper has defined the g objective and its optimal value on any given coordinates. We can use coords and evaluate it with g such that it is approximately equal to g*.

For instance, look at this code, this test passed when asserted 3000 times with random seed. We will have to create an API to expose the g function instead of manually writing, for eg.

I'm not sure it's feasible to test e.g. NSGAII on each problem as part of the CI and at the same time keep the runtime low. At the end its just to make sure our implementation is correct and not providing a comprehensive benchmark. If people want to test out other problems, they can do that and for that they can pretty much look at an existing test and adapt it accordingly. So I think we should select one or two and test it against NSGAII. Since we do it just for a limited number of functions, I don't think we have to create another API to expose g. I like to keep it simple so having it explicitly written down in a single method instead of using some problem specific API would make it a lot easier to use the test code as an example.

@jonpsy
Copy link
Member Author

jonpsy commented May 26, 2021

Very well, I'll comment that the g objective function is taken from the ZDT1 implementation and to refer the docs for optimal g value. Reg. the CI-timeout, if you recall in NSGA-II improvisation we were able to bring down the numGenerations from 1500 =>300 to achieve similar results, which should've loaned us a huge amount of CI-time. However, I agree with few examples should be enough, so I'll only test against ZDT_ONE as per the code I pasted before, do we agree?

@zoq
Copy link
Member

zoq commented May 26, 2021

Very well, I'll comment that the g objective function is taken from the ZDT1 implementation and to refer the docs for optimal g value. Reg. the CI-timeout, if you recall in NSGA-II improvisation we were able to bring down the numGenerations from 1500 =>300 to achieve similar results, which should've loaned us a huge amount of CI-time. However, I agree with few examples should be enough, so I'll only test against ZDT_ONE as per the code I pasted before, do we agree?

Sounds like a good plan to me.

@jonpsy
Copy link
Member Author

jonpsy commented May 27, 2021

Great, only ZDT5 is left. It's a lot confusing if you could weigh in that'd be awesome.

@zoq
Copy link
Member

zoq commented May 28, 2021

Great, only ZDT5 is left. It's a lot confusing if you could weigh in that'd be awesome.

I looked at the pymoo implementation (https://github.com/msu-coinlab/pymoo/blob/master/pymoo/problems/multi/zdt.py#L99) which looks like a reasonable approach to me, also I got the impression that a lot of frameworks just don't implement the function. Anyway, I would just go with the pymoo interpretation, let me know if you like me to look into something specific.

@jonpsy
Copy link
Member Author

jonpsy commented May 28, 2021

I didn't quite understand what they're doing. To begin with, x_1 should be a bitstring of length 30 and x2 .... x11 should be a bitstring of length 5. I'm not sure how they're ensuring that the coordinates are a bitstring or are they converting it in some way? Also the paper was clear that there are 11 dimensions, but this implementation somehow calculates numDimensions in a very obscure way.

To make things worse, here's pagmos implementation, especially two parts

a)

    // Convert the input vector into rounded values (integers)
    vector_double x;
    std::transform(x_double.begin(), x_double.end(), std::back_inserter(x),
                   [](double item) { return std::round(item); });

So basically, if coordinates are 0.03 it becomes 0. But how could they guarantee coordinates would be within 0-1?

b)

 // Counts how many 1s are there in the first (30 dim)
  u[0] = static_cast<vector_double::size_type>(std::count(x.begin(), x.begin() + 30, 1.));

Not getting what the "first 30" dim is supposed to mean.

And here's the excerpt from the original paper.

ZDT-T5

The paper is guaranteeing that the num dimension is 11 but neither of the two implementation seem to follow it. So weird.

@zoq
Copy link
Member

zoq commented May 30, 2021

Thanks for the extra insights, will dig into the two implementations and the paper, and will let you know what I think.

@jonpsy
Copy link
Member Author

jonpsy commented Jun 1, 2021

As discussed, let's skip ZDT5 as of now. Let me know if you find any other changes peculiar 👍 @zoq

@zoq
Copy link
Member

zoq commented Jun 1, 2021

As discussed, let's skip ZDT5 as of now. Let me know if you find any other changes peculiar @zoq

Yes, let us fix the build before we merge this 👍

@jonpsy
Copy link
Member Author

jonpsy commented Jun 6, 2021

Allright, this should work smooth like a butter.
@zoq Yoooooo, its working!

@jonpsy
Copy link
Member Author

jonpsy commented Jun 7, 2021

I think this is ready.

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.

Looks great to me, no more comments from my side.

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 db90638 into mlpack:master Jun 14, 2021
@zoq
Copy link
Member

zoq commented Jun 14, 2021

Thanks again, great work 👍

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.

None yet

2 participants