diff --git a/src/JuMP.jl b/src/JuMP.jl index 5d242854308..1253842ef79 100644 --- a/src/JuMP.jl +++ b/src/JuMP.jl @@ -13,6 +13,7 @@ isdefined(Base, :__precompile__) && __precompile__() module JuMP importall Base.Operators +import Base.map import MathProgBase @@ -123,7 +124,7 @@ type Model <: AbstractModel conDict::Dict{Symbol,Any} # dictionary from constraint names to constraint objects varData::ObjectIdDict - getvalue_counter::Int # number of times we call getvalue on a JuMPContainer, so that we can print out a warning + map_counter::Int # number of times we call getvalue, getdual, getlowerbound and getupperbound on a JuMPContainer, so that we can print out a warning operator_counter::Int # number of times we add large expressions # Extension dictionary - e.g. for robust @@ -177,7 +178,7 @@ function Model(;solver=UnsetSolver(), simplify_nonlinear_expressions::Bool=false Dict{Symbol,Any}(), # varDict Dict{Symbol,Any}(), # conDict ObjectIdDict(), # varData - 0, # getvalue_counter + 0, # map_counter 0, # operator_counter Dict{Symbol,Any}(), # ext ) @@ -394,10 +395,12 @@ end # internal method that doesn't print a warning if the value is NaN _getValue(v::Variable) = v.m.colVal[v.col] +getvaluewarn(v) = Base.warn("Variable value not defined for $(getname(v)). Check that the model was properly solved.") + function getvalue(v::Variable) ret = _getValue(v) if isnan(ret) - Base.warn("Variable value not defined for $(getname(v)). Check that the model was properly solved.") + getvaluewarn(v) end ret end @@ -434,11 +437,17 @@ function getvalue(arr::Array{Variable}) end # Dual value (reduced cost) getter + +# internal method that doesn't print a warning if the value is NaN +_getDual(v::Variable) = v.m.redCosts[v.col] + +getdualwarn(::Variable) = error("Variable bound duals (reduced costs) not available. Check that the model was properly solved and no integer variables are present.") + function getdual(v::Variable) if length(v.m.redCosts) < MathProgBase.numvar(v.m) - error("Variable bound duals (reduced costs) not available. Check that the model was properly solved and no integer variables are present.") + getdualwarn(v) end - return v.m.redCosts[v.col] + return _getDual(v) end const var_cats = [:Cont, :Int, :Bin, :SemiCont, :SemiInt] @@ -538,11 +547,16 @@ LinearConstraint(ref::LinConstrRef) = ref.m.linconstr[ref.idx]::LinearConstraint linearindex(x::ConstraintRef) = x.idx +# internal method that doesn't print a warning if the value is NaN +_getDual(c::LinConstrRef) = c.m.linconstrDuals[c.idx] + +getdualwarn(::LinConstrRef) = error("Dual solution not available. Check that the model was properly solved and no integer variables are present.") + function getdual(c::LinConstrRef) if length(c.m.linconstrDuals) != MathProgBase.numlinconstr(c.m) - error("Dual solution not available. Check that the model was properly solved and no integer variables are present.") + warnnan(_getDual) end - return c.m.linconstrDuals[c.idx] + return _getDual(c) end # Returns the number of non-infinity and nonzero bounds on variables @@ -796,13 +810,14 @@ function getconstraint(m::Model, conname::Symbol) end # usage warnings -function getvalue_warn(x::JuMPContainer{Variable}) +function mapcontainer_warn(f, x::JuMPContainer{Variable}) isempty(x) && return v = first(values(x)) m = v.m - m.getvalue_counter += 1 - if m.getvalue_counter > 400 - Base.warn_once("getvalue has been called on a collection of variables a large number of times. For performance reasons, this should be avoided. Instead of getvalue(x)[a,b,c], use getvalue(x[a,b,c]) to avoid temporary allocations.") + m.map_counter += 1 + if m.map_counter > 400 + # It might not be f that was called the 400 first times but most probably it is f + Base.warn_once("$f has been called on a collection of variables a large number of times. For performance reasons, this should be avoided. Instead of $f(x)[a,b,c], use $f(x[a,b,c]) to avoid temporary allocations.") end return end diff --git a/src/JuMPContainer.jl b/src/JuMPContainer.jl index 987063f9e73..1da9e744008 100644 --- a/src/JuMPContainer.jl +++ b/src/JuMPContainer.jl @@ -42,6 +42,9 @@ type JuMPContainerData condition end +# Needed by getvaluewarn when called by _mapInner +getname(data::JuMPContainerData) = data.name + #JuMPDict{T,N}(name::AbstractString) = # JuMPDict{T,N}(Dict{NTuple{N},T}(), name) @@ -98,25 +101,40 @@ pushmeta!(x::JuMPContainer, sym::Symbol, val) = (x.meta[sym] = val) getmeta(x::JuMPContainer, sym::Symbol) = x.meta[sym] # duck typing approach -- if eltype(innerArray) doesn't support accessor, will fail -for accessor in (:getdual, :getlowerbound, :getupperbound) - @eval $accessor(x::Union{JuMPContainer,Array}) = map($accessor,x) +for accessor in (:getdual, :getlowerbound, :getupperbound, :getvalue) + @eval $accessor(x::AbstractArray) = map($accessor,x) +end +# With JuMPContainer, we take care in _mapInner of the warning if NaN values are returned +# by the accessor so we use the inner accessor that does not generate warnings +for (accessor, inner) in ((:getdual, :_getDual), (:getlowerbound, :getlowerbound), (:getupperbound, :getupperbound), (:getvalue, :_getValue)) + @eval $accessor(x::JuMPContainer) = _map($inner,x) end + _similar(x::Array) = Array(Float64,size(x)) _similar{T}(x::Dict{T}) = Dict{T,Float64}() _innercontainer(x::JuMPArray) = x.innerArray _innercontainer(x::JuMPDict) = x.tupledict -function _getValueInner(x) +# Warning for getter returning NaN +function _warnnan(f, data) + if f === _getValue + getvaluewarn(data) + elseif f === _getDual + getdualwarn(data) + end +end + +function _mapInner(f, x::JuMPContainer) vars = _innercontainer(x) vals = _similar(vars) data = printdata(x) warnedyet = false for I in eachindex(vars) - tmp = _getValue(vars[I]) + tmp = f(vars[I]) if isnan(tmp) && !warnedyet - warn("Variable value not defined for entry of $(data.name). Check that the model was properly solved.") + _warnnan(f, data.name) warnedyet = true end vals[I] = tmp @@ -127,9 +145,10 @@ end JuMPContainer_from(x::JuMPDict,inner) = JuMPDict(inner) JuMPContainer_from(x::JuMPArray,inner) = JuMPArray(inner, x.indexsets) -function getvalue(x::JuMPContainer) - getvalue_warn(x) - ret = JuMPContainer_from(x,_getValueInner(x)) +# The name _map is used instead of map so that this function is called instead of map(::Function, ::JuMPArray) +function _map(f, x::JuMPContainer) + mapcontainer_warn(f, x) + ret = JuMPContainer_from(x, _mapInner(f, x)) # I guess copy!(::Dict, ::Dict) isn't defined, so... for (key,val) in x.meta ret.meta[key] = val diff --git a/src/solvers.jl b/src/solvers.jl index 4adf3c80e67..2ef94492255 100644 --- a/src/solvers.jl +++ b/src/solvers.jl @@ -121,7 +121,7 @@ function solve(m::Model; suppress_warnings=false, isempty(kwargs) || error("Unrecognized keyword arguments: $(join([k[1] for k in kwargs], ", "))") # Clear warning counters - m.getvalue_counter = 0 + m.map_counter = 0 m.operator_counter = 0 # Remember if the solver was initially unset so we can restore diff --git a/test/variable.jl b/test/variable.jl index a05e3c00b77..41f5412bfe5 100644 --- a/test/variable.jl +++ b/test/variable.jl @@ -224,3 +224,19 @@ end @test MathProgBase.numvar(m) == 4 end end + +@testset "[variable] getvalue on sparse array #889" begin + m = Model() + @variable(m, x) + @variable(m, y) + X = sparse([1, 3], [2, 3], [x, y]) + + @test typeof(X) == SparseMatrixCSC{Variable, Int} + + setvalue(x, 1) + setvalue(y, 2) + + Y = getvalue(X) + @test typeof(Y) == SparseMatrixCSC{Float64, Int} + @test Y == sparse([1, 3], [2, 3], [1, 2]) +end