Bug fix in pareto.py, Pareto::divide_conquer_nd

[smanist]

The algorithm in Pareto::divide_conquer_nd fails when two points in the Pareto set have the same value in a certain dimension. An example is included in the modified pareto.py: The Pareto set d21 contains three points, two of which have a value of 2.0 in the first dimension. Ordering the three points results in different ways results in different values of the hypervolume (28 and 32), which are all wrong (should be 29).

The issue is in the dominance test associated with _is_test_required method and pseudo_pf. The array pseudo_pf assigns different ranks to the same values. Therefore, by reordering the Pareto set in the test case, different pseudo Pareto sets are generated for the same Pareto set.

I figured out two ways for the fix. One is to fix pseudo_pf by sorting the Pareto set such that the same values are assigned the same rank (e.g. using scipy.stats.rankdata). The other is to fix the dominance test by checking the actual Pareto set. The first one leads to more iterations in the algorithm. Therefore, I implemented the second approach in pareto.py with a minimum modification - although some simplifications of the code are possible.

[icouckuy]

Thanks. Maybe you can turn that code into a test in https://github.com/GPflow/GPflowOpt/blob/master/testing/unit/test_pareto.py

Either adapt test_hypervolume to include this particular case, or make a second test_hypervolume.

[codecov-io]

Codecov Report

Merging #107 into master will increase coverage by <.01%. The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #107      +/-   ##
==========================================
+ Coverage   99.91%   99.91%   +<.01%     
==========================================
  Files          18       18              
  Lines        1118     1121       +3     
==========================================
+ Hits         1117     1120       +3     
  Misses          1        1

Impacted Files	Coverage Δ
gpflowopt/pareto.py	100% <100%> (ø)	:arrow_up:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 8c03e23...79cffab. Read the comment docs.

[smanist]

Hi, I have removed unnecessary lines as @nknudde requested and added a unit test case as @icouckuy suggested. However, I did not find the script/interface for running/checking the unit test case. I hope it works.

[icouckuy]

@smanist can you update your master to the last one. Hopefully this will also trigger all tests so I can merge this (in the future it is perhaps better to have a separate branch for a PR instead of master). Thanks.

[smanist]

Hi @icouckuy, github shows that my fork is three commits ahead of the master. Anyway, it looks like by now the code has passed the Travis CI check and is waiting for the coverage check results. Also, how will the change request be resolved? Automatically by github or manually by @nknudde?