Follow up of "custom data" (#538)

* add AbstractCustomData and set/get inc_val * fix bugs * remove duplicate methods * delete unnecessary prefixes and fix some bugs * revert some changes and update docstring
atoptima · Jun 10, 2021 · 37bdbb6 · 37bdbb6
1 parent ea80081
commit 37bdbb6
Show file tree

Hide file tree

Showing 11 changed files with 33 additions and 37 deletions.
diff --git a/src/Algorithm/colgen.jl b/src/Algorithm/colgen.jl
@@ -375,12 +375,12 @@ function solve_sps_to_gencols!(
         updatereducedcosts!(reform, redcostshelper, smooth_dual_sol)
     end
 
-    # udate the incumbent values of constraints
+    # update the incumbent values of constraints
     for (_, constr) in getconstrs(masterform)
-        constr.curdata.inc_val = 0.0
+        setcurincval!(masterform, constr, 0.0)
     end
     for (constrid, val) in smooth_dual_sol
-        getconstr(masterform, constrid).curdata.inc_val = val
+        setcurincval!(masterform, constrid, val)
     end
 
     ### BEGIN LOOP TO BE PARALLELIZED

diff --git a/src/ColunaBase/ColunaBase.jl b/src/ColunaBase/ColunaBase.jl
@@ -12,7 +12,7 @@ import Printf
 
 # interface.jl
 export AbstractModel, AbstractProblem, AbstractSense, AbstractMinSense, AbstractMaxSense,
-    AbstractSpace, AbstractPrimalSpace, AbstractDualSpace, getstorage
+    AbstractSpace, AbstractPrimalSpace, AbstractDualSpace, AbstractCustomData, getstorage
 
 # nestedenum.jl
 export NestedEnum, @nestedenum, @exported_nestedenum

diff --git a/src/ColunaBase/solsandbounds.jl b/src/ColunaBase/solsandbounds.jl
@@ -210,13 +210,15 @@ function convert_status(coluna_status::SolutionStatus)
     return MOI.OTHER_RESULT_STATUS
 end
 
+abstract type AbstractCustomData end
+
 # Solution
 struct Solution{Model<:AbstractModel,Decision,Value} <: AbstractDict{Decision,Value}
     model::Model
     bound::Float64
     status::SolutionStatus
     sol::DynamicSparseArrays.PackedMemoryArray{Decision,Value}
-    custom_data
+    custom_data::Union{Nothing, AbstractCustomData}
 end
 
 """
@@ -226,7 +228,7 @@ end
         values::Vector,
         solution_values::Float64,
         status::SolutionStatus,
-        [custom_data]
+        [custom_data]::Union{Nothing, AbstractCustomData}
     )
 
 Create a solution to the `model`. Other arguments are: 
@@ -237,7 +239,7 @@ Create a solution to the `model`. Other arguments are:
 """
 function Solution{Mo,De,Va}(
     model::Mo, decisions::Vector{De}, values::Vector{Va}, solution_value::Float64, 
-    status::SolutionStatus, custom_data = nothing
+    status::SolutionStatus, custom_data::Union{Nothing, AbstractCustomData} = nothing
 ) where {Mo<:AbstractModel,De,Va}
     sol = DynamicSparseArrays.dynamicsparsevec(decisions, values)
     return Solution(model, solution_value, status, sol, custom_data)

diff --git a/src/MOIcallbacks.jl b/src/MOIcallbacks.jl
@@ -22,7 +22,7 @@ function MOI.submit(
     cost::Float64,
     variables::Vector{MOI.VariableIndex},
     values::Vector{Float64},
-    custom_data = nothing
+    custom_data::Union{Nothing, AbstractCustomData} = nothing
 )
     form = cb.callback_data.form
     S = getobjsense(form)
@@ -100,7 +100,7 @@ function MOI.submit(
     cb::Union{MOI.UserCut{Algorithm.RobustCutCallbackContext}, MOI.LazyConstraint{Algorithm.RobustCutCallbackContext}},
     func::MOI.ScalarAffineFunction{Float64},
     set::Union{MOI.LessThan{Float64}, MOI.GreaterThan{Float64}, MOI.EqualTo{Float64}},
-    custom_data = nothing
+    custom_data::Union{Nothing, AbstractCustomData} = nothing
 )
     form = cb.callback_data.form
     rhs = MathProg.convert_moi_rhs_to_coluna(set)

diff --git a/src/MathProg/constraint.jl b/src/MathProg/constraint.jl
@@ -53,15 +53,15 @@ mutable struct Constraint <: AbstractVarConstr
     curdata::ConstrData
     moirecord::MoiConstrRecord
     art_var_ids::Vector{VarId}
-    custom_data::Any
+    custom_data::Union{Nothing, AbstractCustomData}
 end
 
 const ConstrId = Id{Constraint}
 
 function Constraint(
     id::ConstrId, name::String;
     constr_data = ConstrData(), moi_index::MoiConstrIndex = MoiConstrIndex(),
-    custom_data = nothing
+    custom_data::Union{Nothing, AbstractCustomData} = nothing
 )
     return Constraint(
         id, name, constr_data, ConstrData(constr_data), MoiConstrRecord(index = moi_index), 

diff --git a/src/MathProg/formulation.jl b/src/MathProg/formulation.jl
@@ -172,7 +172,7 @@ function setvar!(
     is_explicit::Bool = true,
     moi_index::MoiVarIndex = MoiVarIndex(),
     members::Union{ConstrMembership,Nothing} = nothing,
-    custom_data = nothing,
+    custom_data::Union{Nothing, AbstractCustomData} = nothing,
     id = generatevarid(duty, form),
     branching_priority::Float64 = 1.0
 )
@@ -300,7 +300,7 @@ function setcol_from_sp_primalsol!(
     masterform::Formulation, spform::Formulation, sol_id::VarId, name::String,
     duty::Duty{Variable}; lb::Float64 = 0.0, ub::Float64 = Inf, kind::VarKind = Continuous,
     inc_val::Float64 = 0.0, is_active::Bool = true, is_explicit::Bool = true,
-    moi_index::MoiVarIndex = MoiVarIndex(), custom_data = nothing
+    moi_index::MoiVarIndex = MoiVarIndex(), custom_data::Union{Nothing, AbstractCustomData} = nothing
 )
     cost = getprimalsolcosts(spform)[sol_id]
     master_coef_matrix = getcoefmatrix(masterform)
@@ -411,7 +411,7 @@ function setconstr!(
     moi_index::MoiConstrIndex = MoiConstrIndex(),
     members = nothing, # todo Union{AbstractDict{VarId,Float64},Nothing}
     loc_art_var_abs_cost::Float64 = 0.0,
-    custom_data = nothing,
+    custom_data::Union{Nothing, AbstractCustomData} = nothing,
     id = generateconstrid(duty, form)
 )
     if getduty(id) != duty

diff --git a/src/MathProg/manager.jl b/src/MathProg/manager.jl
@@ -18,7 +18,7 @@ mutable struct FormulationManager
     coefficients::ConstrVarMatrix # rows = constraints, cols = variables
     expressions::VarVarMatrix # cols = variables, rows = expressions
     primal_sols::VarVarMatrix # cols = primal solutions with varid, rows = variables
-    primal_sols_custom_data::Dict{VarId,Any}
+    primal_sols_custom_data::Dict{VarId, AbstractCustomData}
     primal_sol_costs::DynSparseVector{VarId} # primal solutions with varid map to their cost
     dual_sols::ConstrConstrMatrix # cols = dual solutions with constrid, rows = constrs
     dual_sol_rhss::DynSparseVector{ConstrId} # dual solutions with constrid map to their rhs

diff --git a/src/MathProg/solutions.jl b/src/MathProg/solutions.jl
@@ -6,14 +6,14 @@ const DualSolution{M} = Solution{M, ConstrId, Float64}
 
 function PrimalSolution(
     form::M, decisions::Vector{De}, vals::Vector{Va}, val::Float64, status::SolutionStatus,
-    custom_data = nothing
+    custom_data::Union{Nothing, AbstractCustomData} = nothing
 ) where {M<:AbstractFormulation,De,Va}
     return Solution{M,De,Va}(form, decisions, vals, val, status, custom_data)
 end
 
 function DualSolution(
     form::M, decisions::Vector{De}, vals::Vector{Va}, val::Float64, status::SolutionStatus,
-    custom_data = nothing
+    custom_data::Union{Nothing, AbstractCustomData} = nothing
 ) where {M<:AbstractFormulation,De,Va}
     return Solution{M,De,Va}(form, decisions, vals, val, status, custom_data)
 end

diff --git a/src/MathProg/varconstr.jl b/src/MathProg/varconstr.jl
@@ -152,7 +152,6 @@ function setcurrhs!(form::Formulation, constr::Constraint, rhs::Float64)
 end
 setcurrhs!(form::Formulation, constrid::ConstrId, rhs::Float64) = setcurrhs!(form, getconstr(form, constrid), rhs)
 
-
 # Variable & Constraints
 ## kind
 """
@@ -264,7 +263,8 @@ end
     getperenincval(formulation, varconstr)
 
 Return the incumbent value as defined by the user of a variable or a constraint in a formulation. 
-The incumbent value is ?
+The incumbent value is the primal value associated to a variable or the dual value associated to
+a constraint.
 """
 getperenincval(form::Formulation, varid::VarId) = getperenincval(form, getvar(form, varid))
 getperenincval(form::Formulation, var::Variable) = var.perendata.inc_val

diff --git a/src/MathProg/variable.jl b/src/MathProg/variable.jl
@@ -78,7 +78,7 @@ mutable struct Variable <: AbstractVarConstr
     curdata::VarData
     branching_priority::Float64
     moirecord::MoiVarRecord
-    custom_data::Any
+    custom_data::Union{Nothing, AbstractCustomData}
 end
 
 const VarId = Id{Variable}
@@ -87,7 +87,7 @@ getid(var::Variable) = var.id
 
 function Variable(
     id::VarId, name::String; var_data = VarData(), moi_index::MoiVarIndex = MoiVarIndex(),
-    custom_data = nothing, branching_priority = 1.0
+    custom_data::Union{Nothing, AbstractCustomData} = nothing, branching_priority = 1.0
 )
     return Variable(
         id, name, var_data, VarData(var_data), branching_priority,

diff --git a/test/custom_var_cuts_tests.jl b/test/custom_var_cuts_tests.jl
@@ -12,11 +12,11 @@ given by length(s). The custom cut used to cut the fractional solution is
                 sum(λ_s for s in sols if length(s) >= 2) <= 1.0
 where sols is the set of possible combinations of items in a bin.
 =#
-struct MyCustomVarData
+struct MyCustomVarData <: ColunaBase.AbstractCustomData
     nb_items::Int
 end
 
-struct MyCustomCutData
+struct MyCustomCutData <: ColunaBase.AbstractCustomData
     min_items::Int
 end
 
@@ -72,15 +72,9 @@ function custom_var_cuts_test()
 
         model, x, y, dec = build_toy_model(coluna)
 
-        custom_var_is_added = false
-        custom_cut_is_added = false
-
         function my_pricing_callback(cbdata)
-            if !custom_var_is_added
-                Coluna.MathProg.addcustomvars!(cbdata.form.parent_formulation, MyCustomVarData)
-                custom_var_is_added = true
-            end
-
+            addcustomvars!(cbdata.form.parent_formulation, MyCustomVarData)
+
             # Get the reduced costs of the original variables
             I = [1, 2, 3]
             b = BD.callback_spid(cbdata, model)
@@ -91,7 +85,10 @@ function custom_var_cuts_test()
             custduals = Tuple{Int, Float64}[]
             for (_, constr) in getconstrs(cbdata.form.parent_formulation)
                 if typeof(constr.custom_data) == MyCustomCutData
-                    push!(custduals, (constr.custom_data.min_items, constr.curdata.inc_val))
+                    push!(custduals, (
+                        constr.custom_data.min_items,
+                        getcurincval(cbdata.form.parent_formulation, constr)
+                    ))
                 end
             end
 
@@ -135,10 +132,7 @@ function custom_var_cuts_test()
         )
 
         function custom_cut_sep(cbdata)
-            if !custom_cut_is_added
-                Coluna.MathProg.addcustomconstrs!(cbdata.form, MyCustomCutData)
-                custom_cut_is_added = true
-            end
+            addcustomconstrs!(cbdata.form, MyCustomCutData)
 
             # compute the constraint violation
             viol = -1.0
@@ -166,5 +160,5 @@ function custom_var_cuts_test()
         @show JuMP.objective_value(model)
         @test JuMP.termination_status(model) == MOI.OPTIMAL
     end
-
+    
 end