Xpress use optimizer function #4888
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
can you rebase this against |
Mizux
added
Solver: XPRESS
Now that we go through `XPRSoptimize()` this should work out of the box.
Co-authored-by: Francesco Cavaliere <[email protected]>
Co-authored-by: Francesco Cavaliere <[email protected]>
Co-authored-by: Francesco Cavaliere <[email protected]>
Co-authored-by: Francesco Cavaliere <[email protected]>
Co-authored-by: Francesco Cavaliere <[email protected]>
Co-authored-by: Francesco Cavaliere <[email protected]>
Co-authored-by: Francesco Cavaliere <[email protected]>
Co-authored-by: Francesco Cavaliere <[email protected]>
Co-authored-by: Francesco Cavaliere <[email protected]>
djunglas
force-pushed
the
xpress-use-optimizer-function
branch
from
b56c9cd to
440c4b8
Compare
@lperron, I rebased on |
|
I have no idea with CLA
I will check
Laurent Perron | Operations Research | ***@***.*** | (33) 1 42 68 53
00
Le mer. 22 oct. 2025 à 09:46, Daniel Junglas ***@***.***> a
écrit :
… *djunglas* left a comment (google/or-tools#4888)
<#4888 (comment)>
can you rebase this against main ? The directory structure has changed.
In particular, the xpress environment code is now in
ortools/third_party_solvers/
@lperron <https://github.com/lperron>, I rebased on main as requested
(luckily this was easy).
About the CLA I am supposed to sign: do these things expire? I am pretty
sure I contributed before and signed a CLA for that purpose. I can sign a
new one but if I remember correctly then last time this required a longish
discussion with our legal department, so if the old CLA can be made
working, I would prefer that.
—
Reply to this email directly, view it on GitHub
<#4888 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACUPL3LXGESMGJHXRK4FPHL3Y4Y7DAVCNFSM6AAAAACJZFKROWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTIMZQHA4DAMZYGM>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
There was a problem hiding this comment.
Thank you so much for this amazing work!
It's a very big PR and I can only imagine the amount of work it took you guys to develop and test all this!
I tried to review as much as possible the code overall; I could not read every single line of code since there is a lot of changes (especially in g_xpress & xpress_solver).
I did, however, use Xpress 9.6.0 to run the unit tests on my machine & do small manual tests for some features. This allowed me to leave you some feedback.
I have not yet tested all this with real-life problems; we'll come back here if anything pops up!
As a side note, your PR is very big, and I noticed that you did not list all the new features in its decription, so I made you a list of forgotten features:
- Multi-objective support (but only the main one can contain quadratic terms)
- Quadratic constraints support
- First order support
- Support for the following generic solve parameters: presolve, cut-off, thread count, time limit, node limit, absolute and relative gap tolerance, cuts, heuristics
- Extra Xpress-specific parameter to force post-solve ("FORCE_POSTSOLVE")
Best,
Peter
| // size (nVars, nCons, and nVars respectively) | ||
| absl::Status GetLpSol(absl::Span<double> primals, absl::Span<double> duals, | ||
| absl::Span<double> reducedCosts); | ||
| absl::Status MipOptimize(); |
There was a problem hiding this comment.
you can delete this function
|
|
||
| // We test SOS1 constraints with both explicit weights and default weights. | ||
| TEST_P(SimpleLogicalConstraintTest, CanBuildSos1Model) { | ||
| if (GetParam().solver_type == SolverType::kXpress) { |
There was a problem hiding this comment.
I would suggest adding test suite parameters for these skipped tests (like supports_integer_variables, supports_indicator_constraints, etc).
maybe we can get feedback from someone on the OR-Tools team on this.
| * Also, ortools supports SOSs with identical elements and assumes that | ||
| * something like { x, x } is reduced to just { x }. This is debatable: | ||
| * If you consider { x, x } a set, then clearly it is the same as { x }. | ||
| * But if you consider "at most one of x and x may be non-zero", then { x, x } | ||
| * implies x=0. This is not how ortools interprets SOSs with duplicate entries | ||
| * (see the tests). | ||
| * We do not check for duplicate entries here, but Xpress will choke on them. |
There was a problem hiding this comment.
Concerning identical elements, I agree that it should be up to the OR-Tools user to ensure that they provide "clean" inputs, i.e. without duplicates (Gurobi did stop supporting this in their API). This is a pretty advanced feature, and one may assume that the user is being / must be careful when using it. In other words, I would expect this function to fail in case of duplicates.
I suggest that we debate this with the OR-Tools team and remove this comment.
| * Note that in ortools an SOS constraint is made up from expressions and not | ||
| * just variables. Here, we only support SOSs in which each expression is just | ||
| * a single variable with coefficient 1. |
There was a problem hiding this comment.
Looking at what Google did for Gurobi; we'd have to create a variable equal to the expression first, and then use that variable to define the SOS, right? (asking in case we add this feature later)
There was a problem hiding this comment.
Now that you've added support for MIP, hints, branching priorities, and lazy constraints, you should activate the relevant tests. First, remove the following lines from this file:
GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(SimpleMipTest);
GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(MipSolutionHintTest);
GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(LazyConstraintsTest);
GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(BranchPrioritiesTest);Then, activate these tests by instantiating the tests suites. I tested it ou on my PC by adding the following code:
INSTANTIATE_TEST_SUITE_P(XpressSimpleMipTest, SimpleMipTest,
testing::Values(SolverType::kXpress));
INSTANTIATE_TEST_SUITE_P(
XpressMipSolutionHintTest, MipSolutionHintTest,
testing::Values(SolutionHintTestParams(
SolverType::kXpress, SolveParameters(),
SolverType::kXpress, SolveParameters(),
"User solution (.*) stored")));
INSTANTIATE_TEST_SUITE_P(XpressLazyConstraintsTest, LazyConstraintsTest,
testing::Values(LazyConstraintsTestParams(
SolverType::kXpress, SolveParameters())));
INSTANTIATE_TEST_SUITE_P(XpressBranchPrioritiesTest, BranchPrioritiesTest,
testing::Values(BranchPrioritiesTestParams(
SolverType::kXpress, SolveParameters())));unfortunately, some added tests fail and should be looked into in detail.
As for the test suites, I'm not 100% sure I was exhaustive, I suggest that you also take a look at the other tests marked with a GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST. (unfortunately, I think that you won't be able to enable TimeLimitTest yet, since it requires Callback event support)
| std::vector<double> quad_coef; | ||
| ASSIGN_OR_RETURN(int next, xpress_->GetIntAttr(XPRS_ORIGINALROWS)); | ||
| for (const auto& [ortoolsId, quad] : constraints) { | ||
| // Xpress has no function to multiple quadratic rows in one shot, so we |
There was a problem hiding this comment.
| // Xpress has no function to multiple quadratic rows in one shot, so we | |
| // Xpress has no function to add multiple quadratic rows in one shot, so we |
| /*qp_support=*/QpSupportType::kConvexQp, | ||
| /*supports_incrementalism_not_modifying_qp=*/false, | ||
| /*supports_qp_incrementalism=*/false, | ||
| /*use_integer_variables=*/i != 0)); |
There was a problem hiding this comment.
would be clearer (IMO) if it was just seperated into two true/false cases, like with the other parameters
| } else { | ||
| // Auxiliary objective in multi-objective. Xpress does not support | ||
| // different objective senses for different objectives. So if the sense | ||
| // does not match we set the weight to -1.0 to inver the objective |
There was a problem hiding this comment.
| // does not match we set the weight to -1.0 to inver the objective | |
| // does not match we set the weight to -1.0 to invert the objective |
| export_model.compare(export_model.length() - 4, 4, ".svf") == 0) { | ||
| RETURN_IF_ERROR(xpress_->SaveAs(export_model.c_str())); | ||
| } else { | ||
| RETURN_IF_ERROR(xpress_->WriteProb(export_model.c_str())); |
There was a problem hiding this comment.
maybe consider extracting row & column names first? otherwise the file is not easy to read (rows & columns keep their default names, i.e. R1, R2, ... & C1, C2, ...)
| return util::InvalidArgumentErrorBuilder() | ||
| << "more lazy constraints than rows"; | ||
|
|
||
| RETURN_IF_ERROR(shared_ctx.xpress->LoadDelayedRows(delayedRows)); |
There was a problem hiding this comment.
I noticed on a manually-built case that Xpress does not behave well if the indicator constraint is associated to a binary variable that has lb=ub. I guess Xpress treats such a variable as continuous or integer under the hood. I feel that this can be a blocking issue for some users (for example if the user wants to add a binary variable with its lb & ub values read form some data source or equation evaluation).
5 participants
This pull requests adds support for a lot of MIP features for the Xpress solver in ortools:
XpressParametersclass)EXPORT_MODELparameterWe also changed the LP part: we now always call XPRSoptimize(), which is a more modern way of using Xpress and letting Xpress decide what is the best algorithm to run.
Even with this pull request we still do not support the following things for the Xpress solver
We had to adjust some tests because it seemed they did not honor all the parameters passed into them. It is probably a good idea to double check these changes very carefully.