Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for MOSEK, linear, conic and mip #4452

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

ulfworsoe
Copy link

As per our correspondence, here is a PR for MOSEK support. I have not signed the contributor license yet - let me know if that will be necessary - otherwise, feel free to copy anything in this PR.

It should support most major features: Linear interface and math_opt, conic constraints, quadratic terms, integer variables, indicator constraints, deleting items (we do not actually remove constraints, only disable them).

Copy link

google-cla bot commented Nov 27, 2024

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.

@rma350
Copy link
Collaborator

rma350 commented Nov 27, 2024

Looks great. At a high level, I think this is pretty similar to what we had done internally (wrapped the C API with ABSL/C++ bindings first).

I see that you clear variable/constraints instead of deleting them in your C++ wrapper. Your API does support deletion yes? I understand that in some settings, clearing is faster than deletion, but I guess our perspective is that the user can clear themselves if they want clear. If they really need deletion (e.g. they are managing a very large number of cuts/columns), then we want to offer it to them. Also, with your deletion API, do variables have stable ids, or indices, and all the indices change on each deletion?

A big challenge that is not yet complete is to pass the integration tests, e.g. have a file like this:

https://github.com/google/or-tools/blob/stable/ortools/math_opt/solvers/gscip_solver_test.cc

(The gurobi version of this file appears not be exported to the open source project? Not sure why.) This always ends up being harder than it looks, due to a combination of the tests being a bit fragile and the solvers all being a little different (we try to run the same tests for all the solvers, with some solver specific configuraiton).

Specifically re MPSolver, inside of Google we are basically done adding features here, but we are continuing to work on the MPModelProto path e.g. here:

https://github.com/google/or-tools/tree/stable/ortools/linear_solver/proto_solver
https://github.com/google/or-tools/blob/stable/ortools/linear_solver/solve_mp_model.h

(but nothing conic). You would need to check with lperron, maybe we don't even want to merge any more mpsolver changes that don't just delegate to solving with the proto.

The people working the Mosek stuff are US based, so we will probably look more closely after thanksgiving.

@@ -268,6 +268,11 @@ endif()
CMAKE_DEPENDENT_OPTION(USE_GUROBI "Use the Gurobi solver" ON "BUILD_CXX" OFF)
message(STATUS "Gurobi support: ${USE_GUROBI}")

## MOSEK
# see: https://mosek.com
CMAKE_DEPENDENT_OPTION(USE_HIGHS "Use the MOSEK solver" ON "BUILD_CXX" OFF)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/USE_HIGHS/USE_MOSEK/ ;)

# locations, and if that fails, assume it is installed in a system location.
# For option 2 and 3, the newest MOSEK version will be chosen if more are available.

if(MOSEK_PLATFORM_DIR)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would use MOSEK_ROOT_DIR to follow convention

From CMake head repository:

$ git grep -n "ROOT_DIR\b" | wc -l
209
$ git grep -n "PLATFORM_DIR\b" | wc -l
0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants