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

Adds Dual Transformer and Joss-Ann Transformer #746 #758

Merged
merged 19 commits into from
Nov 6, 2016

Conversation

theref
Copy link
Member

@theref theref commented Nov 5, 2016

Adds Dual Transformer and Joss-Ann Transformer. These are both required when creating the fingerprint of a strategy as described in http://doi.org/10.1109/ITW.2010.5593352

These are no tests for these yet, but I thought I'd open a pull request to make sure people are happy with the implementation.

Edit - there are now tests.

def dual_strategy(self, opponent):
from copy import deepcopy
fresh_opponent = deepcopy(opponent)
self.original.play(fresh_opponent)
Copy link
Member

@meatballs meatballs Nov 5, 2016

Choose a reason for hiding this comment

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

should this be self.original_player.play?

That appears to be the name of the attribute which is set at line 187 in strategy_transformers.py

Copy link
Member

Choose a reason for hiding this comment

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

I believe this method is not actually necessary (the transformer does not use it). Can it just be removed?

Copy link
Member

Choose a reason for hiding this comment

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

(There's no need to modify the player class at all, it can all just be done as a transformer.)

def noisy_wrapper(player, opponent, action, noise=0.05):
"""Applies flip_action at the class level."""
r = random.random()
if r < noise:
return flip_action(action)
return action


Copy link
Member

Choose a reason for hiding this comment

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

👍 for fixing PEP8 errors!

MixedTransformer = StrategyTransformerFactory(
mixed_wrapper, name_prefix="Mutated")


def joss_ann_wrapper(player, opponent, proposed_action, probability):
"""Wraps the players strategy function to produce the Joss-Ann.
Copy link
Member

Choose a reason for hiding this comment

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

Why is it called the Joss-Ann? Is that a reference to some paper somewhere? If so, can we have a link in here?

Copy link
Member

Choose a reason for hiding this comment

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

Can we include the bibliographic reference too as well (so with [Ashlock...]_ and the relevant info in the bibliography.rst file.

return action


JossAnnTransformer = StrategyTransformerFactory(joss_ann_wrapper,
Copy link
Member

@meatballs meatballs Nov 5, 2016

Choose a reason for hiding this comment

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

Can we have both parameters on one new line to match the other transformer definitions?


The Dual of a strategy will return the exact opposite set of moves to the
original strategy when both are faced with the same history.

Copy link
Member

@meatballs meatballs Nov 5, 2016

Choose a reason for hiding this comment

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

This needs a reference to Ashlock's paper so we can see where it comes from.

Copy link
Member

Choose a reason for hiding this comment

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

👍

Copy link
Member

@drvinceknight drvinceknight left a comment

Choose a reason for hiding this comment

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

Various things.

Could you add a title to this PR and some context for any reviewer who might not be familiar.

Also: I suggest you add tests before you refactor, it'll make things easier to review but it will also make the refactoring simpler.

MixedTransformer = StrategyTransformerFactory(
mixed_wrapper, name_prefix="Mutated")


def joss_ann_wrapper(player, opponent, proposed_action, probability):
"""Wraps the players strategy function to produce the Joss-Ann.
Copy link
Member

Choose a reason for hiding this comment

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

Can we include the bibliographic reference too as well (so with [Ashlock...]_ and the relevant info in the bibliography.rst file.

probability += (remaining_probability,)
options = [C, D, proposed_action]
action = choice(options, p=probability)
return action
Copy link
Member

Choose a reason for hiding this comment

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

Could we not implement this as a particular case of the Mixed Strategy transformer (by mixing between the probe and Cooperator and Defector)? This would reduce duplication.

def dual_strategy(self, opponent):
from copy import deepcopy
fresh_opponent = deepcopy(opponent)
self.original.play(fresh_opponent)
Copy link
Member

Choose a reason for hiding this comment

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

I believe this method is not actually necessary (the transformer does not use it). Can it just be removed?


The Dual of a strategy will return the exact opposite set of moves to the
original strategy when both are faced with the same history.

Copy link
Member

Choose a reason for hiding this comment

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

👍

@theref
Copy link
Member Author

theref commented Nov 6, 2016

Can we include the bibliographic reference too as well (so with [Ashlock...]_ and the relevant info in the bibliography.rst file.

So should I put a link to the relevant paper (in both the Dual and JossAnn Transformers) and then also add something to /Axelrod/docs/reference/bibliography.rst?

@drvinceknight
Copy link
Member

Yes please.

@drvinceknight
Copy link
Member

(And in the docs you'll be able to point to the reference in the bibliography. Sphinx will render it nicely.)

@theref theref changed the title Issue 746 Adds Dual Transformer and Joss-Ann Transformer #746 Nov 6, 2016
@meatballs meatballs merged commit 10b3123 into Axelrod-Python:master Nov 6, 2016
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