-
Notifications
You must be signed in to change notification settings - Fork 319
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
[Feature] Add OpenSpielWrapper
and OpenSpielEnv
#2345
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/rl/2345
Note: Links to docs will display an error until the docs builds have been completed. ❌ 3 New Failures, 7 Unrelated FailuresAs of commit 1c90d04 with merge base e82a69f (): NEW FAILURES - The following jobs have failed:
FLAKY - The following jobs failed but were likely due to flakiness present on trunk:
BROKEN TRUNK - The following jobs failed but were present on the merge base:👉 Rebase onto the `viable/strict` branch to avoid these failures
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
A few notes:
Also, some of the games in OpenSpiel do not work properly in |
2ca9acf
to
da9ad79
Compare
a55b54e
to
3aa6ed9
Compare
OpenSpielWrapper
and OpenSpielEnv
OpenSpielWrapper
and OpenSpielEnv
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work thanks for that, I left a couple of comments already.
Is the TODO blocking on this?
torchrl/envs/libs/openspiel.py
Outdated
done = torch.tensor( | ||
self._env.is_terminal(), device=self.device, dtype=torch.bool | ||
) | ||
state = self._env.serialize() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's put a flag around that to make the state optional. Maybe "stateless=True"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if we call the flag output_state
? I think that name makes it more obvious what exactly it does--it adds the state to the output of reset
and step
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added the output_state
flag, let me know if you'd rather call it stateless
or something else
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I renamed to return_state
, because that's what PettingZooWrapper
calls it
e9cd3f0
to
7ea1a05
Compare
At the moment, this only supports games where all actions are taken by the players, like in chess or tic-tac-toe. But I've realized that OpenSpiel also has a concept of chance nodes, where a random non-player action is taken. For instance, in Kuhn poker, the initial dealing of the cards is a chance node. In liar's dice, the outcome of rolling dice is a chance node. OpenSpiel has some methods to obtain all the possible chance actions and the associated probability distribution (shown in this example). For now, I'll raise an error if a loaded game contains chance nodes and leave it as a TODO. I might wait to add support for it in a follow-up PR--unless you would prefer for me to add it in this PR. |
7ea1a05
to
16ccdaa
Compare
16ccdaa
to
1c90d04
Compare
Yes I think having a dynamic space would be the way to go. See #2143 |
That's an amazing feature. Happy to integrate it separately! |
Actually, I'm not so sure that we would need dynamic action specs after all. It is true that |
I think I've addressed everything that needed to be fixed so far. Let me know if there is anything else |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazing thanks a mil for this!
Description
Adds environment wrapper classes for OpenSpiel.
OpenSpielWrapper.reset
supports resetting to a specified state.Motivation and Context
Part of #2133
Types of changes
What types of changes does your code introduce? Remove all that do not apply:
Checklist
Go over all the following points, and put an
x
in all the boxes that apply.If you are unsure about any of these, don't hesitate to ask. We are here to help!