drvinceknight
commented
10 years ago comment:1
Listening in.
Closed d3dfe940-4ef4-4242-81e0-6916cce90212 closed 9 years ago
drvinceknight
commented
10 years ago Listening in.
d3dfe940-4ef4-4242-81e0-6916cce90212
commented
10 years ago d3dfe940-4ef4-4242-81e0-6916cce90212
commented
10 years ago New commits:
59193a2 | creates directory structure |
147768b | add basic install and version |
4a8074b | installs gambit without python extension |
16948eb | updates spkg-install to install python api |
0df6e07 | deletes files |
fac6806 | updatees spkg.txt |
72bdb70 | adds dependencies to spkg.txt |
a57121d | Merge branch '16466' into t/16466/add_gambit_as_an_optional_package |
d3dfe940-4ef4-4242-81e0-6916cce90212
commented
10 years ago Commit: a57121d
edd8e884-f507-429a-b577-5d554626c0fe
commented
10 years ago Hi, i did not try to install the package, but here are a few suggestions for SPKG.txt :
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313
commented
10 years ago 7ed8c4ca-6d56-4ae9-953a-41e42b4ed313
commented
10 years ago edd8e884-f507-429a-b577-5d554626c0fe
commented
10 years ago I installed it without problem, and could try some python examples, so it looks good to go. As minor cosmetic changes, could you just:
* Website: http://www.gambit-project.org/
* Mailing list: http://sourceforge.net/p/gambit/mailman/gambit-devel/(those mailing-lists are not easy to find from the website, and may be useful if one need to report some issues upstream)
edd8e884-f507-429a-b577-5d554626c0fe
commented
10 years ago Reviewer: Thierry Monteil
edd8e884-f507-429a-b577-5d554626c0fe
commented
10 years ago Author: James Campbell
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313
commented
10 years ago 7ed8c4ca-6d56-4ae9-953a-41e42b4ed313
commented
10 years ago 
kcrisman
commented
10 years ago Awesome work! Sorry I was awol for a few days on the game theory stuff - and likely to be again, though I will try to ask about it at the Social Choice and Welfare conference next week.
edd8e884-f507-429a-b577-5d554626c0fe
commented
10 years ago Description changed:
---
+++
@@ -1,2 +1,3 @@
Include [gambit](http://gambit.sourceforge.net/gambit13/index.html) as one the optional packages that can be installed with sage.
-The tarball can be found [here](http://gambit.sourceforge.net/gambit13/index.html).
+The tarball can be found [here](http://sourceforge.net/projects/gambit/files/gambit13/13.1.2/gambit-13.1.2.tar.gz/download).
+Thanks Karl! James has been making good progress. We're getting in to the intricacies of the project but what's really great is that Ted Turocy (head gambit person) is very keen and making certain tweaks to gambit to make things play nice with Sage). Will keep you up to speed as we carry on :)
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313
commented
10 years ago 7ed8c4ca-6d56-4ae9-953a-41e42b4ed313
commented
10 years ago d3dfe940-4ef4-4242-81e0-6916cce90212
commented
10 years ago Description changed:
---
+++
@@ -1,3 +1,3 @@
Include [gambit](http://gambit.sourceforge.net/gambit13/index.html) as one the optional packages that can be installed with sage.
-The tarball can be found [here](http://sourceforge.net/projects/gambit/files/gambit13/13.1.2/gambit-13.1.2.tar.gz/download).
+The tarball can be found [here](http://www.vincent-knight.com/static/gambit/gambit-14.0.2.tar.gz).
Hi Vince, the tarball you proposes for upload is not the same as the upstream one (whose sha1sum is 47696dce362653ed5536b7b18dec475bbd5f4313). It is better use the original one, that can be downloaded at http://sourceforge.net/projects/gambit/files/gambit14/14.0.2/. If you need to modify it to let it work with Sage, you should add patches in the SAGE_ROOT/build/pkgs/gambit/patches/ directory and apply them from the spkg-install script.
While you are at it, could you please add a newline at the end of the files package-version.txt, spkg-install and SPKG.txt ?
d3dfe940-4ef4-4242-81e0-6916cce90212
commented
10 years ago At the moment there exists a Gambit branch that is exclusively for optimising gambit for sage. Any changes that are made there will hopefully be merged into Gambit's next release (due at the end of August I believe).
The changes that have been made so far are requited for ticket #16333 and so we thought it best to switch to temporarily hosting our own version of the Gambit tarball so that when people come to review that ticket, they will be using the correct version of Gambit.
Once the next release is sorted this should not be a problem and the tarball can revert back to the one at sourceforge.net
kcrisman
commented
10 years ago So... what's the status on this? I finally have time to look at these tickets seriously again and would really like to do so over the next couple days - especially since the knot theory and Android app summer projects were successful :)
drvinceknight
commented
10 years ago Description changed:
---
+++
@@ -1,3 +1,3 @@
Include [gambit](http://gambit.sourceforge.net/gambit13/index.html) as one the optional packages that can be installed with sage.
-The tarball can be found [here](http://www.vincent-knight.com/static/gambit/gambit-14.0.2.tar.gz).
+The tarball can be found [here](http://vincent-knight.com/other/gambit-14.0.2.tar.gz).
Replying to @kcrisman:
So... what's the status on this? I finally have time to look at these tickets seriously again and would really like to do so over the next couple days - especially since the knot theory and Android app summer projects were successful :)
That would be really great Karl!
Things have been pretty busy recently and I'm only just getting back to trying to find reviewers for the two main tickets (I'm not counting this one).
I've actually had to change the url for this one to: http://vincent-knight.com/other/gambit-14.0.2.tar.gz
I'm not entirely sure I've done that right though... James took care all this and I can't say I looked in to the package stuff... He's currently away from a machine but should be able to sort this particular ticket (relevant to #16333) when he's back or if anyone is eager to review I'll figure out asap! :)
kcrisman
commented
10 years ago Thierry, what platform were you testing on? On Mac OS X 10.7 I get
sage: import gambit
sage: gambit.Game.new_tree()
EFG 2 R "" { }
""
t "" 0
sage: gambit.new_tree()
---------------------------------------------------------------------------
AttributeError: 'module' object has no attribute 'new_tree'contrary to expectations from the documentation. Maybe I installed something wrong...
Suggestion for update: since gambit has a test suite, an spkg-check thing would be good.
edd8e884-f507-429a-b577-5d554626c0fe
commented
10 years ago I a running Debian GNU/Linux stable (wheezy).
I just checkout on the branch i reviewed (which is commit 55d779abbce6e354f5803befc3cb05b465c03f01 based on Sage 6.3.beta3), rebuilt sage, and installed gambit package (version 13.1.2), and i got:
sage: import gambit
sage: gambit.Game.new_tree()
---------------------------------------------------------------------------
AttributeError Traceback (most recent call last)
<ipython-input-2-f105b844ddea> in <module>()
----> 1 gambit.Game.new_tree()
AttributeError: 'module' object has no attribute 'Game'
sage: gambit.new_tree()
EFG 2 R "" { }
""
t "" 0I would really prefer that Sage ships an official binary offered by upstream, not a customized temporary one. What is the current status of your Sage's specific branch being merged ? If it is for not too long, i think it is worth waiting this merge.
kcrisman
commented
10 years ago Wow, exactly the opposite. Huh.
True, this is not so high priority yet especially as the branches are a little mixed up.
drvinceknight
commented
10 years ago Replying to @kcrisman:
Wow, exactly the opposite. Huh.
Hi both, very strange that you're getting the weird behaviour.
Completely agree that it would be idea for this to just be 'a normal' upstream binary of gambit but the reason it's this personalised version is after conversation with the main gambit dev. There needed to be a slight tweak to how the Python api worked, with a view to including this change to future versions of gambit.
When it came to hosting it the gambit maintainer said it was best (I forget if there was an actual reason) to just have it on my site. As a point of action, I'll check some things with James and get in touch with the gambit maintainer to see if we can fix this.
I'm beginning to wonder if since the main purpose of this is for #16333, but that most of #16333 works without gambit, it would be possible to modify/open new tickets so that #16333 can be review 'sans gambit' and a new ticket is opened to take care of getting gambit as an optional package in Sage and to check that the one implementation of the algorithm also works.
True, this is not so high priority yet especially as the branches are a little mixed up.
Agree that this is not high priority based on above, I've left you a message on #16331 re confusion of branches: not too sure what to do there/here... I think we were quite eager to avoid a branch mess but might have managed to create one anyway :) Sorry...
Thank you both very much for looking through this.
kcrisman
commented
10 years ago I'm beginning to wonder if since the main purpose of this is for #16333, but that most of #16333 works without gambit, it would be possible to modify/open new tickets so that #16333 can be review 'sans gambit' and a new ticket is opened to take care of getting gambit as an optional package in Sage and to check that the one implementation of the algorithm also works.
I think that might be good.
tscrim
commented
10 years ago Replying to @drvinceknight:
Completely agree that it would be idea for this to just be 'a normal' upstream binary of gambit but the reason it's this personalised version is after conversation with the main gambit dev. There needed to be a slight tweak to how the Python api worked, with a view to including this change to future versions of gambit.
The standard approach is to create a patch (or if there are more logical grouping, multiple patches) which apply to a clean version of gambit. As these changes get released into gambit, we remove (a part of) the patch.
I'm beginning to wonder if since the main purpose of this is for #16333, but that most of #16333 works without gambit, it would be possible to modify/open new tickets so that #16333 can be review 'sans gambit' and a new ticket is opened to take care of getting gambit as an optional package in Sage and to check that the one implementation of the algorithm also works.
I'm also for separating things out here considering the spkg doesn't seem like it will be ready soon.
drvinceknight
commented
10 years ago Ok thanks, I think it would be helpful (before I start changing/opening tickets and risk breaking everything) to just confirm a few things with someone/anyone. Would either of you be free to talk about this for 5 minutes?
kcrisman
commented
10 years ago Ok thanks, I think it would be helpful (before I start changing/opening tickets and risk breaking everything) to just confirm a few things with someone/anyone. Would either of you be free to talk about this for 5 minutes?
Not now, sorry :( But my recommendation, assuming it is technically not too difficult, is to
drvinceknight
commented
10 years ago Replying to @kcrisman:
Ok thanks, I think it would be helpful (before I start changing/opening tickets and risk breaking everything) to just confirm a few things with someone/anyone. Would either of you be free to talk about this for 5 minutes?
Not now, sorry :( But my recommendation, assuming it is technically not too difficult, is to
- Make #16333 for the basic functionality for games, even if some things are not implemented
- Make this ticket be about both the optional package and making it work with Sage
- Make some other ticket be about any other add ons you are thinking of.
This sounds sensible to me but my understanding is that we can't change the name of a trac ticket (or at least we shouldn't). If that's the case is it enough to explain in the comments what the change is to be?
If I'm wrong and it's fine to change the name of a ticket should I just go ahead and do that?
tscrim
commented
10 years ago Replying to @drvinceknight:
Ok thanks, I think it would be helpful (before I start changing/opening tickets and risk breaking everything) to just confirm a few things with someone/anyone. Would either of you be free to talk about this for 5 minutes?
I might have a little bit of time tonight. Send me an e-mail (tscrim at ucdavis.edu) and we can figure something out.
Replying to @kcrisman:
Not now, sorry :( But my recommendation, assuming it is technically not too difficult, is to
- Make #16333 for the basic functionality for games, even if some things are not implemented
- Make this ticket be about both the optional package and making it work with Sage
- Make some other ticket be about any other add ons you are thinking of.
I concur.
Apropos of this, I really feel like the patch workflow (as opposed to never-rewrite-history branch workflow) is better in situations like this. Too confusing otherwise.
You can start a new branch and merge in up to some commit by using the sha1 (or cherry-pick a single commit). Although I wouldn't worry so much about having the perfect "clean" history and instead use git diff and
git log --no-merges ^developto see all commits not in develop.
kcrisman
commented
10 years ago This sounds sensible to me but my understanding is that we can't change the name of a trac ticket (or at least we shouldn't). If that's the case is it enough to explain in the comments what the change is to be?
If I'm wrong and it's fine to change the name of a ticket should I just go ahead and do that?
Yes, absolutely one can do that, as long as it's clear why and related to the original thing. For instance, often a ticket will start off as "fix trivial error in foo - easy" and by the end become "rewrite entire functionality of bar" which then happens to have fixing the trivial error as a corollary - because that was the only way to fix the supposedly trivial error!
drvinceknight
commented
10 years ago Ok this sounds like a simple enough fix :)
I'm going camping for 4 days tomorrow so unless I get to speak to tscrim (thank you very much for the offer) I'll leave making the change till next week just in case I make a mess that I'm not around to fix.
kcrisman
commented
10 years ago I'm going camping for 4 days tomorrow so unless I get to speak to tscrim (thank you very much for the offer) I'll leave making the change till next week just in case I make a mess that I'm not around to fix.
Nice! Yes, next week is better for me as well.
drvinceknight
commented
10 years ago Just had a very helpful conversation with tscrim and I here is how this mess will be fixed:
All these tickets have been set to 'needs work'. Fixing #16954 should not take too long (it's just cleaning and removing code).
kcrisman
commented
10 years ago Okay, I got back to this. Unfortunately, now it doesn't even build. ? This is on Mac OS X 10.7.
creating build/tools/lcp
gcc -fno-strict-aliasing -g -O2 -DNDEBUG -g -fwrapv -O3 -Wall -I../.. -I.. -I/Users/.../sage/local/include/python2.7 -c gambit/lib/libgambit.cpp -o build/temp.macosx-10.7-x86_64-2.7/gambit/lib/libgambit.o
gambit/lib/libgambit.cpp: In function 'PyObject* __pyx_pf_6gambit_3lib_9libgambit_9LCPSolver_4solve(__pyx_obj_6gambit_3lib_9libgambit_LCPSolver*, __pyx_obj_6gambit_3lib_9libgambit_Game*, PyObject*, PyObject*)':
gambit/lib/libgambit.cpp:45630:35: error: '>>' should be '> >' within a nested template argument list
error: command 'gcc' failed with exit status 1Changed reviewer from Thierry Monteil to Thierry Monteil, Karl-Dieter Crisman
drvinceknight
commented
9 years ago Getting back to this (apologies for the delay).
We're now pointing towards a 'standard' release of gambit which should work fine in the modifications to the NormalFormGame class that we'll start working on (shouldn't take long as code is pretty much ready).
Assuming this version of gambit installs fine and is ok (this branch is merge with latest branch of develop and gambit has built on two separate OSs), what would be the best way to proceed to getting gambit in to the NormalFormGame class.
I think there are two approaches:
NormalFormGame so that it has an interface to gambit. I think that this was the plan at some point but looking at some dev guidance it seems that adding a package should be a single self contained ticket.develop and work on a new branch of that on a different ticket (this is my preferred option as I think it's least likely to break stuff...)Thanks :)
New commits:
819060a | adds needed files |
a7968d9 | completes SPKG.txt |
dee2c53 | changes package number |
018fe86 | changes SPKG and completes install |
9bf529b | updates checksums |
1005e9d | small changes |
drvinceknight
commented
9 years ago Description changed:
---
+++
@@ -1,3 +1,3 @@
-Include [gambit](http://gambit.sourceforge.net/gambit13/index.html) as one the optional packages that can be installed with sage.
-The tarball can be found [here](http://vincent-knight.com/other/gambit-14.0.2.tar.gz).
+Include [http://sourceforge.net/projects/gambit/files/gambit14/14.1.0/](http://sourceforge.net/projects/gambit/files/gambit14/14.1.0/) as one the optional packages that can be installed with sage.
+The tarball can be found [http://sourceforge.net/projects/gambit/files/gambit14/14.1.0/gambit-14.1.0.tar.gz/download](http://sourceforge.net/projects/gambit/files/gambit14/14.1.0/gambit-14.1.0.tar.gz/download).
Changed branch from u/jcampbell/add_gambit_as_an_optional_package to u/vinceknight/gambit
drvinceknight
commented
9 years ago Changed author from James Campbell to James Campbell, Vince Knight
drvinceknight
commented
9 years ago (Karl, I'm hoping that this version of gambit installs fine on your OS, has run fine on my MAC OS 10.9.5)
tscrim
commented
9 years ago FTR, this installed on my Ubuntu 12.04 LTS system.
kcrisman
commented
9 years ago You already have #16333 for the purposes of actually writing the gambit interface, as long as a few examples we test here work out properly this time I think this ticket should be just for adding gambit. Maybe you could put in a couple absolutely minimal gambit-optional doctests to verify that it imports and creates a game, I guess. This could be in a file gambit.py that just serves as a way for people to test and use it independently of Sage (though see below because of the preparser):
For instance, I still get the following (it does compile now):
sage: sage: import gambit
sage: sage: gambit.Game.new_tree()
EFG 2 R "" { }
""
t "" 0
sage: g.players[0].label="A" # turns out not to work in Sage because of preparer, that's okay
---------------------------------------------------------------------------
TypeError: collection indexes must be int or str, not Integer
sage: g.players[int(0)].label="A"I can't find the previously non-working version in the documentation now, maybe that was changed. For posterity, it doesn't work now.
sage: sage: gambit.new_tree()
---------------------------------------------------------------------------
AttributeError Traceback (most recent call last)
<ipython-input-3-46627785e350> in <module>()
----> 1 gambit.new_tree()
AttributeError: 'module' object has no attribute 'new_tree'Travis, did you try any actual gambit examples? If these work I guess that would be enough.
Vincent, I think that the gambit documentation still leaves something to be desired. For instance,
In [1]: g = gambit.Game.read_game("e02.nfg")doesn't work right, because the file is not there (I guess). In R and similar things there is a standard way to access such built-in examples. Not necessary per se, but it would be nice if examples worked right.
drvinceknight
commented
9 years ago Replying to @kcrisman:
You already have #16333 for the purposes of actually writing the gambit interface, as long as a few examples we test here work out properly this time I think this ticket should be just for adding gambit. Maybe you could put in a couple absolutely minimal gambit-optional doctests to verify that it imports and creates a game, I guess. This could be in a file gambit.py that just serves as a way for people to test and use it independently of Sage (though see below because of the preparser):
For instance, I still get the following (it does compile now):
sage: sage: import gambit sage: sage: gambit.Game.new_tree() EFG 2 R "" { } "" t "" 0 sage: g.players[0].label="A" # turns out not to work in Sage because of preparer, that's okay --------------------------------------------------------------------------- TypeError: collection indexes must be int or str, not Integer sage: g.players[int(0)].label="A"I can't find the previously non-working version in the documentation now, maybe that was changed. For posterity, it doesn't work now.
sage: sage: gambit.new_tree() --------------------------------------------------------------------------- AttributeError Traceback (most recent call last) <ipython-input-3-46627785e350> in <module>() ----> 1 gambit.new_tree() AttributeError: 'module' object has no attribute 'new_tree'Travis, did you try any actual gambit examples? If these work I guess that would be enough.
Vincent, I think that the gambit documentation still leaves something to be desired. For instance,
In [1]: g = gambit.Game.read_game("e02.nfg")doesn't work right, because the file is not there (I guess). In R and similar things there is a standard way to access such built-in examples. Not necessary per se, but it would be nice if examples worked right.
Not too sure where the gambit.py would go but like the idea of writing tests for it: would I put it in the game_theory folder and just make sure it doesn't come up in the docs etc...?
For simplicity here is some code you can just paste in to a Sage session to test gambit:
A prisoner's dilemma:
sage: import gambit
sage: g = gambit.Game.new_table([2,2])
sage: g[int(0), int(0)][int(0)] = int(8)
sage: g[int(0), int(0)][int(1)] = int(8)
sage: g[int(0), int(1)][int(0)] = int(2)
sage: g[int(0), int(1)][int(1)] = int(10)
sage: g[int(1), int(0)][int(0)] = int(10)
sage: g[int(1), int(0)][int(1)] = int(2)
sage: g[int(1), int(1)][int(0)] = int(5)
sage: g[int(1), int(1)][int(1)] = int(5)
sage: solver = gambit.nash.ExternalLCPSolver()
sage: solver.solve(g)
[<NashProfile for '': [0.0, 1.0, 0.0, 1.0]>]A battle of the sexes:
e: import gambit
sage: g = gambit.Game.new_table([2,2])
sage: g[int(0), int(0)][int(0)] = int(2)
sage: g[int(0), int(0)][int(1)] = int(1)
sage: g[int(0), int(1)][int(0)] = int(0)
sage: g[int(0), int(1)][int(1)] = int(0)
sage: g[int(1), int(0)][int(0)] = int(0)
sage: g[int(1), int(0)][int(1)] = int(0)
sage: g[int(1), int(1)][int(0)] = int(1)
sage: g[int(1), int(1)][int(1)] = int(2)
sage: solver = gambit.nash.ExternalLCPSolver()
sage: solver.solve(g)
[<NashProfile for '': [1.0, 0.0, 1.0, 0.0]>,
<NashProfile for '': [0.6666666667, 0.3333333333, 0.3333333333, 0.6666666667]>,
<NashProfile for '': [0.0, 1.0, 0.0, 1.0]>]I assume this is the sort of stuff (with more documentation) that would go in to the gambit.py file.
For info, these are examples we had checked ourselves and is what is required for us to get the LCP solver working in NormalFormGame.
Include http://sourceforge.net/projects/gambit/files/gambit14/14.1.0/ as one the optional packages that can be installed with sage. The tarball can be found http://sourceforge.net/projects/gambit/files/gambit14/14.1.0/gambit-14.1.0.tar.gz/download.
Component: packages: optional
Author: James Campbell, Vince Knight
Branch/Commit:
eac34e0Reviewer: Thierry Monteil, Karl-Dieter Crisman, Travis Scrimshaw
Issue created by migration from https://trac.sagemath.org/ticket/16466