Skip to content

Conversation

@akifcorduk
Copy link
Contributor

@akifcorduk akifcorduk commented Nov 25, 2025

This PR implements the first part of the paper from the paper: Preprocessing and Cutting Planes with Conflict Graphs.
This part contains only the preprocessing parts and clique cuts will follow in a separate PR:

  • Clique detection by converting constraints into sorted knapsack constraints. This allows fast clique detection.
  • Additional cliques in the same constraint by utilizing sorting structures.
  • Clique extension/merging across the conflict graph.
  • Clique covering and problem modification.

The data structures and query functions are implemented and will be used as a basis for clique cuts and usage in heuristics.

Pending: benchmark results/comparison

Summary by CodeRabbit

Release Notes

  • New Features

    • Enhanced presolve engine with clique-based conflict graph analysis for improved constraint detection and optimization.
    • Improved constraint propagation and handling during MIP problem initialization.
  • Chores

    • Updated build configuration for benchmarking executables.
    • Refined internal constraint data handling structures.

✏️ Tip: You can customize this high-level summary in your review settings.

@akifcorduk akifcorduk added this to the 26.02 milestone Nov 25, 2025
@akifcorduk akifcorduk added non-breaking Introduces a non-breaking change improvement Improves an existing functionality labels Nov 25, 2025
@copy-pr-bot
Copy link

copy-pr-bot bot commented Nov 25, 2025

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@coderabbitai
Copy link

coderabbitai bot commented Nov 25, 2025

📝 Walkthrough

Walkthrough

This PR introduces comprehensive clique-based presolve functionality for MIP solving. It adds new CSR matrix methods, creates a large clique-management infrastructure module, integrates clique preprocessing into the problem pipeline, and provides host-to-device problem transfer mechanisms.

Changes

Cohort / File(s) Summary
Build Configuration
cpp/CMakeLists.txt, cpp/src/mip/CMakeLists.txt
Added include directory paths for solve_MIP executable and moved clique_table.cu into non-LP build set.
CSR Matrix Enhancements
cpp/src/dual_simplex/sparse_matrix.hpp, cpp/src/dual_simplex/sparse_matrix.cpp
Added public methods get_constraint_range() and insert_row() to CSR matrix class; added scatter_dense overload; updated copyright year and fixed documentation typo.
Clique-based Presolve Infrastructure
cpp/src/mip/presolve/conflict_graph/clique_table.cuh, cpp/src/mip/presolve/conflict_graph/clique_table.cu
Introduced 650+ lines of new clique-management implementation including knapsack constraint normalization, clique construction/extension/pruning, dominance checking, adjacency queries, and diagnostic utilities with full template instantiation support.
Problem & Host Interface
cpp/src/mip/problem/problem.cuh, cpp/src/mip/problem/problem.cu
Added set_constraints_from_host_user_problem() method for host-to-device problem transfer; enhanced constraint resizing to preserve dual state.
Presolve & Solver Integration
cpp/src/mip/diversity/diversity_manager.cu, cpp/src/mip/solver.cu
Integrated clique-table handling into presolve pipeline; added invocation of constraint synchronization in solver initialization.
Presolve Utilities
cpp/src/mip/presolve/probing_cache.cu, cpp/src/mip/presolve/probing_cache.cuh
Added unordered_set include and documented probing injection behavior; no functional changes.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Suggested labels

mip

Suggested reviewers

  • aliceb-nv
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Clique Table and Preprocessing' directly and accurately summarizes the primary changes, which focus on implementing clique detection, management, and preprocessing functionality for MIP solving based on conflict graphs.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@rg20
Copy link
Contributor

rg20 commented Jan 22, 2026

@akifcorduk is this still relevant?

@rgsl888prabhu rgsl888prabhu changed the base branch from main to release/26.02 January 22, 2026 16:48
@akifcorduk akifcorduk marked this pull request as ready for review January 29, 2026 11:59
@akifcorduk akifcorduk requested review from a team as code owners January 29, 2026 11:59
@akifcorduk akifcorduk requested review from aliceb-nv and rg20 January 29, 2026 11:59
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 14

🤖 Fix all issues with AI agents
In @.github/workflows/build.yaml:
- Line 47: Replace all reusable workflow references that use the floating ref
"@main" with a pinned release tag or commit SHA; specifically update the uses
entries like "uses:
rapidsai/shared-workflows/.github/workflows/conda-cpp-build.yaml@main" (and the
other similar uses lines noted) to a stable tag or SHA (e.g., `@vX.Y.Z` or
@<commit-sha>) so the build.yaml and related workflow invocations become
deterministic; ensure you update every occurrence listed in the comment (lines
referencing conda-cpp-build.yaml, wheels-publish.yaml, etc.) to the chosen
pinned ref.

In @.github/workflows/pr.yaml:
- Line 37: Replace all instances of reusable workflow references that end with
`@main` (e.g., the uses line "uses:
rapidsai/shared-workflows/.github/workflows/pr-builder.yaml@main" and the other
14 similar entries) with a specific release tag or commit SHA from the
rapidsai/shared-workflows repo; update each occurrence to the chosen stable tag
or SHA, ensure you pick the intended release for each workflow, and verify CI
runs still pass after changing them.

In @.github/workflows/test.yaml:
- Around line 29-31: Replace all `uses:` references that currently point to
`@main` with a pinned tag or commit SHA to make CI deterministic; specifically
update the reusable workflow invocations for conda-cpp-tests,
conda-python-tests, wheels-test (both occurrences) and custom-job to reference a
stable release tag or commit hash (e.g., replace
`rapidsai/shared-workflows/.github/workflows/conda-cpp-tests.yaml@main` with the
same path at a chosen tag or SHA), and ensure each `uses:` line in the file is
changed accordingly so you deliberately control upgrades of the shared
workflows.

In @.github/workflows/trigger-breaking-change-alert.yaml:
- Line 18: Replace the unpinned reusable workflow reference that currently uses
"rapidsai/shared-workflows/.github/workflows/breaking-change-alert.yaml@main"
with a pinned release tag or commit SHA; update the "uses:" value to point to a
specific tag (e.g., `@vX.Y.Z`) or a commit SHA to ensure immutable behavior and
supply-chain safety, and commit the change so the workflow invocation is no
longer referencing `@main`.

In `@cpp/src/dual_simplex/sparse_matrix.cpp`:
- Around line 597-607: Add a debug assertion in csr_matrix_t<i_t,
f_t>::insert_row to ensure the input vectors have the same length so j and x
remain aligned: verify vars.size() == coeffs.size() (similar to the assert used
in append_column), and fail fast if they differ before mutating row_start, m,
nz_max, j, or x.

In `@cpp/src/mip/presolve/conflict_graph/clique_table.cu`:
- Around line 486-499: Compute the number of removed constraints before calling
erase instead of using the invalidated iterator new_end; for example, calculate
n_of_removed_constraints by calling std::distance(problem.row_sense.begin(),
new_end) or by counting true entries in removal_marker, then call
problem.row_sense.erase(new_end, problem.row_sense.end()) and log
n_of_removed_constraints; ensure the same approach is used for problem.rhs if
needed and reference the existing variables new_end, problem.row_sense.erase,
removal_marker, and n_of_removed_constraints to locate where to change the code.
- Around line 142-150: The current binary detection loop sets all_binary by
checking problem.var_types[...] == dual_simplex::variable_type_t::INTEGER which
misses other discrete/binary kinds; update the predicate in the loop (the block
that sets all_binary inside the for over constraint_range using A.j[j]) to treat
any non-continuous variable with bounds [0,1] as binary (i.e., replace the
specific INTEGER check with a test that var_types[...] !=
dual_simplex::variable_type_t::CONTINUOUS and retain the lower==0 && upper==1
checks) so clique detection runs for true binaries.
- Around line 309-329: Negated literals are currently handled incorrectly: when
var_idx >= problem.num_cols we set coeff = -1 but then do rhs_offset -= coeff
which increases RHS; instead apply the negated coefficient to the RHS by adding
coeff (i.e. change rhs_offset -= coeff to rhs_offset += coeff) so that a negated
literal (coeff == -1) reduces rhs by 1; locate the block around rhs_offset,
coeff, var_idx and update the arithmetic before computing rhs and inserting via
A.insert_row / problem.rhs.
- Around line 294-299: In clique_table_t::check_adjacency the code mistakenly
treats var indices as clique IDs by calling
var_clique_map_first[var_idx1].count(var_idx2) etc.; fix it by (1)
early-guarding self-adjacency (if var_idx1 == var_idx2 return false or true
according to intended semantics — likely false to avoid self-links), (2) keep
the direct adjacency check using
adj_list_small_cliques[var_idx1].count(var_idx2), and (3) replace the
var_clique_map_* checks with a proper intersection test between clique-ID sets
for the two variables: iterate the smaller of var_clique_map_first[var_idx1] ∪
var_clique_map_addtl[var_idx1] and var_clique_map_first[var_idx2] ∪
var_clique_map_addtl[var_idx2] and test membership in the other’s sets (use
count on the appropriate var_clique_map_* sets); return true if any shared
clique ID is found. Ensure you reference and use var_clique_map_first,
var_clique_map_addtl and adj_list_small_cliques in the corrected logic.
- Around line 31-77: In find_cliques_from_constraint: avoid accessing
kc.entries[k-1] when k==0 and prevent original_clique_start_idx from being
negative/creating an invalid lower_bound range. Fix by changing the first while
to require k>=1 (so kc.entries[k-1] is safe) and after computing
original_clique_start_idx, if it < 0 or equals 0 (clique spans whole constraint)
return/short-circuit so you don't use
kc.entries.begin()+original_clique_start_idx in lower_bound; also validate
original_clique_start_idx < kc.entries.size() before calling lower_bound in the
secondary loop and skip adding addtl_cliques when the search range would be
empty.

In `@docs/cuopt/source/cuopt-c/quick-start.rst`:
- Around line 52-55: The conda install lines currently set the wrong CUDA
toolkit value (they use the cuOpt version); update both occurrences of the conda
command that set "cuda-version=26.04.*" to the correct CUDA toolkit version
string (e.g., "cuda-version=12.9" or "cuda-version=13.0") so the two install
lines that reference libcuopt=26.04.* correctly specify the CUDA toolkit via the
cuda-version parameter.

In `@docs/cuopt/source/cuopt-python/quick-start.rst`:
- Around line 44-47: Update the two conda install lines that currently use the
incorrect cuda-version=26.04.*: replace the CUDA 13 command to use
cuda-version=13.1 and the CUDA 12 command to use cuda-version=12.9 so the two
install commands are distinct and match the conda environment files; modify the
existing conda install lines (the ones starting with "conda install -c rapidsai
-c conda-forge -c nvidia cuopt=26.04.* cuda-version=...") to use
cuda-version=13.1 for the CUDA 13 section and cuda-version=12.9 for the CUDA 12
section.

In `@docs/cuopt/source/cuopt-server/quick-start.rst`:
- Around line 32-38: The pip install lines for nightly packages are missing
closing single quotes for the package specifiers (e.g.,
'cuopt-server-cu13==26.4.*' and 'cuopt-sh-client==26.04.*' and the CUDA 12
equivalents). Edit the quick-start.rst snippet to add the missing trailing
single quotes to each package string so each package specifier is wrapped in
matching single quotes (fix both the CUDA 13 and CUDA 12 pip install blocks).
- Around line 16-23: The pip install lines for cuopt-sh-client are missing their
trailing single quotes which will break the commands; update the two occurrences
of 'cuopt-sh-client==26.04.* (one under CUDA 13 and one under CUDA 12) to
include the closing single quote so they read 'cuopt-sh-client==26.04.*'
ensuring both pip install command strings are properly quoted.
🧹 Nitpick comments (2)
cpp/CMakeLists.txt (2)

501-532: Consolidate duplicated include-directory calls for solve_MIP.

You now set include dirs twice for the same target (the later block repeats the private src path). This is redundant and easy to drift—merge into a single call.

♻️ Proposed cleanup
-  target_include_directories(solve_MIP
-    PRIVATE
-    "${CMAKE_CURRENT_SOURCE_DIR}/src"
-  )

501-506: Remove PUBLIC from the solve_MIP include directories.

solve_MIP is an executable, and no other targets link to it. PUBLIC usage requirements on executables have no effect since executables cannot be linked to by other targets. Use PRIVATE exclusively to keep the configuration clear and consistent with CMake best practices.

Additionally, the use of $<BUILD_INTERFACE:...> is unnecessary for executables—this generator expression is typically used for exported library targets. Consider simplifying to a standard path.

🔧 Suggested adjustment
  target_include_directories(solve_MIP
    PRIVATE
    "${CMAKE_CURRENT_SOURCE_DIR}/src"
-   PUBLIC
-   "$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>"
+   "${CMAKE_CURRENT_SOURCE_DIR}/include>"
  )

@akifcorduk akifcorduk changed the base branch from release/26.02 to main January 30, 2026 15:48
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🤖 Fix all issues with AI agents
In `@cpp/src/dual_simplex/sparse_matrix.cpp`:
- Around line 578-582: Add a debug-only bounds check at the start of
csr_matrix_t<i_t, f_t>::get_constraint_range to ensure cstr_idx is within [0,
row_start.size()-1] (so that accessing row_start[cstr_idx+1] is safe); use the
project's debug/assert macro (or <cassert> assert) to verify cstr_idx >= 0 and
cstr_idx + 1 < this->row_start.size(), then return the pair as before—this keeps
the fast path in release but catches invalid cstr_idx in debug.

In `@cpp/src/mip/presolve/conflict_graph/clique_table.cu`:
- Around line 268-284: The adjacency set returned by get_adj_set_of_var
currently includes the variable itself; after populating adj_set from
var_clique_map_first/first, var_clique_map_addtl/addtl_cliques/first, and
adj_list_small_cliques, remove the self-entry by erasing var_idx (e.g., call
adj_set.erase(var_idx)) before returning so the variable is excluded from its
own adjacency set.
- Around line 350-395: The extend_clique function can add existing clique
members because extension_candidates is filled from
clique_table.get_adj_set_of_var(smallest_degree_var) without filtering; update
extend_clique to skip any candidate already present in the input clique (or
already in new_clique) before sorting/attempting to add—e.g. build a quick
lookup from clique (or check membership when iterating extension_candidates) and
only push non-members into extension_candidates so new_clique never receives
duplicates from extension_candidates; refer to extend_clique,
extension_candidates, new_clique, and clique_table.get_adj_set_of_var to locate
the change.
- Around line 295-318: In clique_table_t::check_adjacency, guard against
self-adjacency and stop using std::binary_search on cliques (they are sorted by
coefficient, not var index); first add an early return if var_idx1 == var_idx2
to avoid treating a variable as adjacent to itself, then replace the
std::binary_search checks over first[clique_idx] and the sliced range
(clique.begin()+addtl.start_pos_on_clique to clique.end()) with an index-based
or iterator linear search (e.g., std::find or std::any_of) so membership is
checked by equality against var_idx2; keep the existing checks of
addtl_cliques[addtl_idx].vertex_idx and the final adj_list_small_cliques lookup
unchanged.

Comment on lines +578 to +582
template <typename i_t, typename f_t>
std::pair<i_t, i_t> csr_matrix_t<i_t, f_t>::get_constraint_range(i_t cstr_idx) const
{
return std::make_pair(this->row_start[cstr_idx], this->row_start[cstr_idx + 1]);
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Add a debug bounds assert for cstr_idx.

Without a guard, an invalid cstr_idx can read past row_start. A debug assert keeps the fast path while catching misuse early.

🔧 Suggested change (debug-only assert)
 template <typename i_t, typename f_t>
 std::pair<i_t, i_t> csr_matrix_t<i_t, f_t>::get_constraint_range(i_t cstr_idx) const
 {
+  assert(cstr_idx >= 0);
+  assert(static_cast<size_t>(cstr_idx + 1) < this->row_start.size());
   return std::make_pair(this->row_start[cstr_idx], this->row_start[cstr_idx + 1]);
 }
🤖 Prompt for AI Agents
In `@cpp/src/dual_simplex/sparse_matrix.cpp` around lines 578 - 582, Add a
debug-only bounds check at the start of csr_matrix_t<i_t,
f_t>::get_constraint_range to ensure cstr_idx is within [0, row_start.size()-1]
(so that accessing row_start[cstr_idx+1] is safe); use the project's
debug/assert macro (or <cassert> assert) to verify cstr_idx >= 0 and cstr_idx +
1 < this->row_start.size(), then return the pair as before—this keeps the fast
path in release but catches invalid cstr_idx in debug.

Comment on lines +268 to +284
template <typename i_t, typename f_t>
std::unordered_set<i_t> clique_table_t<i_t, f_t>::get_adj_set_of_var(i_t var_idx)
{
std::unordered_set<i_t> adj_set;
for (const auto& clique_idx : var_clique_map_first[var_idx]) {
adj_set.insert(first[clique_idx].begin(), first[clique_idx].end());
}

for (const auto& addtl_clique_idx : var_clique_map_addtl[var_idx]) {
adj_set.insert(first[addtl_cliques[addtl_clique_idx].clique_idx].begin(),
first[addtl_cliques[addtl_clique_idx].clique_idx].end());
}

for (const auto& adj_vertex : adj_list_small_cliques[var_idx]) {
adj_set.insert(adj_vertex);
}
return adj_set;
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Exclude var_idx from its own adjacency set.

Including the variable itself inflates degree and feeds self-candidates into extension.

🔧 Proposed fix
   for (const auto& adj_vertex : adj_list_small_cliques[var_idx]) {
     adj_set.insert(adj_vertex);
   }
+  adj_set.erase(var_idx);
   return adj_set;
📝 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
template <typename i_t, typename f_t>
std::unordered_set<i_t> clique_table_t<i_t, f_t>::get_adj_set_of_var(i_t var_idx)
{
std::unordered_set<i_t> adj_set;
for (const auto& clique_idx : var_clique_map_first[var_idx]) {
adj_set.insert(first[clique_idx].begin(), first[clique_idx].end());
}
for (const auto& addtl_clique_idx : var_clique_map_addtl[var_idx]) {
adj_set.insert(first[addtl_cliques[addtl_clique_idx].clique_idx].begin(),
first[addtl_cliques[addtl_clique_idx].clique_idx].end());
}
for (const auto& adj_vertex : adj_list_small_cliques[var_idx]) {
adj_set.insert(adj_vertex);
}
return adj_set;
template <typename i_t, typename f_t>
std::unordered_set<i_t> clique_table_t<i_t, f_t>::get_adj_set_of_var(i_t var_idx)
{
std::unordered_set<i_t> adj_set;
for (const auto& clique_idx : var_clique_map_first[var_idx]) {
adj_set.insert(first[clique_idx].begin(), first[clique_idx].end());
}
for (const auto& addtl_clique_idx : var_clique_map_addtl[var_idx]) {
adj_set.insert(first[addtl_cliques[addtl_clique_idx].clique_idx].begin(),
first[addtl_cliques[addtl_clique_idx].clique_idx].end());
}
for (const auto& adj_vertex : adj_list_small_cliques[var_idx]) {
adj_set.insert(adj_vertex);
}
adj_set.erase(var_idx);
return adj_set;
}
🤖 Prompt for AI Agents
In `@cpp/src/mip/presolve/conflict_graph/clique_table.cu` around lines 268 - 284,
The adjacency set returned by get_adj_set_of_var currently includes the variable
itself; after populating adj_set from var_clique_map_first/first,
var_clique_map_addtl/addtl_cliques/first, and adj_list_small_cliques, remove the
self-entry by erasing var_idx (e.g., call adj_set.erase(var_idx)) before
returning so the variable is excluded from its own adjacency set.

Comment on lines +295 to +318
template <typename i_t, typename f_t>
bool clique_table_t<i_t, f_t>::check_adjacency(i_t var_idx1, i_t var_idx2)
{
// Check first cliques: var_clique_map_first stores clique indices
for (const auto& clique_idx : var_clique_map_first[var_idx1]) {
const auto& clique = first[clique_idx];
if (std::binary_search(clique.begin(), clique.end(), var_idx2)) { return true; }
}

// Check additional cliques: var_clique_map_addtl stores indices into addtl_cliques
for (const auto& addtl_idx : var_clique_map_addtl[var_idx1]) {
const auto& addtl = addtl_cliques[addtl_idx];
const auto& clique = first[addtl.clique_idx];
// addtl clique is: vertex_idx + first[clique_idx][start_pos_on_clique:]
if (addtl.vertex_idx == var_idx2) { return true; }
if (addtl.start_pos_on_clique < static_cast<i_t>(clique.size())) {
if (std::binary_search(clique.begin() + addtl.start_pos_on_clique, clique.end(), var_idx2)) {
return true;
}
}
}

return adj_list_small_cliques[var_idx1].count(var_idx2) > 0;
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Avoid binary_search on unsorted cliques and guard self-adjacency.

Cliques are ordered by coefficient (not var index), so binary_search can yield false negatives/positives. Also avoid treating a var as adjacent to itself.

🔧 Proposed fix
 bool clique_table_t<i_t, f_t>::check_adjacency(i_t var_idx1, i_t var_idx2)
 {
+  if (var_idx1 == var_idx2) { return false; }
   // Check first cliques: var_clique_map_first stores clique indices
   for (const auto& clique_idx : var_clique_map_first[var_idx1]) {
     const auto& clique = first[clique_idx];
-    if (std::binary_search(clique.begin(), clique.end(), var_idx2)) { return true; }
+    if (std::find(clique.begin(), clique.end(), var_idx2) != clique.end()) { return true; }
   }
@@
-    if (addtl.start_pos_on_clique < static_cast<i_t>(clique.size())) {
-      if (std::binary_search(clique.begin() + addtl.start_pos_on_clique, clique.end(), var_idx2)) {
+    if (addtl.start_pos_on_clique < static_cast<i_t>(clique.size())) {
+      if (std::find(clique.begin() + addtl.start_pos_on_clique, clique.end(), var_idx2) !=
+          clique.end()) {
         return true;
       }
     }
   }
🤖 Prompt for AI Agents
In `@cpp/src/mip/presolve/conflict_graph/clique_table.cu` around lines 295 - 318,
In clique_table_t::check_adjacency, guard against self-adjacency and stop using
std::binary_search on cliques (they are sorted by coefficient, not var index);
first add an early return if var_idx1 == var_idx2 to avoid treating a variable
as adjacent to itself, then replace the std::binary_search checks over
first[clique_idx] and the sliced range (clique.begin()+addtl.start_pos_on_clique
to clique.end()) with an index-based or iterator linear search (e.g., std::find
or std::any_of) so membership is checked by equality against var_idx2; keep the
existing checks of addtl_cliques[addtl_idx].vertex_idx and the final
adj_list_small_cliques lookup unchanged.

Comment on lines +350 to +395
template <typename i_t, typename f_t>
bool extend_clique(const std::vector<i_t>& clique,
clique_table_t<i_t, f_t>& clique_table,
dual_simplex::user_problem_t<i_t, f_t>& problem,
dual_simplex::csr_matrix_t<i_t, f_t>& A)
{
i_t smallest_degree = std::numeric_limits<i_t>::max();
i_t smallest_degree_var = -1;
// find smallest degree vertex in the current set packing constraint
for (size_t idx = 0; idx < clique.size(); idx++) {
i_t var_idx = clique[idx];
i_t degree = clique_table.get_degree_of_var(var_idx);
if (degree < smallest_degree) {
smallest_degree = degree;
smallest_degree_var = var_idx;
}
}
std::vector<i_t> extension_candidates;
auto smallest_degree_adj_set = clique_table.get_adj_set_of_var(smallest_degree_var);
extension_candidates.insert(
extension_candidates.end(), smallest_degree_adj_set.begin(), smallest_degree_adj_set.end());
std::sort(extension_candidates.begin(), extension_candidates.end(), [&](i_t a, i_t b) {
return clique_table.get_degree_of_var(a) > clique_table.get_degree_of_var(b);
});
auto new_clique = clique;
for (size_t idx = 0; idx < extension_candidates.size(); idx++) {
i_t var_idx = extension_candidates[idx];
bool add = true;
for (size_t i = 0; i < new_clique.size(); i++) {
// check if the tested variable conflicts with all vars in the new clique
if (!clique_table.check_adjacency(var_idx, new_clique[i])) {
add = false;
break;
}
}
if (add) { new_clique.push_back(var_idx); }
}
// if we found a larger cliqe, insert it into the formulation
if (new_clique.size() > clique.size()) {
clique_table.first.push_back(new_clique);
CUOPT_LOG_DEBUG("Extended clique: %lu from %lu", new_clique.size(), clique.size());
// insert the new clique into the problem as a new constraint
insert_clique_into_problem(new_clique, problem, A);
}
return new_clique.size() > clique.size();
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Skip candidates already in the clique to avoid duplicates.

extension_candidates can contain existing clique members, so duplicates can be added, leading to repeated vars in the generated constraint.

🔧 Proposed fix
   auto new_clique = clique;
+  std::unordered_set<i_t> new_clique_set(new_clique.begin(), new_clique.end());
   for (size_t idx = 0; idx < extension_candidates.size(); idx++) {
     i_t var_idx = extension_candidates[idx];
+    if (new_clique_set.count(var_idx)) { continue; }
     bool add    = true;
     for (size_t i = 0; i < new_clique.size(); i++) {
       // check if the tested variable conflicts with all vars in the new clique
       if (!clique_table.check_adjacency(var_idx, new_clique[i])) {
         add = false;
         break;
       }
     }
-    if (add) { new_clique.push_back(var_idx); }
+    if (add) {
+      new_clique.push_back(var_idx);
+      new_clique_set.insert(var_idx);
+    }
   }
🤖 Prompt for AI Agents
In `@cpp/src/mip/presolve/conflict_graph/clique_table.cu` around lines 350 - 395,
The extend_clique function can add existing clique members because
extension_candidates is filled from
clique_table.get_adj_set_of_var(smallest_degree_var) without filtering; update
extend_clique to skip any candidate already present in the input clique (or
already in new_clique) before sorting/attempting to add—e.g. build a quick
lookup from clique (or check membership when iterating extension_candidates) and
only push non-members into extension_candidates so new_clique never receives
duplicates from extension_candidates; refer to extend_clique,
extension_candidates, new_clique, and clique_table.get_adj_set_of_var to locate
the change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

improvement Improves an existing functionality non-breaking Introduces a non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants