Implemented K32R by Charles Kluepfel. #1138

gaffney2010 · 2017-11-12T11:11:21Z

Strategy submitted to Axelrod's second tournament by Charles Kluepfel (K32R).

Forgot to include in earlier commit.

drvinceknight

Looking good!

Just some initial comments for now after an initial pass through, we'll need to also confirm the logic is correct and corresponds to the Fortran code.

At the moment there are a few things failing in the test suite outside of the unit tests you have written (which look good!):

All the documentation is tested and the 74 stochastic strategies in this file (https://github.com/Axelrod-Python/Axelrod/blob/master/docs/tutorials/advanced/classification_of_strategies.rst) would need to be bumped to 75.
There is a family of strategies that makes use of other strategies. In this case it's using this strategy and needs to update it's test. In this file: https://github.com/Axelrod-Python/Axelrod/blob/master/axelrod/tests/strategies/test_meta.py at line 338 we need actions to be:
```
[(C, C), (C, D), (C, C), (C, D), (D, C)]
```
We will need to update the documentation for the second tournament in this file https://github.com/Axelrod-Python/Axelrod/blob/master/docs/reference/overview_of_strategies.rst to point at this strategy (there's a table in there.

I've pointed at something wrong with the rest syntax in the docstring which is currently broken but there might be one or two other things :) (No big deal we'll figure them out.)

As I mentioned on gitter: make these changes and commit them and then push to your fork, that'll automatically update here :)

drvinceknight · 2017-11-12T11:56:37Z

axelrod/strategies/axelrod_second.py

+    player decides that the opponent is random if
+    C1 >= (C1+C2)/2 - 1.5*sqrt(C1+C2)/2  AND
+    C4 >= (C3+C4)/2 - 1.5*sqrt(C3+C4)/2.
+    If the player decides that she is playing against a random player, then


Gender neutral: "they will" or "the player will".

drvinceknight · 2017-11-12T11:56:50Z

axelrod/strategies/axelrod_second.py

+    If the player decides that she is playing against a random player, then
+    she will always defect.
+
+    Otherwise, she will use a straight-forward set of rules:


Gender neutral.

drvinceknight · 2017-11-12T11:58:49Z

axelrod/strategies/axelrod_second.py

+    Otherwise, she will use a straight-forward set of rules:
+    - If opponent's last three choices are the same, then respond in kind.
+    - If opponent's last two choices are the same, then respond in kind with
+    probability 90%.


these are currently breaking the formatting of the documentation: the second line needs to start at the same level as the If:

- If opponent's last two choices are the same, then respond in kind with probability 90%.

drvinceknight · 2017-11-12T11:59:04Z

axelrod/strategies/axelrod_second.py

+    def __init__(self):
+        super().__init__()
+        self.C1, self.C2, self.C3, self.C4 = 0, 0, 0, 0
+        # These are defined by opponent's reaction to player (me) according to:


No need to repeat this.

drvinceknight · 2017-11-12T12:00:13Z

axelrod/strategies/axelrod_second.py

+                return D
+
+        # Otherwise, straight-forward strategy
+        r = random.random() # Will use later


Let's move the r = random.random() to line 559 so this random sample only happens if it's necessary.

Will do. Changing test file too.

drvinceknight · 2017-11-12T12:00:21Z

axelrod/strategies/axelrod_second.py

+        # Otherwise, straight-forward strategy
+        r = random.random() # Will use later
+
+        # Use the defaults from Python


Can you explain what you mean by this?

drvinceknight · 2017-11-12T12:00:41Z

axelrod/tests/strategies/test_axelrod_second.py

+        self.versus_test(axelrod.Alternator(), expected_actions=actions, seed=3)
+
+        # Now we have to test the detect-random logic, which doesn't pick up
+        # until after 26 trips.  So we need a big sample.


drvinceknight · 2017-11-12T12:01:58Z

axelrod/strategies/axelrod_second.py

+    - Otherwise if opponent's last action was to defect, then defect
+    with probability 60%.
+
+    Names:


Blank line after Names please.

drvinceknight · 2017-11-12T12:17:31Z

axelrod/strategies/axelrod_second.py

+
+                      I did:
+                    Def    Coop
+    She does: Coop   C1  |  C3


Gender neutral please. Go with player for your current I and opponent for your current she. Although it's probably also worth changing this to a bullet point list (only because the rest syntax can be troublesome at times). I'd suggest:

This player keeps track of the the opponent's responses to own behavior: - C1 counts: Opponent cooperates as response to player defecting. - C2 counts: Opponent defects as response to player defecting. - C3 counts: Opponent cooperates as response to player cooperating. - C4 counts: Opponent defects as response to player cooperating.

Could we however change these variable names (unless there's a good reason) to be more explicit. Perhaps:

- `cd_count` counts: Opponent cooperates as response to player defecting. - `dd_count` counts: Opponent defects as response to player defecting. - `cc_count` counts: Opponent cooperates as response to player cooperating. - `dc_count` counts: Opponent defects as response to player cooperating.

Was only using this for sqrt. Seemed like it was causing problems, so I replaced with half powers.

gaffney2010 · 2017-11-12T13:22:45Z

Thanks for all the feedback. I made changes to the code, and will work on documentation for the second tournament a little later. I'm not sure I understand the issue with test_meta.py...

drvinceknight · 2017-11-12T13:31:35Z

I'm not sure I understand the issue with test_meta.py...

There's a strategy in meta.py that makes use of a team of other strategies. It is called MetaLongMemory and with the addition of your strategy it's behaviour has changed so it just needs it's test to be updated. Currently at line 338 of test_meta.py we have:

        actions = [(C, C), (C, D), (D, C), (C, D), (D, C)]
        self.versus_test(opponent=axelrod.Alternator(),
                         expected_actions=actions, seed=1)

So in the third round the expected action is (D, C) however when making use of your strategy MetaLongMemory in fact cooperates in the third round so that line just needs to be changed to be:

        actions = [(C, C), (C, D), (C, C), (C, D), (D, C)]
        self.versus_test(opponent=axelrod.Alternator(),
                         expected_actions=actions, seed=1)

drvinceknight · 2017-11-12T13:40:58Z

I have run fingerprints of this implementation and K32r and it looks like a good match:

>>> import axelrod as axl
>>> import axelrod_fortran as axlf
>>> fortran_player = axlf.Player("k32r")
>>> axl_player = axl.Kluepfel()
>>> repetitions = 10000
>>> fortran_player, axl_player, repetitions
(Player: k32r, libstrategies.so, Kluepfel, 10000)

>>> axl_tf = axl.TransitiveFingerprint(axl_player)
>>> axl_tf.fingerprint(repetitions=repetitions, processes=0)
>>> axl_tf.plot();

>>> fortran_tf = axl.TransitiveFingerprint(fortran_player)
>>> fortran_tf.fingerprint(repetitions=repetitions, processes=0)
>>> fortran_tf.plot();

>>> opponents = [s() for s in axl.basic_strategies]
>>> axl_tf_v_basic = axl.TransitiveFingerprint(axl_player, opponents=opponents)
>>> axl_tf_v_basic.fingerprint(repetitions=repetitions, processes=0)
>>> axl_tf_v_basic.plot(display_names=True);

>>> fortran_tf_v_basic = axl.TransitiveFingerprint(fortran_player, opponents=opponents)
>>> fortran_tf_v_basic.fingerprint(repetitions=repetitions, processes=0)
>>> fortran_tf_v_basic.plot(display_names=True);

Before merging having someone else double check the logic would be great but this is looking good to me 👍

drvinceknight · 2017-11-12T13:42:36Z

Re a7a6c1b

Was only using this for sqrt. Seemed like it was causing problems, so I replaced with half powers.

Using sqrt makes the code ever so slightly more readable in my personal opinion. Not sure what trouble was being caused by using np.sqrt but perhaps use math.sqrt (from standard lib)? (Not a big deal).

drvinceknight · 2017-11-12T13:43:14Z

axelrod/strategies/axelrod_second.py

+
+    After 26 turns, the player then tries to detect a random player.  The
+    player decides that the opponent is random if
+    C1 >= (C1+C2)/2 - 1.5*sqrt(C1+C2)/2  AND


These (and anywhere else) needs to be updated to cd_count, ...

gaffney2010 · 2017-11-12T22:48:26Z

I have run fingerprints of this implementation and K32r and it looks like a good match

In the future is this something I should run when making a strategy?

So in the third round the expected action is (D, C) however when making use of your strategy MetaLongMemory in fact cooperates in the third round so that line just needs to be changed.

gaffney2010 · 2017-11-12T23:35:35Z

Sorry. I'm still trying to figure out this test piece. I ran unittest discover on my end and was able to get the same error, and I can see that making the change fixes it. But it looks like this test gets run for every infinite memory ordinary strategy. How come changing this code doesn't break other strategies? [I think that maybe the unittest detects changed code, but then will this break future changes? I don't see why we should expect all infinite memory ordinary strategies to have this expected action.]

Edited in wrong place before.

Added blank lines around bulleted lists.

gaffney2010 · 2017-11-13T02:34:14Z

Alright. I think I've fixed everything. Please let me know if anything else needs to be fixed. Thanks again for the help.

drvinceknight · 2017-11-13T07:26:58Z

But it looks like this test gets run for every infinite memory ordinary strategy. How come changing this code doesn't break other strategies?

That's not quite what's happening. For info:

the meta strategy has a team of strategies that it all gets to play (in this case it's team is all the infinite memory strategy),
every member of the team plays against the opponent but that's all done internally
when it comes to the meta strategy making a decision it asks every member of the team to declare what it would do and then decides what to do based on that "vote" (in this case it just selects C or D depending on which action gets the majority).

So with the addition of your strategy to the team, the majority of the players in this third round turns out to be C (as opposed to D).

Don't worry too much if that's still unclear, essentially these meta players are black boxes and their tests are more regression tests than anything else. Sometimes when adding new strategies their behaviour (and so their tests) changes :)

drvinceknight

Just the one very minor comment about an inline comment I don't understand but otherwise this looks great to me.

As well as running the fingerprints, I have worked through the logic and I'm happy this has been implemented correctly.

drvinceknight · 2017-11-13T07:29:01Z

axelrod/strategies/axelrod_second.py

+
+        # Otherwise respond to recent history
+
+        # Use the defaults from Python


I don't understand what this inline comment is saying. Perhaps remove?

Yea, I'll remove it.

drvinceknight · 2017-11-13T07:37:58Z

In the future is this something I should run when making a strategy?

If you could that would be super helpful. Depending on how much cpu time you have free feel free to run them with less repetitions.

meatballs · 2017-11-13T10:57:14Z

Excellent Job @gaffney2010 Thank you very much indeed!

gaffney2010 added 4 commits November 12, 2017 04:27

Implemented K32R by Charles Kluepfel.

869272f

Strategy submitted to Axelrod's second tournament by Charles Kluepfel (K32R).

Added references to Kluepfel

0adf450

Appended TestKluepfel

8b8b0d6

Included numpy

26999c0

Forgot to include in earlier commit.

drvinceknight requested changes Nov 12, 2017

View reviewed changes

gaffney2010 added 3 commits November 12, 2017 07:59

Responding to feedback from pull request.

81cfea4

Made changes based on feedback.

20e24e2

Got rid of numpy

a7a6c1b

Was only using this for sqrt. Seemed like it was causing problems, so I replaced with half powers.

drvinceknight requested changes Nov 12, 2017

View reviewed changes

gaffney2010 added 2 commits November 12, 2017 17:43

Changed back to sqrts; documentation to new vars

ccda3c7

Added reference for Kluepfel

9822a5f

Changed MetaLongMemory behavior for Kluepfel

52d7fb6

So in the third round the expected action is (D, C) however when making use of your strategy MetaLongMemory in fact cooperates in the third round so that line just needs to be changed.

gaffney2010 added 3 commits November 12, 2017 19:03

Fixed name of numpy import.

e9f2156

Incremented # stochastic strategies

1b08184

Edited in wrong place before.

Fixed docstrings

7af010d

Added blank lines around bulleted lists.

drvinceknight requested changes Nov 13, 2017

View reviewed changes

Removed comment.

d235bc8

drvinceknight approved these changes Nov 13, 2017

View reviewed changes

drvinceknight added the ready-to-merge label Nov 13, 2017

meatballs merged commit f0d2738 into Axelrod-Python:master Nov 13, 2017

drvinceknight mentioned this pull request Nov 13, 2017

After next release of Axelrod add k32r implementation Axelrod-Python/axelrod-fortran#68

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implemented K32R by Charles Kluepfel. #1138

Implemented K32R by Charles Kluepfel. #1138

gaffney2010 commented Nov 12, 2017

drvinceknight left a comment

drvinceknight Nov 12, 2017

drvinceknight Nov 12, 2017

drvinceknight Nov 12, 2017

drvinceknight Nov 12, 2017

drvinceknight Nov 12, 2017

gaffney2010 Nov 12, 2017

drvinceknight Nov 12, 2017

drvinceknight Nov 12, 2017

drvinceknight Nov 12, 2017

drvinceknight Nov 12, 2017

gaffney2010 commented Nov 12, 2017

drvinceknight commented Nov 12, 2017

drvinceknight commented Nov 12, 2017

drvinceknight commented Nov 12, 2017

drvinceknight Nov 12, 2017

gaffney2010 commented Nov 12, 2017

gaffney2010 commented Nov 12, 2017

gaffney2010 commented Nov 13, 2017

drvinceknight commented Nov 13, 2017

drvinceknight left a comment

drvinceknight Nov 13, 2017

gaffney2010 Nov 13, 2017

drvinceknight commented Nov 13, 2017

meatballs commented Nov 13, 2017


		# Otherwise respond to recent history

		# Use the defaults from Python

Implemented K32R by Charles Kluepfel. #1138

Implemented K32R by Charles Kluepfel. #1138

Conversation

gaffney2010 commented Nov 12, 2017

drvinceknight left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gaffney2010 commented Nov 12, 2017

drvinceknight commented Nov 12, 2017

drvinceknight commented Nov 12, 2017

drvinceknight commented Nov 12, 2017

Choose a reason for hiding this comment

gaffney2010 commented Nov 12, 2017

gaffney2010 commented Nov 12, 2017

gaffney2010 commented Nov 13, 2017

drvinceknight commented Nov 13, 2017

drvinceknight left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

drvinceknight commented Nov 13, 2017

meatballs commented Nov 13, 2017