From 4adbee49fb078867ead1d4acda9ba9d184302fe0 Mon Sep 17 00:00:00 2001
From: dhendryc <92737336+dhendryc@users.noreply.github.com>
Date: Thu, 5 Sep 2024 15:25:05 +0200
Subject: [PATCH] Fixing issue with post solve (#191)

* Round candidate solutions to avoid precision errors.

* Minor change.

* Type correction in function signature.

* Tracking down the post solve issue.

* Move out of the test folder since this is only for tracking down the issue,

* Sanity check regarding the integral variables.

* Clean up new solutions.

* Add output description of the solve function.

* Some explanation and pushed up to user level.

* New flags also for the other solves.

* Push the new flags down to the original solve function.

* Incumbent update is decided in add_solution, nowhere else.

* Change default values for clean solution process.

* BCG produces non integer vertices (#194)

BCG returns non integer vertices in FrankWolfe callback due to simplex descent steps.

* Integer check only in debug mode.

* Delete debugging files.

* Update project.toml.

* Simply round the vertices at the integer entries. No extra post solve required.

* Remove show statement.

---------

Co-authored-by: Deborah Hendrych <hendrych@zib.de>
Co-authored-by: Hendrych <dhendryc@mob-009366.zib.de>
---
 Project.toml              |  2 +-
 src/MOI_bounded_oracle.jl |  4 ++++
 src/callbacks.jl          | 26 ++++++++------------------
 src/custom_bonobo.jl      | 21 +++++++++++++++++----
 src/heuristics.jl         |  9 +--------
 src/interface.jl          | 21 ++++++++++++++-------
 src/managed_blmo.jl       |  4 ++++
 7 files changed, 49 insertions(+), 38 deletions(-)

diff --git a/Project.toml b/Project.toml
index a1c18d335..436ad8370 100644
--- a/Project.toml
+++ b/Project.toml
@@ -1,7 +1,7 @@
 name = "Boscia"
 uuid = "36b166db-dac5-4d05-b36a-e6c4cef071c9"
 authors = ["ZIB IOL"]
-version = "0.1.25"
+version = "0.1.26"
 
 [deps]
 Bonobo = "f7b14807-3d4d-461a-888a-05dd4bca8bc3"
diff --git a/src/MOI_bounded_oracle.jl b/src/MOI_bounded_oracle.jl
index f3e7f74fc..6039cee10 100644
--- a/src/MOI_bounded_oracle.jl
+++ b/src/MOI_bounded_oracle.jl
@@ -635,6 +635,8 @@ function solve(
     use_shadow_set=true,
     custom_heuristics=[Heuristic()],
     rounding_prob=1.0, 
+    clean_solutions=false, 
+    max_clean_iter=10,
     kwargs...,
 )
     blmo = convert(MathOptBLMO, lmo)
@@ -670,6 +672,8 @@ function solve(
         use_shadow_set=use_shadow_set,
         custom_heuristics=custom_heuristics,
         rounding_prob=rounding_prob,
+        clean_solutions=clean_solutions,
+        max_clean_iter=max_clean_iter,
         kwargs...,
     )
 end
diff --git a/src/callbacks.jl b/src/callbacks.jl
index dcb14f3ee..a61c0a9d7 100644
--- a/src/callbacks.jl
+++ b/src/callbacks.jl
@@ -20,7 +20,11 @@ function build_FW_callback(
                 @info "$(state.v)"
                 check_infeasible_vertex(tree.root.problem.tlmo.blmo, tree)
                 @assert is_linear_feasible(tree.root.problem.tlmo, state.v)
-            end
+            end  
+            if state.tt != FrankWolfe.simplex_descent && !is_integer_feasible(tree, state.v)
+                @info "Vertex not integer feasible! Here are the integer variables: $(state.v[tree.root.problem.integer_variables])"
+                @assert is_integer_feasible(tree, state.v)
+            end    
         end
         push!(fw_iterations, state.t)
 
@@ -34,15 +38,8 @@ function build_FW_callback(
                     tree.root.options[:domain_oracle],
                 )
                 if best_val < tree.incumbent
-                    tree.root.updated_incumbent[] = true
                     node = tree.nodes[tree.root.current_node_id[]]
-                    sol = FrankWolfeSolution(best_val, best_v, node, :Solver)
-                    push!(tree.solutions, sol)
-                    if tree.incumbent_solution === nothing ||
-                       sol.objective < tree.incumbent_solution.objective
-                        tree.incumbent_solution = sol
-                    end
-                    tree.incumbent = best_val
+                    add_new_solution!(tree, node, best_val, best_v, :Solver)
                     Bonobo.bound!(tree, node.id)
                 end
             end
@@ -54,19 +51,12 @@ function build_FW_callback(
             return false
         end
 
-        if tree.root.options[:domain_oracle](state.v)
+        if tree.root.options[:domain_oracle](state.v) && state.tt != FrankWolfe.simplex_descent
             val = tree.root.problem.f(state.v)
             if val < tree.incumbent
-                tree.root.updated_incumbent[] = true
                 #TODO: update solution without adding node
                 node = tree.nodes[tree.root.current_node_id[]]
-                sol = FrankWolfeSolution(val, copy(state.v), node, :vertex)
-                push!(tree.solutions, sol)
-                if tree.incumbent_solution === nothing ||
-                   sol.objective < tree.incumbent_solution.objective
-                    tree.incumbent_solution = sol
-                end
-                tree.incumbent = val
+                add_new_solution!(tree, node, val, copy(state.v), :vertex)
                 Bonobo.bound!(tree, node.id)
             end
         end
diff --git a/src/custom_bonobo.jl b/src/custom_bonobo.jl
index 0fcf91775..dcce29444 100644
--- a/src/custom_bonobo.jl
+++ b/src/custom_bonobo.jl
@@ -30,7 +30,6 @@ function Bonobo.optimize!(
     tree::Bonobo.BnBTree{<:FrankWolfeNode};
     callback=(args...; kwargs...) -> (),
 )
-    #println("OWN OPTIMIZE")
     while !Bonobo.terminated(tree)
         node = Bonobo.get_next_node(tree, tree.options.traverse_strategy)
         lb, ub = Bonobo.evaluate_node!(tree, node)
@@ -84,8 +83,6 @@ function Bonobo.update_best_solution!(
 )
     isinf(node.ub) && return false
     node.ub >= tree.incumbent && return false
-    tree.root.updated_incumbent[] = true
-    tree.incumbent = node.ub
 
     Bonobo.add_new_solution!(tree, node)
     return true
@@ -95,10 +92,25 @@ function Bonobo.add_new_solution!(
     tree::Bonobo.BnBTree{N,R,V,S},
     node::Bonobo.AbstractNode,
 ) where {N,R,V,S<:FrankWolfeSolution{N,V}}
-    sol = FrankWolfeSolution(node.ub, Bonobo.get_relaxed_values(tree, node), node, :iterate)
+    add_new_solution!(tree, node, node.ub, Bonobo.get_relaxed_values(tree, node), :iterate)
+end
+
+function add_new_solution!(
+    tree::Bonobo.BnBTree{N,R,V,S},
+    node::Bonobo.AbstractNode,
+    objective::T,
+    solution::V,
+    origin::Symbol,
+) where {N,R,V,S<:FrankWolfeSolution{N,V},T<:Real}
+    sol = FrankWolfeSolution(objective, solution, node, origin)
+    sol.solution = solution
+    sol.objective = objective
+
     push!(tree.solutions, sol)
     if tree.incumbent_solution === nothing || sol.objective < tree.incumbent_solution.objective
+        tree.root.updated_incumbent[] = true
         tree.incumbent_solution = sol
+        tree.incumbent = sol.objective
     end
 end
 
@@ -115,3 +127,4 @@ function Bonobo.get_solution(
     end
     return tree.solutions[result].solution
 end
+
diff --git a/src/heuristics.jl b/src/heuristics.jl
index b3dd786ee..3a5d7b3a6 100644
--- a/src/heuristics.jl
+++ b/src/heuristics.jl
@@ -30,15 +30,8 @@ end
 Add a new solution found from the heuristic to the tree.
 """
 function add_heuristic_solution(tree, x, val, heuristic_name::Symbol)
-    tree.root.updated_incumbent[] = true
     node = tree.nodes[tree.root.current_node_id[]]
-    sol = FrankWolfeSolution(val, x, node, heuristic_name)
-    push!(tree.solutions, sol)
-    if tree.incumbent_solution === nothing ||
-        sol.objective < tree.incumbent_solution.objective
-        tree.incumbent_solution = sol
-    end
-    tree.incumbent = val
+    add_new_solution!(tree, node, val, x, heuristic_name)
     Bonobo.bound!(tree, node.id)
 end
 
diff --git a/src/interface.jl b/src/interface.jl
index 65fe63ad9..b496a1ea8 100644
--- a/src/interface.jl
+++ b/src/interface.jl
@@ -53,7 +53,15 @@ use_shadow_set         - The shadow set is the set of discarded vertices which i
                         performance might improve by disabling this option. 
 custom_heuristics      - List of  custom heuristic from the user.    
 prob_rounding          - The probability for calling the rounding heuristics. Since the feasibility has to be checked, it might
-                        expensive to do this for every node.                    
+                        expensive to do this for every node. 
+clean_solutions        - Flag deciding whether new solutions should be polished. They will be rounded and then a quick Frank-Wolfe run will be started.
+max_clean_iter         - Maximum number of iteration in the Frank-Wolfe call for polishing new solutions.
+                            
+Returns
+
+- x - the solution.
+- tlmo - the blmo wrapped in a TimeTrackingLMO instance.
+- result - dictionary containg the statistics and information for plotting progress plots.
 """
 function solve(
     f,
@@ -89,6 +97,8 @@ function solve(
     use_shadow_set=true,
     custom_heuristics=[Heuristic()],
     rounding_prob=1.0,
+    clean_solutions=false, 
+    max_clean_iter=10,
     kwargs...,
 )
     if verbose
@@ -202,6 +212,8 @@ function solve(
                 :use_shadow_set => use_shadow_set,
                 :heuristics => heuristics, 
                 :heu_ncalls => 0,
+                :max_clean_iter => max_clean_iter,
+                :clean_solutions => clean_solutions,
             ),
         ),
         branch_strategy=branching_strategy,
@@ -234,12 +246,7 @@ function solve(
         @assert is_linear_feasible(blmo, start_solution) &&
                 is_integer_feasible(tree, start_solution)
         node = tree.nodes[1]
-        sol = FrankWolfeSolution(f(start_solution), start_solution, node, :start)
-        push!(tree.solutions, sol)
-        if tree.incumbent_solution === nothing || sol.objective < tree.incumbent_solution.objective
-            tree.incumbent_solution = sol
-            tree.incumbent = sol.objective
-        end
+        add_new_solution!(tree, node, f(start_solution), start_solution, :start)
     end
 
     # build callbacks
diff --git a/src/managed_blmo.jl b/src/managed_blmo.jl
index e2d5e7ebe..e13267355 100644
--- a/src/managed_blmo.jl
+++ b/src/managed_blmo.jl
@@ -275,6 +275,8 @@ function solve(
     use_shadow_set=true,
     custom_heuristics=[Heuristic()],
     rounding_prob=1.0,
+    clean_solutions=false, 
+    max_clean_iter=10,
     kwargs...,
 )
     blmo = ManagedBoundedLMO(sblmo, lower_bounds, upper_bounds, int_vars, n)
@@ -312,6 +314,8 @@ function solve(
         use_shadow_set=use_shadow_set,
         custom_heuristics=custom_heuristics,
         rounding_prob=rounding_prob,
+        clean_solutions=clean_solutions,
+        max_clean_iter=max_clean_iter,
         kwargs...,
     )
 end