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

Add new gradual and discussion of tournament #1299

Merged
merged 8 commits into from
Mar 27, 2020

Conversation

drvinceknight
Copy link
Member

Adds the new Gradual see discussion at #1294

Note I've gone with:

  • OriginalGradual (the version that was in the library and that was used in the 1997 paper)
  • Gradual (the one the authors apparently meant to use).

I've decided to not include mistrust or at least the broken version of mistrust that the authors used in #1294 that's needed to reproduce the results suggested in #1294, the authors meant for mistrust to be Suspicious Tit For Tat which we have in the library (and this name is already noted there).

Closes #1294

Added more to description as well.

I've also added the https://github.com/cristal-smac/ipd repo to our
bibliography. (Using 2018 which was the date of their first commit).
I've added the longer tests from the discussion at
#1294
Beaufils et al.'s tournament (1997)
-----------------------------------

In 1997, [Beaufils1997]_ presented used a tournament to describe a new strategy
Copy link
Member

Choose a reason for hiding this comment

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

typo here: 'presented used'


if len(self.history) == 0:
return C

Copy link
Member

Choose a reason for hiding this comment

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

niggle: too many empty lines (black might need running 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.

b7ff5c6 ran black on the file

@drvinceknight
Copy link
Member Author

No idea why the travis build is not appearing, it is happening here: https://travis-ci.org/github/Axelrod-Python/Axelrod/jobs/666857590?utm_medium=notification&utm_source=github_status

@drvinceknight
Copy link
Member Author

No idea why the travis build is not appearing, it is happening here: https://travis-ci.org/github/Axelrod-Python/Axelrod/jobs/666857590?utm_medium=notification&utm_source=github_status

I suggest not worrying about it (we can still make sure the tests pass of course by looking directly at travis) as we will be moving to GitHub actions (see #1297)

@drvinceknight
Copy link
Member Author

@meatballs meatballs self-requested a review March 25, 2020 16:24
@@ -466,6 +689,9 @@ def test_output_from_literature(self):
Dilemma" Proc. Artif. Life 1996

This test just ensures that the strategy is as was originally defined.

See https://github.com/Axelrod-Python/Axelrod/issues/1294 for another
discussion of this.
Copy link
Member

Choose a reason for hiding this comment

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

Should this be a test similar to test_specific_set_of_results for Gradual? Run the whole tournament and compare the scores?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure why not. 👍

Copy link
Member Author

Choose a reason for hiding this comment

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

Note that this will just repeat the doctest. (I'm fine with that.)

Copy link
Member

Choose a reason for hiding this comment

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

Sure. It already does that but only for two strategies right?

We are testing the output of the paper so I suggest we include all of it, and it will follow the same format as the tests for Gradual.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done in 1c07e88

Copy link
Member

@Nikoleta-v3 Nikoleta-v3 left a comment

Choose a reason for hiding this comment

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

Looks very good to me. Just have a minor point 👍

Instead of just checking two strategies, check the entire set of
reported results in the paper.
Co-Authored-By: Nikoleta Glynatsi <GlynatsiNE@cardiff.ac.uk>
@drvinceknight
Copy link
Member Author

I think this is good to be merged now?

@meatballs meatballs merged commit b8066cf into master Mar 27, 2020
@meatballs meatballs deleted the add-new-gradual-and-discussion-of-tournament branch March 27, 2020 10:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Possible incorrect implementation of Gradual
4 participants