SteinAndRapoport Strategy

[MariosZoulias]

#379

New Strategy to be implemented
Please tell e also how to fix the test problem ;)

Thank you

[drvinceknight]

This is looking ready to take a look at, let's get the tests running first of all :)

This file: https://github.com/Axelrod-Python/Axelrod/blob/master/requirements.txt contains the list of all libraries needed by the library, with your contribution we need to add scipy to it, so change it to look like:

numpy>=1.9.2
matplotlib>=1.4.2
hypothesis>=3.2
tqdm>=3.4.0
prompt-toolkit>=1.0.7
scipy>=0.19.0

At present the tests are not even running on the continuous integration server because it cannot import the library as we haven't told it to install scipy. 👍

[drvinceknight]

Looking solid to me @MariosZoulias, once the test and coverage reports run there might be a few other things that come up.

Can you also modify https://github.com/Axelrod-Python/Axelrod/blob/master/docs/reference/overview_of_strategies.rst, that's the file that generates: http://axelrod.readthedocs.io/en/stable/reference/overview_of_strategies.html?highlight=Stein and we will need to modify the table to say the SteinAndRapoport is now implemented.

[drvinceknight]

Lines 7,8,9 of this file look like:

  7 from .axelrod_first import (                                                                                                                      
  8     Davis, RevisedDowning, Feld, Grofman, Nydegger, Joss, Shubik, Tullock,                                                                        
  9     UnnamedStrategy)  

So you can remove this line and just add SteinAndRapoport to the list imported there.

[drvinceknight]

I think you can remove this inline comment.

[drvinceknight]

else : should be else:

[drvinceknight]

round_number < 16 : should be round_number < 16:

[drvinceknight]

round_number < 5 : should be round_number < 5: (no space between 5 and :)

[drvinceknight]

This isn't quite formatted correctly (see: http://axelrod.readthedocs.io/en/stable/tutorials/contributing/strategy/docstrings.html).

Can you remove one indentation level for the description (it should be at the same level as the """).

Then can you add:

Names:
- SteinAndRapoport [Axelrod1980]_

[MariosZoulias]

Hello i made the changes ;)
The only thing is how can i do the change from not implemented to implemented???
Thank you
Marios

Forget it i think i just did it ;)

[drvinceknight]

Forget it i think i just did it ;)

I've closed #1014 as it would be better if that came as part of this PR. The file you need to modify is part of the repository so it is on your computer: open Axelroddocs/reference/overview_of_strategies.rst/ using any text editor and make the change. Then commit it and push :) (All on the same branch you're using for this PR).

[drvinceknight]

This is not indented on the correct level. It should look like:

class SteinAndRapoport(Player):
    ```
    A player who plays...
    ...
    ```

[drvinceknight]

Ok great, there are some failing tests but nothing we can't work through. To get us started, in Axelrod/docs/conf.py can you add scipy and scipy.stats to the modules that are ignored when building the documentation:

15 import sys                                                                                                                                      
   16 import os                                                                                                                                       
   17                                                                                                                                                 
   18 import mock                                                                                                                                     
   19                                                                                                                                                 
   20 MOCK_MODULES = [                                                                                                                                
>> 21     'scipy', 'scipy.stats', 'numpy', 'numpy.linalg', 'numpy.random', 'matplotlib.pyplot', 'matplotlib',                                         
   22     'matplotlib.transforms', 'mpl_toolkits.axes_grid1', 'dill', 'multiprocess',                                                                    23     'prompt_toolkit', 'prompt_toolkit.token', 'prompt_toolkit.styles',                                                                          
   24     'prompt_toolkit.validation']                                                                                                                   25 for mod_name in MOCK_MODULES:                                                                                                                   
   26     sys.modules[mod_name] = mock.Mock()              

I'm not around today but I'll look through everything else properly tomorrow.

[drvinceknight]

@MariosZoulias I've looked through this carefully and you've done a great job! It isn't completely correct though, instead of making multiple comments on the way I have made the changes and opened a Pull Request to your branch: MariosZoulias#1

If you merge that (on github by clicking on "Merge pull request") then that will automatically add my changes to here. Let me know if you have any questions.

[drvinceknight]

A small side note: your last two commit messages were "More changes" and "More and More changes" on http://axelrod.readthedocs.io/en/stable/tutorials/contributing/guidelines.html we briefly mention some guidelines about commit messages. It is good practice to write informative messages: this blog post https://chris.beams.io/posts/git-commit/ is quite a good description of what is meant by that :)

I'm not asking you to change anything about your previous commit messages just letting you know what's considered good practice :)

[MariosZoulias]

Hello .
Thank you for your support and advices.
I did the Pull Merge Request but when i firstly faced this problem (that the last chi-squared is not saved) i made a static list (which its first element was 0) and every 15 rounds doing the chi-squared i add to the end of the list the number 0 (if plays not random -->so i play titfortat) or 1 (if plays random --> so i defect).
So at every round i was just checking the last element of this list (chisquaredlist(-1) ) .
I thought i had made it :/ but according to the tests you are right ;)
Thanks a lot again

[drvinceknight]

i made a static list (which its first element was 0) and every 15 rounds doing the chi-squared i add to the end of the list the number 0 (if plays not random -->so i play titfortat) or 1 (if plays random --> so i defect).
So at every round i was just checking the last element of this list (chisquaredlist(-1) ) .

Cool, it was a good attempt but that wasn't working as you'd hoped.

As you had implemented it, that list was an internal variable for your method, and everytime the function was being called it was called with a new default version of that list:

def strategy(self , opponent: Player , chi_tests = [0]) -> Action:

So that list was never actually growing. Similarly we only ever cared about the last element of the list so that might as well be a simple boolean attribute and calling that variable is_opponent_random helps with readability of the code :)

You were only testing the exact play on the 15th round, and not the rounds thereafter which is why this slipped through. I added that last test which checks that the player defects for 15 rounds following the first test and then also that it tests again at round 30.

Let me know if you have any other questions.

---

I think we're really close to this being ready but as I PR'd (9c03615) a few changes I'll leave the merging decisions up to @marcharper and @meatballs (when they have time). They might have further requests.

Marc and Owen: one of the commits in this PR is 5a6cc56 which is due to appveyor having issues with the scipy dependency. As described in the commit message, this blog post is where the solution comes from: http://tjelvarolsson.com/blog/how-to-continuously-test-your-python-code-on-windows-using-appveyor/ In essence using conda on appveyor for scipy (and I threw numpy in there as well).

[meatballs]

I think this is a little clumsy and not as clear as it could be. How about:

"""Resets a player to its initial state

This method is called at the beginning of each match (between a pair of players) to reset a player's state to its initial starting point. It ensures that no 'memory' of previous matches is carried forward.

The default method resets a player's history, cooperations, defections and state_distribution. Players which have further attributes need to override this method and ensure those additional attributes are also reset.
"""

[drvinceknight]

Sorry @MariosZoulias, this has turned up in this PR because I put a line break on line 244 (I shouldn't have done that), the text wasn't changed by either of us but if you could make the improvements suggested by @meatballs that would be helpful. This file is in Axelrod/axelrod/player.py.

[meatballs]

Let's sort this out separately. I'll merge this PR and put another one in for the docstring

[drvinceknight]

Thanks for the merge @meatballs and thanks for the work on this @MariosZoulias 👍 I'll create a new release when I get a moment, this is a nice strategy to finally have added to the library.