google-deepmind / open_spiel

OpenSpiel is a collection of environments and algorithms for research in general reinforcement learning and search/planning in games.
Apache License 2.0
4.27k stars 936 forks source link

Occasional failures in oh_hell_test #412

Closed lanctot closed 4 years ago

lanctot commented 4 years ago

@solinas Seems like there are still some problems with Oh Hell test.

Log here: https://travis-ci.org/github/deepmind/open_spiel/jobs/737014971

(Only failed 1/6 of the tests. We fixed the seed, right?)

lanctot commented 4 years ago

Marking the game as having known issues for now: https://github.com/deepmind/open_spiel/commit/d9aca33ea74367253124fffe5f323b18621ea094

I remeber we tried everything regarding accessing memory that wasn't allocated. But it could be accessing memory that was not allocated but still initialized? E.g. maybe something like this:

int x;
if (x == 0) {
  // Stuff
}
lanctot commented 4 years ago

Hmm seems like we're not fixing the seed -- so this could just be the game going on forever. Does the log help?

solinas commented 4 years ago

I think code similar to the example you gave would be caught by valgrind (which returned no errors locally when I checked). The problem with this one is I can't seem to reproduce it locally, so I won't rule it out.

The logs hint that values in the InformationStateTensor relating to the player's hand might be set when they shouldn't be. I'll take a look today and see what I can figure out. I'll also fix any places in the tests where we aren't fixing the seed.

lanctot commented 4 years ago

Yeah my guess is that it's not a memory issue but a sequence that leads to an infinite loop.

lanctot commented 4 years ago

Theory debunked: https://travis-ci.org/github/deepmind/open_spiel/jobs/737014971#L77744

solinas commented 4 years ago

I found an issue that is likely the cause of the failures. Seems like a buffer overflow in the tests.

In the InformationStateTensorTest, only max_num_tricks bools are allocated for the hand:

https://github.com/deepmind/open_spiel/blob/a6ad6ac5877686a1702a2d756edaed2b0aa8d870/open_spiel/games/oh_hell_test.cc#L63

But the hand is assumed to be one-hot encoded in the rest of the function:

https://github.com/deepmind/open_spiel/blob/a6ad6ac5877686a1702a2d756edaed2b0aa8d870/open_spiel/games/oh_hell_test.cc#L96-L98

This would explain why we're seeing a corrupted hand in the Travis logs, but I'm a bit surprised it doesn't cause errors more reliably. The hand vector is allocated on the stack, so that would explain why valgrind didn't pick this up though.

lanctot commented 4 years ago

Ok, cool. Btw you should never use vector<bool>. Do a search and you'll find a bunch of stuff on why, here's one place to start: https://www.reddit.com/r/learnprogramming/comments/qb56c/c_why_is_vectorbool_bad/

I recommend changing to vector<int> and just using 1 and 0 as truth values.