add a new strategy CAPRI #1393

yohm · 2021-08-03T08:02:16Z

I added a new strategy 'CAPRI', which is proposed in the following paper.
https://www.nature.com/articles/s41598-020-73855-x

Would you please review and let me know if you find something that needs to be revised? Thank you.

drvinceknight

Thanks for this, I have made some initial stylistic comments but need to read the paper to offer a comprehensive review (planning on doing that soon as it looks interesting).

A comment: it does look like this strategy is a specific case of a axl.Lookerup (https://github.com/Axelrod-Python/Axelrod/blob/dev/axelrod/strategies/lookerup.py#L236) so it would be possible to implement this as a particular case which could help clear things up.

axelrod/strategies/grudger.py

drvinceknight · 2021-08-04T13:14:31Z

axelrod/strategies/grudger.py

+    C: Cooperate at mutual cooperation.
+        This rule prescribes c at (ccc, ccc).
+    A: Accept punishment when you mistakenly defected from mutual cooperation.
+        This rule prescribes c at (ccd, ccc), (cdc, ccd), (dcc, cdc), and (ccc, dcc).
+    P: Punish your co-player by defecting once when he defected from mutual cooperation.
+        This rule prescribes d at (ccc, ccd), and then c at (ccd, cdc), (cdc, dcc), and (dcc, ccc).
+    R: Recover cooperation when you or your co-player cooperated at mutual defection.
+        This rule prescribes c at (ddd, ddc), (ddc, dcc), (dcc, ccc), (ddc, ddd), (dcc, ddc), (ccc, dcc), (ddc, ddc), and (dcc, dcc).
+    I: In all the ohter cases, defect.


These rules are not immediately clear to me. If it is verbatim from the paper (which I have not read yet) then that should be made clear and some clarification should be added.

(The rules are from the paper)

axelrod/strategies/grudger.py

axelrod/tests/strategies/test_grudger.py

drvinceknight · 2021-08-04T13:19:24Z

axelrod/strategies/grudger.py

+        elif hist == [[C,C],[C,C],[D,C]]:  # Rule: A
+            return C
+        elif hist == [[C,C],[D,C],[C,D]]:
+            return C
+        elif hist == [[D,C],[C,D],[C,C]]:
+            return C
+        elif hist == [[C,D],[C,C],[C,C]]:
+            return C
+        elif hist == [[C,C],[C,C],[C,D]]:  # Rule: P
+            return D
+        elif hist == [[C,C],[C,D],[D,C]]:
+            return C
+        elif hist == [[C,D],[D,C],[C,C]]:
+            return C
+        elif hist == [[D,C],[C,C],[C,C]]:
+            return C
+        elif hist == [[D,D],[D,D],[D,C]]:  # Rule: R1
+            return C
+        elif hist == [[D,D],[D,C],[C,C]]:
+            return C
+        elif hist == [[D,C],[C,C],[C,C]]:
+            return C
+        elif hist == [[D,D],[D,D],[C,D]]:  # Rule: R2
+            return C
+        elif hist == [[D,D],[C,D],[C,C]]:
+            return C
+        elif hist == [[C,D],[C,C],[C,C]]:
+            return C
+        elif hist == [[D,D],[D,D],[C,C]]:  # Rule: R3
+            return C
+        elif hist == [[D,D],[C,C],[C,C]]:
+            return C


Suggested change

elif hist == [[C,C],[C,C],[D,C]]: # Rule: A

return C

elif hist == [[C,C],[D,C],[C,D]]:

return C

elif hist == [[D,C],[C,D],[C,C]]:

return C

elif hist == [[C,D],[C,C],[C,C]]:

return C

elif hist == [[C,C],[C,C],[C,D]]: # Rule: P

return D

elif hist == [[C,C],[C,D],[D,C]]:

return C

elif hist == [[C,D],[D,C],[C,C]]:

return C

elif hist == [[D,C],[C,C],[C,C]]:

return C

elif hist == [[D,D],[D,D],[D,C]]: # Rule: R1

return C

elif hist == [[D,D],[D,C],[C,C]]:

return C

elif hist == [[D,C],[C,C],[C,C]]:

return C

elif hist == [[D,D],[D,D],[C,D]]: # Rule: R2

return C

elif hist == [[D,D],[C,D],[C,C]]:

return C

elif hist == [[C,D],[C,C],[C,C]]:

return C

elif hist == [[D,D],[D,D],[C,C]]: # Rule: R3

return C

elif hist == [[D,D],[C,C],[C,C]]:

return C

if hist == [[C, C], [C, C], [D, C]]: # Rule: A

return C

if hist == [[C, C], [D, C], [C, D]]:

return C

if hist == [[D, C], [C, D], [C, C]]:

return C

if hist == [[C, D], [C, C], [C, C]]:

return C

if hist == [[C, C], [C, C], [C, D]]: # Rule: P

return D

if hist == [[C, C], [C, D], [D, C]]:

return C

if hist == [[C, D], [D, C], [C, C]]:

return C

if hist == [[D, C], [C, C], [C, C]]:

return C

if hist == [[D, D], [D, D], [D, C]]: # Rule: R1

return C

if hist == [[D, D], [D, C], [C, C]]:

return C

if hist == [[D, C], [C, C], [C, C]]:

return C

if hist == [[D, D], [D, D], [C, D]]: # Rule: R2

return C

if hist == [[D, D], [C, D], [C, C]]:

return C

if hist == [[C, D], [C, C], [C, C]]:

return C

if hist == [[D, D], [D, D], [C, C]]: # Rule: R3

return C

if hist == [[D, D], [C, C], [C, C]]:

return C

axelrod/strategies/_strategies.py

drvinceknight · 2021-08-04T13:23:37Z

The tests are currently failing as the documentation does not build because of the way you have formatted your doctstring:

Warning, treated as error:
172
/home/runner/work/Axelrod/Axelrod/axelrod/strategies/grudger.py:docstring of axelrod.strategies.grudger.CAPRI:3:Unexpected indentation.
173
make: *** [Makefile:53: html] Error 2

Co-authored-by: Vince Knight <vince@vknight.org>

yohm · 2021-08-17T01:09:24Z

Thanks for this, I have made some initial stylistic comments but need to read the paper to offer a comprehensive review (planning on doing that soon as it looks interesting).

A comment: it does look like this strategy is a specific case of a axl.Lookerup (https://github.com/Axelrod-Python/Axelrod/blob/dev/axelrod/strategies/lookerup.py#L236) so it would be possible to implement this as a particular case which could help clear things up.

Since all the permutations must be specified to use axl.Lookerup, can I keep the implementation in the current form?

Co-authored-by: Vince Knight <vince@vknight.org>

yohm · 2021-08-17T01:19:41Z

The tests are currently failing as the documentation does not build because of the way you have formatted your doctstring:
Warning, treated as error:
172
/home/runner/work/Axelrod/Axelrod/axelrod/strategies/grudger.py:docstring of axelrod.strategies.grudger.CAPRI:3:Unexpected indentation.
173
make: *** [Makefile:53: html] Error 2

Thank you for pointing that out. Could you tell me how to test the docstring?

drvinceknight · 2021-08-23T10:21:16Z

axelrod/strategies/grudger.py

+    CAPRI is a memory-3 strategy proposed in [Murase2020]_. Its behavior is defined by the following five rules.
+    C: Cooperate at mutual cooperation.
+        This rule prescribes c at (ccc, ccc).
+    A: Accept punishment when you mistakenly defected from mutual cooperation.
+        This rule prescribes c at (ccd, ccc), (cdc, ccd), (dcc, cdc), and (ccc, dcc).
+    P: Punish your co-player by defecting once when he defected from mutual cooperation.
+        This rule prescribes d at (ccc, ccd), and then c at (ccd, cdc), (cdc, dcc), and (dcc, ccc).
+    R: Recover cooperation when you or your co-player cooperated at mutual defection.
+        This rule prescribes c at (ddd, ddc), (ddc, dcc), (dcc, ccc), (ddc, ddd), (dcc, ddc), (ccc, dcc), (ddc, ddc), and (dcc, dcc).
+    I: In all the ohter cases, defect.


Thank you for pointing that out. Could you tell me how to test the docstring?

Of course: top build the documentation, cd docs (go to the docs directory) and run:

make html

You might need to pip install some dependencies.

I have checked locally and below fixes the sphinx build problems:

Suggested change

CAPRI is a memory-3 strategy proposed in [Murase2020]_. Its behavior is defined by the following five rules.

C: Cooperate at mutual cooperation.

This rule prescribes c at (ccc, ccc).

A: Accept punishment when you mistakenly defected from mutual cooperation.

This rule prescribes c at (ccd, ccc), (cdc, ccd), (dcc, cdc), and (ccc, dcc).

P: Punish your co-player by defecting once when he defected from mutual cooperation.

This rule prescribes d at (ccc, ccd), and then c at (ccd, cdc), (cdc, dcc), and (dcc, ccc).

R: Recover cooperation when you or your co-player cooperated at mutual defection.

This rule prescribes c at (ddd, ddc), (ddc, dcc), (dcc, ccc), (ddc, ddd), (dcc, ddc), (ccc, dcc), (ddc, ddc), and (dcc, dcc).

I: In all the ohter cases, defect.

CAPRI is a memory-3 strategy proposed in [Murase2020]_. Its behavior is

defined by the following five rules.

- C: Cooperate at mutual cooperation. This rule prescribes c at (ccc, ccc).

- A: Accept punishment when you mistakenly defected from mutual cooperation.

This rule prescribes c at (ccd, ccc), (cdc, ccd), (dcc, cdc), and (ccc,

dcc).

- P: Punish your co-player by defecting once when he defected from mutual

cooperation. This rule prescribes d at (ccc, ccd), and then c at (ccd,

cdc), (cdc, dcc), and (dcc, ccc).

- R: Recover cooperation when you or your co-player cooperated at mutual

defection. This rule prescribes c at (ddd, ddc), (ddc, dcc), (dcc, ccc),

(ddc, ddd), (dcc, ddc), (ccc, dcc), (ddc, ddc), and (dcc, dcc).

- I: In all the ohter cases, defect.

Names:

- CAPRI: Original Name by Y. Murase et al. [Murase2020]_

nit: ohter -> other on line 362

Thank you for your help. I confirmed that the docstring is compiled without error in my environment.

drvinceknight · 2021-08-23T10:22:22Z

Since all the permutations must be specified to use axl.Lookerup, can I keep the implementation in the current form?

Sure, I don't feel strongly, this could always be refactored at a later date.

axelrod/strategies/grudger.py

axelrod/tests/strategies/test_grudger.py

drvinceknight · 2021-08-23T10:48:40Z

Everything is looking really good, I've left a couple of suggestions/comments, let me know if I can assist with anything.

FYI, re my comment about implementing this as a LookerUp you could also implement it as a particular FSM which would correct to Figure 2 (a) of the paper. Not at all suggesting you need to do this, I just liked that you added that in the paper :) (I like FSMs :))

marcharper · 2021-08-23T19:55:21Z

[No action required] Based on Figure 2 in the paper, it looks like the strategy could also be implemented as a finite state machine.
[Edit: not sure how I missed @drvinceknight's comment on this above!]

FYI the original implementation is here, perhaps that's worth documenting. (I don't see any tests though so I'm not sure if the code would be more helpful than the paper descriptions.)

Co-authored-by: Vince Knight <vince@vknight.org>

yohm · 2021-08-27T04:39:29Z

Everything is looking really good, I've left a couple of suggestions/comments, let me know if I can assist with anything.

Thank you for your help.

FYI, re my comment about implementing this as a LookerUp you could also implement it as a particular FSM which would correct to Figure 2 (a) of the paper. Not at all suggesting you need to do this, I just liked that you added that in the paper :) (I like FSMs :))

If you like FSM, you might find the following paper interesting :)
A method to translate LookerUp strategies into FSM is proposed.
https://www.nature.com/articles/s41598-020-70281-x

yohm · 2021-08-27T04:40:25Z

FYI the original implementation is here, perhaps that's worth documenting. (I don't see any tests though so I'm not sure if the code would be more helpful than the paper descriptions.)

I added a link to the repository in the docstring. Thank you for your suggestion.

drvinceknight · 2021-08-27T08:20:48Z

If you like FSM, you might find the following paper interesting :)
A method to translate LookerUp strategies into FSM is proposed.
https://www.nature.com/articles/s41598-020-70281-x

That looks very cool indeed, have added it to my reading list. @marcharper and @gaffney2010 designed an algorithm for finding the memory length of arbitrary FSMs (https://arxiv.org/abs/1912.04493) which is implemented in the library: https://axelrod.readthedocs.io/en/latest/discussion/strategy_archetypes.html?highlight=finite%20state%20machines#finite-state-machines implementing your algorithm to obtain the FSM from the LookerUp could be neat as well.

drvinceknight · 2021-08-27T08:35:25Z

axelrod/strategies/grudger.py

+        if hist == [[D, D], [D, C], [C, C]]:
+            return C
+        if hist == [[D, C], [C, C], [C, C]]:
+            return C


The CI is now failing due to coverage (which checks that all lines of code are hit by at least one test):

axelrod/strategies/grudger.py 160 2 99% 413, 419

This line (line 413) is not hit and also line 419.

These lines are never used because the conditions are duplicates of the previous lines. I commented out these lines.

to make the coverage 100%, codes that never run are commented out

marcharper · 2021-09-01T21:12:20Z

axelrod/strategies/grudger.py

+
+    def strategy(self, opponent: Player) -> Action:
+        # initial history profile is full cooperation
+        hist = [[C, C], [C, C], [C, C]]


It might be more readable to do something like:

hist = zip(self.history[-3:], opponent.history[-3:]) while len(hist) < 3: hist.push(0, (C, C))

Thank you for your suggestion. I polished up the code with 2252c88

Nikoleta-v3 · 2021-09-03T08:18:32Z

👋🏻 @yohm!

The CI is failing due to coverage:

axelrod/tests/strategies/test_grudger.py 200 5 98% 356-360

because you are not using the method assert_prescription anymore.

Nikoleta-v3 · 2021-09-03T08:29:31Z

Another comment I had was regarding 35d1890

I would delete these lines instead of commenting them out. Then you could add comments like,

if hist == [(D, C), (C, C), (C, C)]:  # Rule P and rule R1
    return(C)

or discuss in the docstring that in the implementation of the strategy we don’t need to check all the conditions for each rule.

p.s. Thank you for sharing this https://www.nature.com/articles/s41598-020-70281-x!

marcharper · 2021-09-03T17:10:51Z

Almost there @yohm !

yohm · 2021-09-07T01:26:30Z

@Nikoleta-v3 @marcharper Thank you for your comments. The code was revised according to your advice. See 3ff428b

drvinceknight · 2021-09-07T14:47:21Z

axelrod/strategies/_strategies.py

@@ -143,6 +143,7 @@
    OppositeGrudger,
    SoftGrudger,
    SpitefulCC,
+    Capri,


Can you move this so that the tuple is in alphabetical order please and then Capri needs to be added to the all_strategies list in this file. (This is why the CI is failing).

drvinceknight · 2021-09-13T08:59:22Z

The CI is failing because one of the meta strategies' behaviours has now changed (as it is using Capri). Could you change line 635 of test_meta.py so that actions is:

        actions = [(C, C), (C, D), (C, C), (D, D), (D, C)]

(Previously it was:

        actions = [(C, C), (C, D), (D, C), (C, D), (D, C)]

)

I have made the change locally and checked that the test_meta tests pass.

yohm · 2021-09-13T15:31:13Z

The CI is failing because one of the meta strategies' behaviours has now changed (as it is using Capri). Could you change line 635 of test_meta.py so that actions is:
        actions = [(C, C), (C, D), (C, C), (D, D), (D, C)]
(Previously it was:
        actions = [(C, C), (C, D), (D, C), (C, D), (D, C)]
)

I have made the change locally and checked that the test_meta tests pass.

Hopefully, it is fixed now.

marcharper · 2021-09-15T05:08:55Z

A couple of doc tests are failing, just need to bump up the counts:

======================================================================
FAIL: ./docs/index.rst
Doctest: index.rst
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/opt/hostedtoolcache/Python/3.7.11/x64/lib/python3.7/doctest.py", line 2204, in runTest
    raise self.failureException(self.format_failure(new.getvalue()))
AssertionError: Failed doctest test for index.rst
  File "./docs/index.rst", line 0

----------------------------------------------------------------------
File "./docs/index.rst", line 55, in index.rst
Failed example:
    len(axl.strategies)
Expected:
    238
Got:
    239


======================================================================
FAIL: ./docs/how-to/classify_strategies.rst
Doctest: classify_strategies.rst
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/opt/hostedtoolcache/Python/3.7.11/x64/lib/python3.7/doctest.py", line 2204, in runTest
    raise self.failureException(self.format_failure(new.getvalue()))
AssertionError: Failed doctest test for classify_strategies.rst
  File "./docs/how-to/classify_strategies.rst", line 0

----------------------------------------------------------------------
File "./docs/how-to/classify_strategies.rst", line 81, in classify_strategies.rst
Failed example:
    len(strategies)
Expected:
    55
Got:
    56


----------------------------------------------------------------------

marcharper · 2021-09-17T15:54:43Z

Looks like the last failing test is due to black -- the formatting axelrod/tests/strategies/test_grudger.py is not what it wants. See here for the command to reformat.

drvinceknight · 2021-09-20T15:46:46Z

This looks like everything is fixed now! Thanks for going through it all @yohm 👍

yohm added 2 commits August 3, 2021 15:51

added test cases for CAPRI

712148d

add reference and docstrings

dbedc90

drvinceknight requested changes Aug 4, 2021

View reviewed changes

yohm and others added 5 commits August 17, 2021 10:05

renamed the class following PEP8

463063e

Co-authored-by: Vince Knight <vince@vknight.org>

Update axelrod/strategies/grudger.py

f03755e

Co-authored-by: Vince Knight <vince@vknight.org>

Update axelrod/strategies/grudger.py

7f1a9ca

Co-authored-by: Vince Knight <vince@vknight.org>

Update axelrod/strategies/_strategies.py

4f470ef

Co-authored-by: Vince Knight <vince@vknight.org>

Update axelrod/tests/strategies/test_grudger.py

e7c7e8c

Co-authored-by: Vince Knight <vince@vknight.org>

Update axelrod/tests/strategies/test_grudger.py

4da4472

Co-authored-by: Vince Knight <vince@vknight.org>

drvinceknight reviewed Aug 23, 2021

View reviewed changes

axelrod/strategies/grudger.py Outdated Show resolved Hide resolved

drvinceknight reviewed Aug 23, 2021

View reviewed changes

axelrod/tests/strategies/test_grudger.py Outdated Show resolved Hide resolved

yohm and others added 2 commits August 27, 2021 13:06

Update docstring for CAPRI

332f946

Co-authored-by: Vince Knight <vince@vknight.org>

fixed the doc & add a link to the original implementation

24a46b1

drvinceknight reviewed Aug 27, 2021

View reviewed changes

commented out unused code

35d1890

to make the coverage 100%, codes that never run are commented out

marcharper reviewed Sep 1, 2021

View reviewed changes

yohm added 2 commits September 3, 2021 09:09

refactoring

2252c88

write test cases for noisy actions by specifying seeds

9c2c8f2

refactoring

3ff428b

Nikoleta-v3 approved these changes Sep 7, 2021

View reviewed changes

drvinceknight requested changes Sep 7, 2021

View reviewed changes

fixed all_strategies list

9c43fcb

fixed test

929c6f9

fixed doc tests

845e6cb

reformat with black

4fc6066

drvinceknight approved these changes Sep 20, 2021

View reviewed changes

drvinceknight added the ready-to-merge label Sep 20, 2021

marcharper merged commit 4ab64ab into Axelrod-Python:dev Sep 20, 2021

marcharper removed the ready-to-merge label Sep 20, 2021

yohm deleted the capri branch September 21, 2021 23:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add a new strategy CAPRI #1393

add a new strategy CAPRI #1393

yohm commented Aug 3, 2021

drvinceknight left a comment

drvinceknight Aug 4, 2021

marcharper Aug 7, 2021

drvinceknight Aug 4, 2021

drvinceknight commented Aug 4, 2021

yohm commented Aug 17, 2021

yohm commented Aug 17, 2021

drvinceknight Aug 23, 2021

marcharper Aug 23, 2021

yohm Aug 27, 2021

drvinceknight commented Aug 23, 2021

drvinceknight commented Aug 23, 2021

marcharper commented Aug 23, 2021 •

edited

Loading

yohm commented Aug 27, 2021

yohm commented Aug 27, 2021

drvinceknight commented Aug 27, 2021 •

edited

Loading

drvinceknight Aug 27, 2021

yohm Sep 1, 2021

marcharper Sep 1, 2021

yohm Sep 3, 2021

Nikoleta-v3 commented Sep 3, 2021

Nikoleta-v3 commented Sep 3, 2021

marcharper commented Sep 3, 2021

yohm commented Sep 7, 2021

drvinceknight Sep 7, 2021

yohm Sep 9, 2021

drvinceknight commented Sep 13, 2021

yohm commented Sep 13, 2021

marcharper commented Sep 15, 2021

marcharper commented Sep 17, 2021

drvinceknight commented Sep 20, 2021

add a new strategy CAPRI #1393

add a new strategy CAPRI #1393

Conversation

yohm commented Aug 3, 2021

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

drvinceknight commented Aug 4, 2021

yohm commented Aug 17, 2021

yohm commented Aug 17, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

drvinceknight commented Aug 23, 2021

drvinceknight commented Aug 23, 2021

marcharper commented Aug 23, 2021 • edited Loading

yohm commented Aug 27, 2021

yohm commented Aug 27, 2021

drvinceknight commented Aug 27, 2021 • 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

Nikoleta-v3 commented Sep 3, 2021

Nikoleta-v3 commented Sep 3, 2021

marcharper commented Sep 3, 2021

yohm commented Sep 7, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

drvinceknight commented Sep 13, 2021

yohm commented Sep 13, 2021

marcharper commented Sep 15, 2021

marcharper commented Sep 17, 2021

drvinceknight commented Sep 20, 2021

marcharper commented Aug 23, 2021 •

edited

Loading

drvinceknight commented Aug 27, 2021 •

edited

Loading