Added checks to identify cause of segfault exposed in #2748

cmake/sources-python.cmake

@@ -199,6 +199,7 @@ set(highs_sources_python
     highs/lp_data/HighsModelUtils.cpp
     highs/lp_data/HighsOptions.cpp
     highs/lp_data/HighsRanging.cpp
+    highs/lp_data/HighsReformulation.cpp
     highs/lp_data/HighsSolution.cpp
     highs/lp_data/HighsSolutionDebug.cpp
     highs/lp_data/HighsSolve.cpp

cmake/sources.cmake

@@ -352,6 +352,7 @@ set(highs_sources
     lp_data/HighsModelUtils.cpp
     lp_data/HighsOptions.cpp
     lp_data/HighsRanging.cpp
+    lp_data/HighsReformulation.cpp
     lp_data/HighsSolution.cpp
     lp_data/HighsSolutionDebug.cpp
     lp_data/HighsSolve.cpp

highs/Highs.h

@@ -1771,8 +1771,6 @@ class Highs {
   HighsStatus lpKktCheck(const HighsLp& lp, const std::string& message = "");
   HighsStatus invertRequirementError(std::string method_name) const;
 
-  HighsStatus handleInfCost();
-  void restoreInfCost(HighsStatus& return_status);
   HighsStatus optionChangeAction();
 
   HighsStatus userScale(HighsUserScaleData& data);
@@ -1800,6 +1798,13 @@ class Highs {
 
   bool tryPdlpCleanup(HighsInt& pdlp_cleanup_iteration_limit,
                       const HighsInfo& presolved_lp_info) const;
+
+  // Reformulation methods
+  HighsStatus doReformulation();
+  void undoReformulation(HighsStatus& optimize_status);
+
+  HighsStatus handleInfiniteCost();
+  void restoreInfiniteCost(HighsStatus& optimize_status);
 };
 
 // Start of deprecated methods not in the Highs class

highs/lp_data/HStruct.h

@@ -119,6 +119,8 @@ struct HighsLpMods {
   std::vector<double> save_inf_cost_variable_upper;
 
   void clear();
+  void clearInfiniteCostRecord();
+  void clearSemiVariableRecord();
   bool isClear();
 };
 

highs/lp_data/Highs.cpp

@@ -977,9 +977,16 @@ HighsStatus Highs::run() {
 HighsStatus Highs::optimizeHighs() {
   // Level 1 of Highs::run()
   //
-  // Move the "mods" to here
-  return this->multi_linear_objective_.size() ? this->multiobjectiveSolve()
-                                              : this->optimizeModel();
+  HighsStatus optimize_status = doReformulation();
+  if (optimize_status == HighsStatus::kError) return optimize_status;
+
+  optimize_status = this->multi_linear_objective_.size()
+                        ? this->multiobjectiveSolve()
+                        : this->optimizeModel();
+
+  undoReformulation(optimize_status);
+
+  return optimize_status;
 }
 
 HighsStatus Highs::optimizeLp() {
@@ -1088,26 +1095,7 @@ HighsStatus Highs::calledOptimizeModel() {
   // Set undo_mods = false so that returnFromOptimizeModel() doesn't undo any
   // mods that must be preserved - such as when solving a MIP node
   bool undo_mods = false;
-  if (model_.lp_.has_infinite_cost_) {
-    // If the model has infinite costs, then try to remove them. The
-    // return_status indicates the success of this operation and, if
-    // it's unsuccessful, the model will not have been modified and
-    // optimizeModel() can simply return an error with model status
-    // HighsModelStatus::kUnknown
-    assert(model_.lp_.hasInfiniteCost(options_.infinite_cost));
-    HighsStatus return_status = handleInfCost();
-    if (return_status != HighsStatus::kOk) {
-      assert(return_status == HighsStatus::kError);
-      setHighsModelStatusAndClearSolutionAndBasis(HighsModelStatus::kUnknown);
-      return return_status;
-    }
-    // Modifications have been performed, so must be undone before
-    // this call to optimizeModel() returns
-    assert(!model_.lp_.has_infinite_cost_);
-    undo_mods = true;
-  } else {
-    assert(!model_.lp_.hasInfiniteCost(options_.infinite_cost));
-  }
+  assert(!model_.lp_.has_infinite_cost_);
 
   // Ensure that all vectors in the model have exactly the right size
   exactResizeModel();
@@ -4727,12 +4715,8 @@ HighsStatus Highs::returnFromOptimizeModel(const HighsStatus run_return_status,
   called_return_from_optimize_model = true;
 
   if (undo_mods) {
-    // Restore any infinite costs
-    this->restoreInfCost(return_status);
-
     // Unapply any modifications that have not yet been unapplied
     this->model_.lp_.unapplyMods();
-    //    undo_mods = false;
   }
 
   // Unless solved as a MIP, report on the solution

highs/lp_data/HighsInterface.cpp

@@ -3069,139 +3069,12 @@ HighsStatus Highs::invertRequirementError(std::string method_name) const {
   return HighsStatus::kError;
 }
 
-HighsStatus Highs::handleInfCost() {
-  HighsLp& lp = this->model_.lp_;
-  if (!lp.has_infinite_cost_) return HighsStatus::kOk;
-  HighsLpMods& mods = lp.mods_;
-  double inf_cost = this->options_.infinite_cost;
-  for (HighsInt k = 0; k < 2; k++) {
-    // Pass twice: first checking that infinite costs can be handled,
-    // then handling them, so that model is unmodified if infinite
-    // costs cannot be handled
-    for (HighsInt iCol = 0; iCol < lp.num_col_; iCol++) {
-      double cost = lp.col_cost_[iCol];
-      if (cost > -inf_cost && cost < inf_cost) continue;
-      double lower = lp.col_lower_[iCol];
-      double upper = lp.col_upper_[iCol];
-      if (lp.isMip()) {
-        if (lp.integrality_[iCol] == HighsVarType::kInteger) {
-          lower = std::ceil(lower);
-          upper = std::floor(upper);
-        }
-      }
-      if (cost <= -inf_cost) {
-        if (lp.sense_ == ObjSense::kMinimize) {
-          // Minimizing with -inf cost so try to fix at upper bound
-          if (upper < kHighsInf) {
-            if (k) lp.col_lower_[iCol] = upper;
-          } else {
-            highsLogUser(options_.log_options, HighsLogType::kError,
-                         "Cannot minimize with a cost on variable %d of %g and "
-                         "upper bound of %g\n",
-                         int(iCol), cost, upper);
-            return HighsStatus::kError;
-          }
-        } else {
-          // Maximizing with -inf cost so try to fix at lower bound
-          if (lower > -kHighsInf) {
-            if (k) lp.col_upper_[iCol] = lower;
-          } else {
-            highsLogUser(options_.log_options, HighsLogType::kError,
-                         "Cannot maximize with a cost on variable %d of %g and "
-                         "lower bound of %g\n",
-                         int(iCol), cost, lower);
-            return HighsStatus::kError;
-          }
-        }
-      } else {
-        if (lp.sense_ == ObjSense::kMinimize) {
-          // Minimizing with inf cost so try to fix at lower bound
-          if (lower > -kHighsInf) {
-            if (k) lp.col_upper_[iCol] = lower;
-          } else {
-            highsLogUser(options_.log_options, HighsLogType::kError,
-                         "Cannot minimize with a cost on variable %d of %g and "
-                         "lower bound of %g\n",
-                         int(iCol), cost, lower);
-            return HighsStatus::kError;
-          }
-        } else {
-          // Maximizing with inf cost so try to fix at upper bound
-          if (upper < kHighsInf) {
-            if (k) lp.col_lower_[iCol] = upper;
-          } else {
-            highsLogUser(options_.log_options, HighsLogType::kError,
-                         "Cannot maximize with a cost on variable %d of %g and "
-                         "upper bound of %g\n",
-                         int(iCol), cost, upper);
-            return HighsStatus::kError;
-          }
-        }
-      }
-      if (k) {
-        mods.save_inf_cost_variable_index.push_back(iCol);
-        mods.save_inf_cost_variable_cost.push_back(cost);
-        mods.save_inf_cost_variable_lower.push_back(lower);
-        mods.save_inf_cost_variable_upper.push_back(upper);
-        lp.col_cost_[iCol] = 0;
-      }
-    }
-  }
-  // Infinite costs have been removed, but their presence in the
-  // original model is known from mods.save_inf_cost_variable_*, so
-  // set lp.has_infinite_cost_ to be false to avoid assert when run()
-  // is called using copy of model in MIP solver (See #1446)
-  lp.has_infinite_cost_ = false;
-
-  return HighsStatus::kOk;
-}
-
 HighsStatus Highs::optionChangeAction() {
   if (this->iis_.valid_ && options_.iis_strategy != this->iis_.strategy_)
     this->iis_.clear();
   return HighsStatus::kOk;
 }
 
-void Highs::restoreInfCost(HighsStatus& return_status) {
-  HighsLp& lp = this->model_.lp_;
-  HighsBasis& basis = this->basis_;
-  HighsLpMods& mods = lp.mods_;
-  HighsInt num_inf_cost = mods.save_inf_cost_variable_index.size();
-  if (num_inf_cost <= 0) return;
-  assert(num_inf_cost);
-  for (HighsInt ix = 0; ix < num_inf_cost; ix++) {
-    HighsInt iCol = mods.save_inf_cost_variable_index[ix];
-    double cost = mods.save_inf_cost_variable_cost[ix];
-    double lower = mods.save_inf_cost_variable_lower[ix];
-    double upper = mods.save_inf_cost_variable_upper[ix];
-    double value = solution_.value_valid ? solution_.col_value[iCol] : 0;
-    if (basis.valid) {
-      assert(basis.col_status[iCol] != HighsBasisStatus::kBasic);
-      if (lp.col_lower_[iCol] == lower) {
-        basis.col_status[iCol] = HighsBasisStatus::kLower;
-      } else {
-        basis.col_status[iCol] = HighsBasisStatus::kUpper;
-      }
-    }
-    assert(lp.col_cost_[iCol] == 0);
-    if (value) this->info_.objective_function_value += value * cost;
-    lp.col_cost_[iCol] = cost;
-    lp.col_lower_[iCol] = lower;
-    lp.col_upper_[iCol] = upper;
-  }
-  // Infinite costs have been reintroduced, so reset to true the flag
-  // that was set false in Highs::handleInfCost() (See #1446)
-  lp.has_infinite_cost_ = true;
-
-  if (this->model_status_ == HighsModelStatus::kInfeasible) {
-    // Model is infeasible with the infinite cost variables fixed at
-    // appropriate values, so model status cannot be determined
-    this->model_status_ = HighsModelStatus::kUnknown;
-    setHighsModelStatusAndClearSolutionAndBasis(this->model_status_);
-    return_status = highsStatusFromHighsModelStatus(model_status_);
-  }
-}
-
 HighsStatus Highs::userScale(HighsUserScaleData& data) {
   if (!options_.user_objective_scale && !options_.user_bound_scale)
     return HighsStatus::kOk;

highs/lp_data/HighsLp.cpp

@@ -491,7 +491,14 @@ void HighsLp::unapplyMods() {
   this->mods_.clear();
 }
 
-void HighsLpMods::clear() {
+void HighsLpMods::clearInfiniteCostRecord() {
+  this->save_inf_cost_variable_index.clear();
+  this->save_inf_cost_variable_cost.clear();
+  this->save_inf_cost_variable_lower.clear();
+  this->save_inf_cost_variable_upper.clear();
+}
+
+void HighsLpMods::clearSemiVariableRecord() {
   this->save_non_semi_variable_index.clear();
   this->save_inconsistent_semi_variable_index.clear();
   this->save_inconsistent_semi_variable_lower_bound_value.clear();
@@ -501,10 +508,11 @@ void HighsLpMods::clear() {
   this->save_relaxed_semi_variable_lower_bound_value.clear();
   this->save_tightened_semi_variable_upper_bound_index.clear();
   this->save_tightened_semi_variable_upper_bound_value.clear();
-  this->save_inf_cost_variable_index.clear();
-  this->save_inf_cost_variable_cost.clear();
-  this->save_inf_cost_variable_lower.clear();
-  this->save_inf_cost_variable_upper.clear();
+}
+
+void HighsLpMods::clear() {
+  this->clearInfiniteCostRecord();
+  this->clearSemiVariableRecord();
 }
 
 bool HighsLpMods::isClear() {