adaptive-intelligent-robotics / QDax

Accelerated Quality-Diversity
https://qdax.readthedocs.io/en/latest/
MIT License
258 stars 42 forks source link

feat (algo): Add MAP-Elites Low-Spread #152

Closed btjanaka closed 1 year ago

btjanaka commented 1 year ago

Related issues: [refer to issues] Resolves #151

[Introduce the overall change made in the PR and list the associated modifications below]

Add MAP-Elites Low-Spread: https://dl.acm.org/doi/abs/10.1145/3583131.3590433

This PR introduces:

Questions

  1. Should we store all n_evals fitnesses and descriptors in the repertoire, or should we just store the aggregated fitness and descriptors? We can also store both the aggregate and the separate values. Storing separate values would require passing in n_evals to MELSRepertoire.init_default so that we can define the shapes of fitnesses and descriptors.
  2. Should we check if one individual dominates the others if inserting multiple individuals into the same cell?
  3. What link should I use for citations? Currently I am using the ACM Digital Library link. I can also use the arXiv link: https://arxiv.org/abs/2303.16207 if preferred.

Checks

Future improvements

[List here potential observations made and/or improvements that could be made in the future. If relevant, open issues for those.]

Lookatator commented 1 year ago

Hi @btjanaka ! Thank you very much for this (super well-documented) PR.

I added @Egiob and @manon-but-yes as reviewers for this PR; they are probably the best people to judge it.

I had an overview of the code and it looks great! I have mainly one high-level comment regarding the implementation of the reevaluation scoring function (and where it should be implemented). I will add it as a comment.

btjanaka commented 1 year ago

Hi @Lookatator, thank you for taking a look at this PR! I addressed your comment on the pairwise distance calculation, but I do not see the comment on scoring function evaluation. Did you make the comment in the tests or the notebook? (the scoring function is in both those places).

Egiob commented 1 year ago

Hi guys! I will do a pass by the end of the week :smile:

Lookatator commented 1 year ago

Hi @Lookatator, thank you for taking a look at this PR! I addressed your comment on the pairwise distance calculation, but I do not see the comment on scoring function evaluation. Did you make the comment in the tests or the notebook? (the scoring function is in both those places).

Sorry for the late reply @btjanaka , I did not have the time to post it yesterday, and I also was hesitating to add additional comments regarding the repertoire management (I think the calculation of the mode and of the spread should be done in the algorithm, and the repertoire addition condition should be receiving the same kinds of inputs as the MOMERepertoire). On further consideration, I will keep those comments for another PR, whose purpose will be to make the repertoire of MELS more modular.

btjanaka commented 1 year ago

Thank you @Egiob! I've resolved all your comments except for the one about moving _dispersion

btjanaka commented 1 year ago

@Lookatator Is there anything else I should do to get this PR merged?

manon-but-yes commented 1 year ago

Hi @btjanaka thank you for this PR! Apologies for the delay I just got back to work.

Lookatator commented 1 year ago

@btjanaka I merged it into a local branch to check potential style issues before merging into develop :)