Improve performance of coefficient modification (#135)

* improve performance of constraint LHS modification

* fix deprecations

* more tests with non-Cint indices

src/grb_common.jl

@@ -19,9 +19,9 @@ ivec(v::IVec) = v
 fvec(v::FVec) = v
 cvec(v::CVec) = v
 
-ivec(v::Vector) = convert(IVec, v)
-fvec(v::Vector) = convert(FVec, v)
-cvec(v::Vector) = convert(CVec, v)
+ivec(v::AbstractVector) = convert(IVec, v)
+fvec(v::AbstractVector) = convert(FVec, v)
+cvec(v::AbstractVector) = convert(CVec, v)
 
 ivec(s::Integer) = Cint[s]
 

src/grb_constrs.jl

@@ -255,7 +255,7 @@ function get_sos(model::Model, start::Integer, len::Integer)
     n = num_vars(model)
     numnzP = Array{Cint}(1)
     cbeg = Array{Cint}(len+1)
-    
+
     ret = @grb_ccall(getsos, Cint, (
         Ptr{Void},
         Ptr{Cint},
@@ -314,11 +314,23 @@ function del_constrs!(model::Model, idx::Vector{Cint})
     end
 end
 
+function chg_coeffs!(model::Model, cidx::Real, vidx::Real, val::Real)
+    ret = @grb_ccall(chgcoeffs, Cint, (
+                     Ptr{Void},
+                     Cint,
+                     Ref{Cint},
+                     Ref{Cint},
+                     Ref{Float64}),
+                     model, 1, cidx - 1, vidx - 1, val)
+    if ret != 0
+        throw(GurobiError(model.env, ret))
+    end
+end
 
-chg_coeffs!{T<:Real, S<:Real}(model::Model, cidx::T, vidx::T, val::S) = chg_coeffs!(model, Cint[cidx], Cint[vidx], Float64[val])
-chg_coeffs!{T<:Real, S<:Real}(model::Model, cidx::Vector{T}, vidx::Vector{T}, val::Vector{S}) = chg_coeffs!(model, convert(Vector{Cint},cidx), convert(Vector{Cint},vidx), fvec(val))
-function chg_coeffs!(model::Model, cidx::Vector{Cint}, vidx::Vector{Cint}, val::FVec)
+chg_coeffs!(model::Model, cidx::AbstractVector{<:Real}, vidx::AbstractVector{<:Real}, val::AbstractVector{<:Real}) =
+    chg_coeffs!(model, ivec(cidx), ivec(vidx), fvec(val))
 
+function chg_coeffs!(model::Model, cidx::IVec, vidx::IVec, val::FVec)
     (length(cidx) == length(vidx) == length(val)) || error("Inconsistent argument dimensions.")
 
     numchgs = length(cidx)
@@ -335,6 +347,7 @@ function chg_coeffs!(model::Model, cidx::Vector{Cint}, vidx::Vector{Cint}, val::
 end
 
 function getcoeff!(val::FVec, model::Model, cidx::Integer, vidx::Integer)
+    Base.depwarn("getcoeff!(val, model, cidx, vidx) is deprecated. Instead you can retrieve a coefficient without allocating a vector by doing `val = getcoeff(model, cidx, vidx)`", :grb_getcoeff)
     @assert length(val) == 1
     ret = @grb_ccall(getcoeff, Cint, (
         Ptr{Void},
@@ -346,8 +359,17 @@ function getcoeff!(val::FVec, model::Model, cidx::Integer, vidx::Integer)
         throw(GurobiError(model.env, ret))
     end
 end
+
 function getcoeff(model::Model, cidx::Integer, vidx::Integer)
-    out = Array{Float64}(1)
-    getcoeff!(out::FVec, model::Model, cidx::Integer, vidx::Integer)
-    return out[1]
-end
+    out = Ref{Float64}()
+    ret = @grb_ccall(getcoeff, Cint, (
+        Ptr{Void},
+        Cint,
+        Cint,
+        Ref{Float64}),
+        model, cidx - 1, vidx - 1, out)
+    if ret != 0
+        throw(GurobiError(model.env, ret))
+    end
+    out[]
+end

test/MathProgBase/large_coefficients.jl

@@ -6,15 +6,15 @@ using Gurobi, MathProgBase, Base.Test
         # Min   1.1e100x
         #       x >= 1
         m = MathProgBase.LinearQuadraticModel(GurobiSolver())
-        MathProgBase.loadproblem!(m, Array(Float64, (0, 1)), [1],[Inf], [1.1e100], Float64[], Float64[], :Min)
+        MathProgBase.loadproblem!(m, Array{Float64}(0, 1), [1],[Inf], [1.1e100], Float64[], Float64[], :Min)
         MathProgBase.optimize!(m)
         @test MathProgBase.getsolution(m) == [1.0]
         @test MathProgBase.getobjval(m) == 1e100
 
         # Max   -1.1e100x
         #       x >= 1
         m = MathProgBase.LinearQuadraticModel(GurobiSolver())
-        MathProgBase.loadproblem!(m, Array(Float64, (0, 1)), [1],[Inf], [-1.1e100], Float64[], Float64[], :Max)
+        MathProgBase.loadproblem!(m, Array{Float64}(0, 1), [1],[Inf], [-1.1e100], Float64[], Float64[], :Max)
         MathProgBase.optimize!(m)
         @test MathProgBase.getsolution(m) == [1.0]
         @test MathProgBase.getobjval(m) == -1e100
@@ -24,28 +24,28 @@ using Gurobi, MathProgBase, Base.Test
         # Min   x
         #       x >= 1.1e30
         m = MathProgBase.LinearQuadraticModel(GurobiSolver())
-        MathProgBase.loadproblem!(m, Array(Float64, (0, 1)), [1.1e30],[Inf], [1], Float64[], Float64[], :Min)
+        MathProgBase.loadproblem!(m, Array{Float64}(0, 1), [1.1e30],[Inf], [1], Float64[], Float64[], :Min)
         MathProgBase.optimize!(m)
         @test MathProgBase.status(m) == :Infeasible
 
         # Max   x
         #       x <= -1.1e30
         m = MathProgBase.LinearQuadraticModel(GurobiSolver())
-        MathProgBase.loadproblem!(m, Array(Float64, (0, 1)), [-Inf],[-1.1e30], [1], Float64[], Float64[], :Max)
+        MathProgBase.loadproblem!(m, Array{Float64}(0, 1), [-Inf],[-1.1e30], [1], Float64[], Float64[], :Max)
         MathProgBase.optimize!(m)
         @test MathProgBase.status(m) == :Infeasible
 
         # Min   x
         #       x >= -1.1e30
         m = MathProgBase.LinearQuadraticModel(GurobiSolver())
-        MathProgBase.loadproblem!(m, Array(Float64, (0, 1)), [-1.1e30],[Inf], [1], Float64[], Float64[], :Min)
+        MathProgBase.loadproblem!(m, Array{Float64}(0, 1), [-1.1e30],[Inf], [1], Float64[], Float64[], :Min)
         MathProgBase.optimize!(m)
         @test MathProgBase.status(m) == :Unbounded
 
         # Max   x
         #       x <= 1.1e30
         m = MathProgBase.LinearQuadraticModel(GurobiSolver())
-        MathProgBase.loadproblem!(m, Array(Float64, (0, 1)), [-Inf],[1.1e30], [1], Float64[], Float64[], :Max)
+        MathProgBase.loadproblem!(m, Array{Float64}(0, 1), [-Inf],[1.1e30], [1], Float64[], Float64[], :Max)
         MathProgBase.optimize!(m)
         @test MathProgBase.status(m) == :Unbounded
 

test/constraint_modification.jl

@@ -0,0 +1,51 @@
+using Gurobi
+using Base.Test
+using Gurobi: getcoeff
+
+
+@testset "changing coefficients" begin
+    A = spzeros(2, 2)
+    A[1, 1] = 1.0
+    A[1, 2] = 2.0
+    A[2, 1] = 3.0
+    b = [0.1, 0.2]
+    f = [0.0, 0.0]
+    model = gurobi_model(Gurobi.Env(); f=f, A=A, b=b)
+
+    @test getcoeff(model, 1, 1) == 1.0
+
+    # Verify that we can pass any kind of `::Integer` to `getcoeff`
+    @test getcoeff(model, 1, Int8(1)) == 1.0
+    @test getcoeff(model, UInt32(1), UInt32(1)) == 1.0
+
+    @test getcoeff(model, 1, 2) == 2.0
+    @test getcoeff(model, 2, 1) == 3.0
+    @test getcoeff(model, 2, 2) == 0.0
+
+    chg_coeffs!(model, 2, 1, 1.5)
+    # before updating the model, we still get the old coefficient value
+    @test getcoeff(model, 2, 1) == 3.0
+    # after updating the model, we see the new coefficient value
+    update_model!(model)
+    @test getcoeff(model, 2, 1) == 1.5
+
+    # Verify that we are automatically converting the scalars to
+    # Cint and Float64 as necessary
+    chg_coeffs!(model, Int8(2), Int128(1), Float32(2.0))
+    update_model!(model)
+    @test getcoeff(model, 2, 1) == 2.0
+
+    chg_coeffs!(model, [1, 2], [1, 1], [5.0, 6.5])
+    update_model!(model)
+    @test getcoeff(model, 1, 1) == 5.0
+    @test getcoeff(model, 2, 1) == 6.5
+    @test getcoeff(model, 1, 2) == 2.0
+    @test getcoeff(model, 2, 2) == 0.0
+
+    chg_coeffs!(model, Int8[1, 2], Int8[1, 1], Float32[1.0, 2.0])
+    update_model!(model)
+    @test getcoeff(model, 1, 1) == 1.0
+    @test getcoeff(model, 2, 1) == 2.0
+    @test getcoeff(model, 1, 2) == 2.0
+    @test getcoeff(model, 2, 2) == 0.0
+end

test/runtests.jl

@@ -31,3 +31,5 @@ end
 @testset "MathOptInterface Tests" begin
     evalfile("MOIWrapper.jl")
 end
+
+include("constraint_modification.jl")