From 9a0c66c80c0523f1c293f1b9b063c246992ad2ca Mon Sep 17 00:00:00 2001
From: Corentin Le Molgat <corentinl@google.com>
Date: Fri, 2 Jun 2023 13:18:34 +0200
Subject: [PATCH] pdlp: sync

---
 ortools/pdlp/primal_dual_hybrid_gradient.cc      | 6 ++++++
 ortools/pdlp/primal_dual_hybrid_gradient_test.cc | 9 +++++++++
 ortools/pdlp/quadratic_program.cc                | 3 +--
 ortools/pdlp/solvers_proto_validation.cc         | 9 ++++++---
 ortools/pdlp/solvers_proto_validation_test.cc    | 9 +++++++++
 5 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/ortools/pdlp/primal_dual_hybrid_gradient.cc b/ortools/pdlp/primal_dual_hybrid_gradient.cc
index 229de54224..2c98bbfc9a 100644
--- a/ortools/pdlp/primal_dual_hybrid_gradient.cc
+++ b/ortools/pdlp/primal_dual_hybrid_gradient.cc
@@ -2808,6 +2808,12 @@ SolverResult PrimalDualHybridGradient(
     return ErrorSolverResult(TERMINATION_REASON_INVALID_PROBLEM,
                              "The objective scaling factor cannot be zero.");
   }
+  if (qp.objective_matrix.has_value() &&
+      qp.objective_matrix->diagonal().minCoeff() < 0.0) {
+    return ErrorSolverResult(TERMINATION_REASON_INVALID_PROBLEM,
+                             "The objective is not convex (i.e., the objective "
+                             "matrix contains negative entries).");
+  }
   if (params.use_feasibility_polishing() && !IsLinearProgram(qp)) {
     return ErrorSolverResult(
         TERMINATION_REASON_INVALID_PARAMETER,
diff --git a/ortools/pdlp/primal_dual_hybrid_gradient_test.cc b/ortools/pdlp/primal_dual_hybrid_gradient_test.cc
index 5ec0a04b16..78b0192e3f 100644
--- a/ortools/pdlp/primal_dual_hybrid_gradient_test.cc
+++ b/ortools/pdlp/primal_dual_hybrid_gradient_test.cc
@@ -1052,6 +1052,15 @@ TEST(PrimalDualHybridGradientTest, DetectsProblemWithInconsistentBounds) {
             TERMINATION_REASON_INVALID_PROBLEM);
 }
 
+TEST(PrimalDualHybridGradientTest, DetectsNonconvexQp) {
+  QuadraticProgram qp = TestDiagonalQp1();
+  qp.objective_matrix->diagonal()[0] = -1.0;
+  SolverResult output =
+      PrimalDualHybridGradient(qp, PrimalDualHybridGradientParams());
+  EXPECT_EQ(output.solve_log.termination_reason(),
+            TERMINATION_REASON_INVALID_PROBLEM);
+}
+
 TEST(PrimalDualHybridGradientTest, DetectsProblemWithInconsistentSizes) {
   QuadraticProgram qp = TinyLp();
   qp.objective_vector.resize(0);
diff --git a/ortools/pdlp/quadratic_program.cc b/ortools/pdlp/quadratic_program.cc
index 78556943c8..688ac904e2 100644
--- a/ortools/pdlp/quadratic_program.cc
+++ b/ortools/pdlp/quadratic_program.cc
@@ -18,7 +18,6 @@
 #include <limits>
 #include <string>
 #include <tuple>
-#include <utility>
 #include <vector>
 
 #include "Eigen/Core"
@@ -205,7 +204,7 @@ absl::StatusOr<QuadraticProgram> QpFromMpModelProto(
     }
     qp.objective_scaling_factor = -1;
   }
-  return std::move(qp);
+  return qp;
 }
 
 absl::Status CanFitInMpModelProto(const QuadraticProgram& qp) {
diff --git a/ortools/pdlp/solvers_proto_validation.cc b/ortools/pdlp/solvers_proto_validation.cc
index 4c9290c8d2..ad880ad5f9 100644
--- a/ortools/pdlp/solvers_proto_validation.cc
+++ b/ortools/pdlp/solvers_proto_validation.cc
@@ -14,6 +14,7 @@
 #include "ortools/pdlp/solvers_proto_validation.h"
 
 #include <cmath>
+#include <limits>
 
 #include "absl/status/status.h"
 #include "absl/strings/str_cat.h"
@@ -271,9 +272,11 @@ absl::Status ValidatePrimalDualHybridGradientParams(
     return InvalidArgumentError(
         "diagonal_qp_trust_region_solver_tolerance is NAN");
   }
-  if (params.diagonal_qp_trust_region_solver_tolerance() < 0.0) {
-    return InvalidArgumentError(
-        "diagonal_qp_trust_region_solver_tolerance must be non-negative");
+  if (params.diagonal_qp_trust_region_solver_tolerance() <
+      10 * std::numeric_limits<double>::epsilon()) {
+    return InvalidArgumentError(absl::StrCat(
+        "diagonal_qp_trust_region_solver_tolerance must be at least ",
+        10 * std::numeric_limits<double>::epsilon()));
   }
   if (params.use_feasibility_polishing() &&
       params.handle_some_primal_gradients_on_finite_bounds_as_residuals()) {
diff --git a/ortools/pdlp/solvers_proto_validation_test.cc b/ortools/pdlp/solvers_proto_validation_test.cc
index 2a954e55a8..b8a05f6adc 100644
--- a/ortools/pdlp/solvers_proto_validation_test.cc
+++ b/ortools/pdlp/solvers_proto_validation_test.cc
@@ -652,6 +652,15 @@ TEST(ValidatePrimalDualHybridGradientParams,
   EXPECT_EQ(status_nan.code(), absl::StatusCode::kInvalidArgument);
   EXPECT_THAT(status_nan.message(),
               HasSubstr("diagonal_qp_trust_region_solver_tolerance"));
+
+  PrimalDualHybridGradientParams params_tiny;
+  params_tiny.set_diagonal_qp_trust_region_solver_tolerance(
+      std::numeric_limits<double>::epsilon());
+  const absl::Status status_tiny =
+      ValidatePrimalDualHybridGradientParams(params_tiny);
+  EXPECT_EQ(status_tiny.code(), absl::StatusCode::kInvalidArgument);
+  EXPECT_THAT(status_tiny.message(),
+              HasSubstr("diagonal_qp_trust_region_solver_tolerance"));
 }
 
 TEST(ValidatePrimalDualHybridGradientParams, FeasibilityPolishingValidOptions) {