Error in HPresolve.cpp

[jajhall]

The line

bool hasRowLower =
model->row_lower_[row] != kHighsInf ||
implRowDualLower[row] > options->dual_feasibility_tolerance;

looks wrong. Surely it should be

bool hasRowLower =
model->row_lower_[row] != -kHighsInf ||
implRowDualLower[row] > options->dual_feasibility_tolerance;

Computing this as true_hasRowLower,

assert(true_hasRowLower == hasRowLower);

is triggered very frequently.

What's your view @fwesselm

Clearly the "correction" could be made and then tested, but it's worth understanding why it doesn't cause more problems.

[fwesselm]

I think the checks were introduced here to avoid making superfluous calls to HPresolve::updateColImpliedBounds.

However, the latter method also does input checking and, thus, the incorrect condition may not cause any severe issues.

I am not sure if this is a performance bottleneck, probably not. It may be worth removing the additional if-checks.

I can do some testing without the checks and see if this causes any regressions.

[jajhall]

You may well be right that the unnecessary call to updateColImpliedBounds that is made if the row lower bound is -inf simply returns without doing anything that would cause an error.

Since model->row_lower_[row] != kHighsInf is always true, the error won't lead to updateColImpliedBounds not being called when it should.

I guessed that it must "fail safe" in this case, otherwise it's such a blatant error that it would have caused a lot of failures.

Thanks @fwesselm

[jajhall]

I've implemented the fix in the fix-1711 branch. It would be good if @fwesselm could run the testing and see if this causes any regressions. I'd rather do this than just remove the checks

[fwesselm]

Testing shows that the proposed fix does not cause any regressions on MIPLIB2017, etc.