Skip to content
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

Add new strategy ShortMem #857

Merged
merged 16 commits into from
Feb 11, 2017
Merged

Conversation

janga1997
Copy link
Member

@janga1997 janga1997 commented Feb 8, 2017

#379
From the suggested list of strategies by @marcharper from this paper.

I'm sure there a few things I missed. @drvinceknight Do take a look whenever free.
I will work on the tests if this is okay so far.

C, D = Actions.C, Actions.D

class ShortMem(Player):
"""A player who only ever defects."""
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this docstring is accurate right?

Also note that we require a few more details in the docstring, you can see an example here: http://axelrod.readthedocs.io/en/latest/tutorials/contributing/strategy/writing_the_new_strategy.html

def strategy(opponent: Player) -> Action:

memoryDepth = self.classifier['memoryDepth']
cooperateRatio = array.count('C')/memoryDepth
Copy link
Member

Choose a reason for hiding this comment

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

You haven't defined array so I expect this strategy would fail to play.

Note that we're going to need some tests for the strategy too, you can see some information on the tests here: http://axelrod.readthedocs.io/en/latest/tutorials/contributing/strategy/writing_test_for_the_new_strategy.html

We're happy to give a hand with tests if you'd like one. Essentially we want code that checks that all the logic you write below works :)

Also, note that a Player has a cooperations attribute that counts the number of cooperations.

Copy link
Member Author

@janga1997 janga1997 Feb 8, 2017

Choose a reason for hiding this comment

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

I thought about that, but we need the number of cooperations and defections in the last 10 steps only, and to the extent that I understand it, it records the Cs and Ds for the whole play.

elif defectRatio - cooperateRatio > 0.3:
return D
else:
return TitForTat().strategy(opponent)
Copy link
Member

Choose a reason for hiding this comment

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

Could you code this explicity please. Something like:

else:
    return opponent.history[-1]

@janga1997
Copy link
Member Author

@drvinceknight I've been meaning to get a little feedback. In this case, is FIFO really relevant to the what this strategy does, because I don't think it is.

@drvinceknight
Copy link
Member

I've been meaning to get a little feedback. In this case, is FIFO really relevant to the what this strategy does, because I don't think it is.

Sorry, what do you mean by FIFO?

@janga1997
Copy link
Member Author

It's an acronym for first in, first out, where the oldest (first) entry, or 'head' of the queue, is processed first. This term was specifically mentioned in the description of this strategy in the paper.

@drvinceknight
Copy link
Member

It's an acronym for first in, first out, where the oldest (first) entry, or 'head' of the queue, is processed first. This term was specifically mentioned in the description of this strategy in the paper.

I think it's ok to simply say "the last ten moves".

Copy link
Member

@drvinceknight drvinceknight left a comment

Choose a reason for hiding this comment

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

The main thing that is missing are tests :) Let us know if we can assist with them.

def strategy(opponent: Player) -> Action:

memoryDepth = self.classifier['memoryDepth']
array = self.history[:-11:-1]
Copy link
Member

Choose a reason for hiding this comment

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

As order doesn't matter you can just take: self.history[-10:].

>>> a = list(range(20))
>>> a[-10:]
[10,11,12,13,14,15,16,17,18,19]

However note that I think you mean for this to be opponent.history.

cooperateRatio = array.count('C')/memoryDepth
defectRatio = array.count('D')/memoryDepth

if len(self.history) <= memoryDepth:
Copy link
Member

Choose a reason for hiding this comment

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

You could move this to before the counting (a tiny efficiency improvement).

"""
A player starts by always cooperating for the first 10 moves.

The opponent answers are stored in the memory. The memory is FIFO and the maximum size of the memory is 10 results. From the tenth round on, the program analyzes the memory, and compare the number of defects and cooperates of the opponent, based in percentage. If cooperation occurs 30% more than defection, it will cooperate.
Copy link
Member

Choose a reason for hiding this comment

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

Could you stick to PEP8 here, so keep the characters to less than 80 per line please :)


The opponent answers are stored in the memory, whose maximum size is 10 results. From the tenth round on, the program analyzes the memory, and compare the number of defects and cooperates of the opponent, based in percentage. If cooperation occurs 30% more than defection, it will cooperate.
If defection occurs 30% more than cooperation, the program will defect. Otherwise, the program follows the TitForTat algorithm.

Copy link
Member

Choose a reason for hiding this comment

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

Can you add something like:

    Names

    - Short Mem: [Axelrod1980]_
    """

Except replace Axelrod1980 by a key that you'd put in https://github.com/Axelrod-Python/Axelrod/blob/master/docs/reference/bibliography.rst

@janga1997
Copy link
Member Author

@drvinceknight I hope its okay now?

@Nikoleta-v3
Copy link
Member

@janga1997 Hello I am also a contributor to the library! Did you remember to add the reference like suggested.

Except replace Axelrod1980 by a key that you'd put in https://github.com/Axelrod-Python/Axelrod/blob/master/docs/reference/bibliography.rst

@janga1997
Copy link
Member Author

@Nikoleta-v3 no I didn't yet. I will do it tomorrow along with the tests, as it late night here. Thanks .

@janga1997
Copy link
Member Author

@drvinceknight @Nikoleta-v3 Do review whenever you're free. I believe I've covered the necessary test cases.

@@ -37,3 +37,4 @@ documentation.
.. [Stewart2012] Stewart, a. J., & Plotkin, J. B. (2012). Extortion and cooperation in the Prisoner’s Dilemma. Proceedings of the National Academy of Sciences, 109(26), 10134–10135. http://doi.org/10.1073/pnas.1208087109
.. [Szabó1992] Szabó, G., & Fáth, G. (2007). Evolutionary games on graphs. Physics Reports, 446(4-6), 97–216. http://doi.org/10.1016/j.physrep.2007.04.004
.. [Tzafestas2000] Tzafestas, E. (2000). Toward adaptive cooperative behavior. From Animals to Animals: Proceedings of the 6th International Conference on the Simulation of Adaptive Behavior {(SAB-2000)}, 2, 334–340.
.. [Andre2013] Andre L. C., Honovan P., Felipe T. and Frederico G. (2013). Iterated Prisoner’s Dilemma - An extended analysis, http://abricom.org.br/wp-content/uploads/2016/03/bricsccicbic2013_submission_202.pdf
Copy link
Member

Choose a reason for hiding this comment

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

I am not sure if the reference is generated correctly because there is an extra : in shortmem.py line 15. Could you please check. 😄

ShortMem: [Andre:2013]_

@janga1997
Copy link
Member Author

@Nikoleta-v3 Is everything fine with the tests and the strategy itself?

@Nikoleta-v3
Copy link
Member

Everything else looks good to me! Happy to be working with you 😄
The core contributors need to give a 👍 now.

Copy link
Member

@drvinceknight drvinceknight left a comment

Choose a reason for hiding this comment

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

One tiny tiny thing left from me. Thanks @Nikoleta-v3 and @janga1997 this is looking good!

"""
A player starts by always cooperating for the first 10 moves.

The opponent answers are stored in the memory, whose maximum size is 10 results. From the tenth round on, the program analyzes the memory, and compare the number of defects and cooperates of the opponent, based in percentage. If cooperation occurs 30% more than defection, it will cooperate.
Copy link
Member

Choose a reason for hiding this comment

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

This all looks good to me, could you keep the number of characters on each line to less than 80 please? So it just needs this docstring to line break a bit earlier.

@drvinceknight
Copy link
Member

How exactly do you make a word wrap at the the 80th column.

It would depend on your editor. There are ways to set it up so it does it automatically but you can also create a visual queue that just shows where the 80 character limit is and then you can line break manually.

Copy link
Member

@marcharper marcharper left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! I think there's one error and some style issues, and otherwise almost ready.

if len(opponent.history) <= 10:
return C

array = opponent.history[:-11:-1]
Copy link
Member

Choose a reason for hiding this comment

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

Take a look at this line, the arrays are off by one.

>>> l = list(range(100))
>>> l[:-11:-1]
[99, 98, 97, 96, 95, 94, 93, 92, 91, 90]
>>> l[-11:]
[89, 90, 91, 92, 93, 94, 95, 96, 97, 98, 99]

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct me if I am wrong, since we are talking about the last ten rounds here, doesn't l[-11:] produce a slice with 11 elements?

Copy link
Member

@drvinceknight drvinceknight Feb 9, 2017

Choose a reason for hiding this comment

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

I think you can just use:

>>> a = range(20)
>>> list(a[-10:])
[10, 11, 12, 13, 14, 15, 16, 17, 18, 19]
>>> len(list(a[-10:]))
10

If you go for a[-10:] I think that improves the readability/ambiguity and good call on the having a test that checks this.

Copy link
Member Author

Choose a reason for hiding this comment

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

I understand about the readability/ambiguity issue, but what exactly is a test that checks this?

Is it self.responses_test([C], [C] * 4 + [D] *6, [D] * 4 + [C] * 6) ?

Copy link
Member

@marcharper marcharper Feb 10, 2017

Choose a reason for hiding this comment

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

Sorry, @drvinceknight's suggestion is better. A test to check would fail on 9 trailing elements but not 10.

Copy link
Member Author

@janga1997 janga1997 Feb 10, 2017

Choose a reason for hiding this comment

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

Sorry for being difficult, but I really don't see the reason for an additional test.

The strategy is to return C from 1-9 moves, and from the 10th move, the rest of the logic kicks in.
I don't understand what you and @drvinceknight mean by

A test to check would fail on 9 trailing elements but not 10

Why would the test fail on 9 trailing elements? It would return C as the strategy describes.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry for being difficult, but I really don't see the reason for an additional test.

Don't worry you're not being difficult, it's good to talk these things over :)

I would definitely suggest you change:

opponent.history[:-11:-1]

to be

opponent.history[-10:]

This just helps with readability.

In terms of the test, I suggest you just add the following one:

self.responses_test([C], [D] *9, [D]  * 9)  # This checks that when there are 9 previous plays you cooperate
self.responses_test([D], [D] *10, [D]  * 10)  # This checks that when there are 10 previous plays the logic kicks in

"""
A player starts by always cooperating for the first 10 moves.

The opponent answers are stored in the memory, whose maximum size is 10
Copy link
Member

Choose a reason for hiding this comment

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

Technically all the history is saved in memory in our library. I would just say that the player analyzes the last ten rounds of play without any details about what's in memory.

answers -> actions

return C

array = opponent.history[:-11:-1]
cooperateRatio = array.count('C')
Copy link
Member

Choose a reason for hiding this comment

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

These aren't ratios, cooperation_counts and defection_counts would be better names, or simply C_counts.

compare the number of defects and cooperates of the opponent, based in
percentage. If cooperation occurs 30% more than defection, it will
cooperate.
If defection occurs 30% more than cooperation, the program will defect.
Copy link
Member

Choose a reason for hiding this comment

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

program --> player

@@ -0,0 +1,34 @@
"""Tests for the Cooperator strategy."""
Copy link
Member

Choose a reason for hiding this comment

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

Cooperator -> shortmem

Please add a test to catch the array length issue above (array is of length 9 instead of 10 but none of the tests were tripped).

Copy link
Member Author

Choose a reason for hiding this comment

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

I replied about the array length issue above.

self.responses_test([C], [C] * 4, [D] * 4)
self.responses_test([C], [C] * 4 + [D] *6, [D] * 4 + [C] * 6)

#Cooperate if in the last ten moves, Cooperations - Defections >= 30
Copy link
Member

Choose a reason for hiding this comment

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

30% and see below

Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't understand what you mean.

Copy link
Member

Choose a reason for hiding this comment

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

We're not calculating percentages here. I get that it's pointless to divide by 10 on both sides but it's not an accurate description, and it might be confusing to look at this code in six months. So I'm asking that the comments elaborate more about what the code's intention.


The opponent answers are stored in the memory, whose maximum size is 10
results. From the tenth round on, the program analyzes the memory, and
compare the number of defects and cooperates of the opponent, based in
Copy link
Member

Choose a reason for hiding this comment

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

defections and cooperations

cooperateRatio = array.count('C')
defectRatio = array.count('D')

if cooperateRatio - defectRatio >= 3:
Copy link
Member

Choose a reason for hiding this comment

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

FYI standard Python style is camel_case instead of CamelCase for variables, search for PEP8 (the style guide).

if len(opponent.history) <= 10:
return C

array = opponent.history[:-11:-1]
Copy link
Member

Choose a reason for hiding this comment

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

Sorry for being difficult, but I really don't see the reason for an additional test.

Don't worry you're not being difficult, it's good to talk these things over :)

I would definitely suggest you change:

opponent.history[:-11:-1]

to be

opponent.history[-10:]

This just helps with readability.

In terms of the test, I suggest you just add the following one:

self.responses_test([C], [D] *9, [D]  * 9)  # This checks that when there are 9 previous plays you cooperate
self.responses_test([D], [D] *10, [D]  * 10)  # This checks that when there are 10 previous plays the logic kicks in

C_counts = array.count('C')
D_counts = array.count('D')

if C_counts - D_counts >= 3:
Copy link
Member

Choose a reason for hiding this comment

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

Another question (not sure how we missed this), is the logic correct here?

You're looking at the difference in counts (subtraction) but the description is saying that there should be 30% more cooperations than defections? So I'm not sure the logic is actually correct.

I think this test should be:

if C_counts / D_counts >= 1.3:

That checks that there are 30% more cooperations.

Copy link
Member Author

Choose a reason for hiding this comment

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

I am starting to doubt it a bit myself.
But doesn't the term more than imply subtraction?

Copy link
Member Author

Choose a reason for hiding this comment

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

After thinking about it, it seems your logic makes more sense w.r.t. the given statement.

When I first thought about it, I made the assumption that percentages are to be calculated w.r.t the sum of C and D (10). My mistake. I would have never found it myself.

Copy link
Member

Choose a reason for hiding this comment

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

But doesn't the term more than imply subtraction?

My suggestion is incorrect, because we're only considering 10 turns this is actually correct. Apologies.

Copy link
Member

Choose a reason for hiding this comment

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

@janga1997 I actually think I'm wrong. I think you logic was correct before.

if D_counts == 0:
return C

if C_counts / (D_counts * 1.0) >= 1.3:
Copy link
Member

Choose a reason for hiding this comment

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

I think the logic was actually correct before (my mistake) I suggest you revert: a7b8d9f

@janga1997
Copy link
Member Author

@drvinceknight Is that everything?

@drvinceknight
Copy link
Member

@drvinceknight Is that everything?

This looks good to me. I'll leave @marcharper to merge it :) 👍

@marcharper marcharper merged commit b174daa into Axelrod-Python:master Feb 11, 2017
drvinceknight added a commit that referenced this pull request Feb 11, 2017
This was missed in #857.
marcharper pushed a commit that referenced this pull request Feb 13, 2017
This was missed in #857.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants