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 EugineNier strategy from LessWrong #1016

Merged
merged 14 commits into from
May 25, 2017
Merged

Add EugineNier strategy from LessWrong #1016

merged 14 commits into from
May 25, 2017

Conversation

souravsingh
Copy link
Contributor

Adds a new strategy EugineNier.

Source here- http://lesswrong.com/lw/7f2/prisoners_dilemma_tournament_results/

return D
return C

if opponent.history[-5] == [D, D, D, D, D]:
Copy link
Member

Choose a reason for hiding this comment

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

This line of code is never hit, line 697 will end the function. It is also incorrect: it should be opponent.history[-5:] == [D, D, D, D, D]

Move these two lines to just after the initial cooperation and can you please add a test case for this particular instance.

@souravsingh
Copy link
Contributor Author

@drvinceknight I have made the changes, but I am not completely sure if the strategy written is entirely correct.

@drvinceknight
Copy link
Member

@drvinceknight I have made the changes,

Tests are failing because you did not push the full changes I requested. I am going to fix the strategy and push.

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.

Some further requests to include a test for that attribute I added (I should have done this: apologies).Please check the code snippets I wrote: I could have included a typo etc...

There might be other things that Marc and/or Owen raise as well but otherwise this looks good to me now.

self.second_play_test(rCC=C, rCD=D, rDC=C, rDD=D)

actions = [(C, C), (C, C), (C, C), (D, C)]
self.versus_test(axelrod.Cooperator(), expected_actions=actions)
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 add tests for the is_defector attribute please? (I should have thought of this when I pushed it @souravsingh).

So this would become:

self.versus_test(axelrod.Cooperator(), expected_actions=actions, 
                 attrs={"is_defector": False})

self.versus_test(axelrod.Cooperator(), expected_actions=actions)

actions = [(C, C), (C, C), (C, C), (C, C)]
self.versus_test(axelrod.Cooperator(), expected_actions=actions,
Copy link
Member

Choose a reason for hiding this comment

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

This would become:

self.versus_test(axelrod.Cooperator(), expected_actions=actions, 
                 attrs={"is_defector": False},
                 match_attributes={"length": -1})


# Plays TfT and defects in last round
actions = [(C, C), (C, D), (D, C), (C, D), (D, C), (D, D)]
self.versus_test(axelrod.Alternator(), expected_actions=actions)
Copy link
Member

Choose a reason for hiding this comment

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

self.versus_test(axelrod.Alternator(), expected_actions=actions,
                 attrs={"is_defector": False})

self.versus_test(axelrod.Alternator(), expected_actions=actions)

actions = [(C, C), (C, D), (D, C), (C, D), (D, C), (C, D)]
self.versus_test(axelrod.Alternator(), expected_actions=actions,
Copy link
Member

Choose a reason for hiding this comment

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

self.versus_test(axelrod.Alternator(), expected_actions=actions,
                 match_attributes={"length": -1},
                 attrs={"is_defector": False})

self.versus_test(axelrod.Alternator(), expected_actions=actions,
match_attributes={"length": -1})

actions = [(C, C), (C, D), (D, C), (C, D), (D, C), (D, D)]
Copy link
Member

Choose a reason for hiding this comment

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

This can be deleted, it's a duplication.

opponent = axelrod.MockPlayer(actions=[C, D, D, D, D, D, C, C])
actions = [(C, C), (C, D), (D, D), (D, D),
(D, D), (D, D), (D, C), (D, C)]
self.versus_test(opponent, expected_actions=actions)
Copy link
Member

Choose a reason for hiding this comment

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

self.versus_test(opponent, expected_actions=actions,
                 attrs={"is_defector": True})

match_attributes={"length": -1})


# Plays TfT and defects in last round
actions = [(C, C), (C, D), (D, C), (C, D), (D, C), (D, D)]
self.versus_test(axelrod.Alternator(), expected_actions=actions)
self.versus_test(axelrod.Alternator(), expected_actions=actions
Copy link
Member

Choose a reason for hiding this comment

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

This will fail a test: your comma is in the wrong place.


actions = [(C, C), (C, D), (D, C), (C, D), (D, C), (C, D)]
self.versus_test(axelrod.Alternator(), expected_actions=actions,
attrs={"is_defector": False}
Copy link
Member

Choose a reason for hiding this comment

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

This is missing a comma.

# Becomes defector after 5 defections
opponent = axelrod.MockPlayer(actions=[C, D, D, D, D, D, C, C])
actions = [(C, C), (C, D), (D, D), (D, D),
(D, D), (D, D), (D, C), (D, C)]
self.versus_test(opponent, expected_actions=actions)
self.versus_test(opponent, expected_actions=actions
Copy link
Member

Choose a reason for hiding this comment

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

Comma

@@ -633,17 +633,17 @@ def test_strategy(self):

# Plays TfT and defects in last round
actions = [(C, C), (C, D), (D, C), (C, D), (D, C), (D, D)]
self.versus_test(axelrod.Alternator(), expected_actions=actions
self.versus_test(axelrod.Alternator(), expected_actions=actions,
attrs={"is_defector": False},)
Copy link
Member

Choose a reason for hiding this comment

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

Remove the trailing comma (it is not needed).

def strategy(self, opponent: Player) -> Action:
if not self.history:
return C
if self.is_defector or opponent.history[-5:] == [D] * 5:
Copy link
Member

Choose a reason for hiding this comment

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

I prefer this split into:

if opponent.history[-5:] == [D] * 5:
    self.is_defector = True
if self.is_defector:
    return D
return opponent.history[-1]

Also, is it five defections ever? Or five consecutively?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe it is five consecutive defections

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 it is actually, here's the source from http://lesswrong.com/lw/7f2/prisoners_dilemma_tournament_results/:

Once the other player defects 5 times, switch to all defect.

So you can change this to be:

699         if (not self.is_defector) and opponent.history.count(D) >= 5:

(Checking not self.is_defector and ensures we only count if it's not a Defector.)

Could you adjust the tests so that we check five defections (not necessarily consecutively):

  643         # Becomes defector after 5 defections                                                                                                   
  644         opponent = axelrod.MockPlayer(actions=[D, C, D, D, D, D, C, C])                                                                         
  645         actions = [(C, D), (D, C), (C, D), (D, D),                                                                                              
  646                    (D, D), (D, D), (D, C), (D, C)]                                                                                              
  647         self.versus_test(opponent, expected_actions=actions)  

If you can please run those tests locally to make sure they work (I have just checked on my machine).

Copy link
Member

Choose a reason for hiding this comment

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

Please use opponent.defections instead of opponent.history.count(D), it's more efficientPlease use opponent.defections instead of opponent.history.count(D), it's more efficient

Copy link
Member

Choose a reason for hiding this comment

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

Please use opponent.defections instead of opponent.history.count(D), it's more efficient

Copy link
Member

Choose a reason for hiding this comment

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

Good call!

Copy link
Member

Choose a reason for hiding this comment

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

Slipped my mind :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Give me a few minutes.

@souravsingh
Copy link
Contributor Author

The changes are now revealing a weird error

FAIL: test_strategy (axelrod.tests.strategies.test_titfortat.TestEugineNier)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/sourav/Axelrod/axelrod/tests/strategies/test_titfortat.py", line 622, in test_strategy
    self.second_play_test(rCC=C, rCD=D, rDC=C, rDD=D)
  File "/home/sourav/Axelrod/axelrod/tests/strategies/test_player.py", line 482, in second_play_test
    rCC, C, C, seed=seed)
  File "/home/sourav/Axelrod/axelrod/tests/strategies/test_player.py", line 361, in test_responses
    test_class.assertEqual(s1, response)
AssertionError: 'D' != 'C'
- D
+ C

@drvinceknight
Copy link
Member

drvinceknight commented May 23, 2017

The changes are now revealing a weird error

The code I wrote passed all tests locally. Can you show us what you wrote?

EDIT: Here is the same code with the modification suggest which is to use opponent.defections:

  696     def strategy(self, opponent: Player) -> Action:                                                                                             
  697         if not self.history:                                                                                                                    
  698             return C                                                                                                                            
  699         if (not self.is_defector) and opponent.defections >= 5:                                                                                 
  700             self.is_defector = True                                                                                                             
  701         if self.is_defector:                                                                                                                    
  702             return D                                                                                                                            
  703         return opponent.history[-1]     

@souravsingh
Copy link
Contributor Author

Code for the strategy test

def test_strategy(self):
        self.first_play_test(C)
        self.second_play_test(rCC=C, rCD=D, rDC=C, rDD=D)

        actions = [(C, C), (C, C), (C, C), (D, C)]
        self.versus_test(axelrod.Cooperator(), expected_actions=actions,
                         attrs={"is_defector": False})

        actions = [(C, C), (C, C), (C, C), (C, C)]
        self.versus_test(axelrod.Cooperator(), expected_actions=actions,
                         attrs={"is_defector": False},
                         match_attributes={"length": -1})


        # Plays TfT and defects in last round
        actions = [(C, C), (C, D), (D, C), (C, D), (D, C), (D, D)]
        self.versus_test(axelrod.Alternator(), expected_actions=actions,
                         attrs={"is_defector": False})

        actions = [(C, C), (C, D), (D, C), (C, D), (D, C), (C, D)]
        self.versus_test(axelrod.Alternator(), expected_actions=actions,
                         attrs={"is_defector": False},
                         match_attributes={"length": -1})

        # Becomes defector after 5 defections
        opponent = axelrod.MockPlayer(actions=[D, C, D, D, D, D, C, C])
        actions = [(C, D), (D, C), (C, D), (D, D),
                   (D, D), (D, D), (D, C), (D, C)]
        self.versus_test(opponent, expected_actions=actions)

@souravsingh
Copy link
Contributor Author

Code for main strategy-

    def __init__(self):
        super().__init__()
        self.is_defector = False

    def strategy(self, opponent: Player) -> Action:
        if not self.history:
            return C
        if not (self.is_defector) and opponent.defections >= 5:
            self.is_defector = True
            return D
        return opponent.history[-1]

    def reset(self):
        super().reset()
        self.is_defector = False

@drvinceknight
Copy link
Member

if not (self.is_defector) and opponent.defections >= 5:
self.is_defector = True
return D

This is incorrect. See our suggestions earlier.

@souravsingh
Copy link
Contributor Author

All tests have passed. Pushing the commit

@drvinceknight
Copy link
Member

All tests have passed.

Great. Well done getting the tests working, in the future you can try and use the tests to help you debug your code: they will point out things that aren't correct.

@souravsingh
Copy link
Contributor Author

With this, I think we have crossed the 200 strategies mark.

@drvinceknight
Copy link
Member

With this, I think we have crossed the 200 strategies mark.

For the full tournament including the parametrized strategies: yup!

Names:

- Alexei's Strategy: [LessWrong2011]_
- Alexei's Strategy: [LessWrong2011]_
Copy link
Member

Choose a reason for hiding this comment

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

Is this correct? There's already a strategy called Alexei's strategy, and the name of this one is EugineNier.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My bad. I will fix it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe it is correct. I checked the source and I didn't find anything wrong with naming.

Copy link
Member

Choose a reason for hiding this comment

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

This is correct, it's the name for Alexei, it's just appearing in the diff because some white space is being cleared up on line 648.

Copy link
Member

Choose a reason for hiding this comment

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

However could you change this to just say:

- Alexei: [LessWrong2011]_

Names:

- Alexei's Strategy: [LessWrong2011]_
- Alexei's Strategy: [LessWrong2011]_
Copy link
Member

Choose a reason for hiding this comment

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

However could you change this to just say:

- Alexei: [LessWrong2011]_


Names:

- EugineNier Strategy: [LessWrong2011]_
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 change this to say:

- Eugine Nier: [LessWrong2011]_

@marcharper marcharper merged commit 592823b into Axelrod-Python:master May 25, 2017
@souravsingh souravsingh deleted the new-strategy branch May 25, 2017 00:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants