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

ZeroDivisionError in FirstByDowning #1285

Closed
marcharper opened this issue Mar 2, 2020 · 3 comments · Fixed by #1290
Closed

ZeroDivisionError in FirstByDowning #1285

marcharper opened this issue Mar 2, 2020 · 3 comments · Fixed by #1290
Labels

Comments

@marcharper
Copy link
Member

There's an implicit assumption that self.defections != 0 when turn > 3 in the calculation of beta but if the Match is noisy this can be violated:

>>> import axelrod as axl
>>> player1 = axl.FirstByDowning()
>>> player2 = axl.Defector()
>>> match = axl.Match((player1, player2), turns=3, noise=0.99)
>>> match.play()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/user/repos/axelrod/Axelrod/axelrod/match.py", line 189, in play
    self.players[0], self.players[1], self.noise)
  File "/home/user/repos/axelrod/Axelrod/axelrod/match.py", line 141, in simultaneous_play
    s1, s2 = player.strategy(coplayer), coplayer.strategy(player)
  File "/home/user/repos/axelrod/Axelrod/axelrod/strategies/axelrod_first.py", line 268, in strategy
    beta = (self.number_opponent_cooperations_in_response_to_D /
ZeroDivisionError: division by zero
@marcharper marcharper added the bug label Mar 2, 2020
@marcharper
Copy link
Member Author

marcharper commented Mar 2, 2020

This was found by hypothesis.

@marcharper
Copy link
Member Author

marcharper commented Mar 2, 2020

In #1288 I replaced the denominator with max(self.defections, 1). That removes the error but I'm not sure if it's precisely the right patch. Presumably it should be at least 2?

@drvinceknight
Copy link
Member

Yeah I think it should be:

max(self.defections, 2)

There's obviously a lot of ambiguity here but to keep things consistent with the interpretation currently on offer in the docstring I think that's the correct way to go.

marcharper added a commit that referenced this issue Mar 3, 2020
drvinceknight added a commit that referenced this issue Mar 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants