Implemented WmAdams strategy (k44r from Axelrod's second) #1142

gaffney2010 · 2017-11-21T09:41:17Z

Fingerprints below.

gaffney2010 · 2017-11-21T09:49:15Z

I accidentally edited/uploaded the version of test_axelrod_second that had the random.seed() calls. I'm trying to fix it, but failing I'm afraid.

drvinceknight · 2017-11-21T09:49:59Z

There's a merge conflict due to difference in test_axelrod_second.

Because you're working on the master branch, I'd suggest you git pull upstream (where you set upstream to be the remote corresponding to this repo) and fix thing locally.

Let me know if you're comfortable sorting this out or if I can assist (ping me on gitter).

drvinceknight · 2017-11-21T09:50:35Z

I accidentally edited/uploaded the version of test_axelrod_second that had the random.seed() calls. I'm trying to fix it, but failing I'm afraid.

No problem, messages crossed :) Going to gitter as it'll be easier to do this in sync.

drvinceknight

Some minor stylistic requests (all looks great thanks).

To fix the merge conflict, assuming you're using a command line interface to git (let me know if not), let's first pull the changes from the upstream branch:

git pull https://github.com/Axelrod-Python/Axelrod.git master

Then there will be some errors in a file somewhere, these errors will just include things like <<<<<, ======== and >>>>>>>>> to denote the conflicts. Open up the files and fix those manually.

After that lets commit things:

git commit

Then you should be done :) Not to worry if there's any trouble, we will always be able to figure it out (despite appearances, with git, it's actually very difficult to get in to a mess that is not fixable :) 👍).

drvinceknight · 2017-11-21T10:04:20Z

axelrod/strategies/axelrod_second.py

+
+class WmAdams(Player):
+    """
+    Strategy submitted to Axelrod's second tournament by William Adams (K49R),


drvinceknight · 2017-11-21T10:24:02Z

axelrod/strategies/axelrod_second.py

+    Strategy submitted to Axelrod's second tournament by William Adams (K49R),
+    and came in fifth in that tournament.
+
+    Count the number of opponent defects after their first move, call 


of opponent defections

drvinceknight · 2017-11-21T10:24:14Z

axelrod/strategies/axelrod_second.py

+
+    Count the number of opponent defects after their first move, call 
+    `c_defect`.  Defect if c_defect equals 4, 7, or 9.  If c_defect > 9,
+    then defect immediately after opponent defect with probability = 


then defect immediately after opponent defects

drvinceknight · 2017-11-21T10:24:46Z

axelrod/strategies/axelrod_second.py

+
+        if number_defects == 4 or number_defects == 7 or number_defects == 9: return D
+        if number_defects > 9 and opponent.history[-1] == D:
+            return random_choice((0.5)**(number_defects-9))


PEP8: ((0.5) ** (number_defects - 9))

drvinceknight · 2017-11-21T10:25:03Z

axelrod/tests/strategies/test_axelrod_second.py

+        actions = [(C, D)] * 4 + [(C, C)] * 100
+        self.versus_test(defect_four, expected_actions=actions)
+
+        actions = [(C, D), (C, D), (C, D), (C, D), (C, D), (D, D), (C, D), (C, D), (D, D), (C, D), (D, D), (C, D), (D, D), (D, D), (D, D), (D, D)]


Can we line break these before 80 characters please (PEP8).

marcharper

Thanks for the contribution!

marcharper · 2017-11-21T13:42:09Z

axelrod/strategies/axelrod_second.py

+    }
+
+    def strategy(self, opponent: Player) -> Action:
+        if len(self.history) <=1:


Space after =

marcharper · 2017-11-21T13:42:38Z

axelrod/strategies/axelrod_second.py

+    def strategy(self, opponent: Player) -> Action:
+        if len(self.history) <=1:
+            return C
+        did_d = np.vectorize(lambda action: float(action == D))


We keep a running count of the number of defections in opponent.defections. Please use that instead, and subtract one if the first move is a defection. That way is O(rounds), the current implementation is O(rounds^2).

Wondered about this. I can fix in my earlier "Cave" implementation too.

If you could that would be great (my bad for missing that in the review :)).

marcharper · 2017-11-21T13:43:18Z

axelrod/strategies/axelrod_second.py

+        did_d = np.vectorize(lambda action: float(action == D))
+        number_defects = np.sum(did_d(opponent.history[1:]))
+
+        if number_defects == 4 or number_defects == 7 or number_defects == 9: return D


nit: if number_defects in [4, 7, 9]: return D is cleaner

gaffney2010 · 2017-11-22T21:43:57Z

I may not get to fixing these until after the Holiday (say Friday). Let me know if I should close the PR and re-open then. Thanks.

drvinceknight · 2017-11-22T22:05:39Z

I may not get to fixing these until after the Holiday (say Friday).

No rush at all, happy thanksgiving for tomorrow 👍

Let me know if I should close the PR and re-open then. Thanks.

Whatever is easier for you, if you're able to fix the merge conflict (we can help) then it's fine to keep it on this PR but if you'd prefer to close and open another PR that's also cool.

Conflicts: axelrod/tests/strategies/test_axelrod_second.py

gaffney2010 · 2017-11-25T22:07:33Z

I've made the changes you've requested. Thanks

drvinceknight · 2017-11-26T11:08:05Z

axelrod/strategies/axelrod_second.py

+        if opponent.history[0] == D:
+            number_defects -= 1
+
+        if number_defects in [4, 7, 9]: return D


@gaffney2010 can you confirm which strategy this is? Looking at https://github.com/Axelrod-Python/TourExec/blob/master/src/strategies/k44r.f I don't immediately follow how this implementation corresponds to k44r?

I can confirm that it's k44r. I'll say more about the translation...

I confirm I get the same fingerprints :)

I think you've just done a good job refactoring and re writing the code to be much more readable, just got me scratching my head to follow along, so one or two pointers for my own benefit just to confirm would be awesome (even if it's not in the PR itself but just in a comment here).

The ones I'm particularly stumped by are the comparisons of number_defects to 4, 7 and 9 and also where number_defects - 9 comes from...

Basically every time a defect comes up, it increments this MC variable. While AM is still a whole number, then MC is either less than AM or equal to AM. [While less than, line 16 returns C.] While MC=AM it defects; this happens for the first time at 4 defects [line 11]. As soon as MC exceeds AM, then AM is halved, and MC is reset to zero. [The 5th defect resets MC and sets AM to 2. So the 7th defect gets MC=AM. Then the 8th defect resets MC and sets AM to 1. So the 9th defect gets MC=AM]

After the 9th defect, the function changes because the AM is no longer whole, so MC never again equal AM (and line 17 is out of play). So as soon as MC is incremented (any defect), lines 19-22 are triggered. [Line 22 was 100% until now.] And MC is immediately reset, so there's no sustained defecting, like there was earlier. [For example, there could be exactly 4 defects for many turns, and the strategy would defect for all those turns. But after the 100th defect, the strategy will defect (with some probability) ONLY once.]

That's awesome and make sense. Thanks for explaining.

number_defects - 9 just makes the AM follow the right halving pattern. AM starts at 4.0 and halves after the 4th and 7th, for AM=1.0. After the 9th defect EACH defect will trigger a halving.

drvinceknight · 2017-11-26T11:42:47Z

I'm happy with all the changes and impressed with the refactor/translation of the original! Thanks @gaffney2010 :) 👍

marcharper · 2017-11-27T02:19:55Z

Changes LGTM, just need to resolve the merge conflict(s)

drvinceknight · 2017-11-27T07:57:26Z

The conflicts were fixed in c20a106. Thanks for the contribution @gaffney2010 👍

Implemented WmAdams strategy (k44r from Axelrod's second)

a643480

drvinceknight requested changes Nov 21, 2017

View reviewed changes

marcharper requested changes Nov 21, 2017

View reviewed changes

gaffney2010 added 2 commits November 25, 2017 15:52

Merge http://github.com/Axelrod-Python/Axelrod

c20a106

Conflicts: axelrod/tests/strategies/test_axelrod_second.py

Fixed formatting issues mentioned in PR

90c529b

gaffney2010 force-pushed the master branch from f11259f to da80753 Compare November 25, 2017 21:06

gaffney2010 added 2 commits November 25, 2017 16:31

Minor formatting fixes.

da80753

Subtracted first defect.

dd91f0a

drvinceknight reviewed Nov 26, 2017

View reviewed changes

drvinceknight approved these changes Nov 26, 2017

View reviewed changes

marcharper approved these changes Nov 27, 2017

View reviewed changes

marcharper mentioned this pull request Nov 27, 2017

Use opponent.defections in Cave and some minor styling changes #1143

Merged

drvinceknight merged commit 8b434a9 into Axelrod-Python:master Nov 27, 2017

drvinceknight mentioned this pull request Nov 27, 2017

After next (> v3.9.0) release of Axelrod: strategies to update Axelrod-Python/axelrod-fortran#70

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implemented WmAdams strategy (k44r from Axelrod's second) #1142

Implemented WmAdams strategy (k44r from Axelrod's second) #1142

gaffney2010 commented Nov 21, 2017

gaffney2010 commented Nov 21, 2017

drvinceknight commented Nov 21, 2017

drvinceknight commented Nov 21, 2017

drvinceknight left a comment

drvinceknight Nov 21, 2017

drvinceknight Nov 21, 2017

drvinceknight Nov 21, 2017

drvinceknight Nov 21, 2017

drvinceknight Nov 21, 2017

marcharper left a comment

marcharper Nov 21, 2017

marcharper Nov 21, 2017 •

edited

Loading

gaffney2010 Nov 22, 2017

drvinceknight Nov 22, 2017

marcharper Nov 21, 2017

gaffney2010 commented Nov 22, 2017

drvinceknight commented Nov 22, 2017

gaffney2010 commented Nov 25, 2017

drvinceknight Nov 26, 2017

gaffney2010 Nov 26, 2017

drvinceknight Nov 26, 2017

drvinceknight Nov 26, 2017

gaffney2010 Nov 26, 2017 •

edited

Loading

drvinceknight Nov 26, 2017

gaffney2010 Nov 26, 2017 •

edited

Loading

drvinceknight commented Nov 26, 2017

marcharper commented Nov 27, 2017

drvinceknight commented Nov 27, 2017

Implemented WmAdams strategy (k44r from Axelrod's second) #1142

Implemented WmAdams strategy (k44r from Axelrod's second) #1142

Conversation

gaffney2010 commented Nov 21, 2017

gaffney2010 commented Nov 21, 2017

drvinceknight commented Nov 21, 2017

drvinceknight commented Nov 21, 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

marcharper left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcharper Nov 21, 2017 • edited Loading

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 22, 2017

drvinceknight commented Nov 22, 2017

gaffney2010 commented Nov 25, 2017

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 Nov 26, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gaffney2010 Nov 26, 2017 • edited Loading

Choose a reason for hiding this comment

drvinceknight commented Nov 26, 2017

marcharper commented Nov 27, 2017

drvinceknight commented Nov 27, 2017

marcharper Nov 21, 2017 •

edited

Loading

gaffney2010 Nov 26, 2017 •

edited

Loading

gaffney2010 Nov 26, 2017 •

edited

Loading