Being more consistent about returning HighsStatus::kWarning when writing a model, solution or basis; formatted

Author: jajhall

check/TestBasis.cpp

@@ -56,8 +56,16 @@ TEST_CASE("Basis-file", "[highs_basis_file]") {
   f << "HiGHS v1" << std::endl;
   f << "None" << std::endl;
   f.close();
-  return_status = highs.readBasis(invalid_basis_file);
-  REQUIRE(return_status == HighsStatus::kOk);
+  // HiGHS v1 basis file is deprecated, but read, so warning is
+  // returned
+  REQUIRE(highs.readBasis(invalid_basis_file) == HighsStatus::kWarning);
+
+  // Write and read a file for an invalid basis
+  f.open(invalid_basis_file, std::ios::out);
+  f << "HiGHS_basis_file v2" << std::endl;
+  f << "None" << std::endl;
+  f.close();
+  REQUIRE(highs.readBasis(invalid_basis_file) == HighsStatus::kOk);
 
   // Write and read a file for incompatible number of columns
   f.open(invalid_basis_file, std::ios::out);

check/TestCheckSolution.cpp

@@ -90,9 +90,9 @@ TEST_CASE("check-set-mip-solution", "[highs_check_solution]") {
   if (dev_run) printf("Num nodes = %d\n", int(scratch_num_nodes));
 
   std::string solution_file = test_name + model + ".sol";
-  if (dev_run) return_status = highs.writeSolution("");
-  return_status = highs.writeSolution(solution_file);
-  REQUIRE(return_status == HighsStatus::kOk);
+  if (dev_run) REQUIRE(highs.writeSolution("") == HighsStatus::kOk);
+  ;
+  REQUIRE(highs.writeSolution(solution_file) == HighsStatus::kOk);
 
   highs.clear();
 
@@ -590,9 +590,15 @@ void runWriteReadCheckSolution(Highs& highs, const std::string& test_name,
   if (dev_run)
     printf("Writing solution in style %d to %s\n", int(write_solution_style),
            solution_file.c_str());
-  if (dev_run) return_status = highs.writeSolution("", write_solution_style);
-  return_status = highs.writeSolution(solution_file, write_solution_style);
-  REQUIRE(return_status == HighsStatus::kOk);
+  // For models without names, Highs::writeSolution will return
+  // HighsStatus::kWarning
+  HighsStatus require_status = highs.getLp().col_names_.size()
+                                   ? HighsStatus::kOk
+                                   : HighsStatus::kWarning;
+  REQUIRE(highs.writeSolution(solution_file, write_solution_style) ==
+          require_status);
+  if (dev_run)
+    REQUIRE(highs.writeSolution("", write_solution_style) == HighsStatus::kOk);
 
   const bool& value_valid = highs.getSolution().value_valid;
   bool valid, integral, feasible;

check/TestFilereader.cpp

@@ -491,7 +491,7 @@ TEST_CASE("handle-blank-space-names", "[highs_filereader]") {
   lp.a_matrix_.index_ = {0, 1, 0, 1};
   lp.a_matrix_.value_ = {1, 2, 2, 4};
   Highs h;
-  //  h.setOptionValue("output_flag", dev_run);
+  h.setOptionValue("output_flag", dev_run);
   REQUIRE(h.passModel(lp) == HighsStatus::kOk);
   h.run();
   // Names will be created when writing the solution
@@ -513,7 +513,6 @@ TEST_CASE("handle-blank-space-names", "[highs_filereader]") {
   std::vector<double> value = {2, 3};
   REQUIRE(h.addRow(5, inf, 2, index.data(), value.data()) == HighsStatus::kOk);
   h.run();
-  printf("h.getLP().row_names_[2] = %s\n", h.getLp().row_names_[2].c_str());
   REQUIRE(h.writeBasis("") == HighsStatus::kWarning);
   REQUIRE(h.writeSolution("", kSolutionStylePretty) == HighsStatus::kOk);
   REQUIRE(h.writeModel("") == HighsStatus::kOk);

check/TestNames.cpp

@@ -111,7 +111,7 @@ TEST_CASE("highs-names", "[highs_names]") {
   local_lp.col_names_.clear();
   local_lp.row_names_.clear();
   highs.passModel(local_lp);
-  REQUIRE(highs.writeSolution(solution_file, 1) == HighsStatus::kOk);
+  REQUIRE(highs.writeSolution(solution_file, 1) == HighsStatus::kWarning);
 
   std::remove(solution_file.c_str());
 }

check/TestQpSolver.cpp

@@ -1121,7 +1121,8 @@ TEST_CASE("rowless-qp", "[qpsolver]") {
   REQUIRE(highs.setOptionValue("qp_regularization_value", 0) ==
           HighsStatus::kOk);
   REQUIRE(highs.run() == HighsStatus::kOk);
-  REQUIRE(highs.writeSolution("", kSolutionStylePretty) == HighsStatus::kOk);
+  REQUIRE(highs.writeSolution("", kSolutionStylePretty) ==
+          HighsStatus::kWarning);
 
   const double required_objective_function_value = -2.25;
 

highs/lp_data/Highs.cpp

@@ -755,9 +755,8 @@ HighsStatus Highs::writeLocalModel(HighsModel& model,
   // Replace any blank names and return error if there are duplicates
   // or names with spaces
   call_status = normaliseNames(this->options_.log_options, lp);
-  return_status =
-        interpretCallStatus(options_.log_options, call_status,
-                            return_status, "normaliseNames");
+  return_status = interpretCallStatus(options_.log_options, call_status,
+                                      return_status, "normaliseNames");
   if (return_status == HighsStatus::kError) return return_status;
 
   // Ensure that the LP is column-wise
@@ -827,9 +826,8 @@ HighsStatus Highs::writeBasis(const std::string& filename) {
   // Replace any blank names and return error if there are duplicates
   // or names with spaces
   call_status = normaliseNames(this->options_.log_options, this->model_.lp_);
-  return_status =
-        interpretCallStatus(options_.log_options, call_status,
-                            return_status, "normaliseNames");
+  return_status = interpretCallStatus(options_.log_options, call_status,
+                                      return_status, "normaliseNames");
   if (return_status == HighsStatus::kError) return return_status;
 
   // Report to user that basis is being written
@@ -3369,9 +3367,8 @@ HighsStatus Highs::writeSolution(const std::string& filename,
   // Replace any blank names and return error if there are duplicates
   // or names with spaces
   call_status = normaliseNames(this->options_.log_options, this->model_.lp_);
-  return_status =
-        interpretCallStatus(options_.log_options, call_status,
-                            return_status, "normaliseNames");
+  return_status = interpretCallStatus(options_.log_options, call_status,
+                                      return_status, "normaliseNames");
   if (return_status == HighsStatus::kError) return return_status;
 
   // Report to user that solution is being written

highs/lp_data/HighsInterface.cpp

@@ -513,7 +513,7 @@ HighsStatus Highs::addColsInterface(
   // Update the basis corresponding to new nonbasic columns
   if (useful_basis) appendNonbasicColsToBasisInterface(ext_num_new_col);
 
-  // Possibly add column names
+  // Possibly add blank column names
   lp.addColNames("", ext_num_new_col);
 
   // Increase the number of columns in the LP
@@ -658,7 +658,7 @@ HighsStatus Highs::addRowsInterface(HighsInt ext_num_new_row,
   // Update the basis corresponding to new basic rows
   if (useful_basis) appendBasicRowsToBasisInterface(ext_num_new_row);
 
-  // Possibly add row names
+  // Possibly add blank row names
   lp.addRowNames("", ext_num_new_row);
 
   // Increase the number of rows in the LP

highs/lp_data/HighsLp.cpp

@@ -345,95 +345,26 @@ void HighsLp::userCostScale(const HighsInt user_cost_scale) {
 }
 
 void HighsLp::addColNames(const std::string name, const HighsInt num_new_col) {
-  // Don't add names if there are no columns, or if the names are
-  // already incomplete
+  // Don't add names if there are no columns being added
   if (this->num_col_ == 0) return;
   HighsInt col_names_size = this->col_names_.size();
-  if (col_names_size < this->num_col_) return;
-  if (!this->col_hash_.name2index.size())
-    this->col_hash_.form(this->col_names_);
+  if (col_names_size <= 0) return;
+  assert(col_names_size == this->num_col_ + num_new_col);
   // Handle the addition of user-defined names later
   assert(name == "");
-  if (this->col_name_prefix_ == "")
-    col_name_prefix_ = kHighsUniqueColNamePrefix;
-  for (HighsInt iCol = this->num_col_; iCol < this->num_col_ + num_new_col;
-       iCol++) {
-    const std::string col_name =
-        this->col_name_prefix_ + std::to_string(this->col_name_suffix_++);
-    bool added = false;
-    auto search = this->col_hash_.name2index.find(col_name);
-    if (search == this->col_hash_.name2index.end()) {
-      // Name not found in hash
-      if (col_names_size == this->num_col_) {
-        // No space (or name) for this col name
-        this->col_names_.push_back(col_name);
-        added = true;
-      } else if (col_names_size > iCol) {
-        // Space for this col name. Only add if name is blank
-        if (this->col_names_[iCol] == "") {
-          this->col_names_[iCol] = col_name;
-          added = true;
-        }
-      }
-    }
-    if (added) {
-      const bool duplicate =
-          !this->col_hash_.name2index.emplace(col_name, iCol).second;
-      assert(!duplicate);
-      assert(this->col_names_[iCol] == col_name);
-      assert(this->col_hash_.name2index.find(col_name)->second == iCol);
-    } else {
-      // Duplicate name or other failure
-      this->col_hash_.name2index.clear();
-      return;
-    }
-  }
+  // Blank names for the new columns were added in
+  // appendColsToLpVectors
 }
 
 void HighsLp::addRowNames(const std::string name, const HighsInt num_new_row) {
-  // Don't add names if there are no rows, or if the names are already
-  // incomplete
+  // Don't add names if there are no rows being added
   if (this->num_row_ == 0) return;
   HighsInt row_names_size = this->row_names_.size();
-  if (row_names_size < this->num_row_) return;
-  if (!this->row_hash_.name2index.size())
-    this->row_hash_.form(this->row_names_);
+  if (row_names_size <= 0) return;
+  assert(row_names_size == this->num_row_ + num_new_row);
   // Handle the addition of user-defined names later
   assert(name == "");
-  if (this->row_name_prefix_ == "")
-    row_name_prefix_ = kHighsUniquerowNamePrefix;
-  for (HighsInt iRow = this->num_row_; iRow < this->num_row_ + num_new_row;
-       iRow++) {
-    const std::string row_name =
-        this->row_name_prefix_ + std::to_string(this->row_name_suffix_++);
-    bool added = false;
-    auto search = this->row_hash_.name2index.find(row_name);
-    if (search == this->row_hash_.name2index.end()) {
-      // Name not found in hash
-      if (row_names_size == this->num_row_) {
-        // No space (or name) for this row name
-        this->row_names_.push_back(row_name);
-        added = true;
-      } else if (row_names_size > iRow) {
-        // Space for this row name. Only add if name is blank
-        if (this->row_names_[iRow] == "") {
-          this->row_names_[iRow] = row_name;
-          added = true;
-        }
-      }
-    }
-    if (added) {
-      const bool duplicate =
-          !this->row_hash_.name2index.emplace(row_name, iRow).second;
-      assert(!duplicate);
-      assert(this->row_names_[iRow] == row_name);
-      assert(this->row_hash_.name2index.find(row_name)->second == iRow);
-    } else {
-      // Duplicate name or other failure
-      this->row_hash_.name2index.clear();
-      return;
-    }
-  }
+  // Blank names for the new rows were added in appendRowsToLpVectors
 }
 
 void HighsLp::deleteColsFromVectors(

highs/lp_data/HighsLpUtils.cpp

@@ -1864,7 +1864,8 @@ void reportLpColVectors(const HighsLogOptions& log_options, const HighsLp& lp) {
   std::string type;
   HighsInt count;
   bool have_integer_columns = (getNumInt(lp) != 0);
-  bool have_col_names = lp.col_names_.size() == static_cast<size_t>(lp.num_col_);
+  bool have_col_names =
+      lp.col_names_.size() == static_cast<size_t>(lp.num_col_);
 
   highsLogUser(log_options, HighsLogType::kInfo,
                "  Column        Lower        Upper         Cost       "
@@ -2515,7 +2516,8 @@ HighsStatus assessLpPrimalSolution(const std::string message,
   row_value.assign(lp.num_row_, 0);
   const bool have_integrality = (lp.integrality_.size() != 0);
   if (!solution.value_valid) return HighsStatus::kError;
-  const bool have_col_names = lp.col_names_.size() == static_cast<size_t>(lp.num_col_);
+  const bool have_col_names =
+      lp.col_names_.size() == static_cast<size_t>(lp.num_col_);
   for (HighsInt iCol = 0; iCol < lp.num_col_; iCol++) {
     const std::string name_string =
         have_col_names ? (" (" + lp.col_names_[iCol] + ")") : "";
@@ -3108,8 +3110,10 @@ HighsLp withoutSemiVariables(const HighsLp& lp_, HighsSolution& solution,
   HighsInt semi_row_num = 0;
   // Insert the new variables and their coefficients
   std::stringstream ss;
-  const bool have_col_names = lp.col_names_.size() == static_cast<size_t>(lp.num_col_);
-  const bool have_row_names = lp.row_names_.size() == static_cast<size_t>(lp.num_row_);
+  const bool have_col_names =
+      lp.col_names_.size() == static_cast<size_t>(lp.num_col_);
+  const bool have_row_names =
+      lp.row_names_.size() == static_cast<size_t>(lp.num_row_);
   const bool have_solution = solution.value_valid;
   if (have_solution) {
     // Create zeroed row values for the new rows

highs/lp_data/HighsModelUtils.cpp

@@ -205,8 +205,13 @@ void writePrimalSolution(FILE* file, const HighsLogOptions& log_options,
                          const HighsLp& lp,
                          const std::vector<double>& primal_solution,
                          const bool sparse) {
+  // Use when writing out the solution file (when names can be assumed
+  // to exist) and the improving solution in the MIP solver (when
+  // names cannot be assumed to exist)
   HighsInt num_nonzero_primal_value = 0;
-  assert(lp.col_names_.size() == static_cast<size_t>(lp.num_col_));
+  const bool have_col_names = lp.col_names_.size() > 0;
+  if (have_col_names)
+    assert(lp.col_names_.size() == static_cast<size_t>(lp.num_col_));
   if (sparse) {
     // Determine the number of nonzero primal solution values
     for (HighsInt iCol = 0; iCol < lp.num_col_; iCol++)
@@ -225,8 +230,12 @@ void writePrimalSolution(FILE* file, const HighsLogOptions& log_options,
     if (sparse && !primal_solution[ix]) continue;
     auto valStr = highsDoubleToString(primal_solution[ix],
                                       kHighsSolutionValueToStringTolerance);
+    // Don't invent names locallty: if none exist, then indicate this
+    // - so that the (sparse) solution line format remains "name value
+    // (index)"
+    const std::string name = have_col_names ? lp.col_names_[ix] : "NoName";
     ss.str(std::string());
-    ss << highsFormatToString("%-s %s", lp.col_names_[ix].c_str(), valStr.data());
+    ss << highsFormatToString("%-s %s", name.c_str(), valStr.data());
     if (sparse) ss << highsFormatToString(" %d", int(ix));
     ss << "\n";
     highsFprintfString(file, log_options, ss.str());
@@ -273,9 +282,9 @@ void writeModelSolution(FILE* file, const HighsLogOptions& log_options,
     for (HighsInt ix = 0; ix < lp.num_row_; ix++) {
       auto valStr = highsDoubleToString(solution.row_value[ix],
                                         kHighsSolutionValueToStringTolerance);
-      // Create a row name
       ss.str(std::string());
-      ss << highsFormatToString("%-s %s\n", lp.row_names_[ix].c_str(), valStr.data());
+      ss << highsFormatToString("%-s %s\n", lp.row_names_[ix].c_str(),
+                                valStr.data());
       highsFprintfString(file, log_options, ss.str());
     }
   }
@@ -296,7 +305,8 @@ void writeModelSolution(FILE* file, const HighsLogOptions& log_options,
       auto valStr = highsDoubleToString(solution.col_dual[ix],
                                         kHighsSolutionValueToStringTolerance);
       ss.str(std::string());
-      ss << highsFormatToString("%-s %s\n", lp.col_names_[ix].c_str(), valStr.data());
+      ss << highsFormatToString("%-s %s\n", lp.col_names_[ix].c_str(),
+                                valStr.data());
       highsFprintfString(file, log_options, ss.str());
     }
     ss.str(std::string());
@@ -306,7 +316,8 @@ void writeModelSolution(FILE* file, const HighsLogOptions& log_options,
       auto valStr = highsDoubleToString(solution.row_dual[ix],
                                         kHighsSolutionValueToStringTolerance);
       ss.str(std::string());
-      ss << highsFormatToString("%-s %s\n", lp.row_names_[ix].c_str(), valStr.data());
+      ss << highsFormatToString("%-s %s\n", lp.row_names_[ix].c_str(),
+                                valStr.data());
       highsFprintfString(file, log_options, ss.str());
     }
   }
@@ -359,8 +370,8 @@ HighsStatus normaliseNames(const HighsLogOptions& log_options, HighsLp& lp) {
   if (call_status == HighsStatus::kError) return call_status;
   HighsStatus return_status = call_status;
   call_status =
-    normaliseNames(log_options, false, lp.num_row_, lp.row_name_prefix_,
-		   lp.row_name_suffix_, lp.row_names_, lp.row_hash_);
+      normaliseNames(log_options, false, lp.num_row_, lp.row_name_prefix_,
+                     lp.row_name_suffix_, lp.row_names_, lp.row_hash_);
   if (call_status != HighsStatus::kOk) return call_status;
   return return_status;
 }