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 unsorted legal actions. #209

Merged
merged 1 commit into from
May 11, 2020

Conversation

jhtschultz
Copy link
Member

The last update broke compatibility with some of my EFG files that used to work. The EFG file specification does not guarantee the action names will be sorted. And even if the action names are always sorted within a given node, they may not have been first encountered in that order, so the action_ids they map to may be unsorted. Consider first encountering the infoset with actions {"B" "C"}, followed by {"A" "B" "C"}. The action_ids are then B->0, C->1, A->2. So when calling LegalActions() on the second infoset, we get [2, 0, 1]. Sorting the action_ids after parsing all the actions within each node fixed the issue.

@lanctot
Copy link
Collaborator

lanctot commented May 7, 2020

Whoops... good catch, thanks John!

@lanctot lanctot self-assigned this May 7, 2020
@tewalds
Copy link
Contributor

tewalds commented May 7, 2020

Is there a general test that could catch this?

@lanctot
Copy link
Collaborator

lanctot commented May 7, 2020

Is there a general test that could catch this?

Yeah pretty sure we already have one-- Ed added it when we added the requirement for sorted actions. John is using .efg files generated from code that he has not been submitted yet, so my guess is it was in one of those. (But maybe we indeed don't have the test.)

@jhtschultz
Copy link
Member Author

Yeah that's how I caught it. It just so happens that none of the current EFG files trigger the error.

@OpenSpiel OpenSpiel merged commit e13c994 into google-deepmind:master May 11, 2020
@jhtschultz jhtschultz deleted the efg_game_fix branch June 30, 2020 01:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants