2_gym_wrappers_saving_loading.ipynb: TimeFeatureWrapper ignores the case of not using TimeLimit wrapper

[rbahumi]

Hi, Sorry for not making a proper pull request but I think it is better this way in the case of ipython notebooks.

The issue, in the case of not using TimeLimit wrapper there is no enforcement of maximum length. This can lead to a negative timestamp. Suggested fix:

class TimeFeatureWrapper(gym.Wrapper):
...
    def step(self, action):
        self._current_step += 1
        obs, reward, done, info = self.env.step(action)

        # Overwrite the done signal when the environment is not wrapped with a TimeLimit object
        if not isinstance(env, TimeLimit) and self._current_step >= self._max_steps:
          done = True
          # Update the info dict to signal that the limit was exceeded
          info['time_limit_reached'] = True    

        return self._get_obs(obs), reward, done, info

In addition, and maybe I'm wrong here, but I think that the time calculation should be the other war around, and the first episode should have a timestamp of 0 and the last a timestamp of 1:

time_feature = 1 - (self._current_step / self._max_steps)  =>  time_feature = 0.0 + (self._current_step / self._max_steps)

[araffin]

TimeLimit wrapper there is no enforcement of maximum length.This can lead to a negative timestamp.

True, however, I don't think that's a problem. Also, we should not change the episode length if the user is not aware of that. The TimeFeatureWrapper is only meant to add a feature, not to change the episode length.

the first episode should have a timestamp of 0 and the last a timestamp of 1:

It does not really matter, I did that to be consistent with this paper

[rbahumi]

Also, we should not change the episode length if the user is not aware of that.

Agree. Thanks for the quick replay.