Fix concurrent root halt on infeasible MILP - #1413
Conversation
|
Hi maintainers, the label checker is waiting for category/breaking labels. Suggested labels for this PR: bug, non-breaking, mip. This is a fresh, narrow root-cause PR for #1396: it only changes concurrent root-solve halt coordination and adds a default-opportunistic cuopt_cli regression for the infeasible MPS crash path. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe root LP solve now requests the concurrent-halt flag for all terminal LP statuses immediately after solving; a new CLI regression test verifies an infeasible MILP returns "Termination Status: Infeasible" in opportunistic concurrent mode. ChangesConcurrent root-halt signal on terminal LP status
🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Infer (1.2.0)cpp/src/branch_and_bound/branch_and_bound.cppcpp/src/branch_and_bound/branch_and_bound.cpp:8:10: fatal error: 'branch_and_bound/branch_and_bound.hpp' file not found ... [truncated 1187 characters] ... nclude" Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
python/libcuopt/libcuopt/tests/test_cli.sh (1)
35-68: ⚡ Quick winPin opportunistic mode explicitly in the regression assertion.
This regression targets the opportunistic concurrent-root path, but the invocation relies on default mode. Please force opportunistic mode and check the status line explicitly so the test keeps covering the intended failure mode even if defaults change.
Suggested update
-cuopt_cli "${tmp_mps}" --time-limit 30 | grep -q "Infeasible" || (echo "Expected infeasible status for opportunistic MILP" && exit 1) +cuopt_cli "${tmp_mps}" --mip-determinism-mode 0 --time-limit 30 \ + | grep -q "Status: Infeasible" \ + || (echo "Expected infeasible status for opportunistic MILP" && exit 1)As per coding guidelines, regression tests here should validate the specific failure mode and status/exit path for this concurrent-root bug class.
🤖 Prompt for 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. In `@python/libcuopt/libcuopt/tests/test_cli.sh` around lines 35 - 68, Update the test invocation that currently runs cuopt_cli ("cuopt_cli \"${tmp_mps}\" --time-limit 30 | grep -q \"Infeasible\"") to explicitly pass the CLI flag that forces opportunistic concurrent-root mode (add the project’s opportunistic-mode flag to the cuopt_cli invocation) and tighten the assertion to check the solver status line explicitly (grep for the status output that contains "Infeasible" rather than relying on defaults). Locate the invocation of cuopt_cli and the grep assertion in the test script (the tmp_mps creation and the final pipeline) and modify those two pieces: add the opportunistic-mode option to the cuopt_cli command and ensure the grep matches the solver status line text.Source: Coding guidelines
🤖 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.
Nitpick comments:
In `@python/libcuopt/libcuopt/tests/test_cli.sh`:
- Around line 35-68: Update the test invocation that currently runs cuopt_cli
("cuopt_cli \"${tmp_mps}\" --time-limit 30 | grep -q \"Infeasible\"") to
explicitly pass the CLI flag that forces opportunistic concurrent-root mode (add
the project’s opportunistic-mode flag to the cuopt_cli invocation) and tighten
the assertion to check the solver status line explicitly (grep for the status
output that contains "Infeasible" rather than relying on defaults). Locate the
invocation of cuopt_cli and the grep assertion in the test script (the tmp_mps
creation and the final pipeline) and modify those two pieces: add the
opportunistic-mode option to the cuopt_cli command and ensure the grep matches
the solver status line text.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 3af57071-7f24-44a9-befe-19d303555774
📒 Files selected for processing (2)
cpp/src/branch_and_bound/branch_and_bound.cpppython/libcuopt/libcuopt/tests/test_cli.sh
3f3e4c1 to
7dd1aa5
Compare
|
Closing in favor of #1416 which better addresses the root cause. |
2 participants
Summary
Why
Testing
Fixes #1396