Refactor HER

[araffin]

NOTE: this branch will be merged with #243 not master

Description

Refactoring of HER (need to account for #297 ) using proper obs dict support.
We could also do the timelimit support (see #284 ) as we are breaking the replay buffer api (we need to provide upgrade script)

closes #287 (image support for HER)
closes #284 (for off-policy algorithms)

TODO:

• Check HER performance
• Investigate offline replay buffer speed drop
• Add documentation
• Perf check for HER + TimeLimit handling (PyBullet)

Motivation and Context

• I have raised an issue to propose this change (required for new features and bug fixes)

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation (update in the documentation)

Checklist:

• I've read the CONTRIBUTION guide (required)
• I have updated the changelog accordingly (required).
• My change requires a change to the documentation.
• I have updated the tests accordingly (required for a bug fix or a new feature).
• I have updated the documentation accordingly.
• I have reformatted the code using make format (required)
• I have checked the codestyle using make check-codestyle and make lint (required)
• I have ensured make pytest and make type both pass. (required)
• I have checked that the documentation builds using make doc (required)

Note: You can run most of the checks using make commit-checks.

Note: we are using a maximum length of 127 characters per line

[araffin]

@ferreirajoaouerj could you take a look at this PR? (this should address your concern from #297).
HER is now just a replay buffer class, so implementing multi-goal algorithms should be simpler, as one would need to define only a new buffer class.

[ferreirajoaouerj]

I'll take a look, thank you for addressing this feature.

[ferreirajoaouerj]

Sorry the delay. Nice that you managed to put everything into the replay buffer. The select in #297 function needs to change the observation['desired_goal'] at the beginning of the episode. I was thinking about the least intrusive way to do that. Maybe the best option is to add an env wrapper, then is easy to also implement reward and we don't need to mess with collect_rollouts for each off-policy algo. Anyway, we could explore these options in contrib since the baseline HER is already up and running.

[araffin]

Thanks for taking a look =)

he select in #297 function needs to change the observation['desired_goal'] at the beginning of the episode. I was thinking about the least intrusive way to do that. Maybe the best option is to add an env wrapper, then is easy to also implement reward and we don't need to mess with collect_rollouts for each off-policy algo.

You are right, an env wrapper would be the way to go for changing/providing the desired goal at the beginning of an episode.

For the reward, the compute_reward is only used in the HerReplayBuffer so it should be quite easy to overwrite ;)