[Bug] HER is not updating the done flag of HER transitions

[Bug] Hindsight experience replay (HER) is not updating the done flag of HER transitions

When sampling HER transitions, the SB3 implementation calculates a new reward but not a new done flag. The idea of HER is to sample successful goal transitions by replacing the desired goal with actually achieved goals in the episode. This means that we need to calculate a new reward for each new HER transition. This is done in https://github.com/DLR-RM/stable-baselines3/blob/7b977d7b0344f2828c5af863814fc82868a8af2f/stable_baselines3/her/her_replay_buffer.py#L354-L366. However, we also need to calculate a new done flag, since a successful goal transition should also result in a finished episode. This could be achieved by adding

  transitions["done"][her_indices, 0] = self.env.env_method(
      "compute_done",
      transitions["next_achieved_goal"][her_indices, 0],
      transitions["desired_goal"][her_indices, 0],
      transitions["info"][her_indices, 0],
  )

just after the re-computation of the reward.

Why is this critical? Most RL algorithms use this done flag to update the Q-values. E.g. SAC in https://github.com/DLR-RM/stable-baselines3/blob/7b977d7b0344f2828c5af863814fc82868a8af2f/stable_baselines3/sac/sac.py#L237

Experimental results

I temporarily fixed the issue with a monkey patch to compare the training results with and without updated done flag. My setup is a robot manipulator similar to https://github.com/qgallouedec/panda-gym/ but it has dynamic obstacles in the scene. Therefore, it can also happen that an episode ends by colliding with the obstacle. I'm using SAC + HER for training the agent.

It would be a bit much to share my entire environment, so I just upload the results with and without done flag update. If you want me to test this change on the panda gym environment, I could also try to get that running.

BEFORE bug fix

AFTER bug fix

Checklist

• I have checked that there is no similar issue in the repo (required)
• I have read the documentation (required)
• I have provided a minimal working example to reproduce the bug (required)