search

RobotLocomotion / drake

Model-based design and verification for robotics.
https://drake.mit.edu
Other
3.24k stars 1.25k forks source link

Upgrade Mosek to Mosek 10 #17026

Closed hongkai-dai closed 1 year ago

hongkai-dai commented 2 years ago

As mentioned in https://drakedevelopers.slack.com/archives/C3L92BM2Q/p1650594410258979

We need to complete following tasks

jwnimmer-tri commented 2 years ago

Will we upgrade while MOSEK 10 is still in Beta, or will we be waiting for an official release first?

hongkai-dai commented 2 years ago

Perfect timing, I just received an email from Mosek author saying that the performance of Mosek on semidefinite programming problem has improved significantly. I just sent him an email asking when the stable version will be released. I will update this thread once I hear back from them.

I tried to change the build file to use version 10.0.16 directly, but got error on compiling clk5 library.

jwnimmer-tri commented 2 years ago

Right. If I recall correctly, Mosek 10 uses a different multithreading substrate, so we'll need to do surgery on our BUILD files (and distribution scripts) to account for the changed dependency.

hongkai-dai commented 2 years ago

Right, in their blog post they mention that in Mosek 9 the computation time with multithread is almost the same as with the single thread. But on mosek 10 multithread is much faster https://themosekblog.blogspot.com/2022/06/on-performance-improvement-of-semi.html

hongkai-dai commented 2 years ago

Its twitter says the stable version is going to be released in mid-August https://twitter.com/mosektw?lang=en

Given the large performance boost on Mosek 10 with multi-thread, I think it makes sense to upgrade to version 10 sooner? Especially because it will take some time to modify our MosekSolver implementation, as some of the old Mosek API are deprecated, so I will need to re-write the code to parse Lorentz cone constraint in mosek_solver.cc. What do you think?

jwnimmer-tri commented 2 years ago

I think it would be a good idea to start a shared development branch for the proposed changes (e.g., "Draft PR" with "allow edits from maintainers" checked), so that we can start developing, testing, and debugging the feature.

As to whether we land that PR in master, while Mosek 10 is still in Beta stage? My current vote is "no" -- we shouldn't ship beta dependencies. However, we can wait to re-assess that once we have the code working and know more about the actual trade-offs.

hongkai-dai commented 2 years ago

Thanks! Yes, I think a developement branch makes sense. I can also test our optimization code on that dev branch.

hongkai-dai commented 2 years ago

I would like to work on this dev branch as soon as possible. In my research I encounter some problems that Mosek 9.3.20 cannot solve, while Mosek 10.0.16 can solve. So I think it makes sense to try out the new version. @jwnimmer-tri I tried to change the version in the mosek build bazel file to 10.0.16 directly but ran into build error with clk5 library. May I ask you to help me on the build files (after the TRI holidays)? Thanks a lot!

jwnimmer-tri commented 2 years ago

@hongkai-dai Open a PR with your progress so far and ensure Allow edits by maintainers is checked "yes". Then we can each push commits to incrementally improve it.

hongkai-dai commented 2 years ago

Sounds great, I filed PR https://github.com/RobotLocomotion/drake/pull/17490

jwnimmer-tri commented 2 years ago

Given the "everything" configuration test failures in #17490 ("RuntimeError: Could not acquire MOSEK license."), it seems like we also need to upgrade Drake CI's MOSEK license to enable Mosek 10? I didn't see a checklist item atop this issue with that task yet.

hongkai-dai commented 2 years ago

Ah you are right. I forgot that the drake CI doesn't use TRI's mosek license. Is the CI using MIT's mosek license?

jwnimmer-tri commented 2 years ago

Yes, that's right. I opened #17514 with the request.

svenevs commented 1 year ago

Change Drake's build system to use Mosek 10

I think we can mark this as complete.

Change solvers::MosekSolver class to use the affine cone constraints.

m1 everything builds will use mosek 10 on macOS arm64. More m1 flavors inbound now that everything finally works, just wanted to mention that we should at last be in a place where changing this can be fully tested in CI on linux, mac, and m1 mac.

jwnimmer-tri commented 1 year ago

Change Drake's build system to use Mosek 10

I think we can mark this as complete.

Yes, as of https://github.com/RobotLocomotion/drake/pull/17490. I'll do that now.

Change solvers::MosekSolver class to use the affine cone constraints.

@hongkai-dai Maybe we should open a use issue for that enhancement? The upgrade is finished.

hongkai-dai commented 1 year ago

@hongkai-dai Maybe we should open a use issue for that enhancement? The upgrade is finished.

I think this is completed in https://github.com/RobotLocomotion/drake/pull/18114. We can close this issue.