Skip to content

37 - Prob end and rewrite of the results #507

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

Merged
merged 49 commits into from
Mar 29, 2016
Merged

37 - Prob end and rewrite of the results #507

merged 49 commits into from
Mar 29, 2016

Conversation

drvinceknight
Copy link
Member

This adds to #496 (which could be closed).

This is mainly a rewrite of the results module. Now it is initialised by being passed a set of interactions (list of dictionaries mapping indices to matches). This has the main advantage of simplifying things. No more results.results, results.output and results.outcome which were various forms of holding the same data, now all data is basically in results.matches.

To do this:

  • I added various methods to the Match class: these can be used to calculate more or less everything.
  • I've rewritten all the method to calculate the results, for simplicity I have moved them out of payoff.py (which I think could be removed) and mostly moved everything out of cooperation.py. I'm not sure I see the advantage of that extra layer but perhaps I'm missing something. All result calculations are done by manipulating matches and aim to make no assumptions about what matches are actually played.

This second point has the advantage of making the probabilistic end tournament very simple and in fact I've gotten rid of the ProbEndResults class as it is not at all needed.

The Results class is now in essence a class that computes a standard set of results on any set of matches (even the tests examples I used in the class are not a standard round robin).

One downside is that it is not taking advantage of the length of a match being constant for a standard tournament but I'd suggest that if need be it's easy to inherit and build a Results class that took advantage of the structure but I don't think that's at present necessary.

Furthermore if at any point we wanted to add to the standard set of results it's just a question of adding a method that only has one type of data source to deal with: the matches. I've kind of done an example of this: the results class now also has a match lengths attribute with just shows the lengths of matches played by each player (useless for standard tournament but neat to see expected results for prob end).

Here's a notebook (which I will at some point tweak): https://github.com/Axelrod-Python/Axelrod-notebooks/blob/37/basic-prob-end-tournament.ipynb

Looking forward to hearing thoughts on this.

PS I actually changed one doctest in the morality metrics. I believe that was not quite being calculated consistently. Might be worth having a specific look at to make sure I'm correct. It's an easy fix back if I'm wrong.

Not terribly useful but I thought it made some of the tournament code
I'm writing a tiny bit more readable.
Working through tests. Work also required on the results.
Am not sure what behaviour I actually want yet so commenting out. Need
to work on the results set.
Every tournament is going to need the potential to have it's own Results
class (not all current graphical outputs make sense for all potential
tournament types).
Just to be able to loop over players if need be.
- normalised_scores_diff_length
- normalised_payoff_diff_length
- Also minor change to wording in docstring (both roots are real)
Replacing with players. This seems to be more in line with the rest of
the library and obviously loses no functionality.

I've chosen to make the internal attribute a list to be in line with
tournament attribute (also players, also a list).
Result class directly calculates everything from Matches
Init takes list of players and set of matches.

No assumptions are made about the structures of the Matches passed.

To do this, the Match class is made more powerful (it basically does all
the work):

- Adding a score method.
- Adding a score per turn method to Match class.
- Adding cooperation count to match class.
I believe there was a bug in the morality metric bug.

It took count of plays vs self but other such metrics do not. This is
easily fixable.
@drvinceknight
Copy link
Member Author

Thanks @marcharper, I've made those changes. For info in 627fe84 I've added a property based test that checks that ProbEndTournament runs fine for a variety of tournaments (replicating the test that found that bug with MindReader and BackStabber).

@meatballs
Copy link
Member

Sorry, got a bit overtaken by incoming stuff today. It will have to be tomorrow now

@drvinceknight
Copy link
Member Author

Sorry,

No problem at all, whenever you have time :)

@meatballs
Copy link
Member

I'm starting to look at this and, overall, I think it looks good. One comment strikes me immediately:

I have moved them out of payoff.py (which I think could be removed) and mostly moved everything out of cooperation.py

If that's the case, then could you remove the redundant methods from those two modules? If either ends up empty as a result, then remove the module.

I think the rest of my comments are likely to be picky stuff!!

@drvinceknight
Copy link
Member Author

If that's the case, then could you remove the redundant methods from those two modules? If either ends up empty as a result, then remove the module.

Yeah: good shout. On it! :)

@drvinceknight
Copy link
Member Author

If either ends up empty as a result, then remove the module.

bea72af has done this (removed both the modules).

outcome (dict): returned from the Tournament class and containing
various sets of results for processing by this class.
matches (list): a list of list of completed matches
(1 for each repetition)
with_morality (bool): a flag to determine whether morality metrics
should be calculated.
"""
Copy link
Member

Choose a reason for hiding this comment

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

This whole docstring needs to be in numpy format as we agreed in #347

Whilst we've not bothered going through existing docstrings and changing them in one go for that ticket, we have been doing so whenever they've been 'touched' as part of other changes.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup: good catch. I'm on it.

@drvinceknight
Copy link
Member Author

I think 796a4ae has got the numpy formatting but let me know if not.

self.good_partner_matrix, len(players), repetitions)
self.eigenjesus_rating = ac.eigenvector(self.normalised_cooperation)
self.eigenmoses_rating = ac.eigenvector(self.vengeful_cooperation)
self.matches = matches # List of dicts mapping tuples of players to matches
Copy link
Member

Choose a reason for hiding this comment

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

the comment here doesn't match with the docstring. I suggest it gets deleted and the docstring rules

Copy link
Member Author

Choose a reason for hiding this comment

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

The docstring is actually incorrect, it's not a list of lists but a list of dictionaries. Fixing that now.

Also fixing docstring def for matches.
@drvinceknight
Copy link
Member Author

402ae74 gets all the numpy formatting.

I've realised that the shorter methods in the Match class (some new, some old) haven't stuck strictly to it. I don't personally mind (they are one line docstrings) but I can patch them up if we want.

@meatballs
Copy link
Member

Not worth the effort, I reckon

@meatballs
Copy link
Member

I'm happy with this once travis passes.

@drvinceknight
Copy link
Member Author

I'm happy with this once travis passes.

🎆 🎉 👍 🙇

@meatballs
Copy link
Member

🎆 🎉 👍 🙇

You might get to return to the real world for a while

@drvinceknight
Copy link
Member Author

You might get to return to the real world for a while

I hope it's still there...

Have just opened #509 and #510 which are logical enhancements and pretty straightforward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants