Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions cpp/include/cuopt/mathematical_optimization/constants.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@
#define CUOPT_MIP_BATCH_PDLP_RELIABILITY_BRANCHING "mip_batch_pdlp_reliability_branching"
#define CUOPT_MIP_STRONG_BRANCHING_SIMPLEX_ITERATION_LIMIT \
"mip_strong_branching_simplex_iteration_limit"
#define CUOPT_MIP_NODE_CUTS "mip_node_cuts"
#define CUOPT_MIP_MAX_RESTARTS "mip_max_restarts"

#define CUOPT_SOLUTION_FILE "solution_file"
#define CUOPT_NUM_CPU_THREADS "num_cpu_threads"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,8 @@ class mip_solver_settings_t {
i_t num_cpu_threads = -1; // -1 means use default number of threads in branch and bound
i_t symmetry = -1;
i_t max_cut_passes = 10; // number of cut passes to make
i_t node_cuts = 1; // 0 = disable, 1 = enable cut generation at B&B nodes

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we rename it to generate_node_cuts? The others imply a number and this implies a boolean decision.

i_t max_restarts = 50; // 0 = disable B&B restarts; max number of restarts otherwise
i_t mir_cuts = -1;
i_t mixed_integer_gomory_cuts = -1;
i_t knapsack_cuts = -1;
Expand Down
380 changes: 337 additions & 43 deletions cpp/src/branch_and_bound/branch_and_bound.cpp

Large diffs are not rendered by default.

27 changes: 26 additions & 1 deletion cpp/src/branch_and_bound/branch_and_bound.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
#include <functional>
#include <future>
#include <memory>
#include <optional>
#include <vector>

namespace cuopt::mathematical_optimization::mip {
Expand All @@ -59,6 +60,7 @@ enum class mip_status_t {
NUMERICAL = 5, // The solver encountered a numerical error
UNSET = 6, // The status is not set
WORK_LIMIT = 7, // The solver reached a deterministic work limit
RESTART = 8, // The solver triggered a restart of the B&B tree
};

template <typename i_t, typename f_t>
Expand Down Expand Up @@ -178,6 +180,16 @@ class branch_and_bound_t {
std::vector<i_t> new_slacks_;
std::vector<simplex::variable_type_t> var_types_;

// Shared global cut pool: both the root cut passes and the per-node cut passes append to it.
// Constructed in solve() once the root LP dimensions are known. The pool self-locks on add_cut.
std::optional<cut_pool_t<i_t, f_t>> global_cut_pool_;
// Shared cut generator bound to global_cut_pool_, reused by the root loop and every worker's
// node cut generation. The Gomory path uses only local scratch (no mutable generator state),
// so a single instance can serve all workers concurrently.
std::optional<cut_generation_t<i_t, f_t>> cut_generation_;
// True when every integer variable is binary ([0,1]); node cut generation is gated on this.
bool is_pure_binary_{false};

// Variable locks (see definition 3.3 from T. Achterberg, “Constraint Integer Programming,”
// PhD, Technischen Universität Berlin, Berlin, 2007. doi: 10.14279/depositonce-1634).
// Here we assume that the constraints are in the form `Ax = b, l <= x <= u`.
Expand Down Expand Up @@ -232,6 +244,10 @@ class branch_and_bound_t {
bool enable_concurrent_lp_root_solve_{false};
std::atomic<int> root_concurrent_halt_{0};
std::atomic<int> node_concurrent_halt_{0};
// Set to 1 to signal all B&B workers to stop so the tree can be restarted. Owned here (unlike the
// reference PR which threads a pointer through the constructor) since the B&B taskgroup is the only
// consumer.
std::atomic<int> restart_concurrent_halt_{0};
Comment on lines +247 to +250

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check for any reader of restart_concurrent_halt_.
rg -nP -C3 '\brestart_concurrent_halt_\b' cpp

Repository: NVIDIA/cuopt

Length of output: 2125


🏁 Script executed:

#!/bin/bash
set -e
printf '%s\n' '--- relevant declarations and uses ---'
rg -nP -C5 '\b(node_concurrent_halt_|restart_concurrent_halt_|concurrent_halt|solver_status_)\b' cpp/src/branch_and_bound/branch_and_bound.hpp cpp/src/branch_and_bound/branch_and_bound.cpp
printf '%s\n' '--- taskgroup and worker setup ---'
rg -nP -C8 'taskgroup|concurrent_halt|node_concurrent_halt_' cpp/src/branch_and_bound/branch_and_bound.cpp

Repository: NVIDIA/cuopt

Length of output: 49065


Remove or consume restart_concurrent_halt_.

The flag is only assigned and reset. No code reads it. Wire it into the restart-stop path, or remove it and rely on node_concurrent_halt_ and solver_status_.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cpp/src/branch_and_bound/branch_and_bound.hpp` around lines 247 - 250,
Resolve the unused restart_concurrent_halt_ flag by either consuming it in the
branch-and-bound restart-stop path to signal worker termination, or removing the
member and its assignments/resets while relying on node_concurrent_halt_ and
solver_status_. Ensure the chosen approach preserves restart behavior and leaves
no dead writes.

bool is_root_solution_set{false};

// Pseudocosts
Expand Down Expand Up @@ -261,6 +277,9 @@ class branch_and_bound_t {
omp_atomic_t<f_t> lower_bound_numerical_;
std::function<void(f_t)> user_bound_callback_;

// Number of restarts performed so far in the current solve().
i_t restart_count_{0};

void print_table_header();
void report_heuristic(f_t obj);
void report(char symbol,
Expand Down Expand Up @@ -293,7 +312,9 @@ class branch_and_bound_t {
f_t& last_objective,
f_t root_relax_objective,
i_t& cut_pool_size,
const std::vector<f_t>& saved_solution);
const std::vector<f_t>& saved_solution,
bool generate_new = true,
i_t* num_cuts_added_out = nullptr);

// Set the solution when found at the root node
void set_solution_at_root(simplex::mip_solution_t<i_t, f_t>& solution,
Expand All @@ -313,6 +334,10 @@ class branch_and_bound_t {
// Repairs low-quality solutions from the heuristics, if it is applicable.
void repair_heuristic_solutions();

// Decide whether to restart the B&B tree based on how much larger the estimated full tree is
// compared to the part explored so far.
bool should_restart(f_t current_abs_gap);

// Launch a new diving worker from a given best-first worker.
bool launch_diving_worker(bfs_worker_t<i_t, f_t>* bfs_worker);

Expand Down
33 changes: 33 additions & 0 deletions cpp/src/branch_and_bound/mip_node.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,17 @@ class search_tree_t {
void update(mip_node_t<i_t, f_t>* node_ptr, node_status_t status)
{
std::lock_guard<omp_mutex_t> lock(mutex);
// Maintain the tree-weight progress metric used by the restart heuristic. A node closed as a
// leaf (fathomed/feasible/infeasible/numerical) contributes 2^-depth of the [0,1] tree weight;
// a node that branched becomes an inner node. This lets should_restart() estimate the full tree
// size from the fraction explored so far.
--num_open_nodes;
if (status == node_status_t::HAS_CHILDREN) {
++num_inner_nodes;
} else {
++num_final_nodes;
progress = progress.load() + std::ldexp(f_t(1), -node_ptr->depth);
}
std::vector<mip_node_t<i_t, f_t>*> stack;
node_ptr->set_status(status, stack);
remove_fathomed_nodes(stack);
Expand Down Expand Up @@ -423,6 +434,20 @@ class search_tree_t {
assert(parent_vstatus.size() == original_lp.num_cols);
parent_node->add_children(std::move(down_child),
std::move(up_child)); // child pointers moved into the tree
num_open_nodes += 2;
}

// Free the whole tree and reset all counters so the search tree can be reused after a restart.
// Child destruction goes through mip_node_t's iterative teardown, so deep trees are safe.
void clean()
{
root.children[0].reset();
root.children[1].reset();
num_nodes = 0;
num_open_nodes = 0;
num_final_nodes = 0;
num_inner_nodes = 0;
progress = 0;
}

void graphviz_node(simplex::logger_t& log,
Expand Down Expand Up @@ -456,6 +481,14 @@ class search_tree_t {
omp_mutex_t mutex;
omp_atomic_t<i_t> num_nodes;

// Restart bookkeeping. num_open_nodes counts nodes still to explore; num_final_nodes and
// num_inner_nodes count closed leaves and branched nodes. progress is the fraction of the [0,1]
// tree weight that has been closed (sum of 2^-depth over closed leaves).
omp_atomic_t<i_t> num_open_nodes = 0;
omp_atomic_t<i_t> num_final_nodes = 0;
omp_atomic_t<i_t> num_inner_nodes = 0;
omp_atomic_t<f_t> progress = 0;

static constexpr bool write_graphviz = false;
};

Expand Down
9 changes: 9 additions & 0 deletions cpp/src/branch_and_bound/node_queue.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,15 @@ class node_queue_t {
return best_first_heap_.empty() ? std::numeric_limits<f_t>::infinity() : lower_bound_.load();
}

// Empty both heaps and reset the tracked lower bound. Used to recycle a worker's queue on restart.
void clear()
{
std::lock_guard lock(mutex_);
best_first_heap_.clear();
diving_heap_.clear();
lower_bound_ = std::numeric_limits<f_t>::infinity();
}

private:
struct heap_entry_t {
mip_node_t<i_t, f_t>* node = nullptr;
Expand Down
11 changes: 10 additions & 1 deletion cpp/src/branch_and_bound/worker.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,23 @@ struct branch_and_bound_stats_t {
f_t start_time = 0.0;
omp_atomic_t<f_t> total_lp_solve_time = 0.0;
omp_atomic_t<int64_t> nodes_explored = 0;
omp_atomic_t<int64_t> nodes_unexplored = 0;
// Cumulative nodes explored across all restarts (nodes_explored is reset each restart).
omp_atomic_t<int64_t> total_nodes_explored = 0;
omp_atomic_t<int64_t> nodes_unexplored = 0;
// Tracks the number of nodes being solved by the workers at a given time
omp_atomic_t<i_t> nodes_being_solved = 0;

omp_atomic_t<int64_t> total_lp_iters = 0;
omp_atomic_t<i_t> nodes_since_last_log = 0;
omp_atomic_t<f_t> last_log = 0.0;

// Restart bookkeeping: snapshots at the last should_restart() check plus the consecutive
// large-tree-estimate counter that gates a restart.
i_t restart_nodes_at_last_check = 0;
f_t restart_progress_at_last_check = 0;
f_t restart_gap_at_last_check = 0;
i_t restart_large_tree_count = 0;

omp_atomic_t<int64_t> orbital_fixing_nodes = 0;
omp_atomic_t<int64_t> orbital_fixings_applied = 0;
omp_atomic_t<int64_t> orbital_conflict_nodes = 0;
Expand Down
5 changes: 4 additions & 1 deletion cpp/src/branch_and_bound/worker_pool.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,12 @@ class worker_pool_t {
const simplex::simplex_solver_settings_t<i_t, f_t>& settings,
const uint64_t rng_offset = 0)
{
assert(!is_initialized_);
assert(num_workers > 0);

// Re-entrant: on restart the root LP dimensions may have changed (re-separated cuts), so the
// workers -- which copy/size their per-node buffers (leaf_problem, basis_factors, ...) from
// original_lp at construction -- must be rebuilt rather than merely reset.
workers_.clear();
workers_.resize(num_workers);
num_idle_workers_ = num_workers;
idle_workers_.clear_resize(num_workers);
Expand Down
79 changes: 79 additions & 0 deletions cpp/src/cuts/cuts.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include <cstdio>
#include <cstdlib>
#include <limits>
#include <mutex>
#include <stdexcept>
#include <tuple>
#include <unordered_set>
Expand Down Expand Up @@ -1164,12 +1165,62 @@ void cut_pool_t<i_t, f_t>::add_cut(cut_type_t cut_type, const inequality_t<i_t,
return;
}

// Serialize appends: the global pool is shared by concurrent per-node cut passes.
std::lock_guard<omp_mutex_t> lock(mutex_);

cut_storage_.append_row(cut_squeezed.vector);
rhs_storage_.push_back(cut_squeezed.rhs);
cut_type_.push_back(cut_type);
cut_age_.push_back(0);
}

template <typename i_t, typename f_t>
i_t cut_pool_t<i_t, f_t>::verify_solution(const std::vector<f_t>& x, f_t tolerance) const
{
i_t num_violated = 0;
f_t max_violation = 0.0;
const i_t num_cuts = cut_storage_.m;
for (i_t row = 0; row < num_cuts; row++) {
const i_t row_start = cut_storage_.row_start[row];
const i_t row_end = cut_storage_.row_start[row + 1];
f_t cut_x = 0.0;
for (i_t p = row_start; p < row_end; p++) {
const i_t j = cut_storage_.j[p];
const f_t cut_coeff = cut_storage_.x[p];
cut_x += cut_coeff * x[j];
}
// Cut is cut'*x >= rhs, so violation is rhs - cut'*x (positive means the solution violates it).
const f_t violation = rhs_storage_[row] - cut_x;
if (violation > tolerance) {
num_violated++;
max_violation = std::max(max_violation, violation);
settings_.log.printf(
"Cut pool verification: cut %d (type %d) violated by optimal solution: cut'x=%.10e < "
"rhs=%.10e (violation %.3e > tol %.1e)\n",
row,
static_cast<int>(cut_type_[row]),
cut_x,
rhs_storage_[row],
violation,
tolerance);
}
}
if (num_violated > 0) {
settings_.log.printf(
"Cut pool verification FAILED: %d of %d cuts violated by the optimal solution (max violation "
"%.3e). Some generated cut is not globally valid.\n",
num_violated,
num_cuts,
max_violation);
} else {
settings_.log.printf(
"Cut pool verification passed: optimal solution satisfies all %d cuts within tol %.1e\n",
num_cuts,
tolerance);
}
return num_violated;
}
Comment on lines +1177 to +1222

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Guard against a short x vector.

verify_solution indexes x[j] for every stored column index. Stored indices are bounded by original_vars_, which is fixed when the pool is constructed. If a caller passes a solution vector shorter than original_vars_, the loop reads out of bounds. Add a size check and return early, so a future caller cannot trigger undefined behavior.

🛡️ Proposed guard
 i_t cut_pool_t<i_t, f_t>::verify_solution(const std::vector<f_t>& x, f_t tolerance) const
 {
+  if (static_cast<i_t>(x.size()) < original_vars_) {
+    settings_.log.printf(
+      "Cut pool verification skipped: solution has %zu entries, pool indexes %d variables\n",
+      x.size(),
+      original_vars_);
+    return 0;
+  }
   i_t num_violated   = 0;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cpp/src/cuts/cuts.cpp` around lines 1177 - 1222, Update
cut_pool_t::verify_solution to validate that x contains at least original_vars_
entries before iterating over stored cuts and indexing x[j]. If the vector is
too short, log an appropriate failure and return early without accessing x;
preserve the existing verification behavior for sufficiently sized solution
vectors.


template <typename i_t, typename f_t>
f_t cut_pool_t<i_t, f_t>::cut_distance(i_t row,
const std::vector<f_t>& x,
Expand Down Expand Up @@ -3431,6 +3482,34 @@ bool cut_generation_t<i_t, f_t>::generate_cuts(const lp_problem_t<i_t, f_t>& lp,
return true;
}

template <typename i_t, typename f_t>
bool cut_generation_t<i_t, f_t>::generate_node_cuts(
const lp_problem_t<i_t, f_t>& lp,
const simplex_solver_settings_t<i_t, f_t>& settings,
csr_matrix_t<i_t, f_t>& Arow,
const std::vector<i_t>& new_slacks,
const std::vector<simplex::variable_type_t>& var_types,
simplex::basis_update_mpf_t<i_t, f_t>& basis_update,
const std::vector<f_t>& xstar,
const std::vector<i_t>& basic_list,
const std::vector<i_t>& nonbasic_list,
f_t start_time)
{
// Node cuts are Gomory-only for now. generate_gomory_cuts appends violated cuts to the shared
// global pool. We only accumulate the count here (reported once per restart); the per-node print
// is intentionally omitted to avoid flooding the log during B&B.
const i_t pool_size_before = cut_pool_.pool_size();
generate_gomory_cuts(
lp, settings, Arow, new_slacks, var_types, basis_update, xstar, basic_list, nonbasic_list);
const i_t pool_size_after = cut_pool_.pool_size();
if (pool_size_after > pool_size_before) {
node_cuts_added_ += pool_size_after - pool_size_before;
}
Comment on lines +3501 to +3507

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Partial locking of cut_pool_t: only add_cut is serialized. mutex_ guards the append in add_cut, but every other member reads the same storage without the lock. Concurrent best-first workers therefore race on non-atomic pool state.

  • cpp/src/cuts/cuts.cpp#L3501-L3507: stop differencing pool_size(). Return the number of appended cuts from add_cut, or increment a counter inside the pool under mutex_, and accumulate that value into node_cuts_added_.
  • cpp/src/cuts/cuts.hpp#L351-L354: extend the comment to state that only add_cut is thread-safe, and that pool_size, score_cuts, get_best_cuts, check_for_duplicate_cuts, and verify_solution require external serialization.
📍 Affects 2 files
  • cpp/src/cuts/cuts.cpp#L3501-L3507 (this comment)
  • cpp/src/cuts/cuts.hpp#L351-L354
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cpp/src/cuts/cuts.cpp` around lines 3501 - 3507, Replace the
pool_size_before/pool_size_after differencing around generate_gomory_cuts in
cpp/src/cuts/cuts.cpp:3501-3507 with a count returned by add_cut or a
mutex-protected appended-cut counter, and use that value to update
node_cuts_added_. In cpp/src/cuts/cuts.hpp:351-354, update the cut_pool_t
thread-safety comment to state that only add_cut is thread-safe and that
pool_size, score_cuts, get_best_cuts, check_for_duplicate_cuts, and
verify_solution require external serialization.

return true;
}



template <typename i_t, typename f_t>
void cut_generation_t<i_t, f_t>::generate_knapsack_cuts(
const lp_problem_t<i_t, f_t>& lp,
Expand Down
28 changes: 28 additions & 0 deletions cpp/src/cuts/cuts.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include <dual_simplex/user_problem.hpp>
#include <linear_algebra/sparse_vector.hpp>
#include <math_optimization/types.hpp>
#include <utilities/omp_helpers.hpp>

#include <algorithm>
#include <array>
Expand Down Expand Up @@ -317,6 +318,12 @@ class cut_pool_t {

i_t pool_size() const { return cut_storage_.m; }

// Verify that x satisfies every cut in the pool (each stored as cut'*x >= rhs) within the given
// absolute tolerance. Returns the number of violated cuts (0 means all satisfied) and logs each
// violation. Used as a consistency check that globally-valid cuts never cut off the optimal
// solution. x must be indexed in the same (original) variable space as the stored cuts.
i_t verify_solution(const std::vector<f_t>& x, f_t tolerance) const;

void print_cutpool_types() { print_cut_types("In cut pool", cut_type_, settings_); }

void check_for_duplicate_cuts();
Expand All @@ -341,6 +348,10 @@ class cut_pool_t {
std::vector<f_t> cut_scores_;
std::vector<i_t> best_cuts_;
const f_t min_cut_distance_{1e-4};

// The global cut pool is shared by concurrent per-node cut passes (one per B&B worker), so
// add_cut must serialize its append to cut_storage_/rhs_storage_/etc.
omp_mutex_t mutex_;
};

template <typename i_t, typename f_t>
Expand Down Expand Up @@ -659,6 +670,20 @@ class cut_generation_t {
variable_bounds_t<i_t, f_t>& variable_bounds,
f_t start_time);

bool generate_node_cuts(const simplex::lp_problem_t<i_t, f_t>& lp,
const simplex::simplex_solver_settings_t<i_t, f_t>& settings,
csr_matrix_t<i_t, f_t>& Arow,
const std::vector<i_t>& new_slacks,
const std::vector<simplex::variable_type_t>& var_types,
simplex::basis_update_mpf_t<i_t, f_t>& basis_update,
const std::vector<f_t>& xstar,
const std::vector<i_t>& basic_list,
const std::vector<i_t>& nonbasic_list,
f_t start_time);

// Cumulative number of node-generated Gomory cuts appended to the pool (for restart logging).
i_t node_cuts_added() const { return node_cuts_added_; }

private:
// Generate all mixed integer gomory cuts
void generate_gomory_cuts(const simplex::lp_problem_t<i_t, f_t>& lp,
Expand Down Expand Up @@ -728,6 +753,9 @@ class cut_generation_t {
f_t start_time);

cut_pool_t<i_t, f_t>& cut_pool_;
// Cumulative number of cuts appended to the pool by generate_node_cuts (node-generated Gomory
// cuts). Read for restart logging to gauge how many node cuts are available for re-separation.
omp_atomic_t<i_t> node_cuts_added_{0};
knapsack_generation_t<i_t, f_t> knapsack_generation_;
flow_cover_generation_t<i_t, f_t> flow_cover_generation_;
const simplex::user_problem_t<i_t, f_t>& user_problem_;
Expand Down
Loading