Added tests for starts and indices to HighsSparseMatrix, and using th…

…em in writeLocalModel
ERGO-Code · Nov 19, 2024 · f94a2c7 · f94a2c7
1 parent d4a4a30
commit f94a2c7
Show file tree

Hide file tree

Showing 6 changed files with 110 additions and 16 deletions.
diff --git a/check/TestFilereader.cpp b/check/TestFilereader.cpp
@@ -361,10 +361,11 @@ TEST_CASE("filereader-dD2e", "[highs_filereader]") {
 
 TEST_CASE("writeLocalModel", "[highs_filereader]") {
   Highs h;
-  //  h.setOptionValue("output_flag", dev_run);
+  h.setOptionValue("output_flag", dev_run);
   std::string write_model_file = "foo.mps";
   HighsModel model;
-  HighsLp& lp = model.lp_;;
+  HighsLp& lp = model.lp_;
+  ;
   lp.num_col_ = 2;
   lp.num_row_ = 3;
   lp.col_cost_ = {8, 10};
@@ -374,29 +375,44 @@ TEST_CASE("writeLocalModel", "[highs_filereader]") {
   lp.a_matrix_.index_ = {0, 1, 2, 0, 1, 2};
   lp.a_matrix_.value_ = {2, 3, 2, 2, 4, 1};
 
-  //  if (dev_run)
-  printf("\nModel with no column lower or upper bounds\n");
+  if (dev_run) printf("\nModel with no column lower or upper bounds\n");
   REQUIRE(h.writeLocalModel(model, write_model_file) == HighsStatus::kError);
   lp.col_lower_ = {0, 0};
 
-  //  if (dev_run)
-  printf("\nModel with no column upper bounds\n");
+  if (dev_run) printf("\nModel with no column upper bounds\n");
   REQUIRE(h.writeLocalModel(model, write_model_file) == HighsStatus::kError);
   lp.col_upper_ = {inf, inf};
 
   // Model has no dimensions for a_matrix_, but these are set in
-  // writeLocalModel. 
-  printf("\nModel with no column or row names\n");
+  // writeLocalModel.
+  if (dev_run) printf("\nModel with no column or row names\n");
   REQUIRE(h.writeLocalModel(model, write_model_file) == HighsStatus::kWarning);
   lp.col_names_ = {"C0", "C1"};
   lp.row_names_ = {"R0", "R1", "R2"};
 
-  printf("\nModel with column and row names\n");
+  if (dev_run) printf("\nModel with column and row names\n");
   REQUIRE(h.writeLocalModel(model, write_model_file) == HighsStatus::kOk);
 
+  // Introduce illegal start
+  if (dev_run) printf("\nModel with start entry > num_nz\n");
+  lp.a_matrix_.start_ = {0, 7, 6};
+  REQUIRE(h.writeLocalModel(model, write_model_file) == HighsStatus::kError);
+
+  // Introduce illegal start
+  if (dev_run) printf("\nModel with start entry -1\n");
+  lp.a_matrix_.start_ = {0, -1, 6};
+  REQUIRE(h.writeLocalModel(model, write_model_file) == HighsStatus::kError);
+  lp.a_matrix_.start_ = {0, 3, 6};
+
   // Introduce illegal index
+  if (dev_run) printf("\nModel with index entry -1\n");
   lp.a_matrix_.index_ = {0, -1, 2, 0, 1, 2};
   REQUIRE(h.writeLocalModel(model, write_model_file) == HighsStatus::kError);
-
+
+  // Introduce illegal index
+  if (dev_run) printf("\nModel with index entry 3 >= num_row\n");
+  lp.a_matrix_.index_ = {0, 1, 3, 0, 1, 2};
+  REQUIRE(h.writeLocalModel(model, write_model_file) == HighsStatus::kError);
+
   std::remove(write_model_file.c_str());
 }
diff --git a/src/lp_data/Highs.cpp b/src/lp_data/Highs.cpp
@@ -705,18 +705,32 @@ HighsStatus Highs::writePresolvedModel(const std::string& filename) {
 HighsStatus Highs::writeLocalModel(HighsModel& model,
                                    const std::string& filename) {
   HighsStatus return_status = HighsStatus::kOk;
+  HighsStatus call_status;
 
   HighsLp& lp = model.lp_;
   // Dimensions in a_matrix_ may not be set, so take them from lp
   lp.setMatrixDimensions();
+
   // Ensure that the LP is column-wise
   lp.ensureColwise();
+
   // Ensure that the dimensions are OK
-  if (!lpDimensionsOk("writeLocalModel", lp, options_.log_options)) return HighsStatus::kError;
+  if (!lpDimensionsOk("writeLocalModel", lp, options_.log_options))
+    return HighsStatus::kError;
+
   if (model.hessian_.dim_ > 0) {
-    HighsStatus call_status = assessHessianDimensions(options_, model.hessian_);
+    call_status = assessHessianDimensions(options_, model.hessian_);
     if (call_status == HighsStatus::kError) return call_status;
   }
+
+  // Check that the matrix starts are OK
+  call_status = lp.a_matrix_.assessStart(options_.log_options);
+  if (call_status == HighsStatus::kError) return call_status;
+
+  // Check that the matrix indices are within bounds
+  call_status = lp.a_matrix_.assessIndexBounds(options_.log_options);
+  if (call_status == HighsStatus::kError) return call_status;
+
   // Check for repeated column or row names that would corrupt the file
   if (model.lp_.col_hash_.hasDuplicate(model.lp_.col_names_)) {
     highsLogUser(options_.log_options, HighsLogType::kError,

diff --git a/src/model/HighsHessianUtils.cpp b/src/model/HighsHessianUtils.cpp
@@ -28,7 +28,8 @@ HighsStatus assessHessian(HighsHessian& hessian, const HighsOptions& options) {
   HighsStatus return_status = HighsStatus::kOk;
   HighsStatus call_status;
 
-  return_status = interpretCallStatus(options.log_options, assessHessianDimensions(options, hessian),
+  return_status = interpretCallStatus(options.log_options,
+                                      assessHessianDimensions(options, hessian),
                                       return_status, "assessHessianDimensions");
   if (return_status == HighsStatus::kError) return return_status;
 
@@ -111,8 +112,9 @@ HighsStatus assessHessianDimensions(const HighsOptions& options,
   // Assess the Hessian dimensions and vector sizes
   vector<HighsInt> hessian_p_end;
   const bool partitioned = false;
-  return assessMatrixDimensions(options.log_options, hessian.dim_, partitioned, hessian.start_,
-				hessian_p_end, hessian.index_, hessian.value_);
+  return assessMatrixDimensions(options.log_options, hessian.dim_, partitioned,
+                                hessian.start_, hessian_p_end, hessian.index_,
+                                hessian.value_);
 }
 
 void completeHessianDiagonal(const HighsOptions& options,

diff --git a/src/util/HighsMatrixUtils.cpp b/src/util/HighsMatrixUtils.cpp
@@ -68,7 +68,7 @@ HighsStatus assessMatrix(
   //
   // Check whether the first start is zero
   if (matrix_start[0]) {
-    highsLogUser(log_options, HighsLogType::kWarning,
+    highsLogUser(log_options, HighsLogType::kError,
                  "%s matrix start vector begins with %" HIGHSINT_FORMAT
                  " rather than 0\n",
                  matrix_name.c_str(), matrix_start[0]);

diff --git a/src/util/HighsSparseMatrix.cpp b/src/util/HighsSparseMatrix.cpp
@@ -722,6 +722,65 @@ void HighsSparseMatrix::deleteRows(
   this->num_row_ = new_num_row;
 }
 
+HighsStatus HighsSparseMatrix::assessStart(const HighsLogOptions& log_options) {
+  // Identify main dimensions
+  HighsInt vec_dim;
+  HighsInt num_vec;
+  if (this->isColwise()) {
+    vec_dim = this->num_row_;
+    num_vec = this->num_col_;
+  } else {
+    vec_dim = this->num_col_;
+    num_vec = this->num_row_;
+  }
+  if (this->start_[0]) {
+    highsLogUser(log_options, HighsLogType::kError,
+                 "Matrix start[0] = %d, not 0\n", int(this->start_[0]));
+    return HighsStatus::kError;
+  }
+  HighsInt num_nz = this->numNz();
+  for (HighsInt iVec = 1; iVec < num_vec; iVec++) {
+    if (this->start_[iVec] < this->start_[iVec - 1]) {
+      highsLogUser(log_options, HighsLogType::kError,
+                   "Matrix start[%d] = %d > %d = start[%d]\n", int(iVec),
+                   int(this->start_[iVec]), int(this->start_[iVec - 1]),
+                   int(iVec - 1));
+      return HighsStatus::kError;
+    }
+    if (this->start_[iVec] > num_nz) {
+      highsLogUser(log_options, HighsLogType::kError,
+                   "Matrix start[%d] = %d > %d = number of nonzeros\n",
+                   int(iVec), int(this->start_[iVec]), int(num_nz));
+      return HighsStatus::kError;
+    }
+  }
+  return HighsStatus::kOk;
+}
+
+HighsStatus HighsSparseMatrix::assessIndexBounds(
+    const HighsLogOptions& log_options) {
+  // Identify main dimensions
+  HighsInt vec_dim;
+  HighsInt num_vec;
+  if (this->isColwise()) {
+    vec_dim = this->num_row_;
+    //    num_vec = this->num_col_;
+  } else {
+    vec_dim = this->num_col_;
+    //    num_vec = this->num_row_;
+  }
+  HighsInt num_nz = this->numNz();
+  for (HighsInt iEl = 1; iEl < num_nz; iEl++) {
+    if (this->index_[iEl] < 0 || this->index_[iEl] >= vec_dim) {
+      highsLogUser(log_options, HighsLogType::kError,
+                   "Matrix index[%d] = %d is not in legal range of [0, %d)\n",
+                   int(iEl), int(this->index_[iEl]), vec_dim);
+      return HighsStatus::kError;
+    }
+  }
+  return HighsStatus::kOk;
+}
+
 HighsStatus HighsSparseMatrix::assess(const HighsLogOptions& log_options,
                                       const std::string matrix_name,
                                       const double small_matrix_value,

diff --git a/src/util/HighsSparseMatrix.h b/src/util/HighsSparseMatrix.h
@@ -66,6 +66,9 @@ class HighsSparseMatrix {
   void deleteRows(const HighsIndexCollection& index_collection);
   HighsStatus assessDimensions(const HighsLogOptions& log_options,
                                const std::string matrix_name);
+  HighsStatus assessStart(const HighsLogOptions& log_options);
+  HighsStatus assessIndexBounds(const HighsLogOptions& log_options);
+
   HighsStatus assess(const HighsLogOptions& log_options,
                      const std::string matrix_name,
                      const double small_matrix_value,