Performance improvements (#207)

* cleanup get bounds * add ReducedCost * update checked macro --------- Co-authored-by: Joaquim Garcia <[email protected]> Co-authored-by: guilhermebodin <[email protected]>
jump-dev · Nov 29, 2023 · bdc5553 · bdc5553
1 parent 4d70faf
commit bdc5553
Show file tree

Hide file tree

Showing 3 changed files with 90 additions and 18 deletions.
diff --git a/src/MOI/MOI_wrapper.jl b/src/MOI/MOI_wrapper.jl
@@ -1375,6 +1375,7 @@ function _throw_if_existing_lower(
     if existing_set !== nothing
         throw(MOI.LowerBoundAlreadySet{existing_set, new_set}(variable))
     end
+    return
 end
 
 function _throw_if_existing_upper(
@@ -1399,6 +1400,7 @@ function _throw_if_existing_upper(
     if existing_set !== nothing
         throw(MOI.UpperBoundAlreadySet{existing_set, new_set}(variable))
     end
+    return
 end
 
 function MOI.add_constraint(
@@ -1483,6 +1485,7 @@ function MOI.delete(
     MOI.throw_if_not_valid(model, c)
     info = _info(model, c)
     _set_variable_upper_bound(model, info, Inf)
+    info.previous_upper_bound = Inf
     if info.bound == LESS_AND_GREATER_THAN
         info.bound = GREATER_THAN
     else
@@ -1491,8 +1494,6 @@ function MOI.delete(
     info.lessthan_name = ""
     model.name_to_constraint_index = nothing
     if info.type == BINARY
-        info.previous_lower_bound = _get_variable_lower_bound(model, info)
-        info.previous_upper_bound = _get_variable_upper_bound(model, info)
         @checked Lib.XPRSchgcoltype(model.inner, Cint(1), Ref(Cint(info.column-1)), Ref(UInt8('B')))
     end
     return
@@ -1513,11 +1514,13 @@ function _set_variable_fixed_bound(model, info, value)
         # No SOC constraints, set directly.
         @assert isnan(info.lower_bound_if_soc)
         @checked Lib.XPRSchgbounds(model.inner, Cint(1), Ref(Cint(info.column-1)), Ref(UInt8('B')), Ref(value))
+        return
     elseif value >= 0.0
         # Regardless of whether there are SOC constraints, this is a valid bound
         # for the SOC constraint and should over-ride any previous bounds.
         info.lower_bound_if_soc = NaN
         @checked Lib.XPRSchgbounds(model.inner, Cint(1), Ref(Cint(info.column-1)), Ref(UInt8('B')), Ref(value))
+        return
     elseif isnan(info.lower_bound_if_soc)
         # Previously, we had a non-negative lower bound (i.e., it was set in the
         # case above). Now we're setting this with a negative one, but there are
@@ -1528,6 +1531,7 @@ function _set_variable_fixed_bound(model, info, value)
         @checked Lib.XPRSchgbounds(model.inner, Cint(1), Ref(Cint(info.column-1)), Ref(UInt8('L')), Ref(0.0))
         @checked Lib.XPRSchgbounds(model.inner, Cint(1), Ref(Cint(info.column-1)), Ref(UInt8('U')), Ref(value))
         info.lower_bound_if_soc = value
+        return
     else
         # Previously, we had a negative lower bound. We're setting this with
         # another negative one, but there are still some SOC constraints.
@@ -1536,6 +1540,7 @@ function _set_variable_fixed_bound(model, info, value)
         @assert info.lower_bound_if_soc < 0.0
         info.lower_bound_if_soc = value
         @checked Lib.XPRSchgbounds(model.inner, Cint(1), Ref(Cint(info.column-1)), Ref(UInt8('U')), Ref(value))
+        return
     end
 end
 
@@ -1554,11 +1559,13 @@ function _set_variable_lower_bound(model, info, value)
         # No SOC constraints, set directly.
         @assert isnan(info.lower_bound_if_soc)
         @checked Lib.XPRSchgbounds(model.inner, Cint(1), Ref(Cint(info.column-1)), Ref(UInt8('L')), Ref(value))
+        return
     elseif value >= 0.0
         # Regardless of whether there are SOC constraints, this is a valid bound
         # for the SOC constraint and should over-ride any previous bounds.
         info.lower_bound_if_soc = NaN
         @checked Lib.XPRSchgbounds(model.inner, Cint(1), Ref(Cint(info.column-1)), Ref(UInt8('L')), Ref(value))
+        return
     elseif isnan(info.lower_bound_if_soc)
         # Previously, we had a non-negative lower bound (i.e., it was set in the
         # case above). Now we're setting this with a negative one, but there are
@@ -1567,11 +1574,13 @@ function _set_variable_lower_bound(model, info, value)
         @assert value < 0.0
         @checked Lib.XPRSchgbounds(model.inner, Cint(1), Ref(Cint(info.column-1)), Ref(UInt8('L')), Ref(0.0))
         info.lower_bound_if_soc = value
+        return
     else
         # Previously, we had a negative lower bound. We're setting this with
         # another negative one, but there are still some SOC constraints.
         @assert info.lower_bound_if_soc < 0.0
         info.lower_bound_if_soc = value
+        return
     end
 end
 
@@ -1635,6 +1644,7 @@ function MOI.delete(
     MOI.throw_if_not_valid(model, c)
     info = _info(model, c)
     _set_variable_lower_bound(model, info, -Inf)
+    info.previous_lower_bound = -Inf
     if info.bound == LESS_AND_GREATER_THAN
         info.bound = LESS_THAN
     else
@@ -1643,8 +1653,6 @@ function MOI.delete(
     info.greaterthan_interval_or_equalto_name = ""
     model.name_to_constraint_index = nothing
     if info.type == BINARY
-        info.previous_lower_bound = _get_variable_lower_bound(model, info)
-        info.previous_upper_bound = _get_variable_upper_bound(model, info)
         @checked Lib.XPRSchgcoltype(model.inner, Cint(1), Ref(Cint(info.column-1)), Ref(UInt8('B')))
     end
     return
@@ -1657,13 +1665,13 @@ function MOI.delete(
     MOI.throw_if_not_valid(model, c)
     info = _info(model, c)
     _set_variable_lower_bound(model, info, -Inf)
+    info.previous_lower_bound = -Inf
     _set_variable_upper_bound(model, info, Inf)
+    info.previous_upper_bound = Inf
     info.bound = NONE
     info.greaterthan_interval_or_equalto_name = ""
     model.name_to_constraint_index = nothing
     if info.type == BINARY
-        info.previous_lower_bound = _get_variable_lower_bound(model, info)
-        info.previous_upper_bound = _get_variable_upper_bound(model, info)
         @checked Lib.XPRSchgcoltype(model.inner, Cint(1), Ref(Cint(info.column-1)), Ref(UInt8('B')))
     end
     return
@@ -1676,13 +1684,13 @@ function MOI.delete(
     MOI.throw_if_not_valid(model, c)
     info = _info(model, c)
     _set_variable_lower_bound(model, info, -Inf)
+    info.previous_lower_bound = -Inf
     _set_variable_upper_bound(model, info, Inf)
+    info.previous_upper_bound = Inf
     info.bound = NONE
     info.greaterthan_interval_or_equalto_name = ""
     model.name_to_constraint_index = nothing
     if info.type == BINARY
-        info.previous_lower_bound = _get_variable_lower_bound(model, info)
-        info.previous_upper_bound = _get_variable_upper_bound(model, info)
         @checked Lib.XPRSchgcoltype(model.inner, Cint(1), Ref(Cint(info.column-1)), Ref(UInt8('B')))
     end
     return
@@ -1739,26 +1747,26 @@ function MOI.set(
     if lower == upper
         if lower !== nothing
             _set_variable_fixed_bound(model, info, lower)
+            info.previous_lower_bound = lower
+            info.previous_upper_bound = upper
         end
     else
         if lower !== nothing
             _set_variable_lower_bound(model, info, lower)
+            info.previous_lower_bound = lower
         end
         if upper !== nothing
             _set_variable_upper_bound(model, info, upper)
+            info.previous_upper_bound = upper
         end
     end
-    info.previous_lower_bound = _get_variable_lower_bound(model, info)
-    info.previous_upper_bound = _get_variable_upper_bound(model, info)
     return
 end
 
 function MOI.add_constraint(
     model::Optimizer, f::MOI.VariableIndex, ::MOI.ZeroOne
 )
     info = _info(model, f)
-    info.previous_lower_bound = _get_variable_lower_bound(model, info)
-    info.previous_upper_bound = _get_variable_upper_bound(model, info)
     lower, upper = info.previous_lower_bound, info.previous_upper_bound
     if info.type == CONTINUOUS
         if lower !== nothing
@@ -1802,8 +1810,12 @@ function MOI.delete(
     MOI.throw_if_not_valid(model, c)
     info = _info(model, c)
     @checked Lib.XPRSchgcoltype(model.inner, Cint(1), Ref(Cint(info.column-1)), Ref(UInt8('C')))
-    _set_variable_lower_bound(model, info, info.previous_lower_bound)
-    _set_variable_upper_bound(model, info, info.previous_upper_bound)
+    if !isnan(info.previous_lower_bound)
+        _set_variable_lower_bound(model, info, info.previous_lower_bound)
+    end
+    if !isnan(info.previous_upper_bound)
+        _set_variable_upper_bound(model, info, info.previous_upper_bound)
+    end
     info.type = CONTINUOUS
     info.type_constraint_name = ""
     model.name_to_constraint_index = nothing
@@ -3080,6 +3092,10 @@ function _farkas_variable_dual(model::Optimizer, col::Int64)
     return sum(v * model.cached_solution.linear_dual[i + 1] for (i, v) in zip(mrwind, dmatval))
 end
 
+#=
+    Duals
+=#
+
 function MOI.get(
     model::Optimizer, attr::MOI.ConstraintDual,
     c::MOI.ConstraintIndex{MOI.VariableIndex, MOI.LessThan{Float64}}
@@ -4384,4 +4400,31 @@ function _get_constraint_names(model)
     end
     con_names = split(all_names, '\0')[1:num_constraints]
     return strip.(con_names)
+end
+
+#=
+    Reduced costs
+=#
+
+struct ReducedCost <: MOI.AbstractVariableAttribute
+    result_index::Int
+    ReducedCost(result_index::Int = 1) = new(result_index)
+end
+
+function MOI.supports(
+    ::Optimizer, ::ReducedCost, ::Type{MOI.VariableIndex})
+    return true
+end
+
+MOI.is_set_by_optimize(::ReducedCost) = true
+
+MOI.attribute_value_type(::ReducedCost) = Float64
+
+function MOI.get(
+    model::Optimizer, attr::ReducedCost, vi::MOI.VariableIndex
+)
+    _throw_if_optimize_in_progress(model, attr)
+    MOI.check_result_index_bounds(model, attr)
+    column = _info(model, vi).column
+    return model.cached_solution.variable_dual[column]
 end
diff --git a/src/utils.jl b/src/utils.jl
@@ -132,10 +132,10 @@ macro checked(expr)
     @assert ( expr.args[1].head == :(.) ) && ( expr.args[1].args[1] == :Lib) "Can only use @checked on Lib.\$function"
     @assert length(expr.args) >= 2 "Lib.\$function must be contain atleast one argument and the first argument must be of type XpressProblem"
     f = replace(String(expr.args[1].args[2].value), "XPRS" => "")
-    return esc(quote
-        r = $(expr)::Cint
-        _check($(expr.args[2]), r)::Nothing
-    end)
+    return quote
+        r = $(esc(expr))::Cint
+        _check($(esc(expr.args[2])), r)::Nothing
+    end
 end
 
 function _check(prob, val::Cint)

diff --git a/test/MathOptInterface/MOI_wrapper.jl b/test/MathOptInterface/MOI_wrapper.jl
@@ -976,6 +976,35 @@ function test_dummy_nlp()
     return nothing
 end
 
+function test_multiple_modifications()
+    model = Xpress.Optimizer(OUTPUTLOG = 0)
+
+    x = MOI.add_variable(model)
+
+    c = MOI.add_constraint(model, x, MOI.GreaterThan(1.0))
+
+    MOI.set(
+        model,
+        MOI.ObjectiveFunction{MOI.ScalarAffineFunction{Float64}}(),
+        2.0 * x,
+    )
+
+    MOI.set(
+        model,
+        MOI.ObjectiveSense(),
+        MOI.MIN_SENSE,
+    )
+
+    MOI.optimize!(model)
+
+    @test MOI.get(model, MOI.VariablePrimal(), x) == 1.0
+
+    @test MOI.get(model, MOI.ConstraintDual(), c) == 2.0
+    @test MOI.get(model, Xpress.ReducedCost(), x) == 2.0
+
+    return
+end
+
 end
 
 TestMOIWrapper.runtests()