-
-
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
Add support enumeration #26
Conversation
src/support_enumeration.jl
Outdated
throw(ArgumentError("Implemented only for 2-player games")) | ||
end | ||
|
||
return _support_enumeration_gen(g.players[1].payoff_array, |
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.
Why not return a Task
?
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 think return a Task
would be better. I'll change this.
4959724
to
9d4bafb
Compare
src/support_enumeration.jl
Outdated
""" | ||
support_enumeration_gen(g::NormalFormGame) | ||
|
||
Generator version of `support_enumeration`. |
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.
"And Julia has a type that is basically equivalent to Python’s generators, called Tasks."
Maybe just "Task version of support_enumeration
"?
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 -- Tasks seem to be the right comparison.
src/support_enumeration.jl
Outdated
# Returns | ||
* `::Task`: runnable task for generating Nash equilibria. | ||
""" | ||
function support_enumeration_gen(g::NormalFormGame) |
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.
Change the function name?
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 also thinking about this, but would support_enumeration_task
be fine?
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, that will be clear enough.
031a006
to
7407a45
Compare
src/support_enumeration.jl
Outdated
minus 1 such pairs. This should thus be used only for small games. | ||
|
||
# Arguments | ||
* `g::NormalFormGame`: NormalFormGame instance. |
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 a blank line.
src/support_enumeration.jl
Outdated
|
||
# Returns | ||
* `::Vector{Tuple{Vector{Float64},Vector{Float64}}}`: Mixed-action | ||
Nash equilibria that are found. |
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.
Correct indentation.
src/support_enumeration.jl
Outdated
* `Tuple{Vector{Float64},Vector{Float64}}`: Tuple of Nash equilibrium | ||
mixed actions. | ||
""" | ||
function _support_enumeration_task{T<:Real}(payoff_matrix1::Matrix{T}, |
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.
Should be named _support_enumeration_producer
or something?
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 think producer
is fine.
src/support_enumeration.jl
Outdated
N = length(g.nums_actions) | ||
if N != 2 | ||
throw(ArgumentError("Implemented only for 2-player games")) | ||
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.
Another approach is to implement the method only for 2-player games, by
function support_enumeration_task(g::NormalFormGame{2})
Which is more "Julian"?
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 think @oyamad's suggestion is more Julian because it leave the door open to implement this routine for games with a different number of players.
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 see. That sounds very cool. I will update it.
src/support_enumeration.jl
Outdated
* `payoff_matrix2::Matrix{T}`: Payoff matrix of player 2. | ||
|
||
# Produces | ||
* `Tuple{Vector{Float64},Vector{Float64}}`: Tuple of Nash equilibrium |
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.
Can we return (or produce) Tuple{Vector{Rational{Int}},Vector{Rational{Int}}}
is T
is Rational
?
(What if T<:Integer
?)
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 think for Rational
it is easy to do this.
If T<:Integer
and I do
julia> A = [1 2; 7 5]
2×2 Array{Int64,2}:
1 2
7 5
julia> b = [1, 1]
2-element Array{Int64,1}:
1
1
julia> A \ b
2-element Array{Float64,1}:
-0.333333
0.666667
julia> sol = Vector{Rational{Int}}(2)
2-element Array{Rational{Int64},1}:
4658402128//4591878432
4591878432//4771073360
julia> A_ldiv_B!(sol, factorize(A), b)
2-element Array{Rational{Int64},1}:
-1501199875790165//4503599627370496
6004799503160661//9007199254740992
I believe it's just transform Float
output into Rational
.
How about we create A
and b
as Rational
if T<:Integer
, and return the Rational
solution?
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.
Define A
as a matrix of Rational
s.
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, if we define A
as matrix of Rational
s, then the solution will also be a vector of Rational
s. For now I am keeping A
and b
consistent with T
of payoff matrixs. Do you think it would be better to set A
and b
to be matrix and vector of Rational
if the payoff matrixs is of T<:Integer
?
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, I don't think so. Keep the eltype of A
and b
to be T
.
For the eltype of sol
, try the following:
S = typeof(zero(T)/one(T))
sol = Vector{S}(k+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.
Contrary to what is written in the manual, the following doesn't work:
julia> A = [1 2; 7 5//1]
2×2 Array{Rational{Int64},2}:
1//1 2//1
7//1 5//1
julia> b = [1, 1//1]
2-element Array{Rational{Int64},1}:
1//1
1//1
julia> A_ldiv_B!(factorize(A), b)
2-element Array{Rational{Int64},1}:
-1//3
2//3
julia> b
2-element Array{Rational{Int64},1}:
1//1
1//1
where the solution should have been contained in b
. The same problem occurs with A_ldiv_B!(sol, factorize(A), b)
with preallocated sol
.
src/support_enumeration.jl
Outdated
A[1:end-1, 1:end-1] = payoff_matrix[own_supp, :][:, opp_supp] | ||
sol = Vector{Float64}(k+1) | ||
try | ||
sol[:] = A \ b |
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.
Is it possible to use A_ldiv_B!
, not to allocate a new array every time?
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.
Do you mean overwriting b
, as it is not used later?
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, I am talking about sol
.
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.
Don't A_ldiv_B!
also need we first allocate sol
in advance? Or do you want to allocate sol
just once out of the function _indiff_mixed_action!
? I am not quite sure what you mean by "every time".
If you are just worrying about the problem we talked in last meeting, What about just
A[1:end-1, 1:end-1] = payoff_matrix[own_supp, :][:, opp_supp]
try
global sol
sol = A \ b
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, I am thinking of allocating sol
just once in the loop of _support_enumeration_task
and passing it to _indiff_mixed_action!
, similarly to A
and b
.
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 think this would be a great improvement in efficiency.
src/support_enumeration.jl
Outdated
|
||
# Returns | ||
* `:::Vector{Int}`: View of `a`. | ||
""" |
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.
It will be helpful if an "Examples" section is added as in the Python version.
test/test_support_enumeration.jl
Outdated
Player([3 2 3; 2 6 1])) | ||
NEs = [([1.0, 0.0, 0.0], [1.0, 0.0]), | ||
([0.8, 0.2, 0.0], [2/3, 1/3]), | ||
([0.0, 1/3, 2/3], [1/3, 2/3])] |
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.
Correct the indentation.
a698625
to
702363d
Compare
Regarding #26 (comment), if the eltype of Anyway, the following should work: b = A_ldiv_B!(A, b) where |
@oyamad This is interesting -- I'm a little surprised it doesn't work for anything but floats. This might be related to the end of this discussion |
src/support_enumeration.jl
Outdated
|
||
Compute mixed-action Nash equilibria with equal support size for a | ||
2-player normal form game by support enumeration. For a | ||
non-degenerate game input, these are all Nash equilibria. |
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 think "these are all the Nash equilibria" sounds better
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.
Agreed about adding the "the"
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.
Thank you folks! Corrected the Python version.
About #26 (Comment), sometimes An example: julia> A = [1 4; 2 3]
2×2 Array{Int64,2}:
1 4
2 3
julia> lufact!(A)
ERROR: InexactError()
in generic_lufact!(::Array{Int64,2}, ::Type{Val{true}}) at ./linalg/lu.jl:55
in lufact!(::Array{Int64,2}) at ./linalg/lu.jl:22 How about we also set the eltype of About the time of LU-factorization of |
Of course, LU factorization involves division, and an int divided by an int is not an int in general. Yes, |
src/support_enumeration.jl
Outdated
m = size(payoff_matrix, 1) | ||
k = length(own_supp) | ||
|
||
A[1:end-1, 1:end-1] = payoff_matrix[own_supp, :][:, opp_supp] |
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.
Don't payoff_matrix[own_supp, opp_supp]
work?
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.
Thanks, it works.
src/support_enumeration.jl
Outdated
""" | ||
function _indiff_mixed_action!{T<:Real}(A::Matrix{T}, b::Vector{T}, | ||
out::Vector{T}, | ||
payoff_matrix::Matrix{T}, |
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 the eltype of payoff_matrix
need to be the same as that of A
?
(I guess the values of payoff_matrix
would be converted when assigned to A
.)
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, it would be automatically converted to be the same with the eltype of A
.
I am doing this because the original eltype of payoff_matrix
is not going to be used anymore, so just drop it might be more concise. And rather than automatically converting the element of payoff_matrix
a lot of time, doing it once here might be more efficient, as I thought.
src/support_enumeration.jl
Outdated
S = typeof(zero(T)/one(T)) | ||
if S != T | ||
payoff_matrices = NTuple{2, Matrix{S}}(payoff_matrices) | ||
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 this part is necessary.
src/support_enumeration.jl
Outdated
actions = (Vector{S}(k), Vector{S}(k)) | ||
A = Matrix{S}(k+1, k+1) | ||
b = Vector{S}(k+1) | ||
while supps[1][end] < nums_actions[1]+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.
I guess supps[1][end] <= nums_actions[1]
will be clearer.
src/support_enumeration.jl
Outdated
|
||
if any(b[1:end-1] .<= 0) | ||
return false | ||
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.
Will there be any difference if replaced with
for i in 1:k
b[i] <= zero(T) && return false
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 think this is better. I thought the for
loop will make it inefficient, but the timing results show that your version is even quicker.
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 about
if findfirst(x -> x<=zero(T), b) != 0
return false
end
According to the source code of findfirst
, it is doing exactly what your code does, and I am wondering if it would be better to use the function that is available in Base.
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.
The loop will be at least as fast as using the findfirst
method with a closure mainly because I've seen type inference issues in cases like this. Under the hood, closures are implemented as new Julia types where the enclosed values become fields of the type. Sometimes type inference doesn't do a perfect job at inferring what the types of these fields should be, slowing the code down by multiple orders of magnitude in some cases.
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.
Thanks for explanation in details! I keep with the for
loop in the new commitment.
src/support_enumeration.jl
Outdated
A[1:end-1, end] = -one(T) | ||
A[end, 1:end-1] = one(T) | ||
A[end, end] = zero(T) | ||
b[1:end-1] = zeros(T, k) |
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.
Better to be consistent by just zero(T)
.
Test cases with |
6969e15
to
2957001
Compare
FYI, the Python version has been revised in QuantEcon/QuantEcon.py#311. |
060fc49
to
f9aa4ad
Compare
@shizejin Good job, thanks. |
closes #18
@oyamad
Here I don't quite understand the three warnings of
::Any
in the body of type-related diagnosing.