-
Notifications
You must be signed in to change notification settings - Fork 269
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
Implement prober4 strategy from PRISON project #743
Conversation
The class implements prober4 strategy from PRISON project.
Calculate defections on the fly. Place the strategy logic in a single function.
Tweak Prober4 property names
Fix bug - reverse self.turned_defector condition. Refine docstring. Tweak formatting. Update parameter name in reset method. Rewrite tests to loop over set of histories.
name = 'Prober 4' | ||
classifier = { | ||
'stochastic': False, | ||
'memory_depth': 1, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
memory_depth: float('inf')
self.init_sequence = [ | ||
C, C, D, C, D, D, D, C, C, D, C, D, C, C, D, C, D, D, C, D | ||
] | ||
self.just_Ds = 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a meta comment for everyone (@drvinceknight @meatballs): we could consider counting CC, CD, DC, DD in the player class like we do for cooperations and defections, since multiple players make use of it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 We should open an issue for that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
def strategy(self, opponent): | ||
if len(opponent.history) == 0: | ||
return self.init_sequence[0] | ||
if len(self.history) == 0: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can just check one of the histories, they should never be of differing lengths.
return self.init_sequence[0] | ||
if len(self.history) == 0: | ||
return self.init_sequence[0] | ||
else: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can drop the else here since the above if
statements return if activated
if not self.turned_defector: | ||
if self.cooperation_pool: | ||
return self.cooperation_pool.pop() | ||
else: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can drop the else
Player.reset(self) | ||
self.just_Ds = 0 | ||
self.unjust_Ds = 0 | ||
self.cooperation_pool = [C] * 5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(minor) I think you could rewrite to eliminate the need for the cooperation_pool
by checking if turns < len(self.init_sequence) + 5
player = axelrod.Prober4 | ||
expected_classifier = { | ||
'stochastic': False, | ||
'memory_depth': 1, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
float('inf')
history2 += [D] | ||
self.responses_test(history1, history2, [D]) | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we test that turned_defector
is being set correctly? responses_test
can take an attrs
parameter (a dictionary) that checks the state of internal variables on a player.
Thanks @mturzanska ! I'm requesting some minor changes, looks pretty good overall! |
I have nothing to add to @marcharper's comments so I'll give this a 👍 from me subject to his requested changes. Marc once you're happy please merge in :) Thanks for another great contribution @mturzanska |
Set memory_depth to float('inf'). Remove redundant history length check. Remove redundant else statements. Remove Prober4.cooperation_pool and replace with history length check. Test Prober4.turned_defector value. Revert the update of strategies counter in doc tests.
No problem. I made the changes and double-checked but let me know if anything got overlooked. |
self.turned_defector = False | ||
|
||
def strategy(self, opponent): | ||
if len(self.history) == 0: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might be better as simply:
if not self.history:
Related to #379