Fix #3170: discard a redundant variable bound instead of substituting it as if tight

[EamonHetherton]

Fixes #3170.

The first commit adds the two reproducers as a failing regression test, on its own, so the defect
is visible without any fix applied. The second commit contains the fix.

The models

• check/instances/3170-1.mps — 39x30, 10 binaries. Solved incorrectly with presolve=off alone:
  reports -21262719.4302 against an optimum of -21446579.364727, a relative error of 0.86%,
  about 86x the default relative gap tolerance, so not a tolerance artefact.
• check/instances/3170-2.mps — 40x24, 8 binaries. Needs the tighter option set used in the test;
  reports -516096644.644 against an optimum of -516307714.782682.

Both optima were verified independently of the MIP search, by enumerating every binary assignment
and solving the resulting LP for each.

The defect

HighsTransformedLp::transform() substitutes a variable upper bound x <= a*z + b by the slack
y = a*z + b - x, and passes upper[j] = ub - lb to the cut generator as y's upper bound. That
is sound only if the variable bound is at least as tight as the simple bound, i.e.
maxValue() <= ub, so the code calls cleanupVub() to re-establish that when it does not hold.

cleanupVub() only tightens a variable bound that is not redundant. When
minValue() >= ub - feastol it reports redundant = true and deliberately leaves the bound
unchanged, for the caller to discard — which is what cleanupVarbounds() does. The caller here
ignored that flag and substituted the bound anyway, so the slack could range far above the
ub - lb it was declared to have, and the cut derived from it was not globally valid. Such a cut
can remove the optimum, after which the search closes and reports the incumbent as Optimal at a
zero gap.

cleanupVlb() has the identical contract and the identical caller bug.

The fix

Discard the variable bound when cleanup reports it redundant, and defensively whenever the
assumption still fails afterwards, falling back to the simple bound. Nothing is lost, since a
redundant variable bound is by definition no stronger than the simple bound.

      if (redundant ||
          bestVub[col].second.maxValue() > ub + mip.mipdata_->feastol) {
        bestVub[col].first = -1;
        ubDist[col] = simpleUbDist[col];
        boundDist[col] = std::min(lbDist[col], ubDist[col]);
      }

and symmetrically for the variable lower bound.

Two things to be aware of when reviewing

Only the variable upper bound path is exercised by the tests. The variable lower bound half is
fixed by symmetry, since cleanupVlb() has the same contract and the same caller bug, but neither
attached model reaches it.

The second disjunct is belt-and-braces. After a successful cleanupVub() the invariant holds,
so || maxValue() > ub + feastol should not fire; the redundant flag alone is what these models
hit. Happy to drop it if you would rather the change were minimal.

Adverse effects

The concern with this change is that it discards a variable bound that would otherwise have been
used for cut generation, which could weaken cuts. Measured on latest:

	result
35 MIP models in check/instances	identical status, objective, node count and LP iteration count — every one
400 generated MIPs	identical objectives, none differing
9 models attached to open correctness issues	identical
full unit test suite	1259332 assertions in 335 test cases, pass

So there is no collateral change: no weakened cuts, no extra nodes, no extra iterations anywhere
the defect is not present. The fix appears to be inert unless the defect actually fires. That is
not a proof that it never activates — those 400 models come from the same generator that produces
triggers for this defect at roughly 1 in 500, so most likely none of them hit it.

One thing that might look like a regression but is not: on the larger model this was first found
on, the node count rises with the fix (7 to 18). That is the search doing its job — without the fix
an invalid cut was closing the tree early.

Testing

• the new issue-3170 test fails on latest at the first commit and passes at the second;
• both attached models return the brute-force optimum under every option set tried;
• the numbers in the table above.

[jajhall]

Thanks for this. We don't accept AI-generated code, but I expect that @Opt-Mucca and @fwesselm will identify what needs to be changed and fix it themselves.

[EamonHetherton]

@jajhall ok, I have two other issues I'm working through for a minimal reproduction, one is already logged as an issue with more detail to come. Essentially I am verifying a larger model across hundreds of thousands of solves with different inputs and occasionally HiGHS was returning a noticeably sub-optimal solution and found three discrete issues causing them. Would you rather I just report the issue as best I can or open a pull request with a demonstrated fix (albeit with AI assistance) for you to cherry pick from?

[jajhall]

If you can generate a fix, then it it certainly helps. Our developers may well implement it as it is, but any adverse effects need to be considered

[EamonHetherton]

Thanks, I'll continue as is then; log the issue with as much detail as I can gather and open a PR with a failing test followed by a proposed fix. Cherry pick as you like for what goes upstream. (and i'll rebase to latest too)