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
15 changes: 15 additions & 0 deletions cpp/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,20 @@ FetchContent_MakeAvailable(pslp)
set(BUILD_SHARED_LIBS ${BUILD_SHARED_LIBS_SAVED})


# dejavu - header-only graph automorphism library for MIP symmetry detection
# https://github.com/markusa4/dejavu (header-only, skip its CMakeLists.txt)
FetchContent_Declare(
dejavu
GIT_REPOSITORY "https://github.com/markusa4/dejavu.git"
GIT_TAG "v2.1"
GIT_PROGRESS TRUE
EXCLUDE_FROM_ALL
SYSTEM
SOURCE_SUBDIR _nonexistent
)
FetchContent_MakeAvailable(dejavu)
message(STATUS "dejavu (graph automorphism): ${dejavu_SOURCE_DIR}")

include(${rapids-cmake-dir}/cpm/rapids_logger.cmake)
# generate logging macros
rapids_cpm_rapids_logger(BUILD_EXPORT_SET cuopt-exports INSTALL_EXPORT_SET cuopt-exports)
Expand Down Expand Up @@ -469,6 +483,7 @@ target_include_directories(cuopt PRIVATE

target_include_directories(cuopt SYSTEM PRIVATE
"${pslp_SOURCE_DIR}/include"
"${dejavu_SOURCE_DIR}"
)

target_include_directories(cuopt
Expand Down
162 changes: 161 additions & 1 deletion cpp/src/branch_and_bound/branch_and_bound.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <branch_and_bound/branch_and_bound.hpp>
#include <branch_and_bound/mip_node.hpp>
#include <branch_and_bound/pseudo_costs.hpp>
#include <branch_and_bound/symmetry.hpp>

#include <cuts/cuts.hpp>
#include <mip_heuristics/presolve/conflict_graph/clique_table.cuh>
Expand Down Expand Up @@ -248,11 +249,13 @@ branch_and_bound_t<i_t, f_t>::branch_and_bound_t(
const simplex_solver_settings_t<i_t, f_t>& solver_settings,
f_t start_time,
const probing_implied_bound_t<i_t, f_t>& probing_implied_bound,
std::shared_ptr<detail::clique_table_t<i_t, f_t>> clique_table)
std::shared_ptr<detail::clique_table_t<i_t, f_t>> clique_table,
mip_symmetry_t<i_t, f_t>* symmetry)
: original_problem_(user_problem),
settings_(solver_settings),
probing_implied_bound_(probing_implied_bound),
clique_table_(std::move(clique_table)),
symmetry_(symmetry),
original_lp_(user_problem.handle_ptr, 1, 1, 1),
Arow_(1, 1, 0),
incumbent_(1),
Expand Down Expand Up @@ -1388,6 +1391,163 @@ dual::status_t branch_and_bound_t<i_t, f_t>::solve_node_lp(
worker->leaf_edge_norms = edge_norms_;

if (feasible) {


// Perform orbital fixing
if (symmetry_ != nullptr) {
// First get the set of variables that have been branched down and branched up on
std::vector<i_t> branched_zero;
std::vector<i_t> branched_one;
branched_zero.reserve(node_ptr->depth);
branched_one.reserve(node_ptr->depth);
mip_node_t<i_t, f_t>* node = node_ptr;
while (node != nullptr && node->branch_var >= 0) {
if (node->branch_var_upper == 0.0) {
branched_zero.push_back(node->branch_var);
symmetry_->marked_b0[node->branch_var] = 1;
} else if (node->branch_var_lower == 1.0) {
branched_one.push_back(node->branch_var);
symmetry_->marked_b1[node->branch_var] = 1;
} else {
assert(false); // Unexpected non-binary variable. Only binaries supported in symmetry handling.
}
node = node->parent;
}
Comment on lines +1404 to +1415
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Improper error handling for non-binary variables.

The assert(false) on line 1412 is problematic:

  • In release builds, asserts are compiled out, so this branch silently falls through without adding the variable to either branched_zero or branched_one
  • In debug builds, this crashes the solver

If non-binary integer variables are encountered, the code should handle this explicitly rather than relying on an assertion.

🛡️ Suggested fix: explicit skip with optional debug logging
         if (node->branch_var_upper == 0.0) {
           branched_zero.push_back(node->branch_var);
           symmetry_->marked_b0[node->branch_var] = 1;
         } else if (node->branch_var_lower == 1.0) {
           branched_one.push_back(node->branch_var);
           symmetry_->marked_b1[node->branch_var] = 1;
         } else {
-          assert(false); // Unexpected non-binary variable. Only binaries supported in symmetry handling.
+          // Non-binary integer variable - skip for symmetry handling (only binaries supported)
+          // Continue walking up the tree without including this variable
         }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
while (node != nullptr && node->branch_var >= 0) {
if (node->branch_var_upper == 0.0) {
branched_zero.push_back(node->branch_var);
symmetry_->marked_b0[node->branch_var] = 1;
} else if (node->branch_var_lower == 1.0) {
branched_one.push_back(node->branch_var);
symmetry_->marked_b1[node->branch_var] = 1;
} else {
assert(false); // Unexpected non-binary variable. Only binaries supported in symmetry handling.
}
node = node->parent;
}
while (node != nullptr && node->branch_var >= 0) {
if (node->branch_var_upper == 0.0) {
branched_zero.push_back(node->branch_var);
symmetry_->marked_b0[node->branch_var] = 1;
} else if (node->branch_var_lower == 1.0) {
branched_one.push_back(node->branch_var);
symmetry_->marked_b1[node->branch_var] = 1;
} else {
// Non-binary integer variable - skip for symmetry handling (only binaries supported)
// Continue walking up the tree without including this variable
}
node = node->parent;
}


{
for (i_t j = 0; j < symmetry_->num_original_vars; j++) {
if (var_types_[j] == variable_type_t::CONTINUOUS) continue;
if (symmetry_->marked_b1[j] == 0 && worker->leaf_problem.lower[j] == 1.0) {
symmetry_->f1.push_back(j);
symmetry_->marked_f1[j] = 1;
}
if (symmetry_->marked_b0[j] == 0 && worker->leaf_problem.upper[j] == 0.0) {
symmetry_->f0.push_back(j);
symmetry_->marked_f0[j] = 1;
}
}

// Compute Stab(G, B1) and its orbits
std::vector<i_t> new_base;
new_base.reserve(symmetry_->num_original_vars);
for (i_t j: branched_one) {
new_base.push_back(j);
symmetry_->marked_variables[j] = 1;
}
for (i_t j = 0; j < symmetry_->num_original_vars; j++) {
if (symmetry_->marked_variables[j] == 0) {
new_base.push_back(j);
}
}
for (i_t j: branched_one) {
symmetry_->marked_variables[j] = 0;
}

symmetry_->schreier->set_base(new_base);

dejavu::groups::orbit orb;
orb.initialize(symmetry_->domain_size);
symmetry_->schreier->get_stabilizer_orbit(static_cast<int>(branched_one.size()), orb);

for (i_t v : branched_one) {
symmetry_->orbit_has_b1[orb.find_orbit(v)] = 1;
}

for (i_t v : branched_zero) {
symmetry_->orbit_has_b0[orb.find_orbit(v)] = 1;
}

for (i_t v: symmetry_->continuous_variables) {
symmetry_->orbit_has_continuous[orb.find_orbit(v)] = 1;
}

for (i_t v: symmetry_->f0) {
symmetry_->orbit_has_f0[orb.find_orbit(v)] = 1;
}

for (i_t v: symmetry_->f1) {
symmetry_->orbit_has_f1[orb.find_orbit(v)] = 1;
}

std::vector<i_t> fix_zero; // The set L0 of variables that can be fixed to 0
std::vector<i_t> fix_one; // The set L1 of variables that can be fixed to 1

for (i_t j = 0; j < symmetry_->num_original_vars; j++) {
i_t o = orb.find_orbit(j);
if (orb.orbit_size(o) < 2) continue;

if (symmetry_->orbit_has_b1[o] == 1 || symmetry_->orbit_has_continuous[o] == 1) {
// The orbit contains variables in B1 or continuous variables
// So we can't fix any variables in this orbit to 0
continue;
}

if (symmetry_->orbit_has_b0[o] == 1 || symmetry_->orbit_has_f0[o] == 1) {
// The orbit of this variable contains variables in B0 or F0
// So we can fix this variable to zero (provided its not already in B0 or F0)
if (symmetry_->marked_b0[j] == 0 && symmetry_->marked_f0[j] == 0) {
fix_zero.push_back(j);
}
}

if (symmetry_->orbit_has_f1[o] == 1) {
// The orbit of this variable contains variables in F1
// So we can fix this variable to one (provided its not already in F1)
if (symmetry_->marked_f1[j] == 0) {
fix_one.push_back(j);
}
}
}

// Restore the work arrays
for (i_t v: branched_one) {
symmetry_->orbit_has_b1[orb.find_orbit(v)] = 0;
symmetry_->marked_b1[v] = 0;
}

for (i_t v: branched_zero) {
symmetry_->orbit_has_b0[orb.find_orbit(v)] = 0;
symmetry_->marked_b0[v] = 0;
}

for (i_t v: symmetry_->continuous_variables) {
symmetry_->orbit_has_continuous[orb.find_orbit(v)] = 0;
}

for (i_t v: symmetry_->f0) {
symmetry_->orbit_has_f0[orb.find_orbit(v)] = 0;
symmetry_->marked_f0[v] = 0;
}

for (i_t v: symmetry_->f1) {
symmetry_->orbit_has_f1[orb.find_orbit(v)] = 0;
symmetry_->marked_f1[v] = 0;
}

symmetry_->f0.clear();
symmetry_->f1.clear();

settings_.log.printf(
"Orbital fixing at node %d: fixing %d variables to 0 and %d variables to 1\n",
node_ptr->node_id,
fix_zero.size(),
fix_one.size());
// Finally fix the variables in L0 and L1
for (i_t v: fix_zero) {
settings_.log.printf("Orbital fixing at node %d: fixing variable %d to 0\n", node_ptr->node_id, v);
worker->leaf_problem.lower[v] = 0.0;
worker->leaf_problem.upper[v] = 0.0;
}
for (i_t v: fix_one) {
settings_.log.printf("Orbital fixing at node %d: fixing variable %d to 1\n", node_ptr->node_id, v);
worker->leaf_problem.lower[v] = 1.0;
worker->leaf_problem.upper[v] = 1.0;
}
Comment on lines +1530 to +1545
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Excessive logging in hot path will degrade performance.

This logging runs on every node solve where symmetry is enabled:

  • The summary log (lines 1530-1534) runs for every node
  • The per-variable logs (lines 1537, 1542) can generate thousands of lines per node

This will significantly slow down the solver and flood logs on large instances.

♻️ Suggested fix: use debug logging or remove per-variable logs
-        settings_.log.printf(
-          "Orbital fixing at node %d: fixing %d variables to 0 and %d variables to 1\n",
-          node_ptr->node_id,
-          fix_zero.size(),
-          fix_one.size());
+        if (fix_zero.size() > 0 || fix_one.size() > 0) {
+          settings_.log.debug(
+            "Orbital fixing at node %d: fixing %zu variables to 0 and %zu variables to 1\n",
+            node_ptr->node_id,
+            fix_zero.size(),
+            fix_one.size());
+        }
         // Finally fix the variables in L0 and L1
         for (i_t v: fix_zero) {
-          settings_.log.printf("Orbital fixing at node %d: fixing variable %d to 0\n", node_ptr->node_id, v);
           worker->leaf_problem.lower[v] = 0.0;
           worker->leaf_problem.upper[v] = 0.0;
         }
         for (i_t v: fix_one) {
-          settings_.log.printf("Orbital fixing at node %d: fixing variable %d to 1\n", node_ptr->node_id, v);
           worker->leaf_problem.lower[v] = 1.0;
           worker->leaf_problem.upper[v] = 1.0;
         }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cpp/src/branch_and_bound/branch_and_bound.cpp` around lines 1530 - 1545, The
current orbital-fixing logging (settings_.log.printf) is too verbose in the hot
solve path: avoid printing the per-variable lines for each v in fix_zero/fix_one
and make the summary conditional or lower-severity; update the block around
node_ptr->node_id / fix_zero / fix_one so only the summary is logged at a
debug/trace level (or logged only when fix count > 0 or a verbose flag is set)
and remove or guard the per-variable logs that print each fix and the repeated
accesses to worker->leaf_problem.lower/upper to prevent log flooding and
performance degradation.

}
}
Comment on lines +1396 to +1547
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 16, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Orbital fixing is currently skipped in deterministic B&B.

The new logic only runs in solve_node_lp(). Deterministic mode solves nodes through solve_node_deterministic() and deterministic_dive(), which never touch symmetry_, so that codepath gets symmetry detection overhead but no orbital-fixing benefit. Please either apply the same fixing there or disable symmetry handling for deterministic runs until it is supported end-to-end.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cpp/src/branch_and_bound/branch_and_bound.cpp` around lines 1396 - 1547,
Deterministic B&B never runs the orbital-fixing block (it only runs in
solve_node_lp), so either move the orbital-fixing logic into a reused helper and
call it from solve_node_deterministic() and deterministic_dive(), or disable
symmetry_ in deterministic mode; specifically, extract the symmetry
handling/fixing code currently inside the if (symmetry_ != nullptr) block into a
new function (e.g., perform_orbital_fixing(node_ptr, worker) or
symmetry_->apply_orbital_fixing(...)) and invoke that helper from
solve_node_deterministic() and deterministic_dive() (or set symmetry_ = nullptr
/ skip symmetry setup when deterministic) so deterministic runs no longer pay
overhead without applying fixes. Ensure the helper uses the same symbols
(node_ptr, worker, symmetry_, var_types_, settings_.log) and preserves all
marking/restore semantics.

⚠️ Potential issue | 🔴 Critical

Make orbital-fixing scratch state worker-local.

This block mutates symmetry_->marked_*, symmetry_->orbit_has_*, symmetry_->f0/f1, and also calls symmetry_->schreier->set_base(...) on the shared symmetry_ object. solve_node_lp() runs concurrently across opportunistic B&B workers, so this is a real data race that can corrupt fixings or the Schreier state across nodes. At minimum this needs synchronization; preferably move the scratch vectors and rebased Schreier state into per-worker storage instead of sharing them globally.

As per coding guidelines, "Ensure race conditions are absent in multi-GPU code and multi-threaded server implementations; verify proper synchronization of shared state."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cpp/src/branch_and_bound/branch_and_bound.cpp` around lines 1396 - 1547, The
code mutates shared symmetry_ state
(symmetry_->marked_b0/marked_b1/marked_f0/marked_f1, symmetry_->orbit_has_*,
symmetry_->f0/f1 and calls symmetry_->schreier->set_base) while solve_node_lp()
runs concurrently, causing races; fix by making all scratch state worker-local
and avoid mutating the shared Schreier base: allocate per-worker vectors (e.g.
local_marked_b0, local_marked_b1, local_marked_f0, local_marked_f1,
local_orbit_has_*, local_f0, local_f1) sized symmetry_->num_original_vars and
use those instead of symmetry_->marked_* and symmetry_->orbit_has_*, and either
use a worker-local copy/clone of symmetry_->schreier (or ask Schreier for a
temporary stabilizer/orbit API) and call set_base on that copy; ensure you
update worker-local data when computing orb and apply fixings to
worker->leaf_problem as before; if cloning schreier is impossible, protect the
set_base/get_stabilizer_orbit sequence with a mutex around symmetry_->schreier
to prevent concurrent mutation.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, it needs to be thread local. I'm working on that. Slow your roll code rabbit :)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@chris-maes, understood — happy to wait for your fix! Looking forward to seeing the worker-local scratch state land. 🐇

(\_/)
(•ᴗ•)
/ >🔧

🧠 Learnings used
Learnt from: chris-maes
Repo: NVIDIA/cuopt PR: 1103
File: cpp/src/branch_and_bound/symmetry.hpp:405-430
Timestamp: 2026-04-16T16:27:57.345Z
Learning: In `cpp/src/branch_and_bound/symmetry.hpp` (NVIDIA/cuopt), the dejavu colored-graph automorphism solver preserves vertex colors, so orbits are color-class-restricted. Original variable vertices, constraint vertices, and auxiliary nonzero vertices are assigned strictly non-overlapping color ranges, which guarantees that `orb.orbit_size(j)` for any original variable index `j` is bounded by `num_original_vars`. Review comments assuming orbit mixing across differently-colored vertex types are not applicable in this context.

Learnt from: aliceb-nv
Repo: NVIDIA/cuopt PR: 922
File: cpp/src/branch_and_bound/pseudo_costs.cpp:85-87
Timestamp: 2026-03-03T15:03:14.218Z
Learning: In cuOpt, the dual simplex method is dual-feasible throughout execution in the files under cpp/src/dual_simplex/ and cpp/src/branch_and_bound/. When a solver terminates early with status ITERATION_LIMIT, the current objective value is still a valid (lower) bound for minimization, though looser than the optimum. Treat ITERATION_LIMIT results as safe for strong branching bound tightening, fixings, and cutoff logic in these modules. Apply this guideline to all cpp/src/branch_and_bound and related dual_simplex implementations (i.e., all .cpp files under cpp/src/branch_and_bound and cpp/src/dual_simplex).

Learnt from: aliceb-nv
Repo: NVIDIA/cuopt PR: 986
File: cpp/src/branch_and_bound/branch_and_bound.cpp:8-8
Timestamp: 2026-03-23T11:33:23.998Z
Learning: In this repo (NVIDIA/cuopt), treat nvcc as the supported CUDA toolchain; clang-based compilation/support is not required and may fail/break. During code reviews, do NOT request code changes or add blocking comments for errors that appear only under clang (e.g., header-resolution failures such as 'utilities/determinism_log.hpp not found')—these can be toolchain-related rather than real source issues.




i_t node_iter = 0;
f_t lp_start_time = tic();

Expand Down
7 changes: 6 additions & 1 deletion cpp/src/branch_and_bound/branch_and_bound.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ struct clique_table_t;

namespace cuopt::linear_programming::dual_simplex {

template <typename i_t, typename f_t>
struct mip_symmetry_t;

enum class mip_status_t {
OPTIMAL = 0, // The optimal integer solution was found
UNBOUNDED = 1, // The problem is unbounded
Expand Down Expand Up @@ -80,7 +83,8 @@ class branch_and_bound_t {
const simplex_solver_settings_t<i_t, f_t>& solver_settings,
f_t start_time,
const probing_implied_bound_t<i_t, f_t>& probing_implied_bound,
std::shared_ptr<detail::clique_table_t<i_t, f_t>> clique_table = nullptr);
std::shared_ptr<detail::clique_table_t<i_t, f_t>> clique_table = nullptr,
mip_symmetry_t<i_t, f_t>* symmetry = nullptr);

// Set an initial guess based on the user_problem. This should be called before solve.
void set_initial_guess(const std::vector<f_t>& user_guess) { guess_ = user_guess; }
Expand Down Expand Up @@ -162,6 +166,7 @@ class branch_and_bound_t {
const simplex_solver_settings_t<i_t, f_t> settings_;
const probing_implied_bound_t<i_t, f_t>& probing_implied_bound_;
std::shared_ptr<detail::clique_table_t<i_t, f_t>> clique_table_;
mip_symmetry_t<i_t, f_t>* symmetry_;
std::future<std::shared_ptr<detail::clique_table_t<i_t, f_t>>> clique_table_future_;
std::atomic<bool> signal_extend_cliques_{false};

Expand Down
Loading