Restore per-coefficient bound substitutions before untransforming a path cut (#3171)

[EamonHetherton]

Addresses #3171.

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

The model

check/instances/3171-1.mps — 85 rows, 99 columns, 49 binaries, 300 nonzeros. It is solved
incorrectly under default options, with no option pinning: HiGHS reports Optimal at Gap 0%
with objective 42332.2356068 against an optimum of 42215.5250005, suboptimal by 116.71, a
relative error of 0.276% — about 28x the default relative gap tolerance, so not a tolerance
artefact. It is decided at the root, in zero nodes.

The optimum does not rest on trusting the branch and bound search, or on trusting a patched build:
fixing all 49 integer columns to the values of the better point and solving the remaining LP with
unmodified HiGHS returns Optimal 42215.5250005. So a feasible integral point 116.71 better
than the reported optimum demonstrably exists. --presolve off returns 42215.5250005 as well,
which means the contradiction is visible within a single unmodified binary.

The failure is seed-dependent — it occurs on seeds 0 and 5 of the first 8 — so the test pins
random_seed to 0 (the default) along with threads and parallel, to keep it off the scheduler.

The defect

HighsPathSeparator's path-mixing cut is assembled from several base rows, each transformed by its
own HighsTransformedLp::transform() call, and mapped back by a single untransform() at the end.
HighsTransformedLp keeps the chosen substitution per column in the shared boundTypes array, and
untransform() reads whatever is in there when it runs.

The transform loop transforms base row k before testing whether its right-hand side keeps the
required monotonicity, and drops the row only afterwards:

if (!transLp.transform(aggregatedPath[k].second, tmpUpper, tmpSolval,
                       aggregatedPath[k].first, rhs[k], integralPositive)) {
  pathLen = k;                 // row dropped - but boundTypes already mutated
  break;
}
...
} else if (rhs[k] >= rhs[k - 1] - mip.mipdata_->feastol) {
  pathLen = k;                 // row dropped - but boundTypes already mutated
  break;
}

The discarded row's mutation survives. Since the substitution chosen for a binary depends on the
sign of its coefficient in the row being transformed, a discarded row can flip a binary's
complementation, and untransform() then resubstitutes a different variable than the coefficients
were computed against. Nothing in the current code ties the two together.

The existing assertions in that loop cannot catch it: solval is lbDist[col] for both
kSimpleLb and kVariableLb and ubDist[col] for both kSimpleUb and kVariableUb, and upper
is ub - lb in every case, so neither value changes when the substitution flips.

The change

Record the substitution used for each index when its cut coefficient is computed, and restore those
substitutions immediately before calling untransform(). Also refuse to build the cut when two
base rows of the same path disagree about a shared column, since then no single resubstitution is
correct for all of the coefficients.

This needs BoundType made public on HighsTransformedLp plus a boundType() / setBoundType()
accessor pair; the separator then records transLp.boundType(index) alongside solval / upper
when an index is first seen, flags a disagreement on later occurrences, and restores the recorded
values before untransforming.

Evidence on a production model

Besides the attached reproducer, this fires on a real 10404 x 18899 MIP, where it costs 2978.48 on
the objective. That model still returns the wrong answer with the fixes for #3170 and #3173 both
applied, so the three defects are independent:

build	objective, presolve=on
latest	-12370583.0955	wrong
latest + #3170 + #3173	-12370583.0955	wrong
latest + #3170 + #3173 + this	-12373561.5756	correct

presolve=off returns the correct -12373561.5756 throughout. The node count rises from 103 to
376 with the fix, which is the search doing its job: without it an invalid cut was closing the tree
early.

Instrumenting that model gives the mismatch directly. Instrumenting the
separator to record the substitution used when each coefficient was computed, and compare it
against the one in force at untransform(), gives exactly one mismatched column:

col 14096   substitution at untransform() = kSimpleLb
            y used when the coefficient was computed = 1     (complemented, y = 1 - x)
            y implied by the substitution used        = 0     (y = x - 0)
            coefficient -1   ->   error exactly +1

Evaluating that cut in the transformed space at the known optimum gives violation 0; after
untransform() it gives violation 1. The arithmetic is otherwise sound — the entire error is the
one flipped complementation. With the change, that model returns the correct optimum.

I can supply the model, or the instrumentation patch that makes the mismatch observable in a single
solve, if either would help.

What it took to find the model, and why that matters for review

Delta-debugging the large model does not work: deleting a single row with zero LP dual, which
provably cannot change the root LP optimum, is already enough to destroy the trigger, because the
path the separator walks depends on row and column indices and on hash-table iteration order.

Generating models is what worked, but only after the search was aimed at the actual mechanism.
Roughly 100,000 unshaped random MIPs produced no trigger at all. Three conditions turn out to gate
it, all readable from the code:

• the binary must sit at exactly 0.5 in the LP relaxation — the sign-dependent branch in
  transform() is reached only when simpleLbDist and simpleUbDist tie, so any other LP value
  picks the bound type by distance and no two rows can disagree;
• the separator aggregates cumulatively with positive multipliers, so the binary's running
  coefficient has to change sign between the row that records it and the row that is discarded;
• the discarded row must still leave a path of length two or more, or no cut is produced at all.

Generating for those — odd cycles with equal rewards to pin binaries at 0.5, and alternating
near-cancelling coefficients so the running sum crosses zero — produced 5 substitution flips in
about 250,000 models, of which this is the one that also loses the optimum. The other four generate
the invalid cut without it cutting anything off, which is worth knowing: the defect fires more
often than it changes an answer.

That rarity is a caveat on the test, not on the defect. The test is one seed on one model, and the
conjunction it depends on is narrow, so it is a witness rather than broad coverage. The
inconsistentBoundTypes guard added here is what makes the general situation detectable rather
than silent.

Adverse effects

Measured on latest:

	result
the 33 MIP models in check/instances	identical status, objective, node count and LP iteration count
full unit test suite	1259307 assertions in 335 test cases, pass
the 10404 x 18899 model above	corrected
the attached reproducer	corrected

The change can only suppress a cut, never produce a different one, so the risk is losing cuts that
were previously generated and valid. On the models above none is lost. I have not benchmarked
against MIPLIB or anything at production scale, so I cannot rule out a cut-quality cost where paths
of length two or more are common.

What this does not close

bestVub / bestVlb are shared mutable state as well, and transform() may tighten them in place
via cleanupVub() / cleanupVlb(), so a coefficient computed from an earlier base row can be
untransformed against a tightened bound. This restores the bound type but not the bound values.
I have no reproducer for that case and did not want to widen the change on speculation.

Both are the same design issue: the cut is untransformed by reading state back out of a shared,
still-mutating HighsTransformedLp, rather than carrying the transformation it was built with. If
you would prefer a structural change along those lines, that would close this, the bestVub case,
and #3170's class as well, and this PR is probably not the right starting point for it.

Testing

• the new issue-3171 test fails on latest at the first commit and passes at the second;
• the attached model returns the brute-force-independent optimum under every seed and both
  presolve settings once fixed;
• the numbers in the table above.

🤖 Generated with Claude Code

[Opt-Mucca]

Please see #3179 for fixing this issue. Interested to see if the AI agrees with the fix