-
Notifications
You must be signed in to change notification settings - Fork 264
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
Cache Bug Lurking #779
Comments
I'll look at this more closely tomorrow (if a fix hasn't appeared) but that fix seems straightforward enough? As you say, I think we're mainly ok here because of how the cache is now being used (famous last words!). |
Some code showing where this currently creates a problem (working on a fix):
The above creates 3 players that are not stochastic:
Their class is considered stochastic:
The
This is because the valid key check in the cache checks the cache but the Match checks the players (the match is correct). Note this example shows another error; the name of the Memory one players doesn't take the parameters. |
Closes #779 This makes the underlying read/write of the cache be the string repr. The passed key is the player instances (as opposed to previously going to have to go get the class).
We have to instantiate the strategy before checking if it's stochastic since various strategies have parameters that could change the classification. This could easily be true for the other classifiers such as memory depth and even say length but I'm not sure if there are instances of the latter.
We need to change the cache to use the strategy names as keys and to check the instances for stochasticity rather than the class classifiers. I don't know if this is yet causing problems since we tend to dynamically generate the cache in the Match class now but there is certainly the potential for a problem. Right now I think Matches are ok because
_cache_update_required
is properly checking stochasticity (the players are already instantiated). But the DeterministicCache is not quite doing the same thing -- it's checking the lass classifier rather than the instance classifier. Hopefully the deterministic repetition tests are keeping us safe at the moment.This is the line that I'm worried about:
Right now
key[0]
andkey[1]
are player classes and should be instantiated player names. We could also have an issue with multiple instances of the same class (one stochastic, one not):Since:
The other issue is key collision:
Those are both deterministic and different!
The text was updated successfully, but these errors were encountered: