#16331: Game Theory: Build capacity to solve matching games in to Sage.
-------------------------------------+-------------------------------------
Reporter: vinceknight | Owner:
Type: enhancement | Status: needs_work
Priority: major | Milestone: sage-6.4
Component: game theory | Resolution:
Keywords: Game Theory, | Merged in:
Matching Games, | Reviewers: Karl-Dieter Crisman,
Authors: Vince Knight, | Travis Scrimshaw
James Campbell | Work issues:
Report Upstream: N/A | Commit:
Branch: | 27730d02de114bf1ea38f5b9a099198301fdd188
u/vinceknight/game_theory__build_capacity_to_solve_matching_games_in_to_sage_|
Stopgaps:
Dependencies: |
-------------------------------------+-------------------------------------
Changes (by vinceknight):
* commit: 508d8193ea3d99dda90515bced36c8cd8e747224 =>
27730d02de114bf1ea38f5b9a099198301fdd188
* branch: public/game_theory/solve_matching_games-16331 =>
u/vinceknight/game_theory__build_capacity_to_solve_matching_games_in_to_sage_
Comment:
Replying to [comment:87 kcrisman]:
> More minor things.
> * Confused here:
> {{{
> a bespoke creation of preferences
> }}}
> Is this a Britishism I'm not aware of?
Have reworded and added more docs.
> * I feel like `revr` shouldn't be the name of just reviewers in this
example but then not in the init... unless that is to support something
along the lines of `MatchinGame(suit, revr)`, as the code implies? But
then that should be doctested.
> {{{
>
> def __init__(self, generator, revr=None):
> r"""
> Initialize a matching game and check the inputs.
>
> TESTS::
>
> sage: suit = {0: (3, 4), 1: (3, 4)}
> sage: revr = {3: (0, 1), 4: (1, 0)}
> sage: g = MatchingGame([suit, revr])
> sage: TestSuite(g).run()
> }}}
> Indeed, all input methods should be mentioned fairly clearly somewhere
at the top (maybe even an `INPUT` block? Though this isn't a function...
hmm) and you don't use this on in the huge useful top block either. But
in either case I am pretty sure that
> {{{
> TypeError("generator must be an integer or a pair of 2 dictionaries")
> }}}
> is wrong or not tested.
Tests have been added now but I do think that we make use of this
(apologies if I'm misunderstanding). This setup allows us to either pass a
single number or a set of two dictionaries: both of these are used in the
top block (although I've now added more) and are doctested?
I've added the INPUT block to the top matter.
I have a feeling that I'm misunderstanding you here: so I apologise.
> * I'll let you figure out what is wrong with this one. I guarantee that
with correct input no one will ever notice it, though!
> {{{
> return 'A matching game with {} suitors and {}
reviewers'.format(
> len(self._reviewers), len(self._suitors))
> }}}
This one actually took me a while to figure it out! Now fixed.
> * Really?
> {{{
> def __eq__(self, other):
> return (isinstance(other, MatchingGame)
> and set(self._suitors) == set(other._suitors)
> and set(self._reviewers) == set(other._reviewers))
> }}}
> But... what about the preferences? Is it still the same game if they
are different? (Maybe Travis asked this at some point.)
Looking at git blame this is something that Travis put in so I'm not too
sure. Maybe this has something to do with the TestSuite, which I also
don't know much about...
Sorry to bring you back in to the conversation Travis?
I've added in a test that currently fails when two games are created with
reviewers and suitors with different preferences but won't change the code
unless I'm missing something that Travis can clarify (it could just be
easiest to remove the test if `__eq__` is in fact acting as it should. I'm
happy to tweak the code if needed to make that test pass.
> * Has not, presumably.
> {{{
> Raise an error if the game has been solved yet.
> }}}
Fixed.
> * In `is_complete` I'd still really like some checks for preference
tuples that are the "wrong size" somehow. For instance, someone
accidentally types in `(1,2,2,3)` instead of `(1,2,3)`.
We already have a check in there that ensures the size of both preference
dictionaries are the same but I've now added one to check that you don't
have any repetitions (for the particular case you've raised).
I've tried to think of other cases but perhaps I'm missing them.
> * I'm still not quite happy with the `add_suitor` (presumably also
reviewer) default. I like that you now have the error check for the name
being doubled, but I think that the ''automatic'' addition of a suitor
shouldn't raise this error. If `name = len(self._suitors) + 1` raises
that error, couldn't one just keep adding one until it works? Sorry about
asking this, but I just know we want to make this very user-friendly since
most targeted users of this will not be as familiar with Sage or math
programs...
No need to apologise at all: appreciate the in depth review.
I have changed this so that the automatic addition of a suitor will find a
name that's not already there.
> * I think you need some examples where someone creates a game, and then
all of a sudden realizes they need to add (say) a suitor and reviewer, or
maybe they just do them by hand, but anyway where they need to add
preferences one at a time. You don't really have a method for this, just
`Player.pref` the list of preferences. That should have several good
examples.
There is one example of adding all the preferences one at a time in the
front matter: it's around the `is one example where the preferences are
simply the corresponding` place (line 157: is that what you mean?).
I'm adding another similar one re comment below but let me know if I've
misunderstood.
> * Also, what if someone adds a player to a pre-existing game (well,
two!) and then has to update '''all''' preferences to make it complete?
Is that possible or supported? If not, how do we indicate this?
Have added a longer example showing this.
Note that current behaviour means that if you were to 'mix and match' then
the preferences are all wiped so you'd need to update them (the example
shows this).
The main reason for this is to ensure that no confusion occurs (less
chance of user error).
I don't think that in practice this would happen to often as in practice
if one has the preferences as dictionaries you would simply update the
dictionaries and recreate a game.
----
New commits:
||[http://git.sagemath.org/sage.git/commit/?id=03c87644fea6dbad4d2d7f2d3788e3d33d1e268d
03c8764]||{{{Rewording use of bespoke and adding more docs and tests to
clarify}}}||
||[http://git.sagemath.org/sage.git/commit/?id=18d1a323b29d39c6eed8a6bb02eb592131ad5cc7
18d1a32]||{{{Have added tests for generators}}}||
||[http://git.sagemath.org/sage.git/commit/?id=a5a8888b85765cfc7ff36b27a023232807ba5bcc
a5a8888]||{{{Adding INPUT block}}}||
||[http://git.sagemath.org/sage.git/commit/?id=268e046d219f9b6d09f715f078d8b058fa403639
268e046]||{{{Fixing repr}}}||
||[http://git.sagemath.org/sage.git/commit/?id=c4c6b3f271f49499faabed8ea85b081d6df81863
c4c6b3f]||{{{Adding test that fails for __eq__ method}}}||
||[http://git.sagemath.org/sage.git/commit/?id=de2c6da4717ca9dded5c2c38d4a293a1ba8fe737
de2c6da]||{{{Fixing missing word in docs for is_solved}}}||
||[http://git.sagemath.org/sage.git/commit/?id=b4d6fbea4a92988bc85a22cbe8a963b9502dc0ae
b4d6fbe]||{{{Adding tests and code to pick up preferences that contain
repetitions}}}||
||[http://git.sagemath.org/sage.git/commit/?id=677fd68e39d068912c947d8a7431bd82a93f351d
677fd68]||{{{Add reviewer and suitor default behaviour now makes sure
names are not repeated}}}||
||[http://git.sagemath.org/sage.git/commit/?id=27730d02de114bf1ea38f5b9a099198301fdd188
27730d0]||{{{Writing a big example showing how to add reviewer/suitor to
already created game}}}||
--
Ticket URL: <http://trac.sagemath.org/ticket/16331#comment:93>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/sage-trac.
For more options, visit https://groups.google.com/d/optout.
