Zero Half(Odd-Cycle) cuts by akifcorduk · Pull Request #1428 · NVIDIA/cuopt

akifcorduk · 2026-06-12T12:40:30Z

This branch adds zero-half (odd-cycle / odd-wheel) cuts to the set of cuts. The cut generator runs on the same fractional conflict-graph subgraph that clique cuts use: once per cut pass, prepare_fractional_sub_cg() builds the subgraph then Dijkstra finds violated odd cycles on the auxillarry conflict graph (edge weights summing below 0.5), optionally extends them to odd-wheels by adding fully conflicting variables(similar logic to clique extension). Clique cut was refactored to share that subgraph and the same greedy extension helpers, and implied-bound cuts were moved earlier in the pass so the background clique-table thread has more time to finish before we join it.

A-> this PR
B-> main

Gap Closed Metrics:

Top 10 A>B (A-B), gap_closed_pct

instance	A	B	A-B
brazil3	54.242	12.879	41.364
neos-957323	50.141	13.489	36.652
roll3000	71.977	61.487	10.491
supportcase7	62.035	51.657	10.379
nursesched-medium-hint03	36.597	33.262	3.336
atlanta-ip	17.689	14.890	2.799
sorrell3	3.704	1.961	1.743
neos-873061	62.916	61.324	1.592
physiciansched6-2	1.673	0.157	1.516
neos-3656078-kumeu	31.977	30.583	1.394

Top 10 B>A (B-A), gap_closed_pct

instance	A	B	B-A
rail01	52.512	62.944	10.432
neos-2746589-doon	49.619	58.182	8.563
50v-10	65.878	72.200	6.323
neos-662469	63.135	66.714	3.580
physiciansched3-3	33.387	36.218	2.831
ns1830653	25.524	28.122	2.598
irp	73.107	75.094	1.987
s100	29.239	30.497	1.258
seymour1	24.850	25.933	1.083
qap10	10.571	11.460	0.889

Average and shifted geomean gap closed

A avg=29.648 sgm=8.1513 (shift=1)
B avg=28.924 sgm=7.7679 (shift=1)

Benchmark Results

A Batch 1
total_optimal: 79, avg_mip_gap: 0.3173, geomean_mip_gap: 0.1947, n_low_error: 125
A Batch 2
total_optimal: 77, avg_mip_gap: 0.3175, geomean_mip_gap: 0.1972, n_low_error: 124
B Batch 1
total_optimal: 75, avg_mip_gap: 0.3419, geomean_mip_gap: 0.2062, n_low_error: 119
B Batch2
total_optimal: 75, avg_mip_gap: 0.3126, geomean_mip_gap: 0.1930, n_low_error: 118

On average +3 optimal solutions, -0.4% geomean MIP gap reductions. Overall 30 instances have zero-half cut in the benchmark set.

Brings in upstream's unified OpenMP threading model (PR NVIDIA#1099) and other fixes (NVIDIA#1206 concurrent LP exception cleanup, NVIDIA#1214/NVIDIA#1216 destruction/ capture fixes) while preserving local work on the cut and clique stack. Conflict resolution highlights: - Drop std::future/std::async clique flow; adopt upstream's omp task + omp_atomic_t<bool> signal_extend pattern. - Drop modify_problem parameter from find_initial_cliques (we already removed the code that consumed it); adapt the omp-task call site in branch_and_bound::solve accordingly. - Take upstream's [this, &population] capture for the root-LP CPUFJ improvement callback; the new omp taskwait-before-destruction guarantee makes the prior context-lifetime fix unnecessary. - Take upstream's do_cut_pass refactor of the per-pass LP resolve loop; move our per-pass root_lp_with_cuts publish into do_cut_pass so the benchmark metric is still updated on early exits. - Keep our out-of-line omp_mutex_t definitions in omp_helpers.cpp; the enhanced omp_atomic_t with std::memory_order is taken from upstream.

…apture

akifcorduk · 2026-06-12T14:45:55Z

/ok to test 3edceae

ramakrishnap-nv · 2026-06-12T16:35:59Z

/nvskills-ci

aliceb-nv

Thanks Akif! Let's get Chris' eyes on this as well

One thought - maybe we could start thinking about splitting cuts.cpp into a file per cut family or something. It is becoming quite heavy :)

aliceb-nv · 2026-06-17T16:18:34Z

+  if (clique_table_ == nullptr) {
+    if (signal_extend_) { signal_extend_->store(true, std::memory_order_release); }
+#pragma omp taskwait depend(in : *signal_extend_)
+  }


Isn't there a risk of race here? In "find_initial_cliques", clique_table_out is set to non-null before extend_cliques runs, so you could encounter a scenario where prepare_fractional_sub_cg reads clique_table_, sees it is non-null, and races on the clique table while it is being updated by extend_cliques, right?

You are right. I think in practice it never happened because the other cut generation functions were taking some time and most of the time clique table was non-null. Now it depends on signal_extend being null. But there was a convoluted logic of local, shared out clique_table and that also complicated things, it was because we were using clique generation in presolve before and that included ownership transfer logic. Now that logic is also simplified. Thanks for catching it Alice!

akifcorduk · 2026-06-25T12:07:26Z

/ok to test 0ea3102

coderabbitai

Actionable comments posted: 2

🤖 Prompt for all review comments with 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.

Inline comments:
In `@cpp/src/grpc/codegen/field_registry.yaml`:
- Around line 685-688: The gRPC path currently maps zero_half_cuts directly into
settings.zero_half_cuts without enforcing the local CUOPT_MIP_ZERO_HALF_CUTS
bounds. Add request-side validation in the generated protobuf-to-settings flow
(field_registry.yaml / generated_proto_to_mip_settings.inc path) so
zero_half_cuts is rejected unless it is within [-1, 1], or clamp it before
assigning to the MIP settings. Make sure the fix is applied where the field is
translated into the solver settings, not only in solver_settings.cu.

In `@cpp/src/mip_heuristics/presolve/conflict_graph/clique_table.cu`:
- Around line 727-739: The publication order in clique_table.cu is unsafe
because clique_table_out is assigned in the clique_table extension path before
extend_cliques and fill_var_clique_maps finish mutating the same clique_table
object. Fix this by delaying the clique_table_out assignment until after all
mutations are complete, or by publishing a separate immutable snapshot from the
base table and extending a different object; use the clique_table_out
assignment, extend_cliques, and fill_var_clique_maps symbols to locate the
affected flow.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 93cd4392-45ad-481c-98b7-554bb5f60951

📥 Commits

Reviewing files that changed from the base of the PR and between 3edceae and 0ea3102.

⛔ Files ignored due to path filters (3)

cpp/src/grpc/codegen/generated/cuopt_remote_data.proto is excluded by !**/generated/**
cpp/src/grpc/codegen/generated/generated_mip_settings_to_proto.inc is excluded by !**/generated/**
cpp/src/grpc/codegen/generated/generated_proto_to_mip_settings.inc is excluded by !**/generated/**

📒 Files selected for processing (11)

cpp/include/cuopt/linear_programming/constants.h
cpp/src/branch_and_bound/branch_and_bound.cpp
cpp/src/cuts/cuts.cpp
cpp/src/grpc/codegen/field_registry.yaml
cpp/src/math_optimization/solver_settings.cu
cpp/src/mip_heuristics/presolve/conflict_graph/clique_table.cu
cpp/src/mip_heuristics/presolve/conflict_graph/clique_table.cuh
docs/cuopt/source/cuopt-c/mip/mip-c-api.rst
docs/cuopt/source/mip-settings.rst
skills/cuopt-developer/references/contributing.md
skills/cuopt-developer/references/conventions.md

✅ Files skipped from review due to trivial changes (5)

cpp/include/cuopt/linear_programming/constants.h
skills/cuopt-developer/references/conventions.md
docs/cuopt/source/cuopt-c/mip/mip-c-api.rst
docs/cuopt/source/mip-settings.rst
skills/cuopt-developer/references/contributing.md

🚧 Files skipped from review as they are similar to previous changes (2)

cpp/src/branch_and_bound/branch_and_bound.cpp
cpp/src/cuts/cuts.cpp

akifcorduk · 2026-06-25T12:44:18Z

/ok to test 758e045

chris-maes · 2026-06-25T13:11:25Z

+// aborts/terminates right after. Each channel below enables it through its own
+// DEBUG_* flag and supplies its own prefix; when the flag is 0 the call expands
+// to a no-op that still consumes its arguments.
+#define CUTS_DEBUG_LOG(prefix, ...)    \


Nit: We have a settings.log.debug already. Could this be used instead of this macro?

It seems strange to have cut specific debugging/logging.

The setting.log.debug prints trace logs underneath because it was too verbose. I wanted to have some log in between which is CUTS_DEBUG_LOG. I can the trace logging to setting.log.trace so we can have finer control on the logs on another PR. I can convert this to settings.log.debug for now.

chris-maes · 2026-06-25T13:13:11Z

+                          f_t* work_estimate,
+                          f_t max_work_estimate)
+{
+  for (const auto candidate : candidates) {


Nit: prefer an actual type here instead of auto. Using a type makes the code self-documenting.

chris-maes · 2026-06-25T13:13:33Z

+    if (toc(start_time) >= time_limit) { return; }
+    bool add   = true;
+    i_t checks = 0;
+    for (const auto v : selected) {


Same here. Prefer actual type to auto

chris-maes · 2026-06-25T13:14:32Z

+  scratch.ensure_size(static_cast<std::size_t>(total_idx));
+  ++scratch.gen;
+  const std::uint64_t gen = scratch.gen;
+  auto& dist              = scratch.dist;


Prefer type to auto

chris-maes · 2026-06-25T13:15:12Z

+      ZERO_HALF_DEBUG("dijkstra_odd_cycle work_limit hit pops=%lld", static_cast<long long>(pops));
+      return false;
+    }
+    for (const auto v_local : neigh) {


Prefer type to auto

chris-maes · 2026-06-25T13:15:47Z

+
+  std::unordered_set<i_t> seen_local;
+  seen_local.reserve(local_seq.size());
+  for (const auto lv : local_seq) {


Prefer type to auto

chris-maes · 2026-06-25T13:15:59Z

+  cycle_vertices.reserve(local_seq.size());
+  std::unordered_set<i_t> seen_var;
+  seen_var.reserve(local_seq.size());
+  for (const auto lv : local_seq) {


Preferr type to auto

chris-maes · 2026-06-25T13:16:24Z

+  std::unordered_set<i_t> cycle_members(cycle_vertices.begin(), cycle_vertices.end());
+  std::vector<i_t> candidates;
+  candidates.reserve(adj_set.size());
+  for (const auto candidate : adj_set) {


Prefer type to auto

chris-maes · 2026-06-25T13:19:04Z

 }

+template <typename i_t, typename f_t>
+void cut_generation_t<i_t, f_t>::prepare_fractional_sub_cg(


Nit: The code would be easier to read if the CG acronym was written out in full. Does CG stand for cut graph?

The acronym is conflict_graph. I can extend it.

chris-maes · 2026-06-25T13:19:48Z

+    total_adj_entries += adj_set.size();
+    auto& adj = sub_cg_.adj_local[idx];
+    adj.reserve(adj_set.size());
+    for (const auto neighbor : adj_set) {


Prefer type to auto

chris-maes · 2026-06-25T13:20:00Z

+    {
+      std::unordered_set<i_t> adj_global;
+      adj_global.reserve(adj.size());
+      for (const auto neighbor : adj) {


Prefer type to auto

chris-maes · 2026-06-25T13:20:58Z

+    }
+  }
+
+  // Build the fractional conflict-graph subgraph once (resolving the async


Ah it stands for conflict graph. My ignorance/confusion is a reason to write it out in full.

chris-maes · 2026-06-25T13:23:37Z

+      added_per_var++;
+      // mark all CG vertices that participated so we do not re-derive the same
+      // cycle from a different source vertex
+      for (const auto v : cycle_vertices) {


Prefer explicit type here instead of auto

chris-maes

Thanks for adding these cuts in @akifcorduk

LGTM. Very minor style comments. I did not review the math.

akifcorduk · 2026-06-26T09:57:27Z

/ok to test b90197c

akifcorduk added 30 commits April 24, 2026 14:01

main baseline test

d65be83

fix thread count

eb5d586

initial version of odd-cycle cuts

c03842b

Merge branch 'main' of github.com:NVIDIA/cuopt into main_baselin

056552f

with gap computation

e7bf32c

measure main branch

5335b65

test clique changes

0b04683

merge clique changes

72bb299

remove deterministic guards

bc5006f

clique fixes and common subgraph usage

3f42c82

fix complement bug

af65630

Merge branch 'main_baselin' into zero_half

86e7888

fix compile error

f2004bc

fix omp

3f0ace1

with additional fix

7995451

add cuda error recovery for capture

8ad61dd

test CI

0a5149b

fix logger

06352db

Merge branch 'main' of github.com:NVIDIA/cuopt into cuda_graph_side_c…

36e74d2

…apture

restore the api and use api suitable for <12.3

4d2fb18

more comments

dba39c8

Merge branch 'cuda_graph_side_capture' into main_baselin

0a4571f

fix ping pong graph major, non-major logic

56b6e84

Merge branch 'cuda_graph_side_capture' into main_baselin

b83836b

Merge branch 'main' of github.com:NVIDIA/cuopt into main_Baselin

ce6d499

fix timer, better jaccard

4de79a0

simplify comments

a17ffa3

cut stats

e5bc991

Revert PDLP/PDHG cuda-graph changes to baseline

b5bd4b2

handle compile errors

3edceae

akifcorduk requested review from chris-maes and removed request for mlubin June 12, 2026 14:46

ramakrishnap-nv approved these changes Jun 12, 2026

View reviewed changes

aliceb-nv reviewed Jun 17, 2026

View reviewed changes

akifcorduk added 2 commits June 24, 2026 20:42

fix reviews

0bee11f

handle race condition and clear clique table representations

0ea3102

coderabbitai Bot reviewed Jun 25, 2026

View reviewed changes

Comment thread cpp/src/grpc/codegen/field_registry.yaml

Comment thread cpp/src/mip_heuristics/presolve/conflict_graph/clique_table.cu

Merge branch 'main' of github.com:NVIDIA/cuopt into zero_half

758e045

chris-maes reviewed Jun 25, 2026

View reviewed changes

Comment thread cpp/src/cuts/cuts.cpp Outdated

chris-maes reviewed Jun 25, 2026

View reviewed changes

chris-maes approved these changes Jun 25, 2026

View reviewed changes

handle reviews

b90197c

Uh oh!

Conversation

akifcorduk commented Jun 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Top 10 A>B (A-B), gap_closed_pct

Top 10 B>A (B-A), gap_closed_pct

Average and shifted geomean gap closed

Benchmark Results

Uh oh!

akifcorduk commented Jun 12, 2026

Uh oh!

ramakrishnap-nv commented Jun 12, 2026

Uh oh!

aliceb-nv left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

akifcorduk commented Jun 25, 2026

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

akifcorduk commented Jun 25, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chris-maes left a comment

Choose a reason for hiding this comment

Uh oh!

akifcorduk commented Jun 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

akifcorduk commented Jun 12, 2026 •

edited

Loading