Update new Render API

[younik]

Update to new Render API openai/gym#2671

Todo:

• make human rendering automatic

[jjshoots]

Almost LGTM so far, other than some of the comments I've left.

I think self.render_mode and possibly the warning can be moved into env.py itself, this would remove a lot of clutter from the many environment files.

[jjshoots]

LGTM now, if @WillDudley is happy with this then I think we can merge. (Possibly after the clarification needed from Mark)

[WillDudley]

lgtm :)

I've pinged Ben on Discord asking for his sanity check on the r_env revision if he has time, but nonetheless I approve the merge. If Ben notices we've misinterpreted it we can easily hotfix it.

[jjshoots]

Cool, @younik could you fix black? Then I'll merge this PR, thanks a ton for the help!

[benblack769]

Looking at the r_env thing, it looks necessary to me:

        env = raw_env(**kwargs)
        env = wrappers.CaptureStdoutWrapper(env)
    else:
        env = raw_env(**kwargs)

    env = wrappers.TerminateIllegalWrapper(
        env, illegal_reward=HanabiScorePenalty(env)
    )

Note the code path:

        env = raw_env(**kwargs)
        env = wrappers.CaptureStdoutWrapper(env)
        HanabiScorePenalty(env)

HanabiScorePenalty looks at the hanabi_env attribute, which is part of the raw env, but not part of the wrapped env. If you add an unwrapped, i.e.

        env = raw_env(**kwargs)
        env = wrappers.CaptureStdoutWrapper(env)
        HanabiScorePenalty(env.unwrapped)

It should work

(note that the code was written before the unwrapped attribute was implemented)

[younik]

Thanks, @benblack769, it makes sense! With this PR, I also added a custom implementation of __getattr__ for wrappers. Thus, the following code runs fine:

from pettingzoo.classic.hanabi_v4 import raw_env
from pettingzoo.classic.hanabi.hanabi import HanabiScorePenalty
from pettingzoo.utils import wrappers

env = raw_env(render_mode="human")
env = wrappers.CaptureStdoutWrapper(env)
env = HanabiScorePenalty(env)
float(env)

@jjshoots I fixed pre-commit.

[younik]

@benblack769 for reference

[benblack769]

I wouldn't add it casually in the middle of a big PR like this, its worth thinking over whether this plays nicely with the conversion wrappers, with the parallel wrappers, with the OrderEnforcing wrapper which has its own __getattr__ overload, etc.

PZ has somewhat more complex wrapper system than gym.

That being said, I think people have converted me over to the concept that this is a good idea.

[WillDudley]

Personally I don't quite follow this. Why does adding an attribute error for hidden attributes negate the need for env.unwrapped?

[jjshoots]

It's not the error that negates the need for unwrapped, it's the getattr(self.env, name), that's basically unwrapped.

[WillDudley]

Ah, I see, lgtm then. Merge?

[jjshoots]

I'm waiting to see if @benblack769 has anything to say about that new getattr.

[jjshoots]

@benblack769 Could you have a look at that getattr thing and see if it's reasonable?

[jjshoots]

Thanks for the help everyone! Merging since I think everything's sound.

[Trinkle23897]

Hi there, a lot of people complain about the API changes you have made. Is there a decent solution to address those issues?

[younik]

Hi there, a lot of people complain about the API changes you have made. Is there a decent solution to address those issues?

Hello,
We changed the API of rendering in openai/gym#2671
This became a breaking change in gym 0.26, then we migrate also PettingZoo.

I explained the changes and how to update environments here: https://younis.dev/blog/render-api/
I am happy to help more with it if needed.

[WillDudley]

We also have a tutorial but it needs some minor bugfixes which I don't have capacity to do.

The tutorials can be found here: https://github.com/Farama-Foundation/PettingZoo/tree/master/tutorials/Tianshou

@Trinkle23897 It would help if you could take a look at them, otherwise I'll find someone to delegate the task to. I'll also bump up the task priority, I wasn't aware of the complaints.

[Trinkle23897]

It would be nice if someone can address this issue!

[WillDudley]

We've upped the priority on the tutorial