Add tests to improve code coverage (#204)

Author: odow

src/MosekTools.jl

@@ -435,28 +435,29 @@ function MOI.optimize!(m::Optimizer)
             ),
         )
     end
-    # We need to sort the solutions, so that an optimal one is first (if it
-    # exists). The priority is:
-    #  1. MSK_SOL_STA_INTEGER_OPTIMAL or MSK_SOL_STA_OPTIMAL
-    #  2. MSK_SOL_ITG, MSK_SOL_BAS, MSK_SOL_ITR
-    function solution_priority(sol)
-        solsta_priority =
-            sol.solsta == Mosek.MSK_SOL_STA_INTEGER_OPTIMAL ||
-            sol.solsta == Mosek.MSK_SOL_STA_OPTIMAL
-        if sol.whichsol == Mosek.MSK_SOL_ITG
-            return (solsta_priority, 3)
-        elseif sol.whichsol == Mosek.MSK_SOL_BAS
-            return (solsta_priority, 2)
-        else
-            @assert sol.whichsol == Mosek.MSK_SOL_ITR
-            return (solsta_priority, 1)
-        end
-    end
     # Sort solutions largest priority to smallest
-    sort!(m.solutions; by = solution_priority, rev = true)
+    sort!(m.solutions; by = _solution_priority, rev = true)
     return
 end
 
+# We need to sort the solutions, so that an optimal one is first (if it
+# exists). The priority is:
+#  1. MSK_SOL_STA_INTEGER_OPTIMAL or MSK_SOL_STA_OPTIMAL
+#  2. MSK_SOL_ITG, MSK_SOL_BAS, MSK_SOL_ITR
+function _solution_priority(sol)
+    solsta_priority =
+        sol.solsta == Mosek.MSK_SOL_STA_INTEGER_OPTIMAL ||
+        sol.solsta == Mosek.MSK_SOL_STA_OPTIMAL
+    if sol.whichsol == Mosek.MSK_SOL_ITG
+        return (solsta_priority, 3)
+    elseif sol.whichsol == Mosek.MSK_SOL_BAS
+        return (solsta_priority, 2)
+    else
+        @assert sol.whichsol == Mosek.MSK_SOL_ITR
+        return (solsta_priority, 1)
+    end
+end
+
 # MOI.Name
 
 MOI.supports(::Optimizer, ::MOI.Name) = true

src/attributes.jl

@@ -637,36 +637,40 @@ const _TERMINATION_STATUS_MAP = Dict(
     Mosek.MSK_RES_TRM_INTERNAL_STOP.value => MOI.OTHER_ERROR,
 )
 
+# Mosek.jl defines `MosekEnum <: Integer` but it does not define
+# `hash(::MosekEnum)`. This means creating a dictionary fails. Instead of fixing
+# in Mosek.jl, or pirating a Base.hash(::Mosek.MosekEnum, ::UInt64) method here,
+# we just use the `.value::Int32` field as the key.
+const _PROSTA_STATUS_MAP = Dict(
+    Mosek.MSK_PRO_STA_UNKNOWN.value => MOI.OTHER_ERROR,
+    Mosek.MSK_PRO_STA_PRIM_AND_DUAL_FEAS.value => MOI.LOCALLY_SOLVED,
+    Mosek.MSK_PRO_STA_PRIM_FEAS.value => MOI.LOCALLY_SOLVED,
+    # We proved only dual feasibility? What this one returns is up for debate.
+    Mosek.MSK_PRO_STA_DUAL_FEAS.value => MOI.OTHER_ERROR,
+    Mosek.MSK_PRO_STA_PRIM_INFEAS.value => MOI.INFEASIBLE,
+    Mosek.MSK_PRO_STA_DUAL_INFEAS.value => MOI.DUAL_INFEASIBLE,
+    Mosek.MSK_PRO_STA_PRIM_AND_DUAL_INFEAS.value => MOI.INFEASIBLE,
+    Mosek.MSK_PRO_STA_ILL_POSED.value => MOI.OTHER_ERROR,
+    Mosek.MSK_PRO_STA_PRIM_INFEAS_OR_UNBOUNDED.value =>
+        MOI.INFEASIBLE_OR_UNBOUNDED,
+)
+
 function MOI.get(m::Optimizer, attr::MOI.TerminationStatus)
     if m.trm === nothing
         return MOI.OPTIMIZE_NOT_CALLED
-    elseif m.trm == Mosek.MSK_RES_OK
-        # checking `any(sol -> sol.solsta == Mosek.MSK_SOL_STA_PRIM_INFEAS_CER, m.solutions)`
-        # doesn't work for MIP as there is not certificate, i.e. the solutions status is
-        # `UNKNOWN`, only the problem status is `INFEAS`.
-        if any(
-            sol -> sol.prosta in
-            [Mosek.MSK_PRO_STA_PRIM_INFEAS, Mosek.MSK_PRO_STA_ILL_POSED],
-            m.solutions,
-        )
-            return MOI.INFEASIBLE
-        elseif any(
-            sol -> sol.prosta == Mosek.MSK_PRO_STA_DUAL_INFEAS,
-            m.solutions,
-        )
-            return MOI.DUAL_INFEASIBLE
-        elseif any(
-            sol -> sol.prosta == Mosek.MSK_PRO_STA_PRIM_INFEAS_OR_UNBOUNDED,
-            m.solutions,
-        )
-            return MOI.INFEASIBLE_OR_UNBOUNDED
-        elseif any(
-            sol -> sol.solsta in
-            [Mosek.MSK_SOL_STA_OPTIMAL, Mosek.MSK_SOL_STA_INTEGER_OPTIMAL],
-            m.solutions,
-        )
+    elseif m.trm == Mosek.MSK_RES_OK && length(m.solutions) > 0
+        # The different solutions _could_ have different `.prosta`, but we just
+        # return the information of the first one. This makes the most sense
+        # because `result_index` defaults to 1, and we sort the solutions in
+        # `MOI.optimize!` to ensure that the first solution is OPTIMAL, if one
+        # exists.
+        sol = first(m.solutions)
+        if sol.solsta == Mosek.MSK_SOL_STA_OPTIMAL
+            return MOI.OPTIMAL
+        elseif sol.solsta == Mosek.MSK_SOL_STA_INTEGER_OPTIMAL
             return MOI.OPTIMAL
         end
+        return _PROSTA_STATUS_MAP[sol.prosta.value]
     end
     return get(_TERMINATION_STATUS_MAP, m.trm.value, MOI.OTHER_ERROR)
 end

src/constraint.jl

@@ -569,17 +569,18 @@ _check_bound_compat(::Optimizer, ::MOI.VariableIndex, ::MOI.Integer) = nothing
 
 function MOI.add_constraint(
     m::Optimizer,
-    xs::MOI.VariableIndex,
-    dom::D,
-) where {D<:Union{ScalarLinearDomain,MOI.Integer}}
-    msk_idx = mosek_index(m, xs)
-    if !(msk_idx isa ColumnIndex)
-        error("Cannot add $D constraint on a matrix variable")
+    x::MOI.VariableIndex,
+    set::S,
+) where {S<:Union{ScalarLinearDomain,MOI.Integer}}
+    index = mosek_index(m, x)
+    if index isa MatrixIndex
+        msg = "Cannot add $S constraint on a matrix variable."
+        throw(MOI.AddConstraintNotAllowed{MOI.VariableIndex,S}(msg))
     end
-    _check_bound_compat(m, xs, dom)
-    set_flag(m, xs, D)
-    _add_variable_constraint(m, msk_idx, dom)
-    return MOI.ConstraintIndex{MOI.VariableIndex,D}(xs.value)
+    _check_bound_compat(m, x, set)
+    set_flag(m, x, S)
+    _add_variable_constraint(m, index, set)
+    return MOI.ConstraintIndex{MOI.VariableIndex,S}(x.value)
 end
 
 _cone_type(::Type{MOI.ExponentialCone}) = Mosek.MSK_CT_PEXP
@@ -746,8 +747,8 @@ function MOI.get(
     nnz, cols, vals = Mosek.getarow(m.task, row(m, ci))
     @assert nnz == length(cols) == length(vals)
     terms = MOI.ScalarAffineTerm{Float64}[
-        MOI.ScalarAffineTerm(vals[i], index_of_column(m, cols[i])) for
-        i in 1:nnz
+        MOI.ScalarAffineTerm(v, _col_to_index(m, c)) for
+        (v, c) in zip(vals, cols)
     ]
     return MOI.ScalarAffineFunction(terms, 0.0)
 end
@@ -775,10 +776,8 @@ function MOI.get(
     ::MOI.ConstraintFunction,
     ci::MOI.ConstraintIndex{MOI.VectorOfVariables,S},
 ) where {S<:VectorCone}
-    return MOI.VectorOfVariables([
-        index_of_column(m, col) for
-        col in reorder(columns(m, ci).values, S, true)
-    ])
+    cols = reorder(columns(m, ci).values, S, true)
+    return MOI.VectorOfVariables(_col_to_index.(m, cols))
 end
 
 function MOI.get(
@@ -791,15 +790,17 @@ function MOI.get(
         throw(MOI.GetAttributeNotAllowed(attr))
     end
     r = rows(m, ci)
-    (frow, fcol, fval) = Mosek.getaccftrip(m.task)
+    frow, fcol, fval = Mosek.getaccftrip(m.task)
     constants = Mosek.getaccb(m.task, ci.value)
     set = MOI.Utilities.set_with_dimension(S, length(r))
-    terms = [
-        MOI.VectorAffineTerm(
-            reorder(frow[i] - first(r) + 1, set, false),
-            MOI.ScalarAffineTerm(fval[i], index_of_column(m, fcol[i])),
-        ) for i in eachindex(frow) if frow[i] in r
-    ]
+    terms = MOI.VectorAffineTerm{Float64}[]
+    for (frowi, fcoli, fvali) in zip(frow, fcol, fval)
+        if frowi in r
+            row = reorder(frowi - first(r) + 1, set, false)
+            term = MOI.ScalarAffineTerm(fvali, _col_to_index(m, fcoli))
+            push!(terms, MOI.VectorAffineTerm(row, term))
+        end
+    end
     return MOI.VectorAffineFunction(terms, -reorder(constants, S, false))
 end
 

src/variable.jl

@@ -43,12 +43,9 @@ function mosek_index(m::Optimizer, vi::MOI.VariableIndex)
     return m.x_sd[vi.value]
 end
 
-function index_of_column(m::Optimizer, col::Int32)
-    id = m.x_block.back[col]
-    if iszero(id)
-        return nothing
-    end
-    return MOI.VariableIndex(id)
+function _col_to_index(m::Optimizer, col::Int32)
+    @assert !iszero(m.x_block.back[col])
+    return MOI.VariableIndex(m.x_block.back[col])
 end
 
 ## Delete #####################################################################

test/runtests.jl

@@ -873,6 +873,60 @@ function test_ConstraintName()
     return
 end
 
+function test_MSK_RES_ERR_MISSING_LICENSE_FILE()
+    model = Mosek.Optimizer()
+    x = MOI.add_variable(model)
+    try
+        MOI.optimize!(model)
+    catch err
+        @test err isa Mosek.MosekError
+        @test Mosek.Rescode(err.rcode) == Mosek.MSK_RES_ERR_MISSING_LICENSE_FILE
+    end
+    return
+end
+
+function test_throw_add_constraint_not_allowed_scalar()
+    model = Mosek.Optimizer()
+    set = MOI.PositiveSemidefiniteConeTriangle(2)
+    x, _ = MOI.add_constrained_variables(model, set)
+    @test_throws(
+        MOI.AddConstraintNotAllowed{MOI.VariableIndex,MOI.Integer},
+        MOI.add_constraint(model, x[1], MOI.Integer()),
+    )
+    return
+end
+
+function test_solution_priority()
+    for (whichsol, priority) in
+        (Mosek.MSK_SOL_ITG => 3, Mosek.MSK_SOL_BAS => 2, Mosek.MSK_SOL_ITR => 1)
+        for (solsta, flag) in (
+            Mosek.MSK_SOL_STA_INTEGER_OPTIMAL => true,
+            Mosek.MSK_SOL_STA_OPTIMAL => true,
+            Mosek.MSK_SOL_STA_UNKNOWN => false,
+        )
+            solution = MosekTools.MosekSolution(
+                whichsol,
+                solsta,
+                Mosek.MSK_PRO_STA_PRIM_AND_DUAL_FEAS,
+                Mosek.Stakey[],
+                Float64[],
+                Vector{Float64}[],
+                Float64[],
+                Float64[],
+                Float64[],
+                Float64[],
+                Mosek.Stakey[],
+                Float64[],
+                Float64[],
+                Float64[],
+                Float64[],
+            )
+            @test MosekTools._solution_priority(solution) == (flag, priority)
+        end
+    end
+    return
+end
+
 end  # module
 
 TestMosekTools.runtests()