Adding a render hook or a render option in run()

[felixchalumeau]

Hi, some environments have render functions (like MountainCar etc...) but the main run function does not propose any native way to use this render function when possible.

It would be great to add this as a parameter or to create a new hook RenderEpisode (or something like that) which call the render(env) function at each PRE_ACT_STAGE (plus special cases).
I personaly find the hook more flexible and adapted to the package style !

Thanks !

[findmyway]

This is a good proposal. But I find that it is not that easy to provide a general hook here.

Ideally, we can define one like this:

struct RenderEpisode <: AbstractHook end

(h::RenderEpisode)(::PreActStage, agent, env, obs, action) = render!(env)

But in the PreEpisodeStage, we'd like to clear the view (in MountainCar environment, we need to call clearws()), which means we need to introduce another in RLBase:

(h::RenderEpisode)(::PreEpisodetStage, agent, env, obs, action) = clear_render!(env)

And different environments might support multiple outputs for rendering, like text/plain, image/gif. This makes it less realistic to write a general one in RLCore.

So I'd prefer to let users to decide how to render environments. Or you may propose a elegant solution here 😀

(In fact, I'm considering deprecating render! in RLBase)

[felixchalumeau]

Ok ! and you think it would be too messy to maintain if we introduce a condition (for different output) ?

What about the other option that I mentionned ? Adding the render function (if it has been specified for the environment) in the run() function and given render::Bool as a parameter of run [which would be false by default]. Hence, the clearence would not be a problem anymore, don't you think ?

In any case, I am not specialy willing this render function: it is quite convenient in some cases to see what's happening but it's really not a problem and the package already offers great possibilities !

[findmyway]

and you think it would be too messy to maintain if we introduce a condition

yes

Adding the render function (if it has been specified for the environment) in the run() function

That will also work. But I prefer to keep the logic in run as simple as possible.

it is quite convenient in some cases to see what's happening

I agree. However, I find it more easier to write a customized hook to work with a specific environment. We can keep this issue open and see if we can work out an elegant solution later.

[felixchalumeau]

yes

Ok !

That will also work. But I prefer to keep the logic in run as simple as possible.

Yes, I must admit I agree with this principle.

I agree. However, I find it more easier to write a customized hook to work with a specific environment. We can keep this issue open and see if we can work out an elegant solution later.

Ok ! perfect like this !

[findmyway]

After thinking for a while, I decide to remove the render function in ReinforcementLearningBase and extend the Base.display function to make it work as a hook in 24f8a40

Why is render removed?

I think Julia already has a rich collection of functions to describe how to define the output of a customized type. There's no need to add an extra API in RLBase.

Should I use display instead?

Though I replaced the render function with Base.display in JuliaReinforcementLearning/ReinforcementLearningEnvironments.jl@f432634 . It's not a standard of rule to always implement the Base.display function in a customized environment. Developers are free to decide how to show their environments. For example Base.show(io::IO, ::MIME"image/png", ::AbstractEnv)

The only benefit of implementing Base.display is that, display is extended as a hook here 24f8a40 to address @felixchalumeau 's issue. Again, people can always write a customized hook to determine how to show the environment.

Example

hook = ComposedHook(
    (args...) -> sleep(1),
    display
)