Adding AGEMOEA and IGD - Githubissues

mlpack / ensmallen

A header-only C++ library for numerical optimization --

http://ensmallen.org

Other

740 stars 119 forks source link

Adding AGEMOEA and IGD #399

Closed IWNMWE closed 1 month ago

IWNMWE commented 3 months ago

This PR implements the IGD score and AGEMOE optimizer.

Colab Notebook link(IGD test) : https://colab.research.google.com/drive/1fHLPFbOZCmK4Dfwqsw5Ey08lqwRDp9QK?usp=sharing

IWNMWE commented 3 months ago

Hey I plan on changing a bit more of the declarations for the current functions and plan to resolve the review changes and push the changed declarations together

zoq commented 3 months ago

Sounds good to me, thanks for the update.

IWNMWE commented 3 months ago

Hey I have validated its logic with some of the existing problems in mlpack (ZDT suite) and will now proceed to do it with Maf and will put up the PR for the maf problems withing this week. If possible can you review #397 @zoq . Please feel free to also go through the code changes in this PR for any other issues.

zoq commented 2 months ago

Approved #397 and it should be merged in the next 24 hours.

zoq commented 2 months ago

Let me know if this is ready for review.

IWNMWE commented 2 months ago

@zoq Can you please go through this PR and suggest any possible changes.

rcurtin commented 2 months ago

@IWNMWE can you be sure to add documentation to doc/ for the new technique? Thanks :+1:

IWNMWE commented 2 months ago

Can you run the test case with different random seeds -rng-seed "time" and see how often it fails? The Windows build currently fails in the test run, so perhaps we have to adjust the tolerance.

Hey with latest parameter update my tests seem to be running fine on the windows build on of the other methods tests seems to be failing (maybe rng because my changes do not affect that method) will try again after the documentation changes mentioned by @rcurtin

zoq commented 2 months ago

Yeah, some of the tests case fail, due to a bad rng seed, we are trying to minimize the problem by finding some robust hyper parameters.

IWNMWE commented 2 months ago

@zoq although it was running properly in my system for different seeds I added another try to make it more robust

zoq commented 2 months ago

@zoq although it was running properly in my system for different seeds I added another try to make it more robust

Great, I have seen this on my local system as well, it passes all the tests but then on another system it fails.

IWNMWE commented 2 months ago

Looks like some other test is failing should I make a small commit and re run the pipeline ?

zoq commented 2 months ago

@mlpack-jenkins test this please

zoq commented 2 months ago

Looks like some other test is failing should I make a small commit and re run the pipeline ?

Triggered it again

rcurtin commented 1 month ago

Sounds good. The only comments that matter to me before approval are:

NSGA2 documentation change
indicator documentation (just a plan for the other indicators is fine, so long as we do address it at some point)
template type cleanup but that is not a huge deal if we don't do it

Sorry to hold things up a bit on this PR.

IWNMWE commented 1 month ago

@rcurtin I have made the requested changes. The documentation for Indicators I have added to function_types.md and also fixed the NSGA2 documentation. I have made the requested style fixes as well.As mentioned before I plan on doing the ColType changes for NSGA2 and AGEMOEA after I finish NSGA3 implementation (using ColType) due time constraint.

IWNMWE commented 1 month ago

I have made all the required changes except the ColType which I can do the end of august for NSGA2 and AGEMOEA. Other than that I think this PR is good to be merged. Please let me know if there are other changes I have to make @zoq