Remove continuous singletons from double-sided rows

[Opt-Mucca]

General idea: We currently have no explicit reduction for a column s, with 0 cost, and that only appears in a single double-sided row b_0 <= a^Tx + cs <= b_1. Dual fixing removes all cases where b_0 == -kHighsInf or b_1 == kHighsInf, but the column is left alone if b_0 and b_1 are finite. Removing the column may allow additional reductions to be made.

Motivation: I was playing around with an instance, trying to figure out why HiGHS wasn't making a reduction, and thought this would help (spoiler: it did not), but while doing so I stumbled upon an old TODO in the code with the exact suggestion.

@fwesselm Three major questions:

• Do you think this is worth adding? I haven't performance tested this.
• If so should this be kept behind a MIP-only flag?
• I'm currently skipping the case where s has infinite bounds, but in theory we could handle that here. The issue is I'm not sure what to do with infinite values in postsolve.

Edit: Need to look into the failing tests (they're passing locally.....). Windows does not like this change at all.

[codecov]

Codecov Report

❌ Patch coverage is 91.61290% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.18%. Comparing base (819a538) to head (b13679a).
⚠️ Report is 25 commits behind head on latest.

Files with missing lines	Patch %	Lines
highs/presolve/HighsPostsolveStack.cpp	88.13%	7 Missing ⚠️
highs/lp_data/HighsModelUtils.cpp	60.00%	6 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           latest    #2962      +/-   ##
==========================================
- Coverage   73.22%   73.18%   -0.04%     
==========================================
  Files         445      445              
  Lines      107826   108030     +204     
  Branches    17262    17313      +51     
==========================================
+ Hits        78951    79059     +108     
- Misses      28599    28695      +96     
  Partials      276      276              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[fwesselm]

This is great, @Opt-Mucca! I would like to experiment with this a llittle more. Also, there is HPresolve::removeSlacks which handles a "similar" case. If you are OK with it, I would make some more changes (on top of your change).

[Opt-Mucca]

@fwesselm I was worried that I was missing something, so please make any changes you want.

[jajhall]

@Opt-Mucca I've merge the fix for HighsPostsolveStack::ZeroObjSingletonContinuousCol::undo into this branch, and merged in latest. There's some checking code that prints diagnostic information in the event of errors that assert with debug, and this will disappear when some testing on larger MIPs has been done, and the maths has been looked at properly. I'd like to say that I'll have time to think about the latter in the next few days, but I won't.

TEST_CASE("issue-2962", is the example that previously yielded iterations after postsolve due to one of the ZeroObjSingletonContinuousCol being made basic in postsolve, leading to an incorrect number of basic variables.

[jajhall]

This is now draft until the maths has been checked and the error-checking code has been removed

[jajhall]

HPresolve::redundantSingletonColDoubleSidedSlack is now renamed HPresolve::zeroCostSingleton, and handles zero cost singletons with any bounds. It's also applied to equations, which exposed a serious error in HPresolve::dualFixing. This error has been fixed.

zeroCostSingleton and dualFixing are also logged, and can be switched off.

[jajhall]

@fwesselm, this now looks correct for LPs.

My fix-2962 branch fixed an error that caused pds-20 to have 3 iterations after basis postsolve. I've tested on Netlib, Kennington and relaxations of miplib2017 problems and no unexpected iteraitons after postsolve. Interestingly, Netlib problems fit1p and fit2p, and the relaxation of Miplib problem pg are now reduced in presolve, whereas previously they were unreduced.

This is good to merge into latest, particularly as if fixes a bug in basis postsolve

[fwesselm]

Thanks, @jajhall, just a few comments from me.

[fwesselm]

The number of down-locks and up-locks includes the objective coefficient, so numDownLocks == 0 implies that model->col_cost_[col] >= 0. Similarly, numUpLocks == 0 implies model->col_cost_[col] <= 0.

[jajhall]

If I recall correctly, it was correcting an edge case of your logic that prompted my modification. If the cost is of the right sign but sufficiently small, then it doesn't force the column to its bound.

That said, I may have a sign wrong

I'll re-investigate and get back to you

[fwesselm]

I also had another look and I think the following would work and handle zero cost correctly (slight modification of the code from my original PR):

while (true) {
   if (numDownLocks == 0 &&
       (model->col_cost_[col] > 0 || model->col_lower_[col] != -kHighsInf)) {
     HPRESOLVE_CHECKED_CALL(fixColToLower(postsolve_stack, col));
     break;
   }
   if (numUpLocks == 0 &&
       (model->col_cost_[col] < 0 || model->col_upper_[col] != kHighsInf)) {
     HPRESOLVE_CHECKED_CALL(fixColToUpper(postsolve_stack, col));
   }
   break;
 }

[jajhall]

I also had another look and I think the following would work and handle zero cost correctly (slight modification of the code from my original PR):

while (true) {
   if (numDownLocks == 0 &&
       (model->col_cost_[col] > 0 || model->col_lower_[col] != -kHighsInf)) {
     HPRESOLVE_CHECKED_CALL(fixColToLower(postsolve_stack, col));
     break;
   }
   if (numUpLocks == 0 &&
       (model->col_cost_[col] < 0 || model->col_upper_[col] != kHighsInf)) {
     HPRESOLVE_CHECKED_CALL(fixColToUpper(postsolve_stack, col));
   }
   break;
 }

I appreciate the desire to fix variables, but if |cost| <= dual_feasibility_tolerance then the variable is dual feasible at any value, so there's no forcing fix. If a small cost is zero with exact arithmetic, but nonzero due to rounding error then, with your logic, there's a chance of fixing a variable to an infinite bound in error.

I'd rather be cautious and use

while (true) {
if (numDownLocks == 0 &&
(model->col_cost_[col] > options->dual_feasibility_tolerance || model->col_lower_[col] != -kHighsInf)) {
HPRESOLVE_CHECKED_CALL(fixColToLower(postsolve_stack, col));
break;
}
if (numUpLocks == 0 &&
(model->col_cost_[col] < -options->dual_feasibility_tolerance || model->col_upper_[col] != kHighsInf)) {
HPRESOLVE_CHECKED_CALL(fixColToUpper(postsolve_stack, col));
}
break;
}

I think that very few fixing opportunities will be missed, as original costs are likely to be 0 or greater than dual_feasibility_tolerance, and numerical cancelling of a cost is typically exact.

[fwesselm]

@jajhall, my suggestion was not using a tolerance, thank you for adding this!