Skip to content

Fix asynchronous clique table handoff - #1834

Merged
rapids-bot[bot] merged 1 commit into
NVIDIA:mainfrom
hlinsen:fix-async-clique-table-handoff
Sep 1, 2026
Merged

Fix asynchronous clique table handoff#1834
rapids-bot[bot] merged 1 commit into
NVIDIA:mainfrom
hlinsen:fix-async-clique-table-handoff

Conversation

@hlinsen

@hlinsen hlinsen commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

The root-cut generator could retain a stale null snapshot of the clique table while that table was being constructed asynchronously.

Race sequence:

  • branch_and_bound_t::clique_table_ is initially null.
  • An asynchronous task starts building the table.
  • cut_generation_t is constructed and copies the still-null shared_ptr.
  • The asynchronous task publishes the completed table.
  • Root separation waits for the task, but its copied pointer remains null.
  • Conflict-graph clique cuts and the conflict-graph portion of Zero-Half cuts are skipped.
Metric Main Clique fix Change
Root completion 167 166 -1
Total root-time geomean, N=160 3.760 s 3.987 s +6.02%
Total root-time mean 23.879 s 27.087 s +3.208 s
Root gap closed, N=159 30.955% 30.986% +0.030 pp
Root-gap better/tied/worse 11/140/8 Essentially neutral
Instance Root time: main → fix Ratio Root gap: main → fix Change
ns1208400 4.06 → 54.05 s 13.31× 0.000% → 0.000% 0.000 pp
proteindesign122trx11p8 7.34 → 21.46 s 2.92× 20.667% → 23.961% +3.293 pp
proteindesign121hz512p9 10.01 → 24.29 s 2.43× 16.683% → 18.288% +1.605 pp
comp21-2idx 14.46 → 23.62 s 1.63× 50.676% → 54.248% +3.573 pp

Three regressions bought stronger root bounds. ns1208400 incurred the largest cost without any root-gap improvement.

Signed-off-by: Hugo Linsenmaier <hlinsenmaier@gmail.com>
@hlinsen
hlinsen requested a review from aliceb-nv September 1, 2026 06:18
@hlinsen
hlinsen requested a review from a team as a code owner September 1, 2026 06:18
@hlinsen hlinsen added bug Something isn't working non-breaking Introduces a non-breaking change labels Sep 1, 2026
@hlinsen
hlinsen requested a review from mlubin September 1, 2026 06:18
@copy-pr-bot

copy-pr-bot Bot commented Sep 1, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@coderabbitai

coderabbitai Bot commented Sep 1, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 7d1c200b-3599-4e46-b57e-8ab07d51fdd4

📥 Commits

Reviewing files that changed from the base of the PR and between 0d4538c and 7ce3eb2.

📒 Files selected for processing (2)
  • cpp/src/cuts/cuts.hpp
  • cpp/tests/mip/cuts_test.cu

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.


📝 Walkthrough

Walkthrough

The cut generator now retains a reference to the caller-owned clique-table pointer. A MIP regression test verifies asynchronous clique cuts on a triangle set-packing problem.

Changes

Clique-table lifecycle

Layer / File(s) Summary
Caller-owned clique-table contract
cpp/src/cuts/cuts.hpp
cut_generation_t now requires a non-const shared-pointer reference and stores that reference instead of moving a separate pointer.
Root-node clique-cut regression
cpp/tests/mip/cuts_test.cu
The test configures CPU threads and verifies root LP bounds before and after asynchronous clique cuts.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 7ce3e

This localized concurrency fix ensures root separation sees the asynchronously completed clique table, restoring the intended clique and Zero-Half cut behavior without changing external interfaces or deployment requirements. No actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: aliceb-nv, mlubin, akifcorduk

🚥 Pre-merge checks | ✅ 4 | ❌ 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%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title, "Fix asynchronous clique table handoff," clearly and concisely describes the primary change.
Description check ✅ Passed The description directly explains the asynchronous clique-table race, the affected cut generation, the fix objective, and benchmark results.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@hlinsen

hlinsen commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 7ce3eb2

@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown

CI Test Summary

✅ All 31 test job(s) passed.

@hlinsen

hlinsen commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

/merge

@rapids-bot
rapids-bot Bot merged commit 38a9dfa into NVIDIA:main Sep 1, 2026
183 of 197 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working non-breaking Introduces a non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants