Integrate dojo params into Axelrod#1256
Conversation
|
I'll aim to take a close look through on Thursday 👍 |
| interaction_graph: Graph = None, | ||
| reproduction_graph: Graph = None, | ||
| fitness_transformation: Callable = None, | ||
| mutation_method="transition", |
There was a problem hiding this comment.
how do we feel about having booleans instead of passing a string argument?
atomic=False, transition=True
There was a problem hiding this comment.
You can't have both at the same time so I'd prefer not to use individual booleans. I considered using an enum instead.
|
This looks like it'll be a great addition/move. I think we might need to document some of this somewhere, perhaps in the docstrings of the particular sublayers or perhaps better to have a section in the documentation showing how one of them can be used and how it behaves (and pointing at all the others)? |
|
For sure, we'll want to add some more documentation. If we're settled on the API and other changes that I've made then I'll add some docs but I didn't want to yet in case there were some major objections. We may also want to move the rest of the dojo over -- it's basically just the two algorithms and the invocation scripts now. I'd prefer to do that in another PR though. That should be easy, this PR is the deeper integration. |
Yeah sounds good. I think I'm settled on the API, very nice work. Having some example use cases documented will just confirm that for me 👍
I'd prefer to leave the invocation scripts out of the library but I agree with the algorithms 👍 |
…t take a mutation_probability
marcharper
force-pushed
the
integrate_dojo
branch
from
09c21ed to
c7e6b61
Compare
marcharper
force-pushed
the
integrate_dojo
branch
from
a850ec9 to
e95b41c
Compare
drvinceknight
left a comment
drvinceknight
left a comment
There was a problem hiding this comment.
Just this little docstring please, everything looks good to me otherwise. 👍
|
Thanks @marcharper, this looks good to me. 👍 |
|
@meatballs ready for you when you get a chance |
3 participants
Re: #1240, this PR merges the Parameter classes for the training algorithms as evolvable players in Axelrod by introducing a subclass of each of the archetypes. The
init_kwargsattribute now does most of the heavy lifting and there are many new tests to ensure identical play.The dojo will also be simultaneously updated (see this associated PR).
There are a few uncovered lines and mypy tests to be fixed but I think it's ready for someone else to look at.