open_spiel icon indicating copy to clipboard operation
open_spiel copied to clipboard

Comparison between two game states

Open nikhilweee opened this issue 1 year ago • 2 comments

As the question says, my code tries to compare two different game states. When is state1 == state2?

From my limited knowledge of pybind11, I can see that equality for games is determined from this code snippet: https://github.com/deepmind/open_spiel/blob/b5e0bf6495bd8baf7c6011c18fa4fd403e21385d/open_spiel/python/pybind11/pyspiel.cc#L378-L381

However, I cannot find anything similar for states. Any help will be appreciated!

nikhilweee avatar Aug 10 '22 13:08 nikhilweee

Hi @nikhilweee, it is just plainly missing :)

In fact I just discovered that we defined __eq__ is defined for the other ones. The good thing is we have the actual == operator overridden for states, see here: https://github.com/deepmind/open_spiel/blob/c6fafb92021a8a3aa5f9746cdb79e74917ed26a5/open_spiel/spiel.h#L318

So, you we can add an even easier one for states that just does this: return value2 && value == (*value2). Can you submit a PR that adds it?

lanctot avatar Aug 10 '22 20:08 lanctot

Heads-up: we are looking at adding this. Turned out to be slightly more subtle than we thought so we're discussing the addition.

lanctot avatar Aug 15 '22 12:08 lanctot

Hello @lanctot, as a follow up I also notice some inconsistent behaviour.

$ python
Python 3.10.8 | packaged by conda-forge | (main, Nov 22 2022, 08:26:04) [GCC 10.4.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import pyspiel
>>> game = pyspiel.load_game('leduc_poker', {'players': 2})
>>> state = game.new_initial_state()
>>> state_rec = game.deserialize_state(state.serialize())
>>> state == state_rec
False
>>> state.to_string() == state_rec.to_string()
True

From the code it looks like both comparisons should result in True as they seem identical.

https://github.com/deepmind/open_spiel/blob/c6fafb92021a8a3aa5f9746cdb79e74917ed26a5/open_spiel/spiel.h#L312-L320

If it helps, I am using the latest openspiel built directly from the master branch following this post.

nikhilweee avatar Nov 29 '22 15:11 nikhilweee

Thanks @nikhilweee . This is definitely known and intended, currently, because the state objects do not override __eq__. We would need to expose that to pybind11.

When I last looked at adding it, it turned out to create a lot of non-trivial-to-fix errors, and after a discussion with the team about fixing this, it seemed they convinced me that forcing the user to think about the context for state equality is a good thing. For example, why not make change equality to be over the history? The context will matter for the definition of "equals".

So right now the == operator acts like the normal python one (checking if two references point to the same object, which they don't in your example). So depending what exactly you mean by "equal", you could use the to_string comparison right now (if that works for your use case), or state.history() == rec_state.history() for another.. but generally we think letting the user decide is a good thing.

I also find it a bit over-reaching to override the semantics of a comparison operator between objects.. I don't know, that seems too powerful and potentially destructive. Maybe we should even remove State::operator== for all these reasons too.

Can you say more about your use case and how it affects you? We might still resolve this in the future, and how it goes depends on what it means for users.

lanctot avatar Nov 29 '22 17:11 lanctot

Thanks @lanctot, I do seem to have a constricted viewpoint here and after reading through your thoughts I feel like it would be better to let the user decide. I opened this issue because to me it seemed the most natural way to compare two states - especially because __eq__ is overridden for Game and it's possible to do the following:

$ python
Python 3.10.8 | packaged by conda-forge | (main, Nov 22 2022, 08:26:04) [GCC 10.4.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import pyspiel 
>>> game1 = pyspiel.load_game('leduc_poker')
>>> game2 = pyspiel.load_game('leduc_poker')
>>> game3 = pyspiel.load_game('leduc_poker', {'players': 2})
>>> game1 == game2
True
>>> game1 == game3
False

But you're right that there might be differing opinions on decisions like whether to include history while comparing states. I feel like for my use case, which is primarily leduc_poker, it doesn't really matter and I'm happy comparing state.to_string() for now. I'll go ahead and close the issue. Thank you for your feedback!

nikhilweee avatar Nov 29 '22 17:11 nikhilweee

Cool. But I agree: it does seem inconsistent to support it for GameType and Game but not for State especially since State::operator== is defined!

So I will re-open this because I feel like this is still not fully resolved (it's ok to keep open until then). I will try to re-instate the __eq__ for states, keeping it as a simple call to State::operator== and report back once I remember what issues I ran into. And maybe I'll remove the State::operator== if we want to fully decide on forcing the user to decide.

lanctot avatar Nov 29 '22 17:11 lanctot

We decided against this because it was a lot more subtle than we thought. In the end, we agreed it was better for the user to know what is going on and understand exactly what is being compared.

lanctot avatar Mar 26 '23 12:03 lanctot