[Question] What should be done with the GoalEnv and DiscreteEnv classes?

[jkterry1]

Right now, Gym has a GoalEnv class and Env class as base classes in core.py. The GoalEnv class was added as part of the robotics environments, and impose special requirements on the observation space. From what I can tell, this class has not been used outside of Gym's robotics environments and is largely unnecessary. Unless I'm missing something here, removing this class sounds like the way to go.

[jkterry1]

A related issue is what to do with the DiscreteEnv class. It's also included in the toy_text envs not in core.py, which needs to be fixed if we keep it.

[araffin]

From what I can tell, this class has not been used outside of Gym's robotics environments and is largely unnecessary.

It is quite useful when using multi-goal task with Hindsight Experience Replay (many robotics problem can be framed as such).

[jkterry1]

Perhaps I'm missing something here, but why can't the regular Gym class be used? Like, how does this solve a particular problem in this domain?

[araffin]

Like, how does this solve a particular problem in this domain?

It enforces a common API for those type of environments (which is the big strength of Gym):

gym/gym/core.py

Line 182 in ee0a568

for key in ["observation", "achieved_goal", "desired_goal"]:

and

gym/gym/core.py

Line 207 in ee0a568

assert reward == env.compute_reward(ob['achieved_goal'], ob['desired_goal'], info)

[jkterry1]

@araffin do you have any thoughts on the utility of the discrete env?

[araffin]

i don't know much about the DiscreteEnv, but it looks like a good example for tabular RL (so more for pedagogical purposes)

[jkterry1]

So I sat down and looked through the code again. I can see the value of the GoalEnv class, but I'm of the opinion that the DiscreteEnv class should be removed (and the remaining toy_text environments should be modified to use the regular Gym base class).

[brett-daley]

DiscreteEnv appears to implement a small amount of boilerplate tabular RL code, like sampling the next state according to a probability table. Arguably, this is useful in that it maybe saves a few lines of code, but it also probably could be deleted without being missed (only 3 environments use it currently: Cliff Walking, Frozen Lake, and Taxi).

[ZhiqingXiao]

@jkterry1 @carlosluis

Excuse me for my late comments, but please allow me to show my suggestion to keep the class DiscreteEnv.

We know that one of the greatest contributions of Gym is to provide unified interfaces for different environments. Without the uniform interfaces, different environments would have their own APIs and the agents designed for different environments would not be migrated to other environments easily.

The unified interfaces in Gym include:

• the class Env for discrete-time POMDP.
• the class DiscreteEnv for finite MDP.
• the class GoalEnv for goal-conditional RL.

Especially, the class DiscreteEnv is a good example of class hierarchy that correctly reflect the taxonomy of POMDP. It is an excellent implementation of finite MDP that I often demo in the class. Uses can not only use the existing finite MDPs in Gym, but also derive their own finite MDP environments upon it. Furthermore, users can use it to implement the optimal DP agent in a uniform way.

At large, I encourage abstracting the common part of different environments, rather than the opposite.

[michaelfeil]

As we have been using gym.GoalEnv, the release 0.22 breaks compatibility to older third party gym environments or whereever gym.GoalEnv is imported. (https://github.com/search?q=gym.GoalEnv&type=code)

Is the way to go to replace inheritance from Class(gym.GoalEnv) with Class(gym.Env)?

[jkterry1]

Could you elaborate on your problem a bit more? I asked someone else to read this issue to make sure I wasn't going crazy, but we were both slightly confused what compatibility is broken.

[masterdezign]

@michaelfeil You should probably see this #2516

[michaelfeil]

Sorry, my question's formulation was not very precise.

Since gym.GoalEnv is inherited from gym.Env and simply enforces a certain structure. I would like to leave a suggestion to e.g. leave a symbolic link with a decapitation warning, advising to inherit from gym.Env instead of gym.GoalEnv or integrate 'gym.GoalEnv' in their codebase.

[masterdezign]

As far as I have seen, they suggest avoiding gym.GoalEnv in favor of gym.Env.

[masterdezign]

See comments from DLR-RM/stable-baselines3#780: ee71299

My intention was/is to remove the GoalEnv class from Gym in favor of the regular base class. We just kept it in gym-robotics because we didn't want to bother removing it. I would discourage other projects from using it.