JuliaML / OpenAIGym.jl

OpenAI's Gym binding for Julia
Other
105 stars 20 forks source link

Missing the close function #25

Closed albheim closed 4 years ago

albheim commented 5 years ago

When running environments from the REPL the windows are left open and the only way I found of closing them are by importing PyCall and call the python function which feels a bit convoluted.

using PyCall
pycall(env.pyenv[:close], PyAny;)

There is the close keyword that is mentioned in the description for the render function but I can't seem to get that to work, when calling render(env, close=true) I get the error

ERROR: LoadError: PyError ($(Expr(:escape, :(ccall(#= /local/home/albheim/.julia/packages/PyCall/0jMpb/src/pyfncall.jl:44 =# @pysym(:PyObject_Call), PyPtr, (PyPtr, PyPtr, PyPtr), o, pyargsptr, kw))))) <class 'TypeError'>
TypeError("render() got an unexpected keyword argument 'close'")
  File "/local/home/albheim/miniconda3/lib/python3.7/site-packages/gym/core.py", line 275, in render
    return self.env.render(mode, **kwargs)

Stacktrace:
 [1] pyerr_check at /local/home/albheim/.julia/packages/PyCall/0jMpb/src/exception.jl:60 [inlined]
 [2] pyerr_check at /local/home/albheim/.julia/packages/PyCall/0jMpb/src/exception.jl:64 [inlined]
 [3] macro expansion at /local/home/albheim/.julia/packages/PyCall/0jMpb/src/exception.jl:84 [inlined]
 [4] __pycall!(::PyCall.PyObject, ::Ptr{PyCall.PyObject_struct}, ::PyCall.PyObject, ::PyCall.PyObject) at /local/home/albheim/.julia/packages/PyCall/0jMpb/src/pyfncall.jl:44
 [5] _pycall!(::PyCall.PyObject, ::PyCall.PyObject, ::Tuple{}, ::Int64, ::PyCall.PyObject) at /local/home/albheim/.julia/packages/PyCall/0jMpb/src/pyfncall.jl:29
 [6] _pycall!(::PyCall.PyObject, ::PyCall.PyObject, ::Tuple{}, ::Base.Iterators.Pairs{Symbol,Bool,Tuple{Symbol},NamedTuple{(:close,),Tuple{Bool}}}) at /local/home/albheim/.julia/packages/PyCall/0jMpb/src/pyfncall.jl:11
 [7] #pycall#88 at /local/home/albheim/.julia/packages/PyCall/0jMpb/src/pyfncall.jl:86 [inlined]
 [8] (::getfield(PyCall, Symbol("#kw##pycall")))(::NamedTuple{(:close,),Tuple{Bool}}, ::typeof(PyCall.pycall), ::PyCall.PyObject, ::Type{PyCall.PyAny}) at ./none:0
 [9] #render#3(::Base.Iterators.Pairs{Symbol,Bool,Tuple{Symbol},NamedTuple{(:close,),Tuple{Bool}}}, ::Function, ::GymEnv{PyCall.PyArray{Float64,1}}) at /local/home/albheim/.julia/dev/OpenAIGym/src/OpenAIGym.jl:90
 [10] (::getfield(OpenAIGym, Symbol("#kw##render")))(::NamedTuple{(:close,),Tuple{Bool}}, ::typeof(render), ::GymEnv{PyCall.PyArray{Float64,1}}) at ./none:0
 [11] top-level scope at none:0
in expression starting at /local/home/albheim/.julia/dev/OpenAIGym/src/test.jl:5

It would be easy to just add a close function and run pycall(env.pyenv[:close], PyAny;), I could fix that if that is a good enough solution. But if the render(env, close=true) option is the preferred one I'm not sure how to get that to work.

JobJob commented 5 years ago

Thanks for reporting. Yep, not sure about that close=true option, but I believe what we should probably be doing is adding a finalizer that calls env.pyenv[:close] in the GymEnv constructor. This can be simulated like so

function close_ifnotnull(env::GymEnv)
    # TODO check for Python runtime being PyCall._finalized[]
    !ispynull(env.pyenv) && env.pyenv[:close]()
end

env = GymEnv("LunarLander-v2") # 🚀 
finalizer(close_ifnotnull, env)

The only issue is, I've been doing this in my own code and this seems to be causing a segfault when interrupting a julia script in the middle of rendering an episode. I believe that maybe I could avoid it by checking for PyCall._finalized[], but I'm on a funky branch of PyCall atm (that doesn't have https://github.com/JuliaPy/PyCall.jl/pull/603) so I haven't been able to check it yet.

If you can verify that works and want to make a PR, it would be very welcome.

albheim commented 5 years ago

Would this help with in the REPL though? Rather new to Julia so had to look up what finalizer does but it seems it runs the function when nothing can access env anymore, which is not the case in the REPL right? (I might be very wrong here) I tried to implement what you suggested and nothing happened for me, but again, very new to Julia.

I was thinking more along the lines of just adding a function like this and give users the ability to close the window when they want. Base.close(env::AbstractGymEnv) = !ispynull(env.pyenv) && env.pyenv[:close]() Which I guess is not that much easier than env.pyenv[:close]() once you know what to do, but it feels clearer to me.

JobJob commented 5 years ago

Would this help with in the REPL though?

If you set env = nothing, and then call GC.gc() the rendering window should close. But yeah, that's not very ergonomic at the REPL (more useful in scripts), so let's ignore the close on finalize issue for now.

I was thinking more along the lines of just adding a function like this and give users the ability to close the window when they want. Base.close(env::AbstractGymEnv) = !ispynull(env.pyenv) && env.pyenv[:close]()

Yes I think that's probably the best solution for the REPL, PR welcome.

P.S. I found this issue saying that the close kwarg to render has been removed, so we should probably remove it from the docstring for render here.


Other details about GC and render - mostly notes to self:

The close_ifnotnull finalizer shouldn't be needed in general to match the behaviour of Python. Omitting some details, everything should get cleaned up by the Python garbage collector when all references to the env are gone. Unfortunately, I don't think the Python envs are calling close when they are garbage collected, and I'm seeing the windows hanging around in Python too.