Set Xpress specific parameters before generic ones (#129)

* [XPRESS] Extract variable & constraint names only before writing MPS (#111) * Set Xpress specific parameters before generic ones, add tests * Improve comment Co-authored-by: Peter Mitri <[email protected]> * Fix formatting * Formatting --------- Co-authored-by: Florian Omnès <[email protected]> Co-authored-by: Peter Mitri <[email protected]>
rte-france · May 24, 2024 · 9b1c3fe · 9b1c3fe
1 parent e753fb6
commit 9b1c3fe
Show file tree

Hide file tree

Showing 2 changed files with 63 additions and 5 deletions.
diff --git a/ortools/linear_solver/xpress_interface.cc b/ortools/linear_solver/xpress_interface.cc
@@ -1824,13 +1824,15 @@ MPSolver::ResultStatus XpressInterface::Solve(MPSolverParameters const& param) {
   // Set log level.
   XPRSsetintcontrol(mLp, XPRS_OUTPUTLOG, quiet() ? 0 : 1);
   // Set parameters.
-  // NOTE: We must invoke SetSolverSpecificParametersAsString() _first_.
-  //       Its current implementation invokes ReadParameterFile() which in
-  //       turn invokes XPRSreadcopyparam(). The latter will _overwrite_
-  //       all current parameter settings in the environment.
+  // We first set our internal MPSolverParameters from 'param' and then set
+  // any user-specified internal solver parameters via
+  // solver_specific_parameter_string_.
+  // Default MPSolverParameters can override custom parameters while specific
+  // parameters allow a higher level of customization (for example for
+  // presolving) and therefore we apply MPSolverParameters first.
+  SetParameters(param);
   solver_->SetSolverSpecificParametersAsString(
       solver_->solver_specific_parameter_string_);
-  SetParameters(param);
   if (solver_->time_limit()) {
     VLOG(1) << "Setting time limit = " << solver_->time_limit() << " ms.";
     // In Xpress, a time limit should usually have a negative sign. With a

diff --git a/ortools/linear_solver/xpress_interface_test.cc b/ortools/linear_solver/xpress_interface_test.cc
@@ -768,6 +768,14 @@ TEST_F(XpressFixtureLP, SetPrimalTolerance) {
   EXPECT_EQ(getter.getDoubleControl(XPRS_FEASTOL), tol);
 }
 
+TEST_F(XpressFixtureLP, SetPrimalToleranceNotOverridenByMPSolverParameters) {
+  double tol = 1e-4;  // Choose a value different from kDefaultPrimalTolerance
+  std::string xpressParamString = "FEASTOL " + std::to_string(tol);
+  solver.SetSolverSpecificParametersAsString(xpressParamString);
+  solver.Solve();
+  EXPECT_EQ(getter.getDoubleControl(XPRS_FEASTOL), tol);
+}
+
 TEST_F(XpressFixtureLP, SetDualTolerance) {
   MPSolverParameters params;
   double tol = 1e-2;
@@ -776,6 +784,14 @@ TEST_F(XpressFixtureLP, SetDualTolerance) {
   EXPECT_EQ(getter.getDoubleControl(XPRS_OPTIMALITYTOL), tol);
 }
 
+TEST_F(XpressFixtureLP, SetDualToleranceNotOverridenByMPSolverParameters) {
+  double tol = 1e-4;  // Choose a value different from kDefaultDualTolerance
+  std::string xpressParamString = "OPTIMALITYTOL " + std::to_string(tol);
+  solver.SetSolverSpecificParametersAsString(xpressParamString);
+  solver.Solve();
+  EXPECT_EQ(getter.getDoubleControl(XPRS_OPTIMALITYTOL), tol);
+}
+
 TEST_F(XpressFixtureMIP, SetPresolveMode) {
   MPSolverParameters params;
   params.SetIntegerParam(MPSolverParameters::PRESOLVE,
@@ -788,6 +804,17 @@ TEST_F(XpressFixtureMIP, SetPresolveMode) {
   EXPECT_EQ(getter.getIntegerControl(XPRS_PRESOLVE), 1);
 }
 
+TEST_F(XpressFixtureMIP, SetPresolveModeNotOverridenByMPSolverParameters) {
+  // Test all presolve modes of Xpress
+  std::vector<int> presolveModes{-1, 0, 1, 2, 3};
+  for (int presolveMode : presolveModes) {
+    std::string xpressParamString = "PRESOLVE " + std::to_string(presolveMode);
+    solver.SetSolverSpecificParametersAsString(xpressParamString);
+    solver.Solve();
+    EXPECT_EQ(getter.getIntegerControl(XPRS_PRESOLVE), presolveMode);
+  }
+}
+
 TEST_F(XpressFixtureLP, SetLpAlgorithm) {
   MPSolverParameters params;
   params.SetIntegerParam(MPSolverParameters::LP_ALGORITHM,
@@ -804,6 +831,16 @@ TEST_F(XpressFixtureLP, SetLpAlgorithm) {
   EXPECT_EQ(getter.getIntegerControl(XPRS_DEFAULTALG), 4);
 }
 
+TEST_F(XpressFixtureLP, SetLPAlgorithmNotOverridenByMPSolverParameters) {
+  std::vector<int> defaultAlgs{1, 2, 3, 4};
+  for (int defaultAlg : defaultAlgs) {
+    std::string xpressParamString = "DEFAULTALG " + std::to_string(defaultAlg);
+    solver.SetSolverSpecificParametersAsString(xpressParamString);
+    solver.Solve();
+    EXPECT_EQ(getter.getIntegerControl(XPRS_DEFAULTALG), defaultAlg);
+  }
+}
+
 TEST_F(XpressFixtureMIP, SetScaling) {
   MPSolverParameters params;
   params.SetIntegerParam(MPSolverParameters::SCALING,
@@ -816,6 +853,17 @@ TEST_F(XpressFixtureMIP, SetScaling) {
   EXPECT_EQ(getter.getIntegerControl(XPRS_SCALING), 163);
 }
 
+TEST_F(XpressFixtureMIP, SetScalingNotOverridenByMPSolverParameters) {
+  // Scaling is a bitmap on 16 bits in Xpress, test only a random value among
+  // all possible
+  int scaling = 2354;
+
+  std::string xpressParamString = "SCALING " + std::to_string(scaling);
+  solver.SetSolverSpecificParametersAsString(xpressParamString);
+  solver.Solve();
+  EXPECT_EQ(getter.getIntegerControl(XPRS_SCALING), scaling);
+}
+
 TEST_F(XpressFixtureMIP, SetRelativeMipGap) {
   MPSolverParameters params;
   double relativeMipGap = 1e-3;
@@ -824,6 +872,14 @@ TEST_F(XpressFixtureMIP, SetRelativeMipGap) {
   EXPECT_EQ(getter.getDoubleControl(XPRS_MIPRELSTOP), relativeMipGap);
 }
 
+TEST_F(XpressFixtureMIP, SetRelativeMipGapNotOverridenByMPSolverParameters) {
+  double gap = 1e-2;  // Choose a value different from kDefaultRelativeMipGap
+  std::string xpressParamString = "MIPRELSTOP " + std::to_string(gap);
+  solver.SetSolverSpecificParametersAsString(xpressParamString);
+  solver.Solve();
+  EXPECT_EQ(getter.getDoubleControl(XPRS_MIPRELSTOP), gap);
+}
+
 TEST(XpressInterface, setStringControls) {
   std::vector<std::tuple<std::string, int, std::string>> params = {
       {"MPSRHSNAME", XPRS_MPSRHSNAME, "default_value"},