FullyObsWrapper mission attribute not updated

[ghost]

Hello there! I've found a small bug with the FullyObsWrapper I think. Cheers!

import gym_minigrid
from gym_minigrid.wrappers import FullyObsWrapper
import gym

env = gym.make('MiniGrid-Fetch-5x5-N2-v0')
env.reset()
wrapped_env = FullyObsWrapper(env)
print(env.mission, wrapped_env.mission)
wrapped_env.reset()
print("Unupdated mission variable",wrapped_env.mission)
print("Actual mission variable",wrapped_env.unwrapped.mission)

[maximecb]

I think the problem is likely here:

self.__dict__.update(vars(env))  # Pass values to super wrapper

This trick just gets values from the env class and sets them on the wrapper, if I'm not mistaken. In order for the .mission field to be updated, we'd need to override the getitem method on the wrappers. Seems doable a-priori. Opinions? How did you run into this problem?

[ghost]

I ran into that problem while trying to integrate the Fetch environment within my homemade rl framework to do some multitask learning. This bug hasn't really caused me any problem; I'm simply retrieving the mission from the unwrapped environment instead.

Your solution sounds fine : Instead of updating the object's dictionary on initialization, the getitem method could go fetch the unwrapped env's attribute if the wrapper does not have an attribute with the same name (such as self.observation_space).

[maximecb]

@DjAntaki would you like to submit a PR with this improvement? :)

Also, curious. You're building your own RL framework, will that include your own implementation of PPO/A3C/DDPG/etc? Will you be using PyTorch?

[ghost]

I wrote the override of the __getattr__ function (which is called when __getattribute__ raises an AttributeError) so that it goes retrieve the attribute of self.unwrapped. I then realised that the Wrapper class of gym already has that feature implemented (file core.py line 219). Commenting self.__dict__.update call you mentioned is sufficient to fix the problem. This raises the question as to what purposes that line served. I can submit a pull request with the calls commented.

My framework uses PyTorch; I have my own implementation of Option-Critic and Mixture-Of-Experts networks, the rest algorithm-wise mostly come from other repositories. I built it out of nostalgia for the TrainingLoop class of the Blocks framework.

Si tu veux on peut causer plus amplement sur le slack du mila, je risque d'y être plus rapide pour répondre. @antakiv

[maximecb]

I wrote the override of the getattr function (which is called when getattribute raises an AttributeError) so that it goes retrieve the attribute of self.unwrapped. I then realised that the Wrapper class of gym already has that feature implemented (file core.py line 219). Commenting self.dict.update call you mentioned is sufficient to fix the problem. This raises the question as to what purposes that line served. I can submit a pull request with the calls commented.

Ha! I didn't know that. I'm not the one who originally added that line. Possibly, OpenAI Gym wrappers didn't always have this functionality... Or whoever did this has no idea what they were doing. Let's remove them all, and if the automated tests pass, good enough.

My framework uses PyTorch; I have my own implementation of Option-Critic and Mixture-Of-Experts networks, the rest algorithm-wise mostly come from other repositories. I built it out of nostalgia for the TrainingLoop class of the Blocks framework.

Very cool. I generally find most frameworks out there overcomplicated. They require too much boiletplate and have too many dependencies, are too easily broken. That would be my two cents. Minimize dependencies, keep the API simple and avoid implementing features people won't need.

Did not realize you were at Mila!