Skip to content

Fix false optimality declaration in the determinsitic mode - #1954

Open
aliceb-nv wants to merge 2 commits into
NVIDIA:mainfrom
aliceb-nv:determinsitic-limit-fix
Open

aliceb-nv wants to merge 2 commits into
NVIDIA:mainfrom
aliceb-nv:determinsitic-limit-fix

Conversation

@aliceb-nv

Copy link
Copy Markdown
Contributor

When the solve is interrupted due to a time limit in the deterministic B&B code, remaining nodes abort their solve and return status PENDING which wasn't properly handled by the deterministic codepath, dropping all such nodes from the tree and causing the dual bound to become +inf. If a primal solution was found, it would thus immediately be declared (incorrectly) as optimal.

This PR fixes this issue.

Description

Issue

Checklist

  • I am familiar with the Contributing Guidelines.
  • Testing
    • New or existing tests cover these changes
    • Added tests
    • Created an issue to follow-up
    • NA
  • Documentation
    • The documentation is up to date with these changes
    • Added new documentation
    • NA

@aliceb-nv aliceb-nv added this to the 26.10 milestone Sep 21, 2026
@aliceb-nv
aliceb-nv requested a review from a team as a code owner September 21, 2026 12:27
@aliceb-nv aliceb-nv added bug Something isn't working non-breaking Introduces a non-breaking change labels Sep 21, 2026
@copy-pr-bot

copy-pr-bot Bot commented Sep 21, 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.

@aliceb-nv

Copy link
Copy Markdown
Contributor Author

/ok to test d8ed289

@coderabbitai

coderabbitai Bot commented Sep 21, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

Understand this PR’s impact

Explore downstream dependencies and potential security impact with Blast Radius.

View blast radius →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: NVIDIA/cuopt/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 8de0d0fd-5ae4-493a-b094-27d75b1951fb

📥 Commits

Reviewing files that changed from the base of the PR and between d8ed289 and c950f30.

📒 Files selected for processing (1)
  • cpp/src/branch_and_bound/branch_and_bound.cpp
🚧 Files skipped from review as they are similar to previous changes (1)
  • cpp/src/branch_and_bound/branch_and_bound.cpp

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


📝 Walkthrough

Walkthrough

The status declaration in run_deterministic_bfs_loop is aligned with adjacent declarations. Behavior is unchanged.

Changes

Deterministic BFS execution

Layer / File(s) Summary
Status declaration alignment
cpp/src/branch_and_bound/branch_and_bound.cpp
The declaration alignment in run_deterministic_bfs_loop is adjusted without changing behavior.

Priority: ➖ Normal

Estimated code review effort: 1 (Trivial) | ~2 minutes

Change: Bug fix

🚥 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 1 functions across 1 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 clearly identifies the main change: preventing false optimality declarations in deterministic mode. It contains a minor spelling error in "determinsitic," but the meaning remains clear.
Description check ✅ Passed The description directly explains the deterministic branch-and-bound issue, including the handling of PENDING nodes, the incorrect dual bound, and the resulting false optimality declaration.
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 💡 1
🧪 Generate unit tests (beta)
  • Create a new PR
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR

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

@Kh4ster Kh4ster left a comment

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.

Great minimal fix, thanks @aliceb-nv !

@aliceb-nv

Copy link
Copy Markdown
Contributor Author

/ok to test c950f30

@github-actions

github-actions Bot commented Sep 21, 2026

Copy link
Copy Markdown

CI Test Summary

✅ All 18 test job(s) passed. (3 skipped)

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