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

fix: remove descriptors squeeze() to fix 1D-BD tasks #41

Merged
merged 4 commits into from
Jul 5, 2022

Conversation

manon-but-yes
Copy link
Collaborator

This PR aims to fix the container addition for 1D-BD tasks. With the current container implementation, 1D-BD tasks such as the Hopper-uni task for example (hopper_uni) returns the following error:

Screenshot 2022-06-29 at 10 31 27

This is due to the squeeze of the descriptors done in the add() method of the repertoire. These squeeze are necessary for the fitnesses and indices as these arrays are explicitly expand_dims at the beginning of the add() method. However it is not needed for the descriptors that are not modified in the add() method.
Thus as a fix I propose to remove the squeeze for the descriptors.

This change should not impact the results, but just in case I attached the experimental comparison on GPU with the original container implementation and the new proposed container implementation on the ant_omni and walker2d_uni tasks. I ran 5 replications, the difference between runs is due to the non-deterministic nature of computation on GPU. I also added the results for the hopper_uni after the fix.

Screenshot 2022-06-29 at 11 08 35

@limbryan
Copy link
Collaborator

This issue should be present in MOME repertoire as well. To be confirmed with @felixchalumeau. The corresponding line can be found here: https://github.com/adaptive-intelligent-robotics/QDax/blob/develop/qdax/core/containers/mome_repertoire.py#L293

@felixchalumeau
Copy link
Collaborator

Indeed! I fixed it in MOME as well by precising the axis that must be squeezed. Also, I add parametrized tests for MAP-Elites and MOME to be sure that 1 and 2 dimensions BDs are tested.

@Lookatator Lookatator added this to the QDax v0.1.0 milestone Jul 5, 2022
@felixchalumeau felixchalumeau merged commit fc78101 into develop Jul 5, 2022
@felixchalumeau felixchalumeau mentioned this pull request Jul 5, 2022
@limbryan limbryan deleted the hotfix/bd-dimensions branch July 11, 2022 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants