Commit 3174edf
add an outer_bound_only option to solve_one (#802)
* add an outer_bound_only option to solve_one
* Have the Lagrangian spokes ask only for an outer bound
Thread the new solve_one outer_bound_only option through solve_loop
(SPOpt and PHBase) and switch the Lagrangian spokes onto it, so their
solves skip loading a solution they never look at.
Whether a solution is needed is a property of the spoke rather than of
each call, so it is an `outer_bound_only` attribute on _LagrangianMixin
rather than an argument on lagrangian(). Three cases opt out:
- ReducedCostsSpoke reads x and the rc suffix off the solved
subproblems (class attribute).
- subgradient_while_waiting makes Compute_Xbar read the x values left
behind by the previous solve (set in LagrangianOuterBound.main).
- lagranger, until the first nonants arrive from the hub: it computes
xbar from its own x until it has the hub's.
Also in solve_one's outer_bound_only branch: mark solution_available
False (no solution was loaded, so the Vars hold pre-solve values, and
the staleness check and PRIOR_SOLUTION warmstart must not treat them as
a solution), re-raise a solver exception instead of letting it surface
as an AttributeError on the missing results object, and fix the assert,
which asserted a tuple and so could never fire.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Fix invalid outer bounds and the cgbase positional solve_loop call
Two bugs, both caught by CI on the previous commit.
Gate the outer_bound_only bound extraction behind the existing
not_good_enough_results check instead of running ahead of it. Without
prox, a Lagrangian subproblem is routinely unbounded, and the results
object then carries no usable bound: gurobi leaves Lower_bound None (so
Ebound died on prob * None) while other solvers return garbage, which
was published as a valid outer bound -- test_cvar_lagrangian_outer_bound
saw 1.9e9 against a true optimum of -220700. A bad solve now takes the
path it took before this option existed: gripe, no solution, and
outer_bound left at its previous value, which is still a valid outer
bound since any Lagrangian dual value bounds the original problem.
The remaining None check can only fire on an otherwise usable solve.
Pass solve_loop's arguments by keyword in cgbase. It called super()
positionally, so inserting outer_bound_only ahead of warmstart in the
signature slid warmstart=True into the outer_bound_only slot, tripping
the assert. cgbase was the only positional caller.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Test outer_bound_only first, gated on bound rather than solution
not_good_enough_results asks whether there is a usable *solution*, which
is the wrong question for a bound-only solve: a subproblem stopped by a
time limit or a gap can carry no incumbent at all and still report the
outer bound the caller is there for. Gating the bound-only path behind
it threw those bounds away.
So test outer_bound_only first, with its own gate. no_outer_bound_results
is the bound-shaped analog of not_good_enough_results: same disqualifying
terminations (infeasible, unbounded, error), minus the two clauses about
solution presence and solution status. When it does disqualify a solve
there is no meaningful bound to report -- solvers variously return None,
a sentinel, or garbage -- so outer_bound keeps its previous value, which
is still a valid outer bound.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Guard the outer bound extraction with a try block
The helper rules on the routine, condition-detectable outcomes; the try
block catches what it cannot foresee (an empty results.Problem, say) and
re-raises with the scenario and status attached. Raising is right there:
no_outer_bound_results has already accounted for the bound-less solves,
so anything still throwing is a surprise rather than a routine outcome.
Note this try block is not what catches an unbounded subproblem. Pyomo
assigns None (gurobi) or the last iterate's objective (cplex) to
Problem[0].Lower_bound rather than raising, so nothing propagates out of
the attribute access; the termination condition is the only signal that
survives both plugins, and the None check backstops the rest.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Address the Copilot review: raise instead of assert, cgbase signature
Raise a ValueError rather than assert on outer_bound_only with
need_solution. python -O strips asserts, and this guard earns its keep
precisely there: it is what caught cgbase passing warmstart into the
outer_bound_only slot, whose silent form is bound-only solves in code
that builds columns from the solutions. Verified it now fires under both
python and python -O. This matches the convention already spelled out in
reduced_costs_spoke.py.
Accept and forward outer_bound_only in CGBase.solve_loop, which had
drifted from the base signature -- that drift is what made the
positional call break in the first place. Column generation needs the
solutions, so the option is not useful there; it is accepted to keep the
override honest, and a caller who asks for it raises out of solve_one
because need_solution defaults True. xhat_eval is deliberately left
alone: its signature is a different shape and it defaults
need_solution=False, so the guard would not catch misuse there.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Stop screening TerminationCondition.error out of the outer bound
xpress reports a MIP that ran out of time with no incumbent as
status=aborted, TerminationCondition=error (mip_no_sol_found in
xpress_direct.py), and then hands over xprob_attrs.bestbound anyway --
a perfectly good outer bound. Screening on error threw it away and the
Lagrangian spoke died in Ebound with no bound to report, which is the
case outer_bound_only exists to serve.
Screen as little as possible here: a bound from a solve that went badly
is the whole point of asking for a bound only. Infeasible and unbounded
stay screened, because there the danger is not a missing value but a
misleading one -- cplex fills in the last iterate's objective for an
unbounded LP, a plausible finite number that is not a bound.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Start subproblems at the vacuous outer bound instead of no attribute
Only solve_one ever wrote outer_bound, so until it did there was nothing
to read, and Ebound died with "'ScalarBlock' object has no attribute
'outer_bound'" whenever a first solve reported no bound. Give every
subproblem the bound it actually holds before its first solve: -inf
minimizing, +inf maximizing. That is what is known at that point, it
keeps Ebound arithmetic, and being the worst bound there is it never
wins a comparison at the hub. This predates outer_bound_only -- the old
not_good_enough_results path left the attribute unset the same way.
cgbase reads outer_bound as a reduced cost and tested hasattr to decide
whether one existed, which is now always true, so it tests the value
instead: it sums these into sum_redcosts, where an infinity would
quietly poison the total rather than fail loudly as None did.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Address Copilot review: keyword-only outer_bound_only, docstrings, traceback, tests
- Make outer_bound_only keyword-only and restore warmstart to its
original positional slot in solve_one and the three solve_loop
overrides, so existing positional calls can't land on it.
- Correct the docstrings: a bound-only solve that reports no bound keeps
the previous (possibly vacuous) outer bound rather than raising.
- Use a bare raise when an unexpected bound-extraction error surfaces so
the original traceback is preserved.
- Add mpisppy/tests/test_outer_bound_only.py covering the bound-only
path, the need_solution guard, the normal solve, and the keyword-only
contract; wire it into run_coverage.bash and test_pr_and_main.yml.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* Use None, not +/-inf, for a not-yet-computed subproblem bound
Addresses bknueven's review: a subproblem that has not been solved (or whose
bound-only solve reported no bound) has not *computed* a bound, so it should
hold None rather than a vacuous +/-inf dressed up as a real number.
- _set_initial_bounds: seed outer_bound/inner_bound with None.
- Ebound: return None if any scenario (collectively, across ranks) is missing
its outer bound, since the expectation cannot then be formed; the missing
check is Allreduce'd so every rank agrees before the sum reduction.
- Teach the consumers to treat None as 'no bound to offer': the Lagrangian and
Lagranger spokes skip send_bound; PHBase.Iter0 and Subgradient keep the prior
best bound; post_solve_bound prints 'not available'; the cross-scenario cut
extension folds None to the weakest bound for its max/min aggregation.
- Extend test_outer_bound_only with solver-free coverage of the None path:
initial bounds are None, Ebound is None when any bound is missing and the
weighted sum when all are present, and is a finite number after a successful
bound-only solve.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>1 parent a2f0b82 commit 3174edf
13 files changed
Lines changed: 432 additions & 44 deletions
File tree
- .github/workflows
- mpisppy
- cylinders
- extensions
- opt
- tests
- utils
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
98 | 98 | | |
99 | 99 | | |
100 | 100 | | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
101 | 105 | | |
102 | 106 | | |
103 | 107 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
7 | 7 | | |
8 | 8 | | |
9 | 9 | | |
| 10 | + | |
10 | 11 | | |
11 | 12 | | |
12 | 13 | | |
| |||
411 | 412 | | |
412 | 413 | | |
413 | 414 | | |
414 | | - | |
415 | | - | |
| 415 | + | |
| 416 | + | |
| 417 | + | |
| 418 | + | |
| 419 | + | |
| 420 | + | |
| 421 | + | |
416 | 422 | | |
417 | 423 | | |
418 | 424 | | |
| |||
478 | 484 | | |
479 | 485 | | |
480 | 486 | | |
481 | | - | |
| 487 | + | |
| 488 | + | |
| 489 | + | |
482 | 490 | | |
483 | 491 | | |
484 | 492 | | |
| |||
506 | 514 | | |
507 | 515 | | |
508 | 516 | | |
| 517 | + | |
| 518 | + | |
| 519 | + | |
| 520 | + | |
| 521 | + | |
| 522 | + | |
| 523 | + | |
509 | 524 | | |
510 | 525 | | |
511 | 526 | | |
| |||
519 | 534 | | |
520 | 535 | | |
521 | 536 | | |
522 | | - | |
523 | | - | |
524 | | - | |
525 | | - | |
526 | | - | |
527 | | - | |
528 | | - | |
529 | | - | |
| 537 | + | |
| 538 | + | |
| 539 | + | |
| 540 | + | |
| 541 | + | |
| 542 | + | |
| 543 | + | |
| 544 | + | |
| 545 | + | |
530 | 546 | | |
531 | 547 | | |
532 | 548 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
40 | 40 | | |
41 | 41 | | |
42 | 42 | | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
43 | 48 | | |
44 | 49 | | |
45 | 50 | | |
| |||
105 | 110 | | |
106 | 111 | | |
107 | 112 | | |
108 | | - | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
109 | 117 | | |
110 | 118 | | |
111 | 119 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
12 | 12 | | |
13 | 13 | | |
14 | 14 | | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
15 | 21 | | |
16 | 22 | | |
17 | 23 | | |
| |||
24 | 30 | | |
25 | 31 | | |
26 | 32 | | |
27 | | - | |
| 33 | + | |
28 | 34 | | |
29 | 35 | | |
30 | 36 | | |
| |||
41 | 47 | | |
42 | 48 | | |
43 | 49 | | |
44 | | - | |
| 50 | + | |
| 51 | + | |
45 | 52 | | |
46 | 53 | | |
47 | 54 | | |
| |||
63 | 70 | | |
64 | 71 | | |
65 | 72 | | |
66 | | - | |
| 73 | + | |
67 | 74 | | |
68 | | - | |
| 75 | + | |
69 | 76 | | |
70 | | - | |
| 77 | + | |
71 | 78 | | |
72 | 79 | | |
73 | 80 | | |
74 | 81 | | |
75 | | - | |
| 82 | + | |
76 | 83 | | |
77 | 84 | | |
78 | 85 | | |
79 | | - | |
| 86 | + | |
80 | 87 | | |
81 | 88 | | |
82 | 89 | | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
83 | 95 | | |
84 | 96 | | |
85 | 97 | | |
| |||
93 | 105 | | |
94 | 106 | | |
95 | 107 | | |
96 | | - | |
| 108 | + | |
97 | 109 | | |
98 | 110 | | |
99 | 111 | | |
| |||
103 | 115 | | |
104 | 116 | | |
105 | 117 | | |
106 | | - | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
107 | 122 | | |
108 | 123 | | |
109 | 124 | | |
110 | 125 | | |
111 | 126 | | |
112 | 127 | | |
113 | 128 | | |
114 | | - | |
| 129 | + | |
115 | 130 | | |
116 | 131 | | |
117 | 132 | | |
| |||
120 | 135 | | |
121 | 136 | | |
122 | 137 | | |
123 | | - | |
| 138 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
89 | 89 | | |
90 | 90 | | |
91 | 91 | | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
92 | 96 | | |
93 | 97 | | |
94 | 98 | | |
| |||
201 | 205 | | |
202 | 206 | | |
203 | 207 | | |
204 | | - | |
205 | | - | |
206 | | - | |
207 | | - | |
| 208 | + | |
| 209 | + | |
208 | 210 | | |
209 | 211 | | |
210 | 212 | | |
| |||
436 | 438 | | |
437 | 439 | | |
438 | 440 | | |
439 | | - | |
| 441 | + | |
440 | 442 | | |
441 | | - | |
| 443 | + | |
442 | 444 | | |
443 | 445 | | |
444 | 446 | | |
445 | | - | |
446 | | - | |
447 | | - | |
448 | | - | |
449 | 447 | | |
450 | 448 | | |
451 | 449 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
102 | 102 | | |
103 | 103 | | |
104 | 104 | | |
105 | | - | |
106 | | - | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
107 | 114 | | |
108 | 115 | | |
109 | 116 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
102 | 102 | | |
103 | 103 | | |
104 | 104 | | |
105 | | - | |
106 | | - | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
107 | 110 | | |
108 | 111 | | |
109 | 112 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
586 | 586 | | |
587 | 587 | | |
588 | 588 | | |
589 | | - | |
| 589 | + | |
| 590 | + | |
| 591 | + | |
590 | 592 | | |
591 | 593 | | |
592 | 594 | | |
| |||
599 | 601 | | |
600 | 602 | | |
601 | 603 | | |
602 | | - | |
| 604 | + | |
| 605 | + | |
| 606 | + | |
603 | 607 | | |
604 | 608 | | |
605 | 609 | | |
| |||
632 | 636 | | |
633 | 637 | | |
634 | 638 | | |
| 639 | + | |
| 640 | + | |
| 641 | + | |
| 642 | + | |
| 643 | + | |
635 | 644 | | |
636 | 645 | | |
637 | 646 | | |
| |||
662 | 671 | | |
663 | 672 | | |
664 | 673 | | |
| 674 | + | |
665 | 675 | | |
666 | 676 | | |
667 | 677 | | |
| |||
1224 | 1234 | | |
1225 | 1235 | | |
1226 | 1236 | | |
1227 | | - | |
| 1237 | + | |
1228 | 1238 | | |
1229 | 1239 | | |
1230 | 1240 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
129 | 129 | | |
130 | 130 | | |
131 | 131 | | |
| 132 | + | |
132 | 133 | | |
133 | 134 | | |
134 | 135 | | |
| |||
575 | 576 | | |
576 | 577 | | |
577 | 578 | | |
| 579 | + | |
| 580 | + | |
| 581 | + | |
| 582 | + | |
| 583 | + | |
| 584 | + | |
| 585 | + | |
| 586 | + | |
| 587 | + | |
| 588 | + | |
| 589 | + | |
| 590 | + | |
| 591 | + | |
| 592 | + | |
| 593 | + | |
| 594 | + | |
| 595 | + | |
| 596 | + | |
578 | 597 | | |
579 | 598 | | |
580 | 599 | | |
| |||
0 commit comments