-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
ENH: Add AS algorithm. #433
Conversation
@shizejin This looks very nice. Thanks! When you have time it would be great if you could add the demonstration to this site: Perhaps after the code has been merged and the demonstration has been updated (and a little commentary added)... @cc7768 You reviewed the Julia version. Do you have any thoughts on this? |
This is very cool work @shizejin. The insights of AS are very useful -- It is a great example of "a little extra math can save you a lot of computations." Happy to review this -- In fact, I may spend some of the time during the code sprint on Saturday to review this and go back and finish reviewing the Julia version (I noticed today that we never finished doing that) . |
Thanks @cc7768 ! I will add detailed doc string these days so that it makes easier for you to review. About the Julia version, yes it has been suspending for a really long time... Actually, after writing up this Python version, I realized I made some stupid bugs in the Julia version. After you review this Python version I will go back there and modify it. |
@shizejin Looks great! Some initial comments:
|
@oyamad Thanks for comments!
I have one question: now I am returning a |
b543866
to
a9d24dd
Compare
I have added Here is the newest demonstration, with a Cournot Duopoly Game example added. To do:
|
5c5dafc
to
c273176
Compare
This is ready for review. Before the merge, we may want to decide whether to use The former one is favorable in the sense that it is easy to add |
@oyamad would you have time to review this PR? |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-
I like the idea of returning a
ConvexHull
instance. -
I don't have a good suggestion for the name of the method. My only comment is that the name of a function/method should be in lower case (so I would downvote for
AS
). -
Add a reference information for Abreu and Sannikov (2014) somewhere.
-
Inspect the code with pep8online.com or whatever.
-
Raise
NotImplementedError
ifself.N != 2
? -
Consider prepending an underscore to the name of private functions.
self.N = stage_game.N | ||
self.nums_actions = stage_game.nums_actions | ||
|
||
def AS(self, tol=1e-12, max_iter=500, u_init=np.zeros(2)): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am afraid compute
would leave unclear what to compute...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@oyamad I think you're right.
|
|
A unified interface The usage would be as following now:
>>> p1 = gt.Player([[16, 3, 0], [21, 10, -1], [9, 5, -5]])
>>> p2 = gt.Player([[9, 1, 0], [13, 4, -4], [3, 0, -15]])
>>> sg = gt.NormalFormGame([p1, p2])
>>> rpg = gt.RepeatedGame(sg, 0.3)
>>> hull = rpg.equilibrium_payoffs()
>>> hull.points[hull.vertices]
array([[7.33770472e+00, 1.09826253e+01],
[1.12568240e+00, 2.80000000e+00],
[7.33770472e+00, 4.44089210e-16],
[7.86308964e+00, 4.44089210e-16],
[1.97917977e+01, 2.80000000e+00],
[1.55630896e+01, 9.10000000e+00]])
>>> pd_payoff = [[9.0, 1.0], [10.0, 3.0]]
>>> A = gt.Player(pd_payoff)
>>> B = gt.Player(pd_payoff)
>>> sg = gt.NormalFormGame((A, B))
>>> rpg = gt.RepeatedGame(sg, 0.9)
>>> hull = rpg.equilibrium_payoffs(options={'u_init': np.array([3, 3])})
>>> hull.points[hull.vertices]
array([[3. , 3. ],
[9.75, 3. ],
[9. , 9. ],
[3. , 9.75]]) Currently, only >>> hull = rpg.equilibrium_payoffs(method='abreu_sannikov', options={'u_init': np.array([3, 3])})
>>> hull = rpg.equilibrium_payoffs(method='AS', options={'u_init': np.array([3, 3])})
>>> hull = rpg.equilibrium_payoffs(method='outerapproximation', options={'u_init': np.array([3, 3])})
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/Users/hyde/QuantEcon.py/quantecon/game_theory/repeated_game.py", line 69, in equilibrium_payoffs
raise NotImplementedError(msg)
NotImplementedError: method outerapproximation not supported. It will be very easy for us to unify |
|
||
import numpy as np | ||
from scipy.spatial import ConvexHull | ||
from numba import jit, njit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove jit
.
delta : scalar(float) | ||
The common discount rate at which all players discount the future. | ||
|
||
""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add docstring for attributes.
raise NotImplementedError(msg) | ||
|
||
|
||
def _equilibrium_payoffs_abreu_sannikov(rpg, tol=1e-12, max_iter=500, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it make sense to export this function, under the name, say, abreu_sannikov
(this is what I meant in my previous comment)? In the current version, no documentation is exposed for the AS algorithm implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be a little bit redundant if we both export abreu_sannikov
and RepeatedGame.equilibrium_payoffs(method='abreu_sannikov')
?
What scipy.optimize.minimize
does is to make documentation page for each internal function corresponding to each method (e.g. minimize(method='COBYLA')) and then add a reference link in the documentation of scipy.optimize.minimize
(e.g. here). Do you think we can follow this way?
return hull | ||
|
||
|
||
def _best_dev_gains(sg, delta): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's wrong with passing a RepeatedGame
(which is a container of sg
and delta
) as an argument to this function?
best_dev_gains0 = (1-delta)/delta * \ | ||
(np.max(sg.payoff_arrays[0], 0) - sg.payoff_arrays[0]) | ||
best_dev_gains1 = (1-delta)/delta * \ | ||
(np.max(sg.payoff_arrays[1], 0) - sg.payoff_arrays[1]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tuple comprehension seems to be a better approach here.
return _equilibrium_payoffs_abreu_sannikov(self, **options) | ||
else: | ||
msg = f"method {method} not supported." | ||
raise NotImplementedError(msg) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would expect a ValueError
if I pass method='something_nonsense'
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's True. I shall change this exception.
Considering that 'outerapproximation' method is going to be added in the future, shall we set a separate NotImplementedError
exception for it?
@oyamad this PR is looking close. Should I wait to release a new version through |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mmcky I think we can merge this now. We can modify the code afterwards if necessary.
for d in self.game_dicts: | ||
rpg = RepeatedGame(d['sg'], d['delta']) | ||
for method in ('abreu_sannikov', 'AS'): | ||
hull = rpg.equilibrium_payoffs(options={'u_init': d['u']}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shizejin just wondering, where is method
used here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@natashawatkins Thanks for reviewing. I should have added method='abreu_sannikov'
and method='AS'
here. The test passed because currently only "AS" method is supported and is set to be default.
I will fix this in minutes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No problem, I'm just writing a news item about the latest release
@shizejin did you add the notebook to notes.quantecon.org? |
A Python version implementation of AS (Abreu and Sannikov 2014) algorithm. (Julia version is QuantEcon/GameTheory.jl#65).
Comparing to the Julia version, the running time is significantly reduced here by using
scipy.spatial.ConvexHull
. On my computer, the example from AS runs for 30 ms on average, which is even quicker than the original Java implementation (usually runs for 50 ms, sometimes more than 100 ms). (Downloadable from here)A demonstration shows the AS example and Prisoner's dilemma example.
Detailed docstring needed to be added. Will see if it's possible to improve the efficiency further.