search

FluxML / Gym.jl

Gym environments in Julia
MIT License
54 stars 19 forks source link

name fix for _get_obs in env_wrapper.jl #24

Closed kraftpunk97-zz closed 5 years ago

kraftpunk97-zz commented 5 years ago

Currently, there are two definitions of _get_obs...

So when we create a PendulumEnv, and call state() against it, the function calls _get_obs(::PendulumEnv) instead of _get_obs(::AbstractEnv), since PendulumEnv is a subtype of AbstractEnv. This PR changes _get_obs(::AbstractEnv) to getobs(::AbstractEnv).

tejank10 commented 5 years ago

Perhaps the function call is misleading. Actually, it is supposed to call _get_obs(::PendulumEnv). This is because there are two spaces here: state space and observation space. State space is made up of parameters that define the state. Observation space is how the state is observed by the agent. For training a model, we need to use observation state since that is what is being seen by the model.

I believe agent need to see what state the system is in. If it needs to see it can use env._env.state. For all other purposes, agent only cares about what it observes, hence the function was made. Let me know your views on abstracting state from the user. If at all it is required to know about the state,we can make another method to get state.

kraftpunk97-zz commented 5 years ago

Ok, so tell me if I'm on the right track - for some of the environments, there are two types of states. One that is used by the environment to calculate the next state, and an observational state, that is calculated from the original state, and is fed to the system. All these environments have a helper function for calculating this observational state (calling it _get_obs). We want to define another function that works with an environment wrapper, and gets the observational state, if available for that environment. If no observational state is available for the environment, it should just return the original state. Is that correct?

If so, we can try calling _get_obs against every environment, and only if we get a MethodError, then we return the original state.

kraftpunk97-zz commented 5 years ago

@tejank10 Requested changes made. Please verify and see if they are to your liking.

tejank10 commented 5 years ago

Thanks @kraftpunk97 !