Merged latest into this branch and fixed minor conflicts

Author: jajhall

CMakeLists.txt

@@ -500,7 +500,10 @@ if(ZLIB AND NOT TARGET ZLIB::ZLIB)
   find_package(ZLIB 1.2.3)
 endif()
 
-include(CPack)
+if(CPack_CMake_INCLUDED EQUAL 0)
+  include(CPack)
+endif()
+
 set(CPACK_PACKAGE_VERSION_MAJOR "${HIGHS_VERSION_MAJOR}")
 set(CPACK_PACKAGE_VERSION_MINOR "${HIGHS_VERSION_MINOR}")
 set(CPACK_PACKAGE_VERSION_PATCH "${HIGHS_VERSION_PATCH}")

FEATURES.md

@@ -8,6 +8,8 @@ Now handling correctly the case where an infeasible MIP has a feasible relaxatio
 
 Fixed minor bug exposed by [#2441](https://github.com/ERGO-Code/HiGHS/issues/2441) in Highs::setSolution() for a sparse user solution when the moidel is empty, and only clearing the dual data before solving with modified objective in Highs::multiobjectiveSolve() so that user-supplied solution is not cleared.
 
+
 The irreducible infeasibility system (IIS) facility now detects infeasibility due to bounds on constraint activity values being incompatible with constraint bounds. A kIisStrategyLight mode for the iis_strategy option has been introduced so that only infeasibility due to incompatible variable/constraint bounds and constraint activity values is checked for. The LP corresponding to any known IIS is now formed and held as a data member of the HighsIis class. 
 
+Prompted by [#2463](https://github.com/ERGO-Code/HiGHS/issues/2463), the HiGHS solution and basis files now match data to any column and row names in the model, only assuming that the data are aligned with column and row indices if there are no names in the model. This requires a new version (v2) of the HiGHS basis file. Basis files from v1 are still read, but deprecated. Now, when writing out a model, basis or solution, column and row names are added to the model - previously they were created temporarily and inconsistentyly on the fly. If the model has existing names, then distinctive names are created to replace any blank names, but names with spaces or duplicate names yield an error status return.
 

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/TestCAPI.c

@@ -617,7 +617,6 @@ void fullApi() {
 
   // Define all column names to be different
   for (HighsInt iCol = 0; iCol < num_col; iCol++) {
-    const char suffix = iCol + '0';
     char name[5];  // 3 chars prefix, 1 char iCol, 1 char 0-terminator
     sprintf(name, "%s%" HIGHSINT_FORMAT "", col_prefix, iCol);
     const char* name_p = name;
@@ -650,7 +649,6 @@ void fullApi() {
 
   // Define all row names to be different
   for (HighsInt iRow = 0; iRow < num_row; iRow++) {
-    const char suffix = iRow + '0';
     char name[5];  // 3 chars prefix, 1 char iCol, 1 char 0-terminator
     sprintf(name, "%s%" HIGHSINT_FORMAT "", row_prefix, iRow);
     const char* name_p = name;

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();
 
@@ -459,6 +459,118 @@ TEST_CASE("read-miplib-solution", "[highs_check_solution]") {
   h.resetGlobalScheduler(true);
 }
 
+TEST_CASE("read-lp-file-solution", "[highs_check_solution]") {
+  const std::string test_name = Catch::getResultCapture().getCurrentTestName();
+  const std::string model_file_name = test_name + ".lp";
+  const std::string solution_file_name = test_name + ".sol";
+  const bool with_names = false;
+  HighsLp lp;
+  lp.num_col_ = 3;
+  lp.num_row_ = 2;
+  lp.col_cost_ = {0, 1, 1};
+  lp.col_lower_ = {0, 10, 0};
+  lp.col_upper_ = {kHighsInf, kHighsInf, kHighsInf};
+  if (with_names) lp.col_names_ = {"x", "y", "z"};
+  lp.row_lower_ = {1, -kHighsInf};
+  lp.row_upper_ = {kHighsInf, 2};
+  if (with_names) lp.row_names_ = {"r-lo", "r-up"};
+  lp.a_matrix_.start_ = {0, 2, 2, 4};
+  lp.a_matrix_.index_ = {0, 1, 0, 1};
+  lp.a_matrix_.value_ = {1, 1, 1, 1};
+  lp.integrality_ = {HighsVarType::kContinuous, HighsVarType::kContinuous,
+                     HighsVarType::kInteger};
+  Highs h;
+  h.setOptionValue("output_flag", dev_run);
+  REQUIRE(h.passModel(lp) == HighsStatus::kOk);
+  h.run();
+  h.writeModel(model_file_name);
+  h.writeSolution(solution_file_name);
+
+  h.readModel(model_file_name);
+  h.writeModel("");
+  h.readSolution(solution_file_name);
+  h.run();
+
+  std::remove(model_file_name.c_str());
+  std::remove(solution_file_name.c_str());
+
+  h.resetGlobalScheduler(true);
+}
+
+TEST_CASE("read-lp-file-basis", "[highs_check_solution]") {
+  const std::string test_name = Catch::getResultCapture().getCurrentTestName();
+  const std::string model_file_name = test_name + ".lp";
+  const std::string basis_file_name = test_name + ".bas";
+  const bool with_names = false;
+  HighsLp lp;
+  lp.num_col_ = 3;
+  lp.num_row_ = 2;
+  lp.col_cost_ = {0, 1, 1};
+  lp.col_lower_ = {0, 10, 0};
+  lp.col_upper_ = {kHighsInf, kHighsInf, kHighsInf};
+  if (with_names) lp.col_names_ = {"x", "y", "z"};
+  lp.row_lower_ = {1, -kHighsInf};
+  lp.row_upper_ = {kHighsInf, 2};
+  if (with_names) lp.row_names_ = {"r-lo", "r-up"};
+  lp.a_matrix_.start_ = {0, 2, 2, 4};
+  lp.a_matrix_.index_ = {0, 1, 0, 1};
+  lp.a_matrix_.value_ = {1, 1, 1, 1};
+  Highs h;
+  h.setOptionValue("output_flag", dev_run);
+  REQUIRE(h.passModel(lp) == HighsStatus::kOk);
+  h.run();
+  // Optimally x - basic; y - lower; z - lower
+  h.writeModel(model_file_name);
+  h.writeSolution("", 1);
+  h.writeBasis("");
+  h.writeBasis(basis_file_name);
+
+  h.readModel(model_file_name);
+  // Variables now ordered y; z; x
+  h.writeModel("");
+  h.readBasis(basis_file_name);
+  // Old read basis yields initial basis: y - basic; z - lower; x -
+  // lower, using basis for original ordering with new ordering. Not
+  // optimal - in fact basis matrix B = [0] is singular!
+  h.run();
+  REQUIRE(h.getInfo().simplex_iteration_count == 0);
+
+  std::remove(model_file_name.c_str());
+  std::remove(basis_file_name.c_str());
+
+  h.resetGlobalScheduler(true);
+}
+
+TEST_CASE("read-lp-file-rgn", "[highs_check_solution]") {
+  const std::string test_name = Catch::getResultCapture().getCurrentTestName();
+  const std::string filename =
+      std::string(HIGHS_DIR) + "/check/instances/rgn.mps";
+  const std::string model_file_name = test_name + ".lp";
+  const std::string solution_file_name = test_name + ".sol";
+  Highs h;
+  h.setOptionValue("output_flag", dev_run);
+  REQUIRE(h.readModel(filename) == HighsStatus::kOk);
+  REQUIRE(h.run() == HighsStatus::kOk);
+  REQUIRE(h.writeSolution(solution_file_name) == HighsStatus::kOk);
+  REQUIRE(h.writeModel(model_file_name) == HighsStatus::kOk);
+
+  REQUIRE(h.readModel(model_file_name) == HighsStatus::kOk);
+  REQUIRE(h.readSolution(solution_file_name) == HighsStatus::kOk);
+  bool valid;
+  bool integral;
+  bool feasible;
+  REQUIRE(h.assessPrimalSolution(valid, integral, feasible) ==
+          HighsStatus::kOk);
+  REQUIRE(valid);
+  REQUIRE(integral);
+  REQUIRE(feasible);
+
+  std::remove(model_file_name.c_str());
+  std::remove(solution_file_name.c_str());
+
+  h.resetGlobalScheduler(true);
+}
+
 void runWriteReadCheckSolution(Highs& highs, const std::string& test_name,
                                const std::string& model,
                                const HighsModelStatus require_model_status,
@@ -478,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;
@@ -581,3 +699,42 @@ void runSetLpSolution(const std::string model) {
 
   highs.resetGlobalScheduler(true);
 }
+
+TEST_CASE("miplib-sol-file", "[highs_filereader]") {
+  const std::string test_name = Catch::getResultCapture().getCurrentTestName();
+  std::string sol_file = test_name + ".sol";
+  std::string lp_file = test_name + ".lp";
+  FILE* file = fopen(lp_file.c_str(), "w");
+  std::string file_content =
+      "Minimize\n obj: 2 sel_2 + sel_3\nSubject To\nr0: sel_0 - sel_1 + sel_4 "
+      ">= "
+      "2\nEnd\n";
+  if (dev_run) printf("Using .lp file\n%s", file_content.c_str());
+  fprintf(file, "%s", file_content.c_str());
+  fclose(file);
+  Highs h;
+  h.setOptionValue("output_flag", dev_run);
+  REQUIRE(h.readModel(lp_file) == HighsStatus::kOk);
+
+  file = fopen(sol_file.c_str(), "w");
+  file_content =
+      "=obj= 203672547.1\nsel_0 1\nsel_1 0\nsel_2 0\nsel_3 0\nsel_4 1\n";
+  if (dev_run) printf("Using .sol file\n%s", file_content.c_str());
+  fprintf(file, "%s", file_content.c_str());
+  fclose(file);
+  REQUIRE(h.readSolution(sol_file) == HighsStatus::kOk);
+
+  std::vector<double> solution = h.getSolution().col_value;
+  REQUIRE(solution[0] == 0);
+  REQUIRE(solution[1] == 0);
+  REQUIRE(solution[2] == 1);
+  REQUIRE(solution[3] == 0);
+  REQUIRE(solution[4] == 1);
+
+  REQUIRE(h.run() == HighsStatus::kOk);
+
+  std::remove(lp_file.c_str());
+  std::remove(sol_file.c_str());
+
+  h.resetGlobalScheduler(true);
+}

check/TestFilereader.cpp

@@ -382,7 +382,7 @@ TEST_CASE("writeLocalModel", "[highs_filereader]") {
   h.setOptionValue("output_flag", dev_run);
   HighsModel model;
   HighsLp& lp = model.lp_;
-  ;
+
   lp.num_col_ = 2;
   lp.num_row_ = 3;
   lp.col_cost_ = {8, 10};
@@ -403,7 +403,7 @@ TEST_CASE("writeLocalModel", "[highs_filereader]") {
   // Model has no dimensions for a_matrix_, but these are set in
   // writeLocalModel.
   if (dev_run) printf("\nModel with no column or row names\n");
-  REQUIRE(h.writeLocalModel(model, write_model_file) == HighsStatus::kWarning);
+  REQUIRE(h.writeLocalModel(model, write_model_file) == HighsStatus::kOk);
   lp.col_names_ = {"C0", "C1"};
   lp.row_names_ = {"R0", "R1", "R2"};
 
@@ -477,3 +477,119 @@ TEST_CASE("mps-silly-names", "[highs_filereader]") {
   HighsStatus return_status = h.readModel(model_file);
   REQUIRE(return_status == HighsStatus::kOk);
 }
+
+TEST_CASE("handle-blank-space-names", "[highs_filereader]") {
+  HighsLp lp;
+  lp.num_col_ = 2;
+  lp.num_row_ = 2;
+  lp.col_cost_ = {8, 10};
+  lp.col_lower_ = {0, 0};
+  lp.col_upper_ = {inf, inf};
+  lp.row_lower_ = {7, 12};
+  lp.row_upper_ = {inf, inf};
+  lp.a_matrix_.start_ = {0, 2, 4};
+  lp.a_matrix_.index_ = {0, 1, 0, 1};
+  lp.a_matrix_.value_ = {1, 2, 2, 4};
+  Highs h;
+  h.setOptionValue("output_flag", dev_run);
+  REQUIRE(h.passModel(lp) == HighsStatus::kOk);
+  h.run();
+  // Names will be created when writing the solution
+  REQUIRE(h.writeSolution("", 1) == HighsStatus::kWarning);
+  REQUIRE(h.writeModel("") == HighsStatus::kOk);
+
+  lp.col_names_ = {"Column", "Column"};
+  lp.row_names_ = {"Row0", "Row1"};
+  REQUIRE(h.passModel(lp) == HighsStatus::kOk);
+  REQUIRE(h.writeModel("") == HighsStatus::kError);
+
+  lp.col_names_ = {"Column0", ""};
+  REQUIRE(h.passModel(lp) == HighsStatus::kOk);
+  h.run();
+  REQUIRE(h.writeSolution("", kSolutionStyleOldRaw) == HighsStatus::kWarning);
+  REQUIRE(h.writeModel("") == HighsStatus::kOk);
+
+  std::vector<HighsInt> index = {0, 1};
+  std::vector<double> value = {2, 3};
+  REQUIRE(h.addRow(5, inf, 2, index.data(), value.data()) == HighsStatus::kOk);
+  h.run();
+  REQUIRE(h.writeBasis("") == HighsStatus::kWarning);
+  REQUIRE(h.writeSolution("", kSolutionStylePretty) == HighsStatus::kOk);
+  REQUIRE(h.writeModel("") == HighsStatus::kOk);
+
+  lp.row_names_[1] = "Row 1";
+  REQUIRE(h.passModel(lp) == HighsStatus::kOk);
+  h.run();
+  REQUIRE(h.writeSolution("", 1) == HighsStatus::kError);
+  REQUIRE(h.writeModel("") == HighsStatus::kError);
+
+  h.resetGlobalScheduler(true);
+}
+
+TEST_CASE("read-highs-lp-file0", "[highs_filereader]") {
+  // Identified when fixing #2463 that LP file reader cannot handle
+  // case where row names are numeric constants
+  const std::string test_name = Catch::getResultCapture().getCurrentTestName();
+  const std::string model_file_name0 = test_name + "-0.lp";
+  const std::string model_file_name1 = test_name + "-1.lp";
+  HighsLp lp;
+  lp.num_col_ = 1;
+  lp.num_row_ = 3;
+  lp.col_cost_ = {8};
+  lp.col_lower_ = {0};
+  lp.col_upper_ = {inf};
+  lp.row_lower_ = {-21, 31, 43};
+  lp.row_upper_ = {inf, inf, inf};
+  lp.a_matrix_.start_ = {0, 3};
+  lp.a_matrix_.index_ = {0, 1, 2};
+  lp.a_matrix_.value_ = {2, -3, 7};
+  lp.col_names_ = {"col"};
+  lp.row_names_ = {"row", "55", "9.9"};
+  Highs h;
+  h.setOptionValue("output_flag", dev_run);
+  REQUIRE(h.passModel(lp) == HighsStatus::kOk);
+  // Create a .lp file for this model - that has numeric constants as
+  // row names
+  REQUIRE(h.writeModel(model_file_name0) == HighsStatus::kOk);
+  // Make sure that the .lp file for this model can be read OK
+  REQUIRE(h.readModel(model_file_name0) == HighsStatus::kOk);
+  REQUIRE(h.writeModel(model_file_name1) == HighsStatus::kOk);
+
+  std::remove(model_file_name0.c_str());
+  std::remove(model_file_name1.c_str());
+
+  h.resetGlobalScheduler(true);
+}
+
+TEST_CASE("read-highs-lp-file1", "[highs_filereader]") {
+  const std::string test_name = Catch::getResultCapture().getCurrentTestName();
+  std::string filename;
+  filename = std::string(HIGHS_DIR) + "/check/instances/rgn.mps";
+  std::string model_file_name = test_name + ".lp";
+  Highs h;
+  h.setOptionValue("output_flag", dev_run);
+  REQUIRE(h.readModel(filename) == HighsStatus::kOk);
+  REQUIRE(h.writeModel(model_file_name) == HighsStatus::kOk);
+  REQUIRE(h.readModel(model_file_name) == HighsStatus::kOk);
+
+  std::remove(model_file_name.c_str());
+
+  h.resetGlobalScheduler(true);
+}
+
+TEST_CASE("lp-duplicate-variable", "[highs_filereader]") {
+  const std::string test_name = Catch::getResultCapture().getCurrentTestName();
+  std::string lp_file = test_name + ".lp";
+  FILE* file = fopen(lp_file.c_str(), "w");
+  std::string file_content =
+      "Minimize\n obj: 2 x + y + z\nSubject To\nr0: 2 x + y - x + 0 z >= "
+      "2\nr1: y + x - y >= 1\nEnd\n";
+  if (dev_run) printf("Using .lp file\n%s", file_content.c_str());
+  fprintf(file, "%s", file_content.c_str());
+  fclose(file);
+  Highs h;
+  h.setOptionValue("output_flag", dev_run);
+  REQUIRE(h.readModel(lp_file) == HighsStatus::kWarning);
+
+  std::remove(lp_file.c_str());
+}