Skip to content

fix(correctness): LP presolve fabricated bounds from the INF sentinel#877

Merged
jkitchin merged 1 commit into
mainfrom
fix/lp-presolve-inf-sentinel
Jul 26, 2026
Merged

fix(correctness): LP presolve fabricated bounds from the INF sentinel#877
jkitchin merged 1 commit into
mainfrom
fix/lp-presolve-inf-sentinel

Conversation

@jkitchin

Copy link
Copy Markdown
Owner

The defect

crates/discopt-core/src/lp/simplex/presolve.rs decides whether a column's contribution to a row is unbounded by testing the product a_ij * x_j against the sentinel INF = 1e20:

let (cmin, cmax) = if aij > 0.0 { (aij*lo[j], aij*hi[j]) } else { (aij*hi[j], aij*lo[j]) };
if cmin <= -INF { n_min_inf += 1; } else { sum_min_finite += cmin; }

INF is a sentinel, not a true infinity. For |a_ij| < 1 an unbounded column contributes an ordinary finite number — -0.5 * 1e20 = -5e19 — which fails the <= -INF test and is booked as a finite activity. Two things then go wrong at once:

  1. the row's activity range is no longer recognised as unbounded, so the pass derives bounds it has no right to derive; and
  2. the huge magnitude annihilates every smaller term in the running sum (ulp(5e19) = 8192), so the residual sum_min_finite - ck_min evaluates to exactly 0.0 instead of its true value.

The result is an invalid tightening. Presolve is dimension-preserving with no postsolve, so the mutilated box goes straight to the B&B tree and the node LPs, and the search reports OPTIMAL on a box the optimum has been removed from — a false dual bound. The module's own docstring promises the pass "never cuts a feasible (let alone optimal) solution"; it did.

Minimal reproducer

min 4096·x0    s.t.  -0.5·x0 - 1.9073486328125·x1 <= -17.612222262299806
                     x0 in [0, inf),  x1 in [2.56, 1600.0]

x1 = 1600 satisfies the row on its own (activity −3051 ≤ −17.6), so x0 = 0 is feasible and optimal at objective 0. HiGHS agrees. Pre-fix, presolve fabricates x0 >= 35.2244 and the solver returns OPTIMAL at 144279.32 — and 4096 × 35.2244 = 144279.32 exactly.

The threshold is precisely the sentinel: x0 upper bound of 1e19 is correct, while 1e20, 1e21, 1e25 and inf are all wrong.

How it was found, and how the layer was identified

gear4 returned status=optimal objective=bound=184279.32477 gap_certified=True against a true optimum of 1.643428474 (this repo's own test_gear4_false_infeasible.py, and minlplib.solu).

The layer was pinned by a box-monotonicity check that needs no oracle: a valid relaxation bound is antitone in the box, so capping an infinite endpoint (which shrinks the box) cannot lower the bound. On gear4 the raw box gave 184279.32 and a box capped at 1e6 — a strict subset — gave 0.0. Enlarging the box raised the minimum, which is impossible.

That pointed at the relaxation, but the relaxation was exonerated: A, b, c, obj_offset and the objective floor are byte-identical between the two builds; only two column bounds differ. Handing the identical LP to scipy/HiGHS returned 0.0 while the in-house path returned 184279.32. Equilibration was also exonerated (scipy returns 0.0 on both the scaled and unscaled systems). Shrinking the equilibrated LP greedily while the discrepancy persisted reduced 44×16 to the 1×2 case above.

Fix

Infinity is now decided on the bounds (lo <= -INF / hi >= INF) in a shared contrib() helper used by both the activity-sum loop and the per-column loop.

Additionally, each derived bound is widened by an explicit rounding-error budget 8 * eps * max_abs_term, so that a legitimately large but finite bound swamping the smaller terms cannot reproduce the same false tightening by cancellation. For terms of order 1e3 that is ~2e-13 — no practical loss of propagation.

This is a general defect, not a gear4 quirk

In the differential fuzz added here, 11 of 174 random LPs with unbounded columns (6.3%) returned a claimed-OPTIMAL objective above the true minimum before this change; 0 of 174 after.

Box monotonicity over the in-repo corpus went from 1 violation in 17 comparisons to 0 in 18.

Corpus differential (79 instances, time_limit=20, oracle = minlplib.solu)

Both arms were run with the extension rebuilt and a marker assertion proving which build was loaded (144279.32 for unfixed, 0.0 for fixed) before either panel started.

main this PR
oracle crossings 0 0
gap_certified 62 62 — 0 regressions, 0 newly certified
objective drifts 0
bound drifts 0
status changes 0
node-count changes 11 (both directions: nvs14 223→129, ex1263 7455→10005)
total wall 356.4 s 350.7 s (−1.6%, noise)

Exactly the expected shape for a soundness fix: identical answers, a slightly different search once the invalid tightenings stop firing.

Honest blast radius. gear4 through the default solve() path is byte-identical on both arms (1.6434284641, 3 nodes, certified) and no in-repo instance crossed its oracle on either arm. Surfacing this required either the relaxation-level entry (build_milp_relaxation(...).solve()) or the mixed-scope LP engine. It also explains why the finite-root-box guard in lp_spatial_bb was load-bearing rather than conservative: it was masking this defect for the pure-integer engine, which is why removing it in #874 produced a certified optimal at 184279.32.

Tests

python/tests/test_lp_presolve_infinite_bound.py:

  • the shrunk LP across a bound ladder — inf, 1e20, 1e21, 1e25, plus sub-sentinel 1e19/1e15/1e8 as over-correction guards;
  • a HiGHS differential fuzz over LPs with sentinel/infinite bounds, with a comparison counter (assert compared >= 50) so a vacuous run fails instead of passing green.

Fails-before / passes-after verified by rebuilding the extension with the fix reverted: 5 failed / 3 passed before, 8 passed after.

Verification

check result
cargo test -p discopt-core 537 passed, 0 failed (+ 4 presolve-determinism, 1 doc-test)
pytest python/tests/ -m smoke 832 passed, 1 skipped, 2 xpassed
pytest -m slow test_adversarial_recent_fixes.py 10 passed
ruff check / ruff format --check clean
cargo fmt --check / cargo clippy clean

Contributes to #874's blocker (that PR's false certificate on gear4 is this bug, surfaced by widening the engine's scope).

The simplex's FBBT presolve decided whether a column's contribution to a row
was unbounded by testing the PRODUCT a_ij * x_j against the sentinel
INF = 1e20, rather than testing the bound itself. INF is a sentinel, not a
true infinity, so for |a_ij| < 1 an unbounded column contributes an ordinary
finite number -- -0.5 * 1e20 = -5e19 -- which fails a `<= -INF` test and was
booked as a FINITE activity.

Two things then went wrong at once:

1. the row's activity range was no longer recognised as unbounded, so the pass
   derived bounds it had no right to derive; and
2. the huge magnitude annihilated every smaller term in the running activity
   sum (ulp(5e19) = 8192), so the residual `sum_min_finite - ck_min` came back
   as exactly 0.0 instead of its true value.

The result is an INVALID tightening. Presolve is dimension-preserving with no
postsolve, so the mutilated box goes straight to the B&B tree and node LPs, and
the search then reports OPTIMAL on a box from which the optimum has been
removed -- a false dual bound, the class CLAUDE.md 1 treats as unconditionally
fatal. The module's own docstring promises the pass "never cuts a feasible (let
alone optimal) solution"; it did.

Fix: infinity is now decided on the bounds (`lo <= -INF` / `hi >= INF`) in a
shared `contrib()` helper used by both the activity-sum loop and the per-column
loop. Additionally each derived bound is widened by an explicit rounding-error
budget (8 * eps * max_abs_term) so that a legitimately large but FINITE bound
swamping the smaller terms cannot produce the same false tightening; that is
~2e-13 for terms of order 1e3, i.e. no practical loss of propagation.

Discovery path: `gear4` returned status=optimal objective=bound=184279.32477
with gap_certified=True against a true optimum of 1.643428474. Bisected to the
relaxation vs the LP layer by observing that the SAME LP data gave 0.0 from
scipy/HiGHS and 184279.32 from the in-house path, then shrunk to a 1-row,
2-column LP where presolve forces x0 >= 35.2244 on a column whose optimal value
is 0. 4096 * 35.2244 = 144279.32 exactly reproduces the reported objective.

Not gear4-specific. In the differential fuzz added here, 11 of 174 random LPs
with unbounded columns (6.3%) returned a claimed-OPTIMAL objective above the
true minimum before this change; 0 of 174 after. Box monotonicity over the
in-repo corpus (bound on a larger box must not exceed the bound on a subset)
went from 1 violation in 17 comparisons to 0 in 18.

This also explains why the finite-root-box guard in lp_spatial_bb was
load-bearing rather than conservative: it was masking this defect for the
pure-integer engine.

Tests: python/tests/test_lp_presolve_infinite_bound.py -- the shrunk LP across
a bound ladder (inf, 1e20, 1e21, 1e25 and the sub-sentinel 1e19/1e15/1e8 as
over-correction guards) plus a HiGHS differential fuzz with a comparison
counter so a vacuous run fails rather than passes. Verified fails-before /
passes-after by rebuilding the extension with the fix reverted: 5 failed, 3
passed before; 8 passed after.

Verified: cargo test -p discopt-core 537 passed; pytest -m smoke 832 passed,
1 skipped, 2 xpassed; pytest -m slow test_adversarial_recent_fixes.py 10
passed; ruff + cargo fmt + clippy clean.
@jkitchin
jkitchin merged commit c090deb into main Jul 26, 2026
11 checks passed
@jkitchin
jkitchin deleted the fix/lp-presolve-inf-sentinel branch July 26, 2026 19:36
@jkitchin

Copy link
Copy Markdown
Owner Author

Correction to this PR's corpus-differential table — the panel was partly vacuous

Two numbers in the merged description were wrong. The fix itself, the minimal reproducer, the fuzz result and the box-monotonicity result are unaffected; this is a correction to the panel evidence, and it is the exact failure mode the companion PR #876 added a rule against, committed hours earlier.

What went wrong. The panel harness read the dual bound as result.best_bound. SolveResult has no such attribute — it is result.bound. The read was getattr(r, k, None), so every row recorded bound=None, and the derived bound_crosses_oracle check was therefore False everywhere by construction. Consequently:

  • "oracle crossings: 0 / 0" was vacuous — it compared nothing;
  • "bound drifts: 0" was vacuous for the same reason, and is also wrong — there are 4.

Both arms have been re-run on the corrected harness, which now prints and asserts a fired-check counter (n_checked, exiting non-zero below 20) so a run that compares nothing fails instead of passing green. Each arm asserted its build via the minimal-LP probe first (144279.32477276 for pre-#877, 0.0 for this PR).

Corrected table (79 instances, time_limit=20, oracle minlplib.solu)

pre-#877 (7e78ec64) this PR
bound checks that fired 76 76
oracle crossings 0 0
gap_certified 62 62 — 0 regressions, 0 new
objective drifts 0
bound drifts 4 (was reported as 0)
status changes 0
node-count changes 11

The 4 bound drifts, all sound

instance oracle bound pre → post certified objective
ex1233 155010.6713 121555.490 → 120818.798 False → False unchanged
nvs05 5.470934108 3.805337 → 4.099238 False → False unchanged
nvs18 −778.4 −779.398 → −780.173 False → False unchanged
nvs20 230.9221652 227.518 → 226.097 False → False unchanged

All MINIMIZE; every bound on both arms is verified <= oracle. Three loosen marginally and nvs05 tightens — the expected signature of no longer applying invalid presolve tightenings. All four are already-uncertified instances, objectives are identical, and no instance changes certification state. So the conclusion — cert-clean, no certification regression — stands; the change is not bound-neutral, which the original table wrongly implied.

The independent evidence for the fix is untouched and was never routed through this harness: the 1-row/2-column reproducer where presolve fabricates x0 >= 35.2244 on a column whose optimum is 0; the HiGHS differential fuzz at 11/174 → 0/174; box monotonicity 1 violation in 17 → 0 in 18; cargo test 537, smoke 832, adversarial 10; and fails-before/passes-after confirmed by rebuilding with the fix reverted.

Follow-on verification requested in review, now done

Rebasing #874 onto this fix (10 commits, clean) and re-running gear4 with solve(lp_spatial=True), with both markers asserted (the issue #860 widening present and the presolve probe returning 0.0):

gear4 under #874
before this fix optimal, obj = bound = 184279.32, gap_certified=Truefalse certificate
after this fix time_limit, obj 17.514, bound None, gap_certified=False, 2673 nodes — sound

The false certificate is gone, confirming the diagnosis. Note for #874 separately: gear4 is still a quality regression there — main's default path certifies 1.6434284641 in 3 nodes, while the widened engine spends the full 25 s over 2673 nodes and returns an incumbent ~10x worse, uncertified. Soundness is restored; net-positive on that instance is not established.

jkitchin added a commit that referenced this pull request Jul 26, 2026
…site

tighten_nonlinear_bounds has four call sites. This PR gave three of them a
deadline -- the root declared-box pass, the periodic/domain pass, and AMP --
but _apply_nonlinear_tightening_with_status (solver.py) still called it as
tighten_nonlinear_bounds(model, lb, ub), so that site ran the pass to
completion on every invocation.

Measured on watercontamination0202 (106,711 vars / 107,209 rows) at
time_limit=30: that site cost ~23 s per call and fired 3x, ~70 s of a 126 s
cProfile run -- the single largest remaining overrun after the sparse-linearizer
fix, and the reason a deadline on tighten_nonlinear_bounds ALONE did not bound
the solve. After this change tighten_nonlinear_bounds disappears from the top of
the profile entirely.

End-to-end on the instance the issue is about:

  stage                                  T=30      T=60
  before #878                            579.3 s   620.8 s
  #878 as-is                              88.5 s   118.0 s
  #878 + this commit                      47.2 s    76.3 s

i.e. 12.3x and 8.1x better than the pre-#878 baseline. Both arms asserted the
loaded build first (the sparse-linearizer marker present, plus the #877 presolve
probe returning 0.0) so no number here is from the wrong tree.

The deadline used is the solve's ABSOLUTE model._solve_deadline, not a fresh
per-call fraction. A fraction recomputed per call is exactly how the convexity
classifier's budget used to multiply across model objects (this PR's own
finding), and this helper is called per node as well as at the root, so a
fraction here would compound the same way. A missing attribute yields None,
which preserves today's unbounded pass for an untimed solve rather than
collapsing it to a zero budget.

Soundness: unchanged in kind from this PR's existing argument. Every early exit
does strictly less tightening, which leaves the box looser, never wrong; the one
thing a budget can cost is an infeasibility proof, and stats.infeasible then
simply stays unset for B&B to rediscover.

Tests: python/tests/test_875_nbt_call_site_budget.py -- 5 tests pinning the
WIRING rather than wall-clock (so it cannot rot into a machine-speed
assertion): the deadline arrives at tighten_nonlinear_bounds, it equals the
absolute solve deadline, an absent deadline stays None, an expired budget
neither widens a bound nor manufactures an infeasibility, and a source-level
guard fails if any tighten_nonlinear_bounds call in solver.py omits deadline=.
Fails-before/passes-after verified by stashing the fix: 3 failed / 2 passed
before, 5 passed after.

Verified: pytest -m smoke -> 832 passed, 1 skipped, 2 xpassed; pytest -m slow
test_adversarial_recent_fixes.py -> 10 passed; ruff check + format clean; no
Rust touched. Run on this branch with main merged in (so #877 is included).

NOTE: the issue's acceptance criterion (~1.25xT) is still NOT met at 1.57x /
1.27x. The residual is now two constant-cost root phases that each individually
respect the absolute deadline but together start before it and finish after it:
build_uniform_relaxation ~18 s (bounding it is gated behind the default-off,
CLAUDE.md 5 bound-changing root_build_deadline flag) and _classify_model_convexity
~15 s. A per-phase poll cannot fix that -- it needs root-setup sub-budgets that
sum to T. Recorded on the issue.

Contributes to #875
jkitchin added a commit that referenced this pull request Jul 26, 2026
…_MIXED

Review outcome on #874. The widening is sound and stays; what changes is that it
no longer ships by DEFAULT, because it is not net-positive on the default path
(CLAUDE.md 5 bar 2).

The flag already governed the #844 fallback's 35% budget reserve, but the ENGINE
gate was unconditional, so solve(lp_spatial=True) -- a documented public kwarg --
picked up the widened scope with no opt-in. Measured on gear4, which is mixed (4
integer, 2 continuous with infinite upper bounds) and therefore admitted only
under the widening, at a 25 s budget:

  DISCOPT_LP_SPATIAL_MIXED=0 (default): optimal, 1.6434284641, certified,
                                       3 nodes, 1.1 s
  DISCOPT_LP_SPATIAL_MIXED=1:          time_limit, 17.514, UNcertified,
                                       2678 nodes, 25.0 s

The engine accepts a model the default path already certified in 3 nodes, then
spends the whole budget to return an incumbent ~10.7x worse with no certificate.
It is SOUND -- the bound never crosses the oracle, and the false certificate seen
on gear4 earlier in review was the LP-presolve INF-sentinel bug fixed in #877,
not this widening -- but shipping it by default trades a certificate for a worse
incumbent.

Changes:

* solver.py's lp_spatial=True entry point passes
  mixed=_lp_spatial_mixed_fallback_enabled(); the #844 fallback already did.
* solve_lp_spatial_bb(mixed=...) and _is_in_scope(mixed=...) now default to
  False, so a NEW call site inherits the pre-#860 gate rather than silently
  shipping the widening. Same rationale as row_scan_is_anytime defaulting to
  False in the tightening rules.
* The flag docstring now describes both entry points it governs and records the
  gear4 measurement alongside the existing Panel B table.
* Five tests in test_lp_spatial_bb.py exercise the widened CAPABILITY, so they
  now request it explicitly with mixed=True. The two tests that assert the
  pre-#860 gate declines (mixed=False) are unchanged and still pass.

Also in this branch: merged main and resolved two conflicts by COMPOSITION, not
by choosing a side (rationale recorded in comments at both sites).

* modeling/core.py -- main (#861 review) moved the fallback dual-bound merge out
  of the `_fb.objective is not None` block so a sound bound is reported even with
  no incumbent; this branch made the same merge sense-aware (min for MAXIMIZE,
  where the dual bound is an UPPER bound and max would keep the looser one).
  Kept main's placement AND this branch's sense-awareness.
* _jax/incremental_mccormick.py -- main relaxed _validate to compare shape[1]
  with vacuity-filtered row sets, bounds checked first, which is what lets a
  PINNED box validate; this branch added the obj_offset box-independence
  assertion. Kept main's structure AND this branch's assertion.

Tests: python/tests/test_860_mixed_gate_is_opt_in.py -- 6 tests pinning the
GATING rather than wall-clock, so they cannot rot into machine-speed assertions:
mixed=True admits mixed-minimize and integer-maximize while mixed=False declines
both (with pure-integer-minimize as a control that must be admitted either way),
both defaults are False, a source-level guard fails if either production call
site stops threading the flag, and the env-var helper is off when unset.
Fails-before/passes-after verified by stashing the three source files: 2 failed /
4 passed before, 6 passed after.

Verified: pytest -m smoke -> 844 passed, 1 skipped, 2 xpassed; pytest -m slow
test_adversarial_recent_fixes.py -> 10 passed; ruff check python/ clean; ruff
format clean; no Rust touched.

Also measured, and the reason this is not "Closes": at time_limit=60 with the
flag ON, all three instances #860 names are now IN SCOPE (_is_in_scope True,
where they previously failed the gate) but NONE returns an incumbent --
rsyn0805m04hfsg (MAXIMIZE) bound 9741.371 vs oracle 7174.219, gastrans040 bound
0.0, gastrans582_cold13 bound 0.0, objective None on all three, 6 soundness
checks and 0 violations. #860's stated gap is that these three return no
incumbent, so it remains open for the primal work.

Contributes to #860
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant