search

sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.37k stars 466 forks source link

Game Theory: build capacity to use gambit to solve Normal Form Games. #16333

Closed drvinceknight closed 9 years ago

drvinceknight commented 10 years ago

Build on other tickets (#16954 and #16466) to allow for the use of Gambit as an option when calculating Nash Equilibria.

Depends on #16466

Component: game theory

Keywords: Normal Form Games

Author: Vince Knight, James Campbell

Branch: 07b0bab

Reviewer: Jeroen Demeyer, Karl-Dieter Crisman

Issue created by migration from https://trac.sagemath.org/ticket/16333

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 9 years ago

Changed commit from 3d22981 to b336d6f

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 9 years ago

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

ffeab37Have parser documented and tested for the gambit LCP output
f90b13eHave added lots of tests that fail - more can be added
c28b547Changing name of gambit.py to gambit_doc.py and including code that
22fe917makes gambit and lrs tests optional, now pass when not installed
d8a2f1cIntegrating gambit in to NormalFormGame - all tests pass
b336d6fHave added gambit as a standalone package documentation
drvinceknight commented 9 years ago
comment:46

Ok :) Pretty sure I got it that time (tig is pretty sweet!). Thanks again.

jdemeyer commented 9 years ago
comment:47

Never use a bare except:, catch specific exceptions instead.

Also: fill in your Author name.

jdemeyer commented 9 years ago
comment:48

Without gambit installed:

sage -t --long src/sage/game_theory/normal_form_game.py                                                                                                                 
**********************************************************************                                                                                                  
File "src/sage/game_theory/normal_form_game.py", line 632, in sage.game_theory.normal_form_game.NormalFormGame.__init__                                                 
Failed example:                                                                                                                                                         
    error = NormalFormGame({4:6, 6:9})
Expected:
    Traceback (most recent call last):
    ...
    TypeError: Generator function must be a list, gambit game or nothing
Got:
    <BLANKLINE>
    Traceback (most recent call last):
      File "/usr/local/src/sage-git/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 488, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/usr/local/src/sage-git/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 850, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.game_theory.normal_form_game.NormalFormGame.__init__[35]>", line 1, in <module>
        error = NormalFormGame({Integer(4):Integer(6), Integer(6):Integer(9)})
      File "/usr/local/src/sage-git/local/lib/python2.7/site-packages/sage/game_theory/normal_form_game.py", line 652, in __init__
        raise TypeError("Generator function must be a list or nothing")
    TypeError: Generator function must be a list or nothing
**********************************************************************
File "src/sage/game_theory/normal_form_game.py", line 1154, in sage.game_theory.normal_form_game.NormalFormGame.obtain_nash
Failed example:
    g.obtain_nash(algorithm='lrs')
Exception raised:
    Traceback (most recent call last):
      File "/usr/local/src/sage-git/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 488, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/usr/local/src/sage-git/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 850, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.game_theory.normal_form_game.NormalFormGame.obtain_nash[11]>", line 1, in <module>
        g.obtain_nash(algorithm='lrs')
      File "/usr/local/src/sage-git/local/lib/python2.7/site-packages/sage/game_theory/normal_form_game.py", line 1236, in obtain_nash
        raise NotImplementedError("lrs is not installed")
    NotImplementedError: lrs is not installed
**********************************************************************
File "src/sage/game_theory/normal_form_game.py", line 1156, in sage.game_theory.normal_form_game.NormalFormGame.obtain_nash
Failed example:
    g.obtain_nash(algorithm='LCP')
Exception raised:
    Traceback (most recent call last):
      File "/usr/local/src/sage-git/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 488, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/usr/local/src/sage-git/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 850, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.game_theory.normal_form_game.NormalFormGame.obtain_nash[12]>", line 1, in <module>
        g.obtain_nash(algorithm='LCP')
      File "/usr/local/src/sage-git/local/lib/python2.7/site-packages/sage/game_theory/normal_form_game.py", line 1242, in obtain_nash
        raise NotImplementedError("gambit is not installed")
    NotImplementedError: gambit is not installed
**********************************************************************
jdemeyer commented 9 years ago

Changed branch from u/vinceknight/gambit_integration to u/jdemeyer/ticket/16333

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 9 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

0b67999Lazily import all of game theory
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 9 years ago

Changed commit from b336d6f to 0b67999

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 9 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

949bed8Fix documentation formatting
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 9 years ago

Changed commit from 0b67999 to 949bed8

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 9 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

aa5a8f7Remove bare "except:" statements
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 9 years ago

Changed commit from 949bed8 to aa5a8f7

6bdad4c1-1e26-4f2f-a442-a01a2292c181 commented 9 years ago
comment:54

Thanks for that initial explanation of rebase @nathanncohen. So ridiculously powerful! :)

http://www.clipartbest.com/cliparts/LcK/75X/LcK75Xpca.jpeg

Nathann

drvinceknight commented 9 years ago
comment:55

Just looked through those commits (tig for the win! <- you have seriously upgrading my git today). Thanks!

I'm correct in saying that we're now waiting for someone other than you to review right? (I don't review the changes you just made do I? Although I guess I've just done that...).

drvinceknight commented 9 years ago

Author: Vincent Knight, James Campbell

drvinceknight commented 9 years ago
comment:57

Should I/you add you as an author based on those commits? Apologies for the noobish questions. http://s3-ec.buzzfed.com/static/2014-10/26/6/enhanced/webdr08/enhanced-14836-1414320930-8.jpg

jdemeyer commented 9 years ago

Reviewer: Jeroen Demeyer

jdemeyer commented 9 years ago
comment:58

I don't feel like I should be author, consider my patches as "reviewer patches" even though I have no intention to actually review this ticket. I only looked at this ticket because of the is_package_installed() discussion we had. I noticed a few obvious opportunities for improvement, so I fixed those.

drvinceknight commented 9 years ago
comment:59

Great: thanks for clarifying and thanks for the help :)

kcrisman commented 9 years ago
comment:60

Don't worry, a review is coming, I was off internet all day yesterday but will probably look at this tonight :)

kcrisman commented 9 years ago
comment:61

Quite minor comments:

I ended up wrapping presents long enough that I won't get to a final review until Monday, but on the surface it looks like everything is just fine, I just want to make sure there isn't some edge case in the parsing and that the equilibria are okay. I am really surprised I missed so many formatting things before - thanks, Jeroen!

drvinceknight commented 9 years ago

Changed branch from u/jdemeyer/ticket/16333 to u/vinceknight/gambit_integration

drvinceknight commented 9 years ago

Changed commit from aa5a8f7 to 8f3cc37

drvinceknight commented 9 years ago

New commits:

8f3cc37Fixing incorrect optional tag
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 9 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

aa3fc67Fixing wording for LCP algorithm differences
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 9 years ago

Changed commit from 8f3cc37 to aa3fc67

drvinceknight commented 9 years ago
comment:64

Replying to @kcrisman:

Quite minor comments:

  • # optional - LCP - should this be gambit?
  • the output differs to the other algorithms - from the other ones?

Correct on both accounts. Both fixed now (thanks!).

I ended up wrapping presents long enough that I won't get to a final review until Monday, but on the surface it looks like everything is just fine, I just want to make sure there isn't some edge case in the parsing and that the equilibria are okay. I am really surprised I missed so many formatting things before - thanks, Jeroen!

Thanks both and if this waits till after Christmas it's not the end of the world :) Happy holidays both :)

kcrisman commented 9 years ago
comment:65
  • # optional - LCP - should this be gambit?

There were two of these, one still is there ;-)

Thanks both and if this waits till after Christmas it's not the end of the world :) Happy holidays both :)

I have something else to take care of first but hope to get to it before the end of today.

kcrisman commented 9 years ago
comment:66
+It is also possible to generate a Normal Form Game from a gambit Game::
+

This would be a good place to refer to the gambit document or remind why there are all these ints.

is evidenced by the two algorithms returning different solutions::

Maybe better now is "the various algorithms"?

Finally, I think the parser is working fine but could use a little extra tlc if and when games with more than 2 players are implemented, as it is a very long series of things to parse for the reader (I mean the code like profile.append(tuple(gambitstrategy[previousplayerstrategylength: previousplayerstrategylength + len(player.strategies)]))) though I agree it should work correctly.

I don't see any other issues. Thanks for breaking all of these tickets apart! It really made it a lot easier to get figured out.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 9 years ago

Changed commit from aa3fc67 to 0c6cc65

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 9 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

9b59a46Fixed reference to just two algorithms
2533822Have added link to gambit docs from norma form game docs at relevant point
0c6cc65Merge branch 'u/vinceknight/gambit_integration' of git://trac.sagemath.org/sage into rebased_version
drvinceknight commented 9 years ago
comment:68

Replying to @kcrisman:

+It is also possible to generate a Normal Form Game from a gambit Game::
+

This would be a good place to refer to the gambit document or remind why there are all these ints.

I've added a pointer to the gambit documentation. Let me know if you think I've said too much/little.

is evidenced by the two algorithms returning different solutions::

Maybe better now is "the various algorithms"?

Yup: have put that in.

Finally, I think the parser is working fine but could use a little extra tlc if and when games with more than 2 players are implemented, as it is a very long series of things to parse for the reader (I mean the code like profile.append(tuple(gambitstrategy[previousplayerstrategylength: previousplayerstrategylength + len(player.strategies)]))) though I agree it should work correctly.

I agree, I'm hoping that future dev will further integrate all the gambit algorithms, as this happens the parser will be in turn expanded. The next thing I'll be working on is #17392 though :)

I don't see any other issues. Thanks for breaking all of these tickets apart! It really made it a lot easier to get figured out.

Thank you for all your help: I'm planning on finding time to try and review some tickets myself :)

kcrisman commented 9 years ago
comment:69

A few more things, largely about the gambit docs document.

The `python API documentation for gambit
<http://www.gambit-project.org/gambit14/pyapi.html>_`

I think the syntax needs to put the underscore after the backtick.

Otherwise I think we are okay.

kcrisman commented 9 years ago

Changed reviewer from Jeroen Demeyer to Jeroen Demeyer, Karl-Dieter Crisman

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 9 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

70b61abFixing capitalisation of ipython
cc282fbFixing syntax for hyperlink
c9f036aFixing syntaxing
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 9 years ago

Changed commit from 0c6cc65 to c9f036a

drvinceknight commented 9 years ago
comment:72

Sorry for the late reply to this: I've had a pretty nice internet fast :) Hope you all had a great holiday season and happy new year!

Replying to @kcrisman:

A few more things, largely about the gambit docs document.

  • It's capitalized IPython, for whatever reason - the gambit docs thing is not correct that way, I missed it earlier.

Pretty sure I caught these now.

  • Here
The `python API documentation for gambit
<http://www.gambit-project.org/gambit14/pyapi.html>_`

Yup: have fixed.

I think the syntax needs to put the underscore after the backtick.

  • There are several things in single backticks (like `LCP`, but look for them, it's not the only one) that should be in double backticks ``LCP``.

Looked for these and think there was just that one that I recognised. I did find various things in single back ticks but I think there done so correctly, I might well be confused by the syntax though: if you could point out one that is not right that should clarify what I'm misunderstanding and I'll go ahead and fix all the rest. (Apologies for not finding these, have built the docs and read through but think I'm just missing it).

Thanks again :)

Otherwise I think we are okay.

kcrisman commented 9 years ago
comment:73

Sorry for the late reply to this: I've had a pretty nice internet fast :)

A good idea! I also (nearly) did that.

  • It's capitalized IPython, for whatever reason - the gambit docs thing is not correct that way, I missed it earlier.

Pretty sure I caught these now.

This commit actually makes it worse; it changes one IPython and then doesn't correctly fix an ipython. Why don't you back up to before that commit and fix them correctly - all should be first two letters caps, rest lowercase.

Looked for these and think there was just that one that I recognised.

I think you got 'em.

kcrisman commented 9 years ago
comment:74

Just had to check the look of the docs again. In addition to getting the IPython thing right (there are two total in the gambit docs and one in the normal form game file), I notice that in the gambit docs file

Here is how to use the ExternalEnumPureSolver

has ExternalEnumPureSolver in single backticks, but this makes it look like a math variable instead of code. I recommend double backticks or nothing; similarly for

If one really wants to use gambit directly in Sage (without using the NormalFormGame class as a wrapper)

Not sure why I didn't mention those before, I did see them.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 9 years ago

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

f684012Fixing incorrect optional tag
93bd1d6Fixing wording for LCP algorithm differences
4e2afbdFixing syntaxing
b0142f5Have fixed Ipython capitalisation
3a0b97eAdding correct backticks
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 9 years ago

Changed commit from c9f036a to 3a0b97e

drvinceknight commented 9 years ago
comment:76

I think I caught everything although I made a slight mess with the rebase (Still learning... Will get it eventually) so apologies if the commits do not make the most sense. If it's confusing just let me know and I'll fix them.

kcrisman commented 9 years ago
comment:77

You unfortunately reverted or something the fix for one of the lcp optional tests (should be gambit):

+            sage: LCP_eqs == sorted(LCP_eqs)  # optional - LCP
+            True

Sorry, I shouldn't have asked you for some wacky thing, just trying to save a commit...

Otherwise all set!

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 9 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

8f3cc37Fixing incorrect optional tag
aa3fc67Fixing wording for LCP algorithm differences
0c6cc65Merge branch 'u/vinceknight/gambit_integration' of git://trac.sagemath.org/sage into rebased_version
70b61abFixing capitalisation of ipython
cc282fbFixing syntax for hyperlink
c9f036aFixing syntaxing
cc60552Merge conflict
07b0babHave corrected optional test
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 9 years ago

Changed commit from 3a0b97e to 07b0bab

drvinceknight commented 9 years ago
comment:79

Replying to @kcrisman:

You unfortunately reverted or something the fix for one of the lcp optional tests (should be gambit):

+            sage: LCP_eqs == sorted(LCP_eqs)  # optional - LCP
+            True

Sorry, I shouldn't have asked you for some wacky thing, just trying to save a commit...

Not at all: I appreciate the opportunity to try and learn :) Haven't quite gotten it (this push is a single commit for example that seems to have grabbed more...) but I will!

Otherwise all set!

Awesome! The next thing will be that list of games :)

vbraun commented 9 years ago

Changed branch from u/vinceknight/gambit_integration to 07b0bab

kcrisman commented 9 years ago

Changed author from Vincent Knight, James Campbell to Vince Knight, James Campbell

kcrisman commented 9 years ago

Changed commit from 07b0bab to none

kcrisman commented 9 years ago
comment:82

Vince, I guess you are Vince from now on if we want to keep credit to the same individual :)