-
-
Notifications
You must be signed in to change notification settings - Fork 24
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
FEAT: Add tol option to best response related methods #74
Conversation
Thanks @QBatista
|
|
||
# Returns | ||
- `ne::Vector{NTuple{N,Int}}`: Vector of pure action Nash equilibria. | ||
""" | ||
function pure_nash(nfg::NormalFormGame; ntofind=Inf) | ||
function pure_nash(nfg::NormalFormGame; ntofind=Inf, tol::Float64=1e-8) |
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.
tol
is not used in is_nash
!
Codecov Report
@@ Coverage Diff @@
## master #74 +/- ##
==========================================
+ Coverage 94.28% 95.15% +0.87%
==========================================
Files 5 5
Lines 350 351 +1
==========================================
+ Hits 330 334 +4
+ Misses 20 17 -3
Continue to review full report at Codecov.
|
src/normal_form_game.jl
Outdated
@@ -306,7 +306,7 @@ end | |||
""" | |||
best_response(player, opponents_actions, payoff_perturbation) | |||
|
|||
Return the perturbed best response to `opponents_actions`. | |||
Return the perturbed best responses to `opponents_actions`. |
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.
Shouldn't this be singular?
src/normal_form_game.jl
Outdated
throw(ArgumentError( | ||
"tie_breaking must be one of 'smallest' or 'random'" | ||
)) | ||
end |
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 don't think we need these tie_breaking
and tol
for this method. payoff_perturbation
is supposed to break ties (assuming that perturbations are drawn from some continuous distribution).
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 did this to match the Python version which has both. Do you want me to get rid of it?
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.
Yes please.
src/normal_form_game.jl
Outdated
|
||
# Returns | ||
|
||
- `::Bool` | ||
""" | ||
is_nash(g::NormalFormGame{1}, action::Action) = | ||
is_best_response(g.players[1], action, nothing) | ||
is_nash(g::NormalFormGame{1}, action::Action, tol::Float64=1e-8) = |
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.
tol
must be a keyword argument.
src/normal_form_game.jl
Outdated
is_nash(g::NormalFormGame{1}, action::Action, tol::Float64=1e-8) = | ||
is_best_response(g.players[1], action, nothing, tol=tol) | ||
|
||
is_nash(g::NormalFormGame{1}, action_profile::ActionProfile) = |
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 tol
argument.
@oyamad This should be ready -- do you want me to squash into a few logical commits before reviewing it? |
Yes, please. |
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.
Great, thanks!
I added one more small test.
Close #21 #75