Fix bug on probing cache usage of lb bounds prop and probing cache bounds lookup with wrong variable id

cpp/src/mip/local_search/rounding/constraint_prop.cu

@@ -593,7 +593,8 @@ std::vector<thrust::pair<i_t, f_t>> constraint_prop_t<i_t, f_t>::generate_bulk_r
                  "Probing value must be an integer");
     f_t val_to_round = first_probe;
     // check probing cache if some implied bounds exists
-    if (use_probing_cache && bounds_update.probing_cache.contains(unset_var_idx)) {
+    if (use_probing_cache &&
+        bounds_update.probing_cache.contains(*sol.problem_ptr, unset_var_idx)) {
       // check if there are any conflicting bounds
       val_to_round =
         bounds_update.probing_cache.get_least_conflicting_rounding(*sol.problem_ptr,

cpp/src/mip/local_search/rounding/lb_constraint_prop.cu

@@ -287,11 +287,17 @@ std::vector<thrust::pair<i_t, f_t>> lb_constraint_prop_t<i_t, f_t>::generate_bul
                  "Probing value must be an integer");
     f_t val_to_round = first_probe;
     // check probing cache if some implied bounds exists
-    if (use_probing_cache && lb_bounds_update.probing_cache.contains(unset_var_idx)) {
+    if (use_probing_cache &&
+        lb_bounds_update.probing_cache.contains(*orig_sol.problem_ptr, unset_var_idx)) {
       // TODO here we can try checking the amount of implied stack.
       // check if there are any conflicting bounds
-      val_to_round = lb_bounds_update.probing_cache.get_least_conflicting_rounding(
-        lb_bounds_update.host_bounds, unset_var_idx, first_probe, second_probe, int_tol);
+      val_to_round =
+        lb_bounds_update.probing_cache.get_least_conflicting_rounding(*orig_sol.problem_ptr,
+                                                                      lb_bounds_update.host_bounds,
+                                                                      unset_var_idx,
+                                                                      first_probe,
+                                                                      second_probe,
+                                                                      int_tol);
     }
     cuopt_assert(orig_sol.problem_ptr->variable_lower_bounds.element(
                    unset_var_idx, orig_sol.handle_ptr->get_stream()) <= val_to_round + int_tol &&

cpp/src/mip/presolve/lb_probing_cache.cu

@@ -28,16 +28,21 @@ namespace cuopt::linear_programming::detail {
 
 template <typename i_t, typename f_t>
 void lb_probing_cache_t<i_t, f_t>::update_bounds_with_selected(
-  std::vector<f_t>& host_bounds, const cache_entry_t<i_t, f_t>& cache_entry)
+  std::vector<f_t>& host_bounds,
+  const cache_entry_t<i_t, f_t>& cache_entry,
+  const std::vector<i_t>& reverse_original_ids)
 {
   i_t n_bounds_updated = 0;
   for (const auto& [var_idx, bound] : cache_entry.var_to_cached_bound_map) {
-    if (host_bounds[2 * var_idx] < bound.lb) {
-      host_bounds[2 * var_idx] = bound.lb;
+    i_t var_idx_in_current_problem = reverse_original_ids[var_idx];
+    // -1 means that variable was fixed and doesn't exists in the current problem
+    if (var_idx_in_current_problem == -1) { continue; }
+    if (host_bounds[2 * var_idx_in_current_problem] < bound.lb) {
+      host_bounds[2 * var_idx_in_current_problem] = bound.lb;
       n_bounds_updated++;
     }
-    if (host_bounds[2 * var_idx + 1] > bound.ub) {
-      host_bounds[2 * var_idx + 1] = bound.ub;
+    if (host_bounds[2 * var_idx_in_current_problem + 1] > bound.ub) {
+      host_bounds[2 * var_idx_in_current_problem + 1] = bound.ub;
       n_bounds_updated++;
     }
   }
@@ -47,25 +52,31 @@ template <typename i_t, typename f_t>
 i_t lb_probing_cache_t<i_t, f_t>::check_number_of_conflicting_vars(
   const std::vector<f_t>& host_bounds,
   const cache_entry_t<i_t, f_t>& cache_entry,
-  f_t integrality_tolerance)
+  f_t integrality_tolerance,
+  const std::vector<i_t>& reverse_original_ids)
 {
   i_t n_conflicting_var = 0;
   for (const auto& [var_idx, bound] : cache_entry.var_to_cached_bound_map) {
-    if (host_bounds[2 * var_idx] - integrality_tolerance > bound.ub ||
-        host_bounds[2 * var_idx + 1] < bound.lb - integrality_tolerance) {
+    i_t var_idx_in_current_problem = reverse_original_ids[var_idx];
+    // -1 means that variable was fixed and doesn't exists in the current problem
+    if (var_idx_in_current_problem == -1) { continue; }
+    if (host_bounds[2 * var_idx_in_current_problem] - integrality_tolerance > bound.ub ||
+        host_bounds[2 * var_idx_in_current_problem + 1] < bound.lb - integrality_tolerance) {
       ++n_conflicting_var;
     }
   }
   return n_conflicting_var;
 }
 
 template <typename i_t, typename f_t>
-f_t lb_probing_cache_t<i_t, f_t>::get_least_conflicting_rounding(std::vector<f_t>& host_bounds,
-                                                                 i_t var_id,
+f_t lb_probing_cache_t<i_t, f_t>::get_least_conflicting_rounding(problem_t<i_t, f_t>& problem,
+                                                                 std::vector<f_t>& host_bounds,
+                                                                 i_t var_id_on_problem,
                                                                  f_t first_probe,
                                                                  f_t second_probe,
                                                                  f_t integrality_tolerance)
 {
+  i_t var_id      = problem.original_ids[var_id_on_problem];
   auto& cache_row = probing_cache[var_id];
 
   i_t hit_interval_for_first_probe  = -1;
@@ -80,11 +91,14 @@ f_t lb_probing_cache_t<i_t, f_t>::get_least_conflicting_rounding(std::vector<f_t
   i_t n_conflicting_vars = 0;
   // first probe found some interval
   if (hit_interval_for_first_probe != -1) {
-    n_conflicting_vars = check_number_of_conflicting_vars(
-      host_bounds, cache_row[hit_interval_for_first_probe], integrality_tolerance);
+    n_conflicting_vars = check_number_of_conflicting_vars(host_bounds,
+                                                          cache_row[hit_interval_for_first_probe],
+                                                          integrality_tolerance,
+                                                          problem.reverse_original_ids);
     if (n_conflicting_vars == 0) {
       CUOPT_LOG_TRACE("No conflicting vars, returning first probe");
-      update_bounds_with_selected(host_bounds, cache_row[hit_interval_for_first_probe]);
+      update_bounds_with_selected(
+        host_bounds, cache_row[hit_interval_for_first_probe], problem.reverse_original_ids);
       return first_probe;
     }
   }
@@ -97,29 +111,34 @@ f_t lb_probing_cache_t<i_t, f_t>::get_least_conflicting_rounding(std::vector<f_t
                   n_conflicting_vars);
   // check for the other side, if it the interval includes second_probe return that, if not return
   // cutoff point second probe has a hit but it is not the same as first probe
-  i_t other_interval_idx             = 1 - hit_interval_for_first_probe;
-  i_t n_conflicting_vars_other_probe = check_number_of_conflicting_vars(
-    host_bounds, cache_row[other_interval_idx], integrality_tolerance);
+  i_t other_interval_idx = 1 - hit_interval_for_first_probe;
+  i_t n_conflicting_vars_other_probe =
+    check_number_of_conflicting_vars(host_bounds,
+                                     cache_row[other_interval_idx],
+                                     integrality_tolerance,
+                                     problem.reverse_original_ids);
 
   if (n_conflicting_vars_other_probe < n_conflicting_vars) {
     CUOPT_LOG_DEBUG(
       "Better conflicting vars found %d in the other probing region (cache interval)!",
       n_conflicting_vars_other_probe);
-    update_bounds_with_selected(host_bounds, cache_row[other_interval_idx]);
+    update_bounds_with_selected(
+      host_bounds, cache_row[other_interval_idx], problem.reverse_original_ids);
     if (other_interval_idx == hit_interval_for_second_probe) {
       return second_probe;
     } else {
       return cache_row[other_interval_idx].val_interval.val;
     }
   }
-  update_bounds_with_selected(host_bounds, cache_row[hit_interval_for_first_probe]);
+  update_bounds_with_selected(
+    host_bounds, cache_row[hit_interval_for_first_probe], problem.reverse_original_ids);
   return first_probe;
 }
 
 template <typename i_t, typename f_t>
-bool lb_probing_cache_t<i_t, f_t>::contains(i_t var_id)
+bool lb_probing_cache_t<i_t, f_t>::contains(problem_t<i_t, f_t>& problem, i_t var_id)
 {
-  return probing_cache.count(var_id) > 0;
+  return probing_cache.count(problem.original_ids[var_id]) > 0;
 }
 
 template <typename i_t, typename f_t, typename f_t2>

cpp/src/mip/presolve/probing_cache.cu

@@ -33,12 +33,16 @@ i_t probing_cache_t<i_t, f_t>::check_number_of_conflicting_vars(
   const std::vector<f_t>& host_lb,
   const std::vector<f_t>& host_ub,
   const cache_entry_t<i_t, f_t>& cache_entry,
-  f_t integrality_tolerance)
+  f_t integrality_tolerance,
+  const std::vector<i_t>& reverse_original_ids)
 {
   i_t n_conflicting_var = 0;
   for (const auto& [var_idx, bound] : cache_entry.var_to_cached_bound_map) {
-    if (host_lb[var_idx] - integrality_tolerance > bound.ub ||
-        host_ub[var_idx] < bound.lb - integrality_tolerance) {
+    i_t var_idx_in_current_problem = reverse_original_ids[var_idx];
+    // -1 means that variable was fixed and doesn't exists in the current problem
+    if (var_idx_in_current_problem == -1) { continue; }
+    if (host_lb[var_idx_in_current_problem] - integrality_tolerance > bound.ub ||
+        host_ub[var_idx_in_current_problem] < bound.lb - integrality_tolerance) {
       ++n_conflicting_var;
     }
   }
@@ -47,16 +51,22 @@ i_t probing_cache_t<i_t, f_t>::check_number_of_conflicting_vars(
 
 template <typename i_t, typename f_t>
 void probing_cache_t<i_t, f_t>::update_bounds_with_selected(
-  std::vector<f_t>& host_lb, std::vector<f_t>& host_ub, const cache_entry_t<i_t, f_t>& cache_entry)
+  std::vector<f_t>& host_lb,
+  std::vector<f_t>& host_ub,
+  const cache_entry_t<i_t, f_t>& cache_entry,
+  const std::vector<i_t>& reverse_original_ids)
 {
   i_t n_bounds_updated = 0;
   for (const auto& [var_idx, bound] : cache_entry.var_to_cached_bound_map) {
-    if (host_lb[var_idx] < bound.lb) {
-      host_lb[var_idx] = bound.lb;
+    i_t var_idx_in_current_problem = reverse_original_ids[var_idx];
+    // -1 means that variable was fixed and doesn't exists in the current problem
+    if (var_idx_in_current_problem == -1) { continue; }
+    if (host_lb[var_idx_in_current_problem] < bound.lb) {
+      host_lb[var_idx_in_current_problem] = bound.lb;
       n_bounds_updated++;
     }
-    if (host_ub[var_idx] > bound.ub) {
-      host_ub[var_idx] = bound.ub;
+    if (host_ub[var_idx_in_current_problem] > bound.ub) {
+      host_ub[var_idx_in_current_problem] = bound.ub;
       n_bounds_updated++;
     }
   }
@@ -87,11 +97,15 @@ f_t probing_cache_t<i_t, f_t>::get_least_conflicting_rounding(problem_t<i_t, f_t
   i_t n_conflicting_vars = 0;
   // first probe found some interval
   if (hit_interval_for_first_probe != -1) {
-    n_conflicting_vars = check_number_of_conflicting_vars(
-      host_lb, host_ub, cache_row[hit_interval_for_first_probe], integrality_tolerance);
+    n_conflicting_vars = check_number_of_conflicting_vars(host_lb,
+                                                          host_ub,
+                                                          cache_row[hit_interval_for_first_probe],
+                                                          integrality_tolerance,
+                                                          problem.reverse_original_ids);
     if (n_conflicting_vars == 0) {
       CUOPT_LOG_TRACE("No conflicting vars, returning first probe");
-      update_bounds_with_selected(host_lb, host_ub, cache_row[hit_interval_for_first_probe]);
+      update_bounds_with_selected(
+        host_lb, host_ub, cache_row[hit_interval_for_first_probe], problem.reverse_original_ids);
       return first_probe;
     }
   }
@@ -104,9 +118,13 @@ f_t probing_cache_t<i_t, f_t>::get_least_conflicting_rounding(problem_t<i_t, f_t
                   n_conflicting_vars);
   // check for the other side, if it the interval includes second_probe return that, if not return
   // cutoff point second probe has a hit but it is not the same as first probe
-  i_t other_interval_idx             = 1 - hit_interval_for_first_probe;
-  i_t n_conflicting_vars_other_probe = check_number_of_conflicting_vars(
-    host_lb, host_ub, cache_row[other_interval_idx], integrality_tolerance);
+  i_t other_interval_idx = 1 - hit_interval_for_first_probe;
+  i_t n_conflicting_vars_other_probe =
+    check_number_of_conflicting_vars(host_lb,
+                                     host_ub,
+                                     cache_row[other_interval_idx],
+                                     integrality_tolerance,
+                                     problem.reverse_original_ids);
 
   if (n_conflicting_vars_other_probe < n_conflicting_vars) {
     CUOPT_LOG_DEBUG(
@@ -115,7 +133,8 @@ f_t probing_cache_t<i_t, f_t>::get_least_conflicting_rounding(problem_t<i_t, f_t
       var_id,
       first_probe,
       n_conflicting_vars_other_probe);
-    update_bounds_with_selected(host_lb, host_ub, cache_row[other_interval_idx]);
+    update_bounds_with_selected(
+      host_lb, host_ub, cache_row[other_interval_idx], problem.reverse_original_ids);
     if (other_interval_idx == hit_interval_for_second_probe) {
       CUOPT_LOG_DEBUG("Better value on second probe val %f", second_probe);
       return second_probe;
@@ -125,14 +144,15 @@ f_t probing_cache_t<i_t, f_t>::get_least_conflicting_rounding(problem_t<i_t, f_t
       return cache_row[other_interval_idx].val_interval.val;
     }
   }
-  update_bounds_with_selected(host_lb, host_ub, cache_row[hit_interval_for_first_probe]);
+  update_bounds_with_selected(
+    host_lb, host_ub, cache_row[hit_interval_for_first_probe], problem.reverse_original_ids);
   return first_probe;
 }
 
 template <typename i_t, typename f_t>
-bool probing_cache_t<i_t, f_t>::contains(i_t var_id)
+bool probing_cache_t<i_t, f_t>::contains(problem_t<i_t, f_t>& problem, i_t var_id)
 {
-  return probing_cache.count(var_id) > 0;
+  return probing_cache.count(problem.original_ids[var_id]) > 0;
 }
 
 template <typename i_t, typename f_t>

cpp/src/mip/presolve/probing_cache.cuh

@@ -87,14 +87,16 @@ struct cache_entry_t {
 template <typename i_t, typename f_t>
 class probing_cache_t {
  public:
-  bool contains(i_t var_id);
+  bool contains(problem_t<i_t, f_t>& problem, i_t var_id);
   void update_bounds_with_selected(std::vector<f_t>& host_lb,
                                    std::vector<f_t>& host_ub,
-                                   const cache_entry_t<i_t, f_t>& cache_entry);
+                                   const cache_entry_t<i_t, f_t>& cache_entry,
+                                   const std::vector<i_t>& reverse_original_ids);
   i_t check_number_of_conflicting_vars(const std::vector<f_t>& host_lb,
                                        const std::vector<f_t>& host_ub,
                                        const cache_entry_t<i_t, f_t>& cache_entry,
-                                       f_t integrality_tolerance);
+                                       f_t integrality_tolerance,
+                                       const std::vector<i_t>& reverse_original_ids);
   // check if there are any conflicting bounds
   f_t get_least_conflicting_rounding(problem_t<i_t, f_t>& problem,
                                      std::vector<f_t>& host_lb,
@@ -111,15 +113,18 @@ class probing_cache_t {
 template <typename i_t, typename f_t>
 class lb_probing_cache_t {
  public:
-  bool contains(i_t var_id);
+  bool contains(problem_t<i_t, f_t>& problem, i_t var_id);
   void update_bounds_with_selected(std::vector<f_t>& host_bounds,
-                                   const cache_entry_t<i_t, f_t>& cache_entry);
+                                   const cache_entry_t<i_t, f_t>& cache_entry,
+                                   const std::vector<i_t>& reverse_original_ids);
   i_t check_number_of_conflicting_vars(const std::vector<f_t>& host_bounds,
                                        const cache_entry_t<i_t, f_t>& cache_entry,
-                                       f_t integrality_tolerance);
+                                       f_t integrality_tolerance,
+                                       const std::vector<i_t>& reverse_original_ids);
   // check if there are any conflicting bounds
-  f_t get_least_conflicting_rounding(std::vector<f_t>& host_bounds,
-                                     i_t var_id,
+  f_t get_least_conflicting_rounding(problem_t<i_t, f_t>& problem,
+                                     std::vector<f_t>& host_bounds,
+                                     i_t var_id_on_problem,
                                      f_t first_probe,
                                      f_t second_probe,
                                      f_t integrality_tolerance);

cpp/src/mip/problem/problem.cu

@@ -89,8 +89,6 @@ void problem_t<i_t, f_t>::op_problem_cstr_body(const optimization_problem_t<i_t,
     is_binary_variable.resize(n_variables, handle_ptr->get_stream());
     compute_n_integer_vars();
     compute_binary_var_table();
-    original_ids.resize(n_variables);
-    std::iota(original_ids.begin(), original_ids.end(), 0);
   }
   compute_transpose_of_problem();
   // Check after modifications
@@ -157,6 +155,7 @@ problem_t<i_t, f_t>::problem_t(const problem_t<i_t, f_t>& problem_)
     is_binary_pb(problem_.is_binary_pb),
     presolve_data(problem_.presolve_data, handle_ptr->get_stream()),
     original_ids(problem_.original_ids),
+    reverse_original_ids(problem_.reverse_original_ids),
     reverse_coefficients(problem_.reverse_coefficients, handle_ptr->get_stream()),
     reverse_constraints(problem_.reverse_constraints, handle_ptr->get_stream()),
     reverse_offsets(problem_.reverse_offsets, handle_ptr->get_stream()),
@@ -205,6 +204,7 @@ problem_t<i_t, f_t>::problem_t(const problem_t<i_t, f_t>& problem_, bool no_deep
         ? std::move(presolve_data_t{*problem_.original_problem_ptr, handle_ptr->get_stream()})
         : std::move(presolve_data_t{problem_.presolve_data, handle_ptr->get_stream()})),
     original_ids(problem_.original_ids),
+    reverse_original_ids(problem_.reverse_original_ids),
     reverse_coefficients(
       (!no_deep_copy)
         ? rmm::device_uvector<f_t>(problem_.reverse_coefficients, handle_ptr->get_stream())
@@ -1067,11 +1067,15 @@ problem_t<i_t, f_t> problem_t<i_t, f_t>::get_problem_after_fixing_vars(
   // if we are fixing on the original problem, the variable_map is what we want in
   // problem.original_ids but considering the case that we are fixing some variables multiple times,
   // do an assignment from the original_ids of the current problem
-  // TODO if we have more host gather operations, put them in a function
   problem.original_ids.resize(variable_map.size());
+  std::fill(problem.reverse_original_ids.begin(), problem.reverse_original_ids.end(), -1);
   auto h_variable_map = cuopt::host_copy(variable_map);
   for (size_t i = 0; i < variable_map.size(); ++i) {
+    cuopt_assert(h_variable_map[i] < original_ids.size(), "Variable index out of bounds");
     problem.original_ids[i] = original_ids[h_variable_map[i]];
+    cuopt_assert(original_ids[h_variable_map[i]] < reverse_original_ids.size(),
+                 "Variable index out of bounds");
+    problem.reverse_original_ids[original_ids[h_variable_map[i]]] = i;
   }
   RAFT_CHECK_CUDA(handle_ptr->get_stream());
   return problem;
@@ -1301,6 +1305,10 @@ void problem_t<i_t, f_t>::preprocess_problem()
                              0.);
   integer_indices.resize(n_variables, handle_ptr->get_stream());
   is_binary_variable.resize(n_variables, handle_ptr->get_stream());
+  original_ids.resize(n_variables);
+  std::iota(original_ids.begin(), original_ids.end(), 0);
+  reverse_original_ids.resize(n_variables);
+  std::iota(reverse_original_ids.begin(), reverse_original_ids.end(), 0);
   compute_n_integer_vars();
   compute_binary_var_table();
   check_problem_representation(true);

cpp/src/mip/problem/problem.cuh

@@ -213,6 +213,8 @@ class problem_t {
   // this vector refers to the problem after any presolve or preprocessing
   // it is to have correct access to the parent problem when we fix some variables
   std::vector<i_t> original_ids;
+  // reverse original ids
+  std::vector<i_t> reverse_original_ids;
 
   // reverse CSR matrix
   rmm::device_uvector<f_t> reverse_coefficients;