Skip to content
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 vector of scalar variables with in syntax #2657

Merged
merged 9 commits into from
Oct 2, 2021
Merged

Add vector of scalar variables with in syntax #2657

merged 9 commits into from
Oct 2, 2021

Conversation

joaquimg
Copy link
Member

@joaquimg joaquimg commented Aug 7, 2021

Partially solves: #2148

this enables both:

model = Model()
@variable(model, x[1:2] in MOI.Integer())

and

model = Model()
@variable(model, x[1:2] in [MOI.Integer(), MOI.ZeroOne()])

.in does not parse in julia, so @variable(model, x[1:2] .in MOI.GreaterThan.([3, 2])) is not an option.

Also added protection to other cases.

This would be a great replacement for jump-dev/ParametricOptInterface.jl#40

cc @guilhermebodin

@codecov
Copy link

codecov bot commented Aug 7, 2021

Codecov Report

Merging #2657 (0cfa41c) into master (28a57b2) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2657      +/-   ##
==========================================
+ Coverage   93.84%   93.85%   +0.01%     
==========================================
  Files          44       44              
  Lines        5524     5534      +10     
==========================================
+ Hits         5184     5194      +10     
  Misses        340      340              
Impacted Files Coverage Δ
src/Containers/DenseAxisArray.jl 88.77% <100.00%> (ø)
src/macros.jl 94.94% <100.00%> (+0.06%) ⬆️
src/variables.jl 98.62% <100.00%> (+<0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 28a57b2...0cfa41c. Read the comment docs.

Copy link
Member

@odow odow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just my style pet-peeves. JuliaFormatter doesn't have an option for this yet.

@joaquimg
Copy link
Member Author

joaquimg commented Aug 9, 2021

Latest version allows SparseAxisArray

@variable(model, b[i=1:2, j=1:2; i+j==3] in MOI.GreaterThan(3.0))
# and
@variable(model, a[i=1:2, j=1:2; i+j==3] in
    JuMP.Containers.SparseAxisArray(
        Dict(
            (1,2) => MOI.GreaterThan(3.0),
            (2,1) => MOI.GreaterThan(3.0),
)))

But DenseAxisArray like:

@variable(model, z1[1:2, 2:4] in MOI.GreaterThan(3.0))

fails with:

Add vector of variables: Error During Test at C:\Users\joaquimgarcia\.julia\dev\JuMP\test\macros.jl:920
  Got exception outside of a @test
  MethodError: no method matching similar(::Type{Array{VariableRef, N} where N}, ::Tuple{Base.OneTo{Int64}, UnitRange{Int64}})
  Closest candidates are:
    similar(::AbstractArray{T, N} where N, ::Tuple) where T at abstractarray.jl:740
    similar(::Type{A}, ::Type{T}, ::StaticArrays.Size{S}) where {A<:Array, T, S} at C:\Users\joaquimgarcia\.julia\packages\StaticArrays\lpEdQ\src\abstractarray.jl:163
    similar(::Type{T}, ::Union{Integer, AbstractUnitRange}...) where T<:AbstractArray at abstractarray.jl:783
    ...
  Stacktrace:
    [1] similar(#unused#::Base.Broadcast.Broadcasted{Base.Broadcast.ArrayConflict, Tuple{Base.OneTo{Int64}, UnitRange{Int64}}, typeof(add_variable), Tuple{Base.RefValue{Model}, JuMP.Containers.DenseAxisArray{VariableConstrainedOnCreation{MathOptInterface.GreaterThan{Float64}, ScalarVariable{Float64, Float64, Float64, Float64}}, 2, Tuple{Base.OneTo{Int64}, UnitRange{Int64}}, Tuple{JuMP.Containers._AxisLookup{Base.OneTo{Int64}}, JuMP.Containers._AxisLookup{Tuple{Int64, Int64}}}}, JuMP.Containers.DenseAxisArray{String, 2, Tuple{Base.OneTo{Int64}, UnitRange{Int64}}, Tuple{JuMP.Containers._AxisLookup{Base.OneTo{Int64}}, JuMP.Containers._AxisLookup{Tuple{Int64, Int64}}}}}}, #unused#::Type{VariableRef}, dims::Tuple{Base.OneTo{Int64}, UnitRange{Int64}})
      @ Base.Broadcast .\broadcast.jl:202
    [2] similar(bc::Base.Broadcast.Broadcasted{Base.Broadcast.ArrayConflict, Tuple{Base.OneTo{Int64}, UnitRange{Int64}}, typeof(add_variable), Tuple{Base.RefValue{Model}, JuMP.Containers.DenseAxisArray{VariableConstrainedOnCreation{MathOptInterface.GreaterThan{Float64}, ScalarVariable{Float64, Float64, Float64, Float64}}, 2, Tuple{Base.OneTo{Int64}, UnitRange{Int64}}, Tuple{JuMP.Containers._AxisLookup{Base.OneTo{Int64}}, JuMP.Containers._AxisLookup{Tuple{Int64, Int64}}}}, JuMP.Containers.DenseAxisArray{String, 2, Tuple{Base.OneTo{Int64}, UnitRange{Int64}}, Tuple{JuMP.Containers._AxisLookup{Base.OneTo{Int64}}, JuMP.Containers._AxisLookup{Tuple{Int64, Int64}}}}}}, #unused#::Type{VariableRef})
      @ Base.Broadcast .\broadcast.jl:196
    [3] copy
      @ .\broadcast.jl:908 [inlined]
    [4] materialize
      @ .\broadcast.jl:883 [inlined]
    [5] add_variable(model::Model, variables::JuMP.Containers.DenseAxisArray{VariableConstrainedOnCreation{MathOptInterface.GreaterThan{Float64}, ScalarVariable{Float64, Float64, Float64, Float64}}, 2, Tuple{Base.OneTo{Int64}, UnitRange{Int64}}, Tuple{JuMP.Containers._AxisLookup{Base.OneTo{Int64}}, JuMP.Containers._AxisLookup{Tuple{Int64, Int64}}}}, names::JuMP.Containers.DenseAxisArray{String, 2, Tuple{Base.OneTo{Int64}, UnitRange{Int64}}, Tuple{JuMP.Containers._AxisLookup{Base.OneTo{Int64}}, JuMP.Containers._AxisLookup{Tuple{Int64, Int64}}}})
      @ JuMP C:\Users\joaquimgarcia\.julia\dev\JuMP\src\variables.jl:1082

Any ideas?

@odow
Copy link
Member

odow commented Aug 9, 2021

Hmm. I'm guessing I did something wrong here: #2533. Broadcasting over multiple DenseAxisArrays is hard.

@odow
Copy link
Member

odow commented Aug 9, 2021

We probably need to modify this somehow

# We specify `Ax` for the type of `axes` to avoid conflict where `axes` has type `Tuple{Vararg{Int,N}}`.
function Base.similar(
A::DenseAxisArray{T,N,Ax},
::Type{S},
axes::Ax,
) where {T,N,Ax,S}
return construct_undef_array(S, axes)
end
# Avoid conflict with method defined in Julia Base when the axes of the
# `DenseAxisArray` are all `Base.OneTo`:
function Base.similar(
::DenseAxisArray{T,N,Ax},
::Type{S},
axes::Ax,
) where {T,N,Ax<:Tuple{Base.OneTo,Vararg{Base.OneTo}},S}
return construct_undef_array(S, axes)
end

@odow
Copy link
Member

odow commented Sep 29, 2021

@joaquimg I pushed a fix for the broadcasting

@odow odow merged commit 28a0663 into master Oct 2, 2021
@odow odow deleted the jg/at_var branch October 2, 2021 00:31
function Base.BroadcastStyle(::Type{T}) where {T<:DenseAxisArray}
return Broadcast.ArrayStyle{T}()
function Base.BroadcastStyle(::Type{<:DenseAxisArray})
return Broadcast.ArrayStyle{DenseAxisArray}()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was subtle

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants