New strategy: Graaskamp

[kjurgielajtis]

Hi, I tried my best to contribute to issue #1109 :)

[drvinceknight]

Thanks for this :)

As we don't have any information on the "analogy" strategy I think we need to make that pretty clear in the docstring.

I'm not sure the entirety of the implementation is right just yet but I've left a few comments, once they're addressed we might need a few more test cases too.

Also, could you modify the table in https://github.com/Axelrod-Python/Axelrod/blob/master/docs/reference/overview_of_strategies.rst about the first tournament to include your implementation (which appears here: https://axelrod.readthedocs.io/en/stable/reference/overview_of_strategies.html#axelrod-s-first-tournament).

[kjurgielajtis]

Thank You for reply :) Now I see that I've made many mistakes, will correct them as soon as possible.

[drvinceknight]

Thanks for making some of the changes, there are some remaining requests. Let me know if anything's unclear :)

[drvinceknight]

We're getting there, the main test failure is due to the fact that sometimes your strategy doesn't return anything (I've pointed out where to fix that).

Thanks again, it's looking good 👍

[drvinceknight]

Graaskamp: 0.05 (the parameters get automatically added).

[kjurgielajtis]

I'm not sure what should I do here. Should I add parameters manually somehow for testing? Thanks :)

[marcharper]

Yes, you can change the test name to "Graaskamp: 0.05"

[drvinceknight]

These tests are a good start, once you've made the changes I've requested I think we'll need to change one or two and possibly include a few more to ensure we're checking all the described behaviours :) 👍

[kjurgielajtis]

I've added some more tests :) I think I've found a bug. After round 57 when playing with Grudger, Graaskamp only cooperates. I'm trying to fix this.

[drvinceknight]

I think I might have found what's wrong there: #1234 (comment)

[kjurgielajtis]

That's almost everything :) I'm not quite sure how to implement tests for opponents playing randomly because of almost 60 rounds of playing randomly and then taking action. How should it look like?

[drvinceknight]

Two minor points but I think I found what might be wrong with the behaviour against Grudger.

[drvinceknight]

No need for this, you can remove the if statement and simply unindent the rest of the code.

[drvinceknight]

We need to setup a next_random_defection_turn for the first one so this should be something like:

if self.next_random_defection_turn is None:
    self.next_random_defection_turn = random.randint(5, 15) + len(self.history) 

if len(self.history) == self.next_random_defection_turn:
    self.next_random_defection_turn = random.randint(5, 15) + len(self.history)  # resample the next defection turn
    return D
return C

[kjurgielajtis]

Bug where player only cooperates after round 56 still appears. I think it has to do with this line:

    len(self.history) == self.next_random_defection_turn:
        self.next_random_defection_turn` = random.randint(5, 15) + len(self.history)
        return D
    **return C**

[drvinceknight]

@kjurgielajtis I've opened kjurgielajtis#1 which includes a number of little tweaks. I wanted to be sure that any suggestions I made no longer included off by one errors etc so I checked them locally. You can either merge that PR (and the changes will automatically update) or just take a look and include the changes manually.

You'll notice there was an off by 1 error in the initial TfT play and that I've also written a number of tests and included a couple of comments to show how they match each case of the description. (I believe that catches them all but I might have missed something that the next reviewer might notice.)

[drvinceknight]

Closed by #1244