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

Implement state distribution in result set. #742

Merged
merged 12 commits into from
Oct 16, 2016
Merged

Implement state distribution in result set. #742

merged 12 commits into from
Oct 16, 2016

Conversation

drvinceknight
Copy link
Member

Closes #738

Add state distribution and normalised state distribution to the result set.

Also adds the normalised state distribution to the summary.

Closes #738

Add state distribution and normalised state distribution to the result
set.
Also adds the normalised state distribution to the summary.
@@ -326,6 +326,26 @@ def build_ranking(self):
return sorted(range(self.nplayers),
key=lambda i: -nanmedian(self.normalised_scores[i]))

def build_normalised_state_distribution(self):
Copy link
Member

Choose a reason for hiding this comment

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

Needs an underscore? Seems like we're being inconsistent with the build* functions

Copy link
Member Author

Choose a reason for hiding this comment

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

👍

for player in self.state_distribution:
counters = []
for counter in player:
total = sum(counter.values(), 0.0)
Copy link
Member

Choose a reason for hiding this comment

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

Is the 0.0 necessary here? Is it possible for sum to be zero?

Copy link
Member Author

Choose a reason for hiding this comment

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

Looks like it wasn't needed. Have removed it.

@@ -401,6 +423,13 @@ def _update_cooperation(self, p1, p2, cooperations):
self.cooperation[p1][p2] += cooperations[0]
self.cooperation[p2][p1] += cooperations[1]

def _update_state_distribution(self, p1, p2, counter):
self.state_distribution[p1][p2] += counter
Copy link
Member

Choose a reason for hiding this comment

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

Docstring?

Copy link
Member Author

Choose a reason for hiding this comment

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

Added this and a bunch of others that were missing (whilst doing this I got annoyed at my prior lazy self).

def _update_state_distribution(self, p1, p2, counter):
self.state_distribution[p1][p2] += counter

counter[('C', 'D')], counter[('D', 'C')] = (counter[('D', 'C')],
Copy link
Member

Choose a reason for hiding this comment

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

I like the swap but not the line break.... I guess there's no getting around that, but maybe it would be more clear if broken after the = sign

Copy link
Member

Choose a reason for hiding this comment

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

Could shorten with C, D = actions.C, actions.D at the top of file...

"CC_rate", "CD_rate", "DC_rate",
"DD_rate"])

states = [('C', 'C'), ('C', 'D'), ('D', 'C'), ('D', 'D')]
Copy link
Member

Choose a reason for hiding this comment

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

Could use C, D = actions.C, actions.D

for i, player in enumerate(self.normalised_state_distribution):
counts = []
for state in states:
counts.append(sum([opp[state] for j, opp in enumerate(player)
Copy link
Member

Choose a reason for hiding this comment

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

Two lines here? One for the sum, one for the append so there's no line break

counts.append(sum([opp[state] for j, opp in enumerate(player)
if i != j]))
try:
counts = [c / sum(counts) for c in counts]
Copy link
Member

Choose a reason for hiding this comment

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

Any change for an integer division error here?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think so but I've added a property based test to just run a bunch of tournaments and attempt to summarise them...

Copy link
Member Author

Choose a reason for hiding this comment

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

What did you mean by an integer division error? (I might have misunderstood.)

Copy link
Member Author

Choose a reason for hiding this comment

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

I know what you mean. Am pushing a better hypothesis test and a fix.

self.cooperating_rating,
median_wins)]
summary_data = [self.player(rank, *summary_data[i]) for
summary_data = [self.player(rank, *(list(summary_data[i])
Copy link
Member

Choose a reason for hiding this comment

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

This is a bit hard to follow... could use a readability upgrade?

@@ -20,6 +20,10 @@ This tutorial will show you how to access the various results of a tournament:
- Payoff difference means: the mean score differences.
- Cooperation counts: the number of times each player cooperated.
- Normalised cooperation: cooperation count per turn.
- Normalised cooperation: cooperation count per turn.
- State distribution: the count of each type of state of a match
Copy link
Member

Choose a reason for hiding this comment

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

Might be good to explain what we mean by state distribution -- that it's two rounds of play rather than some other amount.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've added that in the wording below. Repeated it for both which might be a bit overkill, happy to tweak further :)

@meatballs meatballs merged commit 2a7edcf into master Oct 16, 2016
@meatballs meatballs deleted the 738 branch October 16, 2016 11:05
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.

None yet

3 participants