Fix MPS writer emitting an empty QCMATRIX name for unnamed quadratic constraints

cpp/src/io/mps_writer.cpp

@@ -21,6 +21,7 @@
 #include <limits>
 #include <map>
 #include <memory>
+#include <unordered_set>
 #include <vector>
 
 namespace cuopt::mathematical_optimization::io {
@@ -239,6 +240,33 @@ void mps_writer_t<i_t, f_t>::write(const std::string& mps_file_path)
 
   if (problem_.get_sense()) { mps_file << "OBJSENSE\n MAXIMIZE\n"; }
 
+  // Unnamed quadratic constraints get QC<q>, made unique against every other row name so the
+  // reader cannot mistake one for another row or the objective.
+  std::vector<std::string> quadratic_row_names(quadratic_constraints.size());
+  {
+    std::unordered_set<std::string> used_names;
+    used_names.insert(problem_.get_objective_name().empty() ? "OBJ"
+                                                            : problem_.get_objective_name());
+    for (size_t k = 0; k < static_cast<size_t>(n_constraints); ++k) {
+      used_names.insert(k < problem_.get_row_names().size() ? problem_.get_row_names()[k]
+                                                            : "R" + std::to_string(k));
+    }
+    for (const auto& qc : quadratic_constraints) {
+      if (!qc.constraint_row_name.empty()) { used_names.insert(qc.constraint_row_name); }
+    }
+    for (size_t q = 0; q < quadratic_constraints.size(); ++q) {
+      std::string name = quadratic_constraints[q].constraint_row_name;
+      if (name.empty()) {
+        name = "QC" + std::to_string(q);
+        while (used_names.count(name) != 0) {
+          name += "_";
+        }
+        used_names.insert(name);
+      }
+      quadratic_row_names[q] = std::move(name);
+    }
+  }
+
   // ROWS section
   mps_file << "ROWS\n";
   mps_file << " N  "
@@ -252,10 +280,9 @@ void mps_writer_t<i_t, f_t>::write(const std::string& mps_file_path)
     mps_file << " " << type << "  " << row_name << "\n";
   }
   for (size_t q = 0; q < quadratic_constraints.size(); ++q) {
-    const auto& qc = quadratic_constraints[q];
-    std::string row_name =
-      qc.constraint_row_name.empty() ? "QC" + std::to_string(q) : qc.constraint_row_name;
-    char const type = qc.constraint_row_type;
+    const auto& qc              = quadratic_constraints[q];
+    const std::string& row_name = quadratic_row_names[q];
+    char const type             = qc.constraint_row_type;
     mps_file << " " << type << "  " << row_name << "\n";
   }
 
@@ -340,9 +367,7 @@ void mps_writer_t<i_t, f_t>::write(const std::string& mps_file_path)
                        : "R" + std::to_string(nnz.first);
         } else if (nnz.first < static_cast<size_t>(n_constraints) + quadratic_constraints.size()) {
           const size_t q = nnz.first - static_cast<size_t>(n_constraints);
-          row_name       = quadratic_constraints[q].constraint_row_name.empty()
-                             ? "QC" + std::to_string(q)
-                             : quadratic_constraints[q].constraint_row_name;
+          row_name       = quadratic_row_names[q];
         } else {
           row_name = "R" + std::to_string(nnz.first);
         }
@@ -372,10 +397,9 @@ void mps_writer_t<i_t, f_t>::write(const std::string& mps_file_path)
     }
   }
   for (size_t q = 0; q < quadratic_constraints.size(); ++q) {
-    const auto& qc = quadratic_constraints[q];
-    std::string row_name =
-      qc.constraint_row_name.empty() ? "QC" + std::to_string(q) : qc.constraint_row_name;
-    const f_t rhs = qc.rhs_value;
+    const auto& qc              = quadratic_constraints[q];
+    const std::string& row_name = quadratic_row_names[q];
+    const f_t rhs               = qc.rhs_value;
     if (std::isfinite(rhs) && rhs != 0.0) {
       mps_file << "    RHS1      " << row_name << " " << rhs << "\n";
     }
@@ -499,8 +523,9 @@ void mps_writer_t<i_t, f_t>::write(const std::string& mps_file_path)
   // QCMATRIX sections for quadratic constraints (QCQP)
   if (problem_.has_quadratic_constraints()) {
     coo_canonicalization_scratch_t<i_t, f_t> qc_scratch;
+    size_t qc_index = 0;
     for (const auto& qc : problem_.get_quadratic_constraints()) {
-      mps_file << "QCMATRIX   " << qc.constraint_row_name << "\n";
+      mps_file << "QCMATRIX   " << quadratic_row_names[qc_index++] << "\n";
       typename mps_data_model_t<i_t, f_t>::quadratic_constraint_t qc_canon = qc;
       canonicalize_coo_matrix(qc_canon.rows, qc_canon.cols, qc_canon.vals, qc_scratch);
       const i_t nnz = static_cast<i_t>(qc_canon.vals.size());

cpp/tests/linear_programming/parser_test.cpp

@@ -1276,6 +1276,106 @@ TEST(mps_roundtrip, empty_constraint_matrix)
   EXPECT_THAT(reloaded.get_row_names(), ElementsAre("empty"));
 }
 
+TEST(mps_roundtrip, unnamed_quadratic_constraint)
+{
+  mps_data_model_t<int, double> model;
+  std::vector<double> values{1.0, 1.0};
+  std::vector<int> indices{0, 1};
+  std::vector<int> offsets{0, 2};
+  std::vector<double> rhs{10.0};
+  std::vector<double> objective{1.0, 1.0};
+  std::vector<double> lower_bounds{0.0, 0.0};
+  std::vector<double> upper_bounds{5.0, 5.0};
+  std::vector<char> row_types{'L'};
+
+  model.set_csr_constraint_matrix(values, indices, offsets);
+  model.set_constraint_bounds(rhs);
+  model.set_objective_coefficients(objective);
+  model.set_variable_lower_bounds(lower_bounds);
+  model.set_variable_upper_bounds(upper_bounds);
+  model.set_row_types(row_types);
+  model.set_variable_names({"x", "y"});
+  model.set_row_names({"lin"});
+
+  const std::vector<double> qc_linear_values{1.0, 1.0};
+  const std::vector<int> qc_linear_indices{0, 1};
+  const std::vector<double> qc_values{1.0, 1.0};
+  const std::vector<int> qc_rows{0, 1};
+  const std::vector<int> qc_cols{0, 1};
+  model.append_quadratic_constraint(
+    1, "", 'L', qc_linear_values, qc_linear_indices, 4.0, qc_values, qc_rows, qc_cols);
+
+  temp_file_t temp_file(".mps");
+  mps_writer_t<int, double> writer(model);
+  writer.write(temp_file.string());
+
+  auto reloaded = read_mps<int, double>(temp_file.string(), false);
+  ASSERT_EQ(1u, reloaded.get_quadratic_constraints().size());
+  EXPECT_EQ("QC0", reloaded.get_quadratic_constraints()[0].constraint_row_name);
+  EXPECT_EQ(4.0, reloaded.get_quadratic_constraints()[0].rhs_value);
+}
+
+TEST(mps_roundtrip, unnamed_quadratic_constraints_avoid_existing_row_names)
+{
+  mps_data_model_t<int, double> model;
+  std::vector<double> values{1.0, 1.0};
+  std::vector<int> indices{0, 1};
+  std::vector<int> offsets{0, 2};
+  std::vector<double> rhs{10.0};
+  std::vector<double> objective{2.0, 3.0};
+  std::vector<double> lower_bounds{0.0, 0.0};
+  std::vector<double> upper_bounds{5.0, 5.0};
+  std::vector<char> row_types{'L'};
+
+  model.set_csr_constraint_matrix(values, indices, offsets);
+  model.set_constraint_bounds(rhs);
+  model.set_objective_coefficients(objective);
+  model.set_variable_lower_bounds(lower_bounds);
+  model.set_variable_upper_bounds(upper_bounds);
+  model.set_row_types(row_types);
+  model.set_variable_names({"x", "y"});
+  model.set_objective_name("QC0");
+  model.set_row_names({"QC1"});
+
+  const std::vector<double> qc_linear_values{1.0, 1.0};
+  const std::vector<int> qc_linear_indices{0, 1};
+  const std::vector<double> qc0_values{1.0, 1.0};
+  const std::vector<int> qc0_rows{0, 1};
+  const std::vector<int> qc0_cols{0, 1};
+  const std::vector<double> qc1_values{3.0, 2.0, 5.0};
+  const std::vector<int> qc1_rows{0, 0, 1};
+  const std::vector<int> qc1_cols{0, 1, 1};
+  model.append_quadratic_constraint(
+    1, "", 'L', qc_linear_values, qc_linear_indices, 4.0, qc0_values, qc0_rows, qc0_cols);
+  model.append_quadratic_constraint(
+    2, "", 'L', qc_linear_values, qc_linear_indices, 6.0, qc1_values, qc1_rows, qc1_cols);
+
+  temp_file_t temp_file(".mps");
+  mps_writer_t<int, double> writer(model);
+  writer.write(temp_file.string());
+
+  auto reloaded = read_mps<int, double>(temp_file.string(), false);
+  EXPECT_EQ("QC0", reloaded.get_objective_name());
+  EXPECT_THAT(reloaded.get_row_names(), ElementsAre("QC1"));
+  EXPECT_THAT(reloaded.get_objective_coefficients(), ElementsAre(2.0, 3.0));
+  EXPECT_EQ(0.0, reloaded.get_objective_offset());
+  ASSERT_EQ(2u, reloaded.get_quadratic_constraints().size());
+  const auto& qc0 = reloaded.get_quadratic_constraints()[0];
+  const auto& qc1 = reloaded.get_quadratic_constraints()[1];
+  EXPECT_THAT(qc0.linear_values, ElementsAre(1.0, 1.0));
+  EXPECT_THAT(qc1.linear_values, ElementsAre(1.0, 1.0));
+  EXPECT_EQ(4.0, qc0.rhs_value);
+  EXPECT_EQ(6.0, qc1.rhs_value);
+  EXPECT_THAT(qc0.vals, ElementsAre(1.0, 1.0));
+  EXPECT_THAT(qc0.rows, ElementsAre(0, 1));
+  EXPECT_THAT(qc0.cols, ElementsAre(0, 1));
+  EXPECT_THAT(qc1.vals, ElementsAre(3.0, 2.0, 5.0));
+  EXPECT_THAT(qc1.rows, ElementsAre(0, 0, 1));
+  EXPECT_THAT(qc1.cols, ElementsAre(0, 1, 1));
+  EXPECT_NE(reloaded.get_quadratic_constraints()[0].constraint_row_name,
+            reloaded.get_quadratic_constraints()[1].constraint_row_name);
+}
+
 TEST(mps_writer_op, rejects_missing_constraint_offsets)
 {
   mps_data_model_t<int, double> model;