Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: tictactoe doc #1121

Closed
wants to merge 1 commit into from
Closed

Conversation

safarnejad
Copy link

Description

No parallel API is not possible in tic-tac-toe. The docstring was wrongfully said that parallel API is available.

Type of change

  • This change requires a documentation update

No parallel API is not possible in tic-tac-toe
@elliottower
Copy link
Contributor

Hey thanks for the contribution. This is the case for all classic environments, I messaged another dev to get a second opinion because as you point out the code itself says parallelizable is false, so the docs should probably say that? Except you can do the turn based aec to parallel wrapper, and in that sense they still do work. I think the wrapper basically says “verify this yourself that the env works properly with this, we can’t guarantee all turn based aec encs will work the same with it”

I know in an unrelated project which uses pettingzoo classic envs, the turn based wrapper is used. Probably should say something on the classic homepage and on each environment explaining things.

@jjshoots
Copy link
Member

I think the is_parallelizable flag was invented before the wrapper was created. In that case, tic tac toe may have been wrongly labelled to be parallel friendly. This change makes sense with what Elliot is mentioning: you can parallel this environment, we just can't guarantee it would work.

@elliottower
Copy link
Contributor

We have been using the turn based to parallel wrapper extensively for another project and it works as intended, so I think I’m going to leave the parallel api true. If you wanted to make a note instead of this change to say that you need to use the wrapper, that would be okay. Otherwise I’ll probably do that

@jjshoots jjshoots closed this Jun 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants