Skip to content

Fixes the cache bug. #782

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

Merged
merged 5 commits into from
Dec 4, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 21 additions & 3 deletions axelrod/deterministic_cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,24 @@ def __init__(self, file_name=None):
if file_name is not None:
self.load(file_name)

def _key_transform(self, key):
"""
Parameters
----------
key: tuple
A 3-tuple: (player instance, player instance, match length)
"""
return key[0].name, key[1].name, key[2]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we check if the player has been instantiated? This could still fail in the same way, no?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is assuming that key[0] and key[1] are instance of the Player class. I have modified the valid key check to check that that's indeed the case.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

     def _is_valid_key(self, key):
          """Validate a proposed dictionary key
 @@ -85,8 +103,8 @@ def _is_valid_key(self, key):
          # integer
          try:
              if not (
 -                issubclass(key[0], Player) and
 -                issubclass(key[1], Player) and
 +                isinstance(key[0], Player) and
 +                isinstance(key[1], Player) and
                  isinstance(key[2], int)


def __delitem__(self, key):
return UserDict.__delitem__(self, self._key_transform(key))

def __getitem__(self, key):
return UserDict.__getitem__(self, self._key_transform(key))

def __contains__(self, key):
return UserDict.__contains__(self, self._key_transform(key))

def __setitem__(self, key, value):
"""Overrides the UserDict.__setitem__ method in order to validate
the key/value and also to set the turns attribute"""
Expand All @@ -60,7 +78,7 @@ def __setitem__(self, key, value):
raise ValueError(
'Value must be a list with length equal to turns attribute')

UserDict.__setitem__(self, key, value)
UserDict.__setitem__(self, self._key_transform(key), value)

def _is_valid_key(self, key):
"""Validate a proposed dictionary key
Expand All @@ -85,8 +103,8 @@ def _is_valid_key(self, key):
# integer
try:
if not (
issubclass(key[0], Player) and
issubclass(key[1], Player) and
isinstance(key[0], Player) and
isinstance(key[1], Player) and
isinstance(key[2], int)
):
return False
Expand Down
2 changes: 1 addition & 1 deletion axelrod/match.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ def __init__(self, players, turns, game=None, deterministic_cache=None,
"""
self.result = []
self.turns = turns
self._cache_key = (players[0].__class__, players[1].__class__, turns)
self._cache_key = (players[0], players[1], turns)
self.noise = noise

if game is None:
Expand Down
4 changes: 3 additions & 1 deletion axelrod/strategies/memoryone.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,10 @@ def __init__(self, four_vector=None, initial=C):
"""
Player.__init__(self)
self._initial = initial
if four_vector:
if four_vector is not None:
self.set_four_vector(four_vector)
if self.name == 'Generic Memory One Player':
self.name = "%s: %s" % (self.name, four_vector)

def set_four_vector(self, four_vector):
if not all(0 <= p <= 1 for p in four_vector):
Expand Down
17 changes: 17 additions & 0 deletions axelrod/tests/integration/test_matches.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,20 @@ def test_outcome_repeats_stochastic(self, strategies, turns, seed):

self.assertEqual(results[0], results[1])
self.assertEqual(results[1], results[2])

def test_matches_with_det_player_for_stochastic_classes(self):
"""A test based on a bug found in the cache.

See: https://github.com/Axelrod-Python/Axelrod/issues/779"""
p1 = axelrod.MemoryOnePlayer((0, 0, 0, 0))
p2 = axelrod.MemoryOnePlayer((1, 0, 1, 0))
p3 = axelrod.MemoryOnePlayer((1, 1, 1, 0))

m = axelrod.Match((p1, p2), turns=3)
self.assertEqual(m.play(), [('C', 'C'), ('D', 'C'), ('D', 'D')])

m = axelrod.Match((p2, p3), turns=3)
self.assertEqual(m.play(), [('C', 'C'), ('C', 'C'), ('C', 'C')])

m = axelrod.Match((p1, p3), turns=3)
self.assertEqual(m.play(), [('C', 'C'), ('D', 'C'), ('D', 'C')])
16 changes: 9 additions & 7 deletions axelrod/tests/unit/test_deterministic_cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,11 @@ class TestDeterministicCache(unittest.TestCase):

@classmethod
def setUpClass(cls):
cls.test_key = (TitForTat, Defector, 3)
cls.test_key = (TitForTat(), Defector(), 3)
cls.test_value = [('C', 'D'), ('D', 'D'), ('D', 'D')]
cls.test_save_file = 'test_cache_save.txt'
cls.test_load_file = 'test_cache_load.txt'
if sys.version_info[0] == 2:
# Python 2.x
cls.test_pickle = b"""(dp0\n(caxelrod.strategies.titfortat\nTitForTat\np1\ncaxelrod.strategies.defector\nDefector\np2\nI3\ntp3\n(lp4\n(S'C'\np5\nS'D'\np6\ntp7\na(g6\ng6\ntp8\na(g6\ng6\ntp9\nas."""
else:
# Python 3.x
cls.test_pickle = b'\x80\x03}q\x00caxelrod.strategies.titfortat\nTitForTat\nq\x01caxelrod.strategies.defector\nDefector\nq\x02K\x03\x87q\x03]q\x04(X\x01\x00\x00\x00Cq\x05X\x01\x00\x00\x00Dq\x06\x86q\x07h\x06h\x06\x86q\x08h\x06h\x06\x86q\tes.'
cls.test_pickle = b'\x80\x03}q\x00X\x0b\x00\x00\x00Tit For Tatq\x01X\x08\x00\x00\x00Defectorq\x02K\x03\x87q\x03]q\x04(X\x01\x00\x00\x00Cq\x05X\x01\x00\x00\x00Dq\x06\x86q\x07h\x06h\x06\x86q\x08h\x06h\x06\x86q\tes.'
with open(cls.test_load_file, 'wb') as f:
f.write(cls.test_pickle)

Expand Down Expand Up @@ -102,3 +97,10 @@ def test_load_error_for_inccorect_format(self):
with self.assertRaises(ValueError):
cache = DeterministicCache()
cache.load(filename)

def test_del_item(self):
cache = DeterministicCache()
cache[self.test_key] = self.test_value
self.assertTrue(self.test_key in cache)
del cache[self.test_key]
self.assertFalse(self.test_key in cache)
6 changes: 3 additions & 3 deletions axelrod/tests/unit/test_match.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ def test_init(self, turns, game):
turns
)
self.assertEqual(
match._cache_key, (axelrod.Cooperator, axelrod.Cooperator, turns))
match._cache_key, (p1, p2, turns))
self.assertEqual(match.turns, turns)
self.assertEqual(match._cache, {})
self.assertEqual(match.noise, 0)
Expand Down Expand Up @@ -96,11 +96,11 @@ def test_play(self):
expected_result = [(C, D), (C, D), (C, D)]
self.assertEqual(match.play(), expected_result)
self.assertEqual(
cache[(axelrod.Cooperator, axelrod.Defector, 3)], expected_result)
cache[(axelrod.Cooperator(), axelrod.Defector(), 3)], expected_result)

# a deliberately incorrect result so we can tell it came from the cache
expected_result = [(C, C), (D, D), (D, C)]
cache[(axelrod.Cooperator, axelrod.Defector, 3)] = expected_result
cache[(axelrod.Cooperator(), axelrod.Defector(), 3)] = expected_result
match = axelrod.Match(players, 3, deterministic_cache=cache)
self.assertEqual(match.play(), expected_result)

Expand Down
21 changes: 21 additions & 0 deletions axelrod/tests/unit/test_memoryone.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,27 @@

C, D = axelrod.Actions.C, axelrod.Actions.D

class TestGenericPlayerOne(unittest.TestCase):
"""
A class to test the naming and classification of generic memory one players
"""
p1 = axelrod.MemoryOnePlayer((0, 0, 0, 0))
p2 = axelrod.MemoryOnePlayer((1, 0, 1, 0))
p3 = axelrod.MemoryOnePlayer((1, 0.5, 1, 0.5))

def test_name(self):
self.assertEqual(self.p1.name,
"Generic Memory One Player: (0, 0, 0, 0)")
self.assertEqual(self.p2.name,
"Generic Memory One Player: (1, 0, 1, 0)")
self.assertEqual(self.p3.name,
"Generic Memory One Player: (1, 0.5, 1, 0.5)")

def test_stochastic_classification(self):
self.assertFalse(self.p1.classifier['stochastic'])
self.assertFalse(self.p2.classifier['stochastic'])
self.assertTrue(self.p3.classifier['stochastic'])


class TestWinStayLoseShift(TestPlayer):

Expand Down
4 changes: 2 additions & 2 deletions docs/tutorials/advanced/using_the_cache.rst
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,11 @@ Let us rerun the above match but using the cache::
We can take a look at the cache::

>>> cache # doctest: +ELLIPSIS
{(<class 'axelrod.strategies.gobymajority.GoByMajority'>, <class 'axelrod.strategies.alternator.Alternator'>, 200): [('C', 'C'), ..., ('C', 'D')]}
{('Soft Go By Majority', 'Alternator', 200): [('C', 'C'), ..., ('C', 'D')]}
>>> len(cache)
1

This maps a triplet of 2 player classes and the match length to the resulting
This maps a triplet of 2 player names and the match length to the resulting
interactions. We can rerun the code and compare the timing::

>>> def run_match_with_cache():
Expand Down