Removed removeSlacks presolve reduction as, without costs, it is a duplication of zeroCostSingleton reduction

Author: Julian Hall

check/TestPresolve.cpp

@@ -638,46 +638,6 @@ TEST_CASE("write-presolved-model", "[highs_test_presolve]") {
   highs1.resetGlobalScheduler(true);
 }
 
-TEST_CASE("presolve-slacks", "[highs_test_presolve]") {
-  // This LP reduces to empty, because the equation is a doubleton
-  HighsLp lp;
-  lp.num_col_ = 2;
-  lp.num_row_ = 1;
-  lp.col_cost_ = {1, 0};
-  lp.col_lower_ = {0, 0};
-  lp.col_upper_ = {kHighsInf, kHighsInf};
-  lp.row_lower_ = {1};
-  lp.row_upper_ = {1};
-  lp.a_matrix_.start_ = {0, 1, 2};
-  lp.a_matrix_.index_ = {0, 0};
-  lp.a_matrix_.value_ = {1, 1};
-  Highs h;
-  h.setOptionValue("output_flag", dev_run);
-  REQUIRE(h.passModel(lp) == HighsStatus::kOk);
-  REQUIRE(h.presolve() == HighsStatus::kOk);
-  REQUIRE(h.getPresolvedLp().num_col_ == 0);
-  REQUIRE(h.getPresolvedLp().num_row_ == 0);
-
-  lp.num_col_ = 4;
-  lp.num_row_ = 2;
-  lp.col_cost_ = {-10, -25, 0, 0};
-  lp.col_lower_ = {0, 0, 0, 0};
-  lp.col_upper_ = {kHighsInf, kHighsInf, kHighsInf, kHighsInf};
-  lp.row_lower_ = {80, 120};
-  lp.row_upper_ = {80, 120};
-  lp.a_matrix_.start_ = {0, 2, 4, 5, 6};
-  lp.a_matrix_.index_ = {0, 1, 0, 1, 0, 1};
-  lp.a_matrix_.value_ = {1, 1, 2, 4, 1, 1};
-  REQUIRE(h.setOptionValue("presolve_remove_slacks", true) == HighsStatus::kOk);
-  REQUIRE(h.passModel(lp) == HighsStatus::kOk);
-  REQUIRE(h.run() == HighsStatus::kOk);
-  REQUIRE(h.presolve() == HighsStatus::kOk);
-  REQUIRE(h.getPresolvedLp().num_col_ == 2);
-  REQUIRE(h.getPresolvedLp().num_row_ == 2);
-
-  h.resetGlobalScheduler(true);
-}
-
 TEST_CASE("presolve-issue-2095", "[highs_test_presolve]") {
   std::string model_file =
       std::string(HIGHS_DIR) + "/check/instances/issue-2095.mps";

highs/lp_data/HighsOptions.h

@@ -1691,9 +1691,10 @@ class HighsOptions : public HighsOptionsStruct {
         advanced, &presolve_rule_logging, false);
     records.push_back(record_bool);
 
-    record_bool = new OptionRecordBool("presolve_remove_slacks",
-                                       "Remove slacks after presolve", advanced,
-                                       &presolve_remove_slacks, false);
+    record_bool =
+        new OptionRecordBool("presolve_remove_slacks",
+                             "Remove slacks after presolve: redundant option!",
+                             advanced, &presolve_remove_slacks, false);
     records.push_back(record_bool);
 
     record_bool =

highs/presolve/HPresolve.cpp

@@ -6547,13 +6547,7 @@ HPresolve::Result HPresolve::presolve(HighsPostsolveStack& postsolve_stack) {
       break;
     }
 
-    if (!reducedToEmpty()) {
-      // Now consider removing slacks
-      if (options->presolve_remove_slacks)
-        HPRESOLVE_CHECKED_CALL(removeSlacks(postsolve_stack));
-
-      report();
-    }
+    if (!reducedToEmpty()) report();
   } else {
     highsLogUser(options->log_options, HighsLogType::kInfo,
                  "\nPresolve is switched off\n");
@@ -6562,49 +6556,6 @@ HPresolve::Result HPresolve::presolve(HighsPostsolveStack& postsolve_stack) {
   return presolveReturn();
 }
 
-HPresolve::Result HPresolve::removeSlacks(
-    HighsPostsolveStack& postsolve_stack) {
-  // SingletonColumns data structure appears not to be retained
-  // throughout presolve
-  for (HighsInt iCol = 0; iCol != model->num_col_; ++iCol) {
-    if (colDeleted[iCol]) continue;
-    if (colsize[iCol] != 1) continue;
-    // Only do this for pure slacks as cost coefficient changes lead
-    // to dual postsolve errors since the basic costs may well change,
-    // leading to changes in the row duals that cannot be determined
-    if (model->col_cost_[iCol]) continue;
-    if (model->integrality_[iCol] == HighsVarType::kInteger) continue;
-    HighsInt coliter = colhead[iCol];
-    HighsInt iRow = Arow[coliter];
-    assert(Acol[coliter] == iCol);
-    assert(!rowDeleted[iRow]);
-    if (!isEquation(iRow)) continue;
-    double lower = model->col_lower_[iCol];
-    double upper = model->col_upper_[iCol];
-    double rhs = model->row_lower_[iRow];
-    double coeff = Avalue[coliter];
-    assert(coeff);
-    // Slack is s = (rhs - a^Tx)/coeff
-    //
-    // Constraint bounds become:
-    //
-    // For coeff > 0 [rhs - coeff * upper, rhs - coeff * lower]
-    //
-    // For coeff < 0 [rhs - coeff * lower, rhs - coeff * upper]
-    model->row_lower_[iRow] =
-        coeff > 0 ? rhs - coeff * upper : rhs - coeff * lower;
-    model->row_upper_[iRow] =
-        coeff > 0 ? rhs - coeff * lower : rhs - coeff * upper;
-    //
-    postsolve_stack.slackColSubstitution(iRow, iCol, rhs, getRowVector(iRow));
-
-    markColDeleted(iCol);
-
-    unlink(coliter);
-  }
-  return Result::kOk;
-}
-
 HPresolve::Result HPresolve::checkTimeLimit() {
   assert(timer);
   if (options->time_limit < kHighsInf && timer->read() >= options->time_limit)

highs/presolve/HPresolve.h

@@ -352,8 +352,6 @@ class HPresolve {
 
   Result presolve(HighsPostsolveStack& postsolve_stack);
 
-  Result removeSlacks(HighsPostsolveStack& postsolve_stack);
-
   Result checkTimeLimit();
 
   Result checkLimits(HighsPostsolveStack& postsolve_stack);

highs/presolve/HPresolveAnalysis.cpp

@@ -32,11 +32,8 @@ void HPresolveAnalysis::setup(const HighsLp* model_,
     presolve_clocks_ = clock;
   }
 
-  // Allow logging if option is set and presolve_remove_slacks option
-  // is not set (since it's not and official pesolve rule with an
-  // entry in PresolveRuleType) and LP presolve is being used
-  allow_logging_ = options_->presolve_rule_logging &&
-                   !options->presolve_remove_slacks && lp_presolve;
+  // Allow logging if option is set and LP presolve is being used
+  allow_logging_ = options_->presolve_rule_logging && lp_presolve;
   // NB logging_on_ is also used to determine whether logging has
   // started to prevent double-accounting. Specifically,
   //

highs/presolve/HighsPostsolveStack.cpp

@@ -1294,64 +1294,6 @@ void HighsPostsolveStack::DuplicateColumn::transformToPresolvedSpace(
   primalSol[col] = primalSol[col] + colScale * primalSol[duplicateCol];
 }
 
-void HighsPostsolveStack::SlackColSubstitution::undo(
-    const HighsOptions& options, const std::vector<Nonzero>& rowValues,
-    HighsSolution& solution, HighsBasis& basis) {
-  bool debug_print = false;
-
-  // compute primal values
-  double colCoef = 0;
-  HighsCDouble rowValue = 0;
-  for (const auto& rowVal : rowValues) {
-    if (rowVal.index == col)
-      colCoef = rowVal.value;
-    else
-      rowValue += rowVal.value * solution.col_value[rowVal.index];
-  }
-
-  assert(colCoef != 0);
-  // Row values aren't fully postsolved, so why do this?
-  solution.row_value[row] =
-      static_cast<double>(rowValue + colCoef * solution.col_value[col]);
-
-  solution.col_value[col] = static_cast<double>((rhs - rowValue) / colCoef);
-
-  // If no dual values requested, return here
-  if (!solution.dual_valid) return;
-
-  // Row retains its dual value, and column has this dual value
-  solution.col_dual[col] = -solution.row_dual[row] * colCoef;
-
-  // Set basis status if necessary
-  if (!basis.valid) return;
-
-  // If row is basic, then slack is basic, otherwise row retains its status
-  HighsBasisStatus save_row_basis_status = basis.row_status[row];
-  if (basis.row_status[row] == HighsBasisStatus::kBasic) {
-    basis.col_status[col] = HighsBasisStatus::kBasic;
-    basis.row_status[row] =
-        computeRowStatus(solution.row_dual[row], RowType::kEq);
-  } else if (basis.row_status[row] == HighsBasisStatus::kLower) {
-    basis.col_status[col] =
-        colCoef > 0 ? HighsBasisStatus::kUpper : HighsBasisStatus::kLower;
-  } else {
-    basis.col_status[col] =
-        colCoef > 0 ? HighsBasisStatus::kLower : HighsBasisStatus::kUpper;
-  }
-  if (debug_print)
-    printf(
-        "HighsPostsolveStack::SlackColSubstitution::undo OgRowStatus = %s; "
-        "RowStatus = %s; ColStatus = %s\n",
-        utilBasisStatusToString(save_row_basis_status).full_.c_str(),
-        utilBasisStatusToString(basis.row_status[row]).full_.c_str(),
-        utilBasisStatusToString(basis.col_status[col]).full_.c_str());
-  if (basis.col_status[col] == HighsBasisStatus::kLower) {
-    assert(solution.col_dual[col] > -options.dual_feasibility_tolerance);
-  } else if (basis.col_status[col] == HighsBasisStatus::kUpper) {
-    assert(solution.col_dual[col] < options.dual_feasibility_tolerance);
-  }
-}
-
 void HighsPostsolveStack::FourierMotzkinObjCol::transformToPresolvedSpace(
     const std::vector<Nonzero>& costEntries,
     std::vector<double>& primalSol) const {

highs/presolve/HighsPostsolveStack.h

@@ -283,16 +283,6 @@ class HighsPostsolveStack {
     void transformToPresolvedSpace(std::vector<double>& primalSol) const;
   };
 
-  struct SlackColSubstitution {
-    double rhs;
-    HighsInt row;
-    HighsInt col;
-
-    void undo(const HighsOptions& options,
-              const std::vector<Nonzero>& rowValues, HighsSolution& solution,
-              HighsBasis& basis);
-  };
-
   struct ZeroObjSingletonContinuousCol {
     double origRowLower;
     double origRowUpper;
@@ -325,7 +315,6 @@ class HighsPostsolveStack {
     kForcingColumnRemovedRow,
     kDuplicateRow,
     kDuplicateColumn,
-    kSlackColSubstitution,
     kZeroObjSingletonContinuousCol,
     kFourierMotzkinBlock,
     kFourierMotzkinObjCol,
@@ -412,9 +401,6 @@ class HighsPostsolveStack {
       case ReductionType::kDuplicateColumn: {
         return "Duplicate column";
       }
-      case ReductionType::kSlackColSubstitution: {
-        return "Slack col substitution";
-      }
       case ReductionType::kImpliedEquation: {
         return "Implied equation";
       }
@@ -607,19 +593,6 @@ class HighsPostsolveStack {
     reductionAdded(ReductionType::kFreeColSubstitution);
   }
 
-  template <typename RowStorageFormat>
-  void slackColSubstitution(HighsInt row, HighsInt col, double rhs,
-                            const HighsMatrixSlice<RowStorageFormat>& rowVec) {
-    rowValues.clear();
-    for (const HighsSliceNonzero& rowVal : rowVec)
-      rowValues.emplace_back(origColIndex[rowVal.index()], rowVal.value());
-
-    reductionValues.push(
-        SlackColSubstitution{rhs, origRowIndex[row], origColIndex[col]});
-    reductionValues.push(rowValues);
-    reductionAdded(ReductionType::kSlackColSubstitution);
-  }
-
   template <typename RowStorageFormat>
   void zeroCostSingleton(HighsInt row, HighsInt col, double origRowLower,
                          double origRowUpper, double new_row_lb,
@@ -1284,13 +1257,6 @@ class HighsPostsolveStack {
           reduction.undo(options, solution, basis);
           break;
         }
-        case ReductionType::kSlackColSubstitution: {
-          SlackColSubstitution reduction;
-          reductionValues_.pop(rowValues_);
-          reductionValues_.pop(reduction);
-          reduction.undo(options, rowValues_, solution, basis);
-          break;
-        }
         case ReductionType::kZeroObjSingletonContinuousCol: {
           ZeroObjSingletonContinuousCol reduction;
           reductionValues.pop(rowValues);