Repository navigation
Conversation
Prioritize saved iterates that satisfy the relaxed objective-gap tolerance and use consistent gap normalization during recovery. Add selection-policy regression tests and detailed objective/complementarity diagnostics. Validation: 13 focused barrier tests and 198 convex QP, SOCP, and Maros benchmarks at 1e-6 tolerances. Both prior numerical-error cases recover valid suboptimal solutions; two pre-existing input parsing errors remain. Signed-off-by: Rajesh Gandham <rgandham@nvidia.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to No actionable saved-iterate selection risk remains from the reviewed change. The PR is mergeable after normal checks. Pre-merge checks |
  |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 8.70% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 23 functions across 4 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
-
Check name Status Explanation Title check The title clearly summarizes the main change: preserving recoverable barrier iterates when objective gaps converge. Description check The description directly explains the iterate-selection change, recovery behavior, affected scenarios, and added tests. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
rg20
added
improvement
Update the cache-refinement test to use the templated barrier cache API introduced upstream. Signed-off-by: Rajesh Gandham <rgandham@nvidia.com>
|
/ok to test 76b20ec |
Signed-off-by: Rajesh Gandham <rgandham@nvidia.com>
|
/ok to test eb12ae2 |
CI Test Summary✅ All 32 test job(s) passed. |
| const bool saved_gap_pass = !check_objective_gap || saved.objective_gap < tolerance.objective_gap; | ||
|
|
||
| // Preserve a recoverable iterate even when better feasibility comes with a worse objective gap. | ||
| if (current_gap_pass != saved_gap_pass) { return current_gap_pass; } |
There was a problem hiding this comment.
So this saves when current_gap_pass is true and save_gap_pass is false. And it does not save when current_gap_pass is false and save_gap_pass is true.
But if they are both true, then you check to see if the current improved upon the saved for primal, dual and complementarity. But you don't check that the objective gap improved.
Is there a reason not to require that all: primal, dual, complementarity, and objective improve?
Signed-off-by: Rajesh Gandham <rgandham@nvidia.com>
Signed-off-by: Rajesh Gandham <rgandham@nvidia.com>
| relative_objective_gap); | ||
| } | ||
| settings.log.printf("Search direction computation failed\n"); | ||
| settings.log.printf("Current and saved solutions do not satisfy relaxed barrier tolerances\n"); |
There was a problem hiding this comment.
You probably want to keep "Search direction computation failed" that explains why we need to stop the solver.
Maybe "Search direction computation failed. No solutions satisfy barrier tolerances"
| !(tolerances[i] > 0) || !(current_residuals[i] < tolerances[i])) { | ||
| return false; | ||
| } | ||
| current_merit = std::max(current_merit, current_residuals[i] / tolerances[i]); |
There was a problem hiding this comment.
This is hard to understand. I think it would be better to first check if the current_residuals satisfy the tolerances. Then check to see if the max residual is less for the current than for the saved.
Signed-off-by: Rajesh Gandham <rgandham@nvidia.com>
|
/ok to test 95face7 |
2 participants
Description
This PR preserves an iterate that satisfies the relaxed tolerances so it can be returned as a suboptimal solution if later iterations encounter numerical difficulties.
Previously, an iterate was saved only when primal feasibility, dual feasibility, and complementarity all improved. This could retain an iterate with an unacceptable objective gap while discarding later iterates that satisfied all relaxed tolerances. The new policy prioritizes preserving a qualifying iterate for recovery.
QPLIB_8547
With relaxed tolerances set to 1e-6:
Before: Iterations 55 and 56 satisfy the relaxed criteria, but are not retained by the old selection policy. When primal feasibility deteriorates at iteration 57, the solver reports a numerical error.
After: The solver preserves iteration 55 and restores it when subsequent iterations deteriorate, returning a valid suboptimal solution
chainsing-50000-2
With relaxed tolerances set to 1e-6:
Before: Iterations 22–24 satisfy the relaxed criteria, but are not retained because their dual residuals are worse than the previously saved iterate’s. That earlier iterate does not satisfy the objective-gap tolerance. Subsequent numerical trouble therefore results in a numerical-error status.
After: The solver preserves a qualifying iterate and restores iteration 23 after the numerical error.
Checklist